Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[python-native-code] breaks Pantsd cache invalidation #14612

Closed
Eric-Arellano opened this issue Feb 25, 2022 · 1 comment · Fixed by #17009 or #17013
Closed

[python-native-code] breaks Pantsd cache invalidation #14612

Eric-Arellano opened this issue Feb 25, 2022 · 1 comment · Fixed by #17009 or #17013
Assignees
Labels
backend: Python Python backend-related issues bug

Comments

@Eric-Arellano
Copy link
Contributor

We should not be using os.environ. It needs to use the engine to request Environment instead.

"--cpp-flags",
default=safe_shlex_split(os.environ.get("CPPFLAGS", "")),

We might need to add a new magic string like <CPPFLAGS>, similar to how we handle <PATH>.

def go_search_paths(self, env: Environment) -> tuple[str, ...]:
def iter_path_entries():
for entry in self._go_search_paths:
if entry == "<PATH>":
path = env.get("PATH")
if path:
yield from path.split(os.pathsep)
else:
yield entry
return tuple(OrderedSet(iter_path_entries()))

While fixing, if we don't use a magic string like <CPPFLAGS>, we should also use default_str so that our docs don't put whatever the value of the docs release is.

--

Even better would be removing [python-native-code] and consolidating it into something like [subprocess-environment] or [python]. The subsystem is confusing and a holdover from v1. I tried consolidating, but gave up because I couldn't avoid breaking the deprecation policy.

consolidating it into something like [subprocess-environment] or [python]

It seems like we've gone in the direction of env vars per language, so [subprocess-environment] is probably a misnomer.

@chrisjrn
Copy link
Contributor

For deprecation purposes, I'm proposing adding a deprecation message for the envvar CPPFLAGS being set if the environment target is a local environment, and an error if not. The magic string approach seems OK for a new method that should explicitly only work on local environments.

Consolidating the various Python subsystems looks like a good idea, probably worth undertaking as a separate ticket though.

chrisjrn pushed a commit that referenced this issue Sep 26, 2022
…ange (#17009)

This adds `PythonNativeCodeEnvironment`, a value created by a rule that encapsulates the environment var fetching needed to make `PythonNativeCodeSubsystem.EnvironmentAware` resolve correctly.

Fixes #14612
chrisjrn pushed a commit that referenced this issue Sep 27, 2022
This extends #17009 by making a generic mechanism to request environment variables when `EnvironmentAware` subsystems are constructed.

Generally speaking, this encapsulates both the requesting and consumption of environment variables into the subsystem itself, and this will save subsystem consumers from knowing which environment variables to request in order to call an option post-processing method. All option post-processing methods should safely be able to be converted to properties too. All very exciting.

Fixes #14612
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend: Python Python backend-related issues bug
Projects
None yet
3 participants