From 73e97c2cb047db4eedaf22af9d472fb253ea64ac Mon Sep 17 00:00:00 2001 From: Stu Hood Date: Mon, 8 Mar 2021 20:30:56 -0800 Subject: [PATCH] Require explicit environment usage (#11641) ### Problem In order to allow concurrent runs of Pants in #11639 (part of #7654), all global mutable singletons must be either 1) thread-local, 2) replaced with explicitly passed values. `os.environ` and `sys.argv` are two of the last cases we're concerned with. ### Solution Although we could hypothetically redefine `os.environ` and `os.getenv` as thread-local, they seem to be a better fit to be explicitly passed. So this change: 1. Renames (and moves) `PantsEnvironment` to `CompleteEnvironment` 2. Adds `Environment` and `EnvironmentRequest`, which filter the `CompleteEnvironment` to a smaller subset, and should be used more frequently by `@rules`. 3. Fixes a few cases of implicit `os.environ` usage in the `PythonSetup` subsystem by explicitly selecting interpreter-selection env vars (`HOME`/`PATH`/`PYENV_ROOT`) Finally, we empty the environment for `pantsd` runs to enforce explicit usage of `CompleteEnvironment`/`Environment`. ### Result The last (?) global mutable variable blocking #7654 is removed. [ci skip-build-wheels] --- pants.toml | 6 + .../backend/awslambda/python/rules_test.py | 3 +- .../protobuf/python/rules_integration_test.py | 5 +- .../import_parser_test.py | 1 + .../python/dependency_inference/rules_test.py | 8 +- .../goals/pytest_runner_integration_test.py | 2 +- .../backend/python/goals/setup_py_test.py | 7 +- .../lint/bandit/rules_integration_test.py | 2 +- .../lint/black/rules_integration_test.py | 2 +- .../docformatter/rules_integration_test.py | 2 +- .../lint/flake8/rules_integration_test.py | 2 +- .../lint/isort/rules_integration_test.py | 2 +- .../lint/pylint/rules_integration_test.py | 2 +- .../lint/python_fmt_integration_test.py | 3 +- .../typecheck/mypy/rules_integration_test.py | 2 +- .../backend/python/util_rules/pex_cli_test.py | 5 +- .../python/util_rules/pex_environment.py | 20 +-- .../backend/python/util_rules/pex_test.py | 5 +- src/python/pants/bin/daemon_pants_runner.py | 26 ++-- src/python/pants/bin/local_pants_runner.py | 23 ++-- src/python/pants/bin/pants_runner.py | 3 +- src/python/pants/core/goals/repl.py | 7 +- .../pants/core/goals/repl_integration_test.py | 3 + src/python/pants/core/goals/run.py | 7 +- src/python/pants/core/goals/run_test.py | 1 + src/python/pants/core/goals/test.py | 6 +- src/python/pants/core/register.py | 2 - src/python/pants/core/target_types_test.py | 4 +- .../core/util_rules/pants_environment.py | 18 +-- .../core/util_rules/subprocess_environment.py | 15 ++- src/python/pants/engine/desktop.py | 9 +- src/python/pants/engine/environment.py | 116 ++++++++++++++++++ .../environment_test.py} | 8 +- src/python/pants/engine/process.py | 19 ++- src/python/pants/init/engine_initializer.py | 9 +- src/python/pants/init/options_initializer.py | 28 +++-- src/python/pants/init/plugin_resolver.py | 16 ++- src/python/pants/option/global_options.py | 12 +- .../pants/option/global_options_test.py | 10 +- .../pants/option/options_bootstrapper.py | 4 +- src/python/pants/pantsd/pants_daemon.py | 23 ++-- src/python/pants/pantsd/pants_daemon_core.py | 17 ++- src/python/pants/python/python_setup.py | 54 ++++---- src/python/pants/python/python_setup_test.py | 61 +++++---- src/python/pants/testutil/rule_runner.py | 32 +++-- src/rust/engine/src/externs/interface.rs | 10 +- src/rust/engine/src/scheduler.rs | 5 +- .../init/test_options_initializer.py | 9 +- .../pants_test/init/test_plugin_resolver.py | 51 +++++--- .../pantsd/test_pants_daemon_core.py | 10 +- 50 files changed, 470 insertions(+), 227 deletions(-) create mode 100644 src/python/pants/engine/environment.py rename src/python/pants/{core/util_rules/pants_environment_test.py => engine/environment_test.py} (79%) diff --git a/pants.toml b/pants.toml index 4493a208242..ea4be73961f 100644 --- a/pants.toml +++ b/pants.toml @@ -136,6 +136,12 @@ pytest_plugins.add = [ ] timeout_default = 60 +[test] +# TODO: These are exposed to tests in order to allow for python interpreter discovery when +# Pants-tests-Pants: in particular, the [python-setup] subsystem consumes them. +# see https://github.com/pantsbuild/pants/issues/11638 +extra_env_vars = ["PYENV_ROOT", "HOME", "PATH"] + [coverage-py] interpreter_constraints = [">=3.7,<3.9"] diff --git a/src/python/pants/backend/awslambda/python/rules_test.py b/src/python/pants/backend/awslambda/python/rules_test.py index 7fb22e412af..1bf25738ca6 100644 --- a/src/python/pants/backend/awslambda/python/rules_test.py +++ b/src/python/pants/backend/awslambda/python/rules_test.py @@ -36,7 +36,8 @@ def create_python_awslambda(rule_runner: RuleRunner, addr: Address) -> Tuple[str [ "--backend-packages=pants.backend.awslambda.python", "--source-root-patterns=src/python", - ] + ], + env_inherit={"PATH", "PYENV_ROOT", "HOME"}, ) target = rule_runner.get_target(addr) built_asset = rule_runner.request(BuiltPackage, [PythonAwsLambdaFieldSet.create(target)]) diff --git a/src/python/pants/backend/codegen/protobuf/python/rules_integration_test.py b/src/python/pants/backend/codegen/protobuf/python/rules_integration_test.py index fda26a88f2c..bf6403c86e3 100644 --- a/src/python/pants/backend/codegen/protobuf/python/rules_integration_test.py +++ b/src/python/pants/backend/codegen/protobuf/python/rules_integration_test.py @@ -46,7 +46,10 @@ def assert_files_generated( ] if mypy: options.append("--python-protobuf-mypy-plugin") - rule_runner.set_options(options) + rule_runner.set_options( + options, + env_inherit={"PATH", "PYENV_ROOT", "HOME"}, + ) tgt = rule_runner.get_target(Address(spec)) protocol_sources = rule_runner.request( HydratedSources, [HydrateSourcesRequest(tgt[ProtobufSources])] diff --git a/src/python/pants/backend/python/dependency_inference/import_parser_test.py b/src/python/pants/backend/python/dependency_inference/import_parser_test.py index 3eb0a6e1c53..87edf25f990 100644 --- a/src/python/pants/backend/python/dependency_inference/import_parser_test.py +++ b/src/python/pants/backend/python/dependency_inference/import_parser_test.py @@ -48,6 +48,7 @@ def assert_imports_parsed( ): if content: rule_runner.create_file(filename, content) + rule_runner.set_options([], env_inherit={"PATH", "PYENV_ROOT", "HOME"}) rule_runner.add_to_build_file("project", "python_library(sources=['**/*.py'])") tgt = rule_runner.get_target(Address("project")) imports = rule_runner.request( diff --git a/src/python/pants/backend/python/dependency_inference/rules_test.py b/src/python/pants/backend/python/dependency_inference/rules_test.py index 082658c2379..60b6fa8b78d 100644 --- a/src/python/pants/backend/python/dependency_inference/rules_test.py +++ b/src/python/pants/backend/python/dependency_inference/rules_test.py @@ -83,7 +83,7 @@ def run_dep_inference( args = ["--backend-packages=pants.backend.python", "--source-root-patterns=src/python"] if enable_string_imports: args.append("--python-infer-string-imports") - rule_runner.set_options(args) + rule_runner.set_options(args, env_inherit={"PATH", "PYENV_ROOT", "HOME"}) target = rule_runner.get_target(address) return rule_runner.request( InferredDependencies, [InferPythonImportDependencies(target[PythonSources])] @@ -129,7 +129,8 @@ def test_infer_python_inits() -> None: "--backend-packages=pants.backend.python", "--python-infer-inits", "--source-root-patterns=src/python", - ] + ], + env_inherit={"PATH", "PYENV_ROOT", "HOME"}, ) rule_runner.create_file("src/python/root/__init__.py") @@ -168,7 +169,8 @@ def test_infer_python_conftests() -> None: target_types=[PythonTests], ) rule_runner.set_options( - ["--backend-packages=pants.backend.python", "--source-root-patterns=src/python"] + ["--backend-packages=pants.backend.python", "--source-root-patterns=src/python"], + env_inherit={"PATH", "PYENV_ROOT", "HOME"}, ) rule_runner.create_file("src/python/root/conftest.py") diff --git a/src/python/pants/backend/python/goals/pytest_runner_integration_test.py b/src/python/pants/backend/python/goals/pytest_runner_integration_test.py index 22c134075bc..8f6896ce210 100644 --- a/src/python/pants/backend/python/goals/pytest_runner_integration_test.py +++ b/src/python/pants/backend/python/goals/pytest_runner_integration_test.py @@ -169,7 +169,7 @@ def run_pytest( args.append("--pytest-config=pytest.ini") if force: args.append("--test-force") - rule_runner.set_options(args, env=env) + rule_runner.set_options(args, env=env, env_inherit={"PATH", "PYENV_ROOT", "HOME"}) inputs = [PythonTestFieldSet.create(test_target)] test_result = rule_runner.request(TestResult, inputs) diff --git a/src/python/pants/backend/python/goals/setup_py_test.py b/src/python/pants/backend/python/goals/setup_py_test.py index 9319b39d735..2b59d259b9a 100644 --- a/src/python/pants/backend/python/goals/setup_py_test.py +++ b/src/python/pants/backend/python/goals/setup_py_test.py @@ -56,7 +56,7 @@ def create_setup_py_rule_runner(*, rules: Iterable) -> RuleRunner: - return RuleRunner( + rule_runner = RuleRunner( rules=rules, target_types=[ PexBinary, @@ -68,6 +68,8 @@ def create_setup_py_rule_runner(*, rules: Iterable) -> RuleRunner: ], objects={"setup_py": PythonArtifact}, ) + rule_runner.set_options([], env_inherit={"PATH", "PYENV_ROOT", "HOME"}) + return rule_runner # We use a trivial test that our SetupKwargs plugin hook works. @@ -501,7 +503,8 @@ def assert_requirements( version_scheme: FirstPartyDependencyVersionScheme = FirstPartyDependencyVersionScheme.EXACT, ): rule_runner.set_options( - [f"--setup-py-generation-first-party-dependency-version-scheme={version_scheme.value}"] + [f"--setup-py-generation-first-party-dependency-version-scheme={version_scheme.value}"], + env_inherit={"PATH", "PYENV_ROOT", "HOME"}, ) tgt = rule_runner.get_target(addr) reqs = rule_runner.request( diff --git a/src/python/pants/backend/python/lint/bandit/rules_integration_test.py b/src/python/pants/backend/python/lint/bandit/rules_integration_test.py index 369abe40c99..52fd39f74f6 100644 --- a/src/python/pants/backend/python/lint/bandit/rules_integration_test.py +++ b/src/python/pants/backend/python/lint/bandit/rules_integration_test.py @@ -72,7 +72,7 @@ def run_bandit( args.append("--bandit-skip") if additional_args: args.extend(additional_args) - rule_runner.set_options(args) + rule_runner.set_options(args, env_inherit={"PATH", "PYENV_ROOT", "HOME"}) results = rule_runner.request( LintResults, [BanditRequest(BanditFieldSet.create(tgt) for tgt in targets)], diff --git a/src/python/pants/backend/python/lint/black/rules_integration_test.py b/src/python/pants/backend/python/lint/black/rules_integration_test.py index 860286cfa18..e24ddf0e140 100644 --- a/src/python/pants/backend/python/lint/black/rules_integration_test.py +++ b/src/python/pants/backend/python/lint/black/rules_integration_test.py @@ -88,7 +88,7 @@ def run_black( args.append("--black-skip") if version: args.append(f"--black-version={version}") - rule_runner.set_options(args) + rule_runner.set_options(args, env_inherit={"PATH", "PYENV_ROOT", "HOME"}) field_sets = [BlackFieldSet.create(tgt) for tgt in targets] lint_results = rule_runner.request(LintResults, [BlackRequest(field_sets)]) input_sources = rule_runner.request( diff --git a/src/python/pants/backend/python/lint/docformatter/rules_integration_test.py b/src/python/pants/backend/python/lint/docformatter/rules_integration_test.py index ae183f50fe3..d75702fa0e7 100644 --- a/src/python/pants/backend/python/lint/docformatter/rules_integration_test.py +++ b/src/python/pants/backend/python/lint/docformatter/rules_integration_test.py @@ -53,7 +53,7 @@ def run_docformatter( args.append(f"--docformatter-args='{passthrough_args}'") if skip: args.append("--docformatter-skip") - rule_runner.set_options(args) + rule_runner.set_options(args, env_inherit={"PATH", "PYENV_ROOT", "HOME"}) field_sets = [DocformatterFieldSet.create(tgt) for tgt in targets] lint_results = rule_runner.request(LintResults, [DocformatterRequest(field_sets)]) input_sources = rule_runner.request( diff --git a/src/python/pants/backend/python/lint/flake8/rules_integration_test.py b/src/python/pants/backend/python/lint/flake8/rules_integration_test.py index 1e758a21c10..6e1989dc647 100644 --- a/src/python/pants/backend/python/lint/flake8/rules_integration_test.py +++ b/src/python/pants/backend/python/lint/flake8/rules_integration_test.py @@ -68,7 +68,7 @@ def run_flake8( args.append("--flake8-skip") if additional_args: args.extend(additional_args) - rule_runner.set_options(args) + rule_runner.set_options(args, env_inherit={"PATH", "PYENV_ROOT", "HOME"}) results = rule_runner.request( LintResults, [ diff --git a/src/python/pants/backend/python/lint/isort/rules_integration_test.py b/src/python/pants/backend/python/lint/isort/rules_integration_test.py index 66c94183dcc..988ff141e77 100644 --- a/src/python/pants/backend/python/lint/isort/rules_integration_test.py +++ b/src/python/pants/backend/python/lint/isort/rules_integration_test.py @@ -66,7 +66,7 @@ def run_isort( args.append(f"--isort-args='{passthrough_args}'") if skip: args.append("--isort-skip") - rule_runner.set_options(args) + rule_runner.set_options(args, env_inherit={"PATH", "PYENV_ROOT", "HOME"}) field_sets = [IsortFieldSet.create(tgt) for tgt in targets] lint_results = rule_runner.request(LintResults, [IsortRequest(field_sets)]) input_sources = rule_runner.request( diff --git a/src/python/pants/backend/python/lint/pylint/rules_integration_test.py b/src/python/pants/backend/python/lint/pylint/rules_integration_test.py index 00394678f52..912b3f58ea8 100644 --- a/src/python/pants/backend/python/lint/pylint/rules_integration_test.py +++ b/src/python/pants/backend/python/lint/pylint/rules_integration_test.py @@ -93,7 +93,7 @@ def run_pylint( args.append("--pylint-skip") if additional_args: args.extend(additional_args) - rule_runner.set_options(args) + rule_runner.set_options(args, env_inherit={"PATH", "PYENV_ROOT", "HOME"}) results = rule_runner.request( LintResults, [PylintRequest(PylintFieldSet.create(tgt) for tgt in targets)], diff --git a/src/python/pants/backend/python/lint/python_fmt_integration_test.py b/src/python/pants/backend/python/lint/python_fmt_integration_test.py index 2a335cff3f2..224314e298d 100644 --- a/src/python/pants/backend/python/lint/python_fmt_integration_test.py +++ b/src/python/pants/backend/python/lint/python_fmt_integration_test.py @@ -45,7 +45,8 @@ def run_black_and_isort( [ "--backend-packages=['pants.backend.python.lint.black', 'pants.backend.python.lint.isort']", *(extra_args or []), - ] + ], + env_inherit={"PATH", "PYENV_ROOT", "HOME"}, ) results = rule_runner.request(LanguageFmtResults, [targets]) return results diff --git a/src/python/pants/backend/python/typecheck/mypy/rules_integration_test.py b/src/python/pants/backend/python/typecheck/mypy/rules_integration_test.py index 9a90ffdeb6e..0227b7821a4 100644 --- a/src/python/pants/backend/python/typecheck/mypy/rules_integration_test.py +++ b/src/python/pants/backend/python/typecheck/mypy/rules_integration_test.py @@ -134,7 +134,7 @@ def run_mypy( args.append("--mypy-skip") if additional_args: args.extend(additional_args) - rule_runner.set_options(args) + rule_runner.set_options(args, env_inherit={"PATH", "PYENV_ROOT", "HOME"}) result = rule_runner.request( TypecheckResults, [MyPyRequest(MyPyFieldSet.create(tgt) for tgt in targets)], diff --git a/src/python/pants/backend/python/util_rules/pex_cli_test.py b/src/python/pants/backend/python/util_rules/pex_cli_test.py index 775498c6a44..1c5b847e421 100644 --- a/src/python/pants/backend/python/util_rules/pex_cli_test.py +++ b/src/python/pants/backend/python/util_rules/pex_cli_test.py @@ -28,7 +28,10 @@ def test_custom_ca_certs(rule_runner: RuleRunner) -> None: with temporary_dir() as tmpdir: certs_file = Path(tmpdir) / "certsfile" certs_file.write_text("Some fake cert") - rule_runner.set_options([f"--ca-certs-path={certs_file}"]) + rule_runner.set_options( + [f"--ca-certs-path={certs_file}"], + env_inherit={"PATH", "PYENV_ROOT", "HOME"}, + ) proc = rule_runner.request( Process, [PexCliProcess(argv=["some", "--args"], description="")], diff --git a/src/python/pants/backend/python/util_rules/pex_environment.py b/src/python/pants/backend/python/util_rules/pex_environment.py index 944681f172f..9b514655e82 100644 --- a/src/python/pants/backend/python/util_rules/pex_environment.py +++ b/src/python/pants/backend/python/util_rules/pex_environment.py @@ -11,6 +11,7 @@ from pants.core.util_rules.subprocess_environment import SubprocessEnvironmentVars from pants.engine import process from pants.engine.engine_aware import EngineAwareReturnType +from pants.engine.environment import Environment, EnvironmentRequest from pants.engine.process import BinaryPath, BinaryPathRequest, BinaryPaths, BinaryPathTest from pants.engine.rules import Get, MultiGet, collect_rules, rule from pants.option.global_options import GlobalOptions @@ -18,7 +19,7 @@ from pants.python.python_setup import PythonSetup from pants.util.frozendict import FrozenDict from pants.util.logging import LogLevel -from pants.util.memo import memoized_property +from pants.util.memo import memoized_method from pants.util.ordered_set import OrderedSet from pants.util.strutil import create_path_env_var @@ -67,12 +68,12 @@ def register_options(cls, register): ), ) - @memoized_property - def path(self) -> Tuple[str, ...]: + @memoized_method + def path(self, env: Environment) -> Tuple[str, ...]: def iter_path_entries(): for entry in self.options.executable_search_paths: if entry == "": - path = os.environ.get("PATH") + path = env.get("PATH") if path: for path_entry in path.split(os.pathsep): yield path_entry @@ -158,6 +159,9 @@ async def find_pex_python( subprocess_env_vars: SubprocessEnvironmentVars, global_options: GlobalOptions, ) -> PexEnvironment: + pex_relevant_environment = await Get( + Environment, EnvironmentRequest(["PATH", "HOME", "PYENV_ROOT"]) + ) # PEX files are compatible with bootstrapping via Python 2.7 or Python 3.5+. The bootstrap # code will then re-exec itself if the underlying PEX user code needs a more specific python # interpreter. As such, we look for many Pythons usable by the PEX bootstrap code here for @@ -166,7 +170,7 @@ async def find_pex_python( Get( BinaryPaths, BinaryPathRequest( - search_path=python_setup.interpreter_search_paths, + search_path=python_setup.interpreter_search_paths(pex_relevant_environment), binary_name=binary_name, test=BinaryPathTest( args=[ @@ -218,8 +222,10 @@ def first_python_binary() -> Optional[PythonExecutable]: return None return PexEnvironment( - path=pex_runtime_env.path, - interpreter_search_paths=tuple(python_setup.interpreter_search_paths), + path=pex_runtime_env.path(pex_relevant_environment), + interpreter_search_paths=tuple( + python_setup.interpreter_search_paths(pex_relevant_environment) + ), subprocess_environment_dict=subprocess_env_vars.vars, # TODO: This path normalization is duplicated with `engine_initializer.py`. How can we do # the normalization only once, via the options system? diff --git a/src/python/pants/backend/python/util_rules/pex_test.py b/src/python/pants/backend/python/util_rules/pex_test.py index 4b58d9340cd..c9cdc0af72b 100644 --- a/src/python/pants/backend/python/util_rules/pex_test.py +++ b/src/python/pants/backend/python/util_rules/pex_test.py @@ -352,7 +352,9 @@ def create_pex_and_get_all_data( additional_args=additional_pex_args, ) rule_runner.set_options( - ["--backend-packages=pants.backend.python", *additional_pants_args], env=env + ["--backend-packages=pants.backend.python", *additional_pants_args], + env=env, + env_inherit={"PATH", "PYENV_ROOT", "HOME"}, ) pex = rule_runner.request(pex_type, [request]) if isinstance(pex, Pex): @@ -468,6 +470,7 @@ def test_pex_environment(rule_runner: RuleRunner, pex_type: type[Pex | VenvPex]) "--subprocess-environment-env-vars=LANG", # Value should come from environment. "--subprocess-environment-env-vars=ftp_proxy=dummyproxy", ), + interpreter_constraints=PexInterpreterConstraints(["CPython>=3.6"]), env={"LANG": "es_PY.UTF-8"}, ) diff --git a/src/python/pants/bin/daemon_pants_runner.py b/src/python/pants/bin/daemon_pants_runner.py index dff4598676c..617bba955bb 100644 --- a/src/python/pants/bin/daemon_pants_runner.py +++ b/src/python/pants/bin/daemon_pants_runner.py @@ -11,12 +11,12 @@ from pants.base.exiter import PANTS_FAILED_EXIT_CODE, ExitCode from pants.bin.local_pants_runner import LocalPantsRunner +from pants.engine.environment import CompleteEnvironment from pants.engine.internals.native import RawFdRunner from pants.engine.internals.native_engine import PySessionCancellationLatch from pants.init.logging import stdio_destination from pants.option.options_bootstrapper import OptionsBootstrapper from pants.pantsd.pants_daemon_core import PantsDaemonCore -from pants.util.contextutil import argv_as, hermetic_environment_as logger = logging.getLogger(__name__) @@ -97,7 +97,11 @@ def should_keep_polling(now): ) def single_daemonized_run( - self, working_dir: str, cancellation_latch: PySessionCancellationLatch + self, + args: Tuple[str, ...], + env: Dict[str, str], + working_dir: str, + cancellation_latch: PySessionCancellationLatch, ) -> ExitCode: """Run a single daemonized run of Pants. @@ -110,22 +114,18 @@ def single_daemonized_run( logger.debug("Connected to pantsd") # Capture the client's start time, which we propagate here in order to get an accurate # view of total time. - env_start_time = os.environ.get("PANTSD_RUNTRACKER_CLIENT_START_TIME", None) + env_start_time = env.get("PANTSD_RUNTRACKER_CLIENT_START_TIME", None) start_time = float(env_start_time) if env_start_time else time.time() - # Clear global mutable state before entering `LocalPantsRunner`. Note that we use - # `sys.argv` and `os.environ`, since they have been mutated to maintain the illusion - # of a local run: once we allow for concurrent runs, this information should be - # propagated down from the caller. - # see https://github.com/pantsbuild/pants/issues/7654 options_bootstrapper = OptionsBootstrapper.create( - env=os.environ, args=sys.argv, allow_pantsrc=True + env=env, args=args, allow_pantsrc=True ) # Run using the pre-warmed Session. - scheduler, options_initializer = self._core.prepare(options_bootstrapper) + complete_env = CompleteEnvironment(env) + scheduler, options_initializer = self._core.prepare(options_bootstrapper, complete_env) runner = LocalPantsRunner.create( - os.environ, + complete_env, options_bootstrapper, scheduler=scheduler, options_initializer=options_initializer, @@ -165,9 +165,9 @@ def __call__( stdin_fileno=stdin_fileno, stdout_fileno=stdout_fileno, stderr_fileno=stderr_fileno, - ), hermetic_environment_as(**env), argv_as((command,) + args): + ): return self.single_daemonized_run( - working_directory.decode(), cancellation_latch + ((command,) + args), env, working_directory.decode(), cancellation_latch ) finally: logger.info(f"request completed: `{' '.join(args)}`") diff --git a/src/python/pants/bin/local_pants_runner.py b/src/python/pants/bin/local_pants_runner.py index 804ca22c30b..1d64bbbb541 100644 --- a/src/python/pants/bin/local_pants_runner.py +++ b/src/python/pants/bin/local_pants_runner.py @@ -4,9 +4,8 @@ from __future__ import annotations import logging -import os from dataclasses import dataclass -from typing import Mapping, Optional, Tuple +from typing import Optional, Tuple from pants.base.build_environment import get_buildroot from pants.base.exception_sink import ExceptionSink @@ -14,7 +13,7 @@ from pants.base.specs import Specs from pants.base.specs_parser import SpecsParser from pants.build_graph.build_configuration import BuildConfiguration -from pants.core.util_rules.pants_environment import PantsEnvironment +from pants.engine.environment import CompleteEnvironment from pants.engine.internals.native import Native from pants.engine.internals.native_engine import PySessionCancellationLatch from pants.engine.internals.scheduler import ExecutionError @@ -67,6 +66,7 @@ def _init_graph_session( options_initializer: OptionsInitializer, options_bootstrapper: OptionsBootstrapper, build_config: BuildConfiguration, + env: CompleteEnvironment, run_id: str, options: Options, scheduler: Optional[GraphScheduler] = None, @@ -75,9 +75,9 @@ def _init_graph_session( native = Native() native.set_panic_handler() graph_scheduler_helper = scheduler or EngineInitializer.setup_graph( - options_bootstrapper, build_config + options_bootstrapper, build_config, env ) - with options_initializer.handle_unknown_flags(options_bootstrapper, raise_=True): + with options_initializer.handle_unknown_flags(options_bootstrapper, env, raise_=True): global_options = options.for_global_scope() return graph_scheduler_helper.new_session( run_id, @@ -86,7 +86,7 @@ def _init_graph_session( session_values=SessionValues( { OptionsBootstrapper: options_bootstrapper, - PantsEnvironment: PantsEnvironment(os.environ), + CompleteEnvironment: env, } ), cancellation_latch=cancellation_latch, @@ -95,7 +95,7 @@ def _init_graph_session( @classmethod def create( cls, - env: Mapping[str, str], + env: CompleteEnvironment, options_bootstrapper: OptionsBootstrapper, options_initializer: Optional[OptionsInitializer] = None, scheduler: Optional[GraphScheduler] = None, @@ -106,13 +106,13 @@ def create( By the time this method runs, logging will already have been initialized in either PantsRunner or DaemonPantsRunner. - :param env: The environment (e.g. os.environ) for this run. + :param env: The environment for this run. :param options_bootstrapper: The OptionsBootstrapper instance to reuse. :param scheduler: If being called from the daemon, a warmed scheduler to use. """ - options_initializer = options_initializer or OptionsInitializer(options_bootstrapper) + options_initializer = options_initializer or OptionsInitializer(options_bootstrapper, env) build_config, options = options_initializer.build_config_and_options( - options_bootstrapper, raise_=True + options_bootstrapper, env, raise_=True ) run_tracker = RunTracker(options) @@ -124,6 +124,7 @@ def create( options_initializer, options_bootstrapper, build_config, + env, run_tracker.run_id, options, scheduler, @@ -132,7 +133,7 @@ def create( # Option values are usually computed lazily on demand, but command line options are # eagerly computed for validation. - with options_initializer.handle_unknown_flags(options_bootstrapper, raise_=True): + with options_initializer.handle_unknown_flags(options_bootstrapper, env, raise_=True): for scope in options.scope_to_flags.keys(): options.for_scope(scope) diff --git a/src/python/pants/bin/pants_runner.py b/src/python/pants/bin/pants_runner.py index c5c800b357d..8897674a3f5 100644 --- a/src/python/pants/bin/pants_runner.py +++ b/src/python/pants/bin/pants_runner.py @@ -11,6 +11,7 @@ from pants.base.exception_sink import ExceptionSink from pants.base.exiter import ExitCode from pants.bin.remote_pants_runner import RemotePantsRunner +from pants.engine.environment import CompleteEnvironment from pants.init.logging import initialize_stdio, stdio_destination from pants.init.util import init_workdir from pants.option.option_value_container import OptionValueContainer @@ -94,6 +95,6 @@ def run(self, start_time: float) -> ExitCode: log_location=init_workdir(global_bootstrap_options), pantsd_instance=False ) runner = LocalPantsRunner.create( - env=self.env, options_bootstrapper=options_bootstrapper + env=CompleteEnvironment(self.env), options_bootstrapper=options_bootstrapper ) return runner.run(start_time) diff --git a/src/python/pants/core/goals/repl.py b/src/python/pants/core/goals/repl.py index 07fcb0b05a3..6bf4cc312f7 100644 --- a/src/python/pants/core/goals/repl.py +++ b/src/python/pants/core/goals/repl.py @@ -9,6 +9,7 @@ from pants.base.build_root import BuildRoot from pants.engine.addresses import Addresses from pants.engine.console import Console +from pants.engine.environment import CompleteEnvironment from pants.engine.fs import Digest, Workspace from pants.engine.goal import Goal, GoalSubsystem from pants.engine.process import InteractiveProcess, InteractiveRunner @@ -91,6 +92,7 @@ async def run_repl( build_root: BuildRoot, union_membership: UnionMembership, global_options: GlobalOptions, + complete_env: CompleteEnvironment, ) -> Repl: transitive_targets = await Get( TransitiveTargets, TransitiveTargetsRequest(all_specified_addresses) @@ -121,10 +123,9 @@ async def run_repl( workspace.write_digest( request.digest, path_prefix=PurePath(tmpdir).relative_to(build_root.path).as_posix() ) + env = {**complete_env, **request.extra_env} result = interactive_runner.run( - InteractiveProcess( - argv=request.args, env=request.extra_env, run_in_workspace=True, hermetic_env=False - ) + InteractiveProcess(argv=request.args, env=env, run_in_workspace=True) ) return Repl(result.exit_code) diff --git a/src/python/pants/core/goals/repl_integration_test.py b/src/python/pants/core/goals/repl_integration_test.py index ac09c304881..389a32c3b32 100644 --- a/src/python/pants/core/goals/repl_integration_test.py +++ b/src/python/pants/core/goals/repl_integration_test.py @@ -47,6 +47,7 @@ def test_repl_with_targets(rule_runner: RuleRunner) -> None: "--backend-packages=pants.backend.codegen.protobuf.python", ], args=["src/python/lib.py"], + env_inherit={"PATH", "PYENV_ROOT", "HOME"}, ) assert result.exit_code == 0 @@ -61,6 +62,7 @@ def test_repl_ipython(rule_runner: RuleRunner) -> None: "--backend-packages=pants.backend.codegen.protobuf.python", ], args=["--shell=ipython", "src/python/lib.py"], + env_inherit={"PATH", "PYENV_ROOT", "HOME"}, ) assert result.exit_code == 0 @@ -72,6 +74,7 @@ def test_repl_bogus_repl_name(rule_runner: RuleRunner) -> None: Repl, global_args=["--backend-packages=pants.backend.python"], args=["--shell=bogus-repl", "src/python/lib.py"], + env_inherit={"PATH", "PYENV_ROOT", "HOME"}, ) assert result.exit_code == -1 assert "'bogus-repl' is not a registered REPL. Available REPLs" in result.stderr diff --git a/src/python/pants/core/goals/run.py b/src/python/pants/core/goals/run.py index 138c37d7fdf..4d9a823137d 100644 --- a/src/python/pants/core/goals/run.py +++ b/src/python/pants/core/goals/run.py @@ -8,6 +8,7 @@ from pants.base.build_root import BuildRoot from pants.engine.console import Console +from pants.engine.environment import CompleteEnvironment from pants.engine.fs import Digest, Workspace from pants.engine.goal import Goal, GoalSubsystem from pants.engine.process import InteractiveProcess, InteractiveRunner @@ -90,6 +91,7 @@ async def run( interactive_runner: InteractiveRunner, workspace: Workspace, build_root: BuildRoot, + complete_env: CompleteEnvironment, ) -> Run: targets_to_valid_field_sets = await Get( TargetRootsToFieldSets, @@ -109,14 +111,13 @@ async def run( ) args = (arg.format(chroot=tmpdir) for arg in request.args) - extra_env = {k: v.format(chroot=tmpdir) for k, v in request.extra_env.items()} + env = {**complete_env, **{k: v.format(chroot=tmpdir) for k, v in request.extra_env.items()}} try: result = interactive_runner.run( InteractiveProcess( argv=(*args, *run_subsystem.args), - env=extra_env, + env=env, run_in_workspace=True, - hermetic_env=False, ) ) exit_code = result.exit_code diff --git a/src/python/pants/core/goals/run_test.py b/src/python/pants/core/goals/run_test.py index ca218c91fa6..0dbb4dbd3a7 100644 --- a/src/python/pants/core/goals/run_test.py +++ b/src/python/pants/core/goals/run_test.py @@ -59,6 +59,7 @@ class TestBinaryTarget(Target): interactive_runner, workspace, BuildRoot(), + rule_runner.environment, ], mock_gets=[ MockGet( diff --git a/src/python/pants/core/goals/test.py b/src/python/pants/core/goals/test.py index 4131a04f022..c273fe64cbf 100644 --- a/src/python/pants/core/goals/test.py +++ b/src/python/pants/core/goals/test.py @@ -15,12 +15,12 @@ FieldSetsWithSources, FieldSetsWithSourcesRequest, ) -from pants.core.util_rules.pants_environment import PantsEnvironment from pants.engine.addresses import Address from pants.engine.collection import Collection from pants.engine.console import Console from pants.engine.desktop import OpenFiles, OpenFilesRequest from pants.engine.engine_aware import EngineAwareReturnType +from pants.engine.environment import CompleteEnvironment from pants.engine.fs import EMPTY_FILE_DIGEST, Digest, FileDigest, MergeDigests, Snapshot, Workspace from pants.engine.goal import Goal, GoalSubsystem from pants.engine.process import FallibleProcessResult, InteractiveProcess, InteractiveRunner @@ -472,10 +472,10 @@ class TestExtraEnv: @rule def get_filtered_environment( - test_subsystem: TestSubsystem, pants_env: PantsEnvironment + test_subsystem: TestSubsystem, complete_env: CompleteEnvironment ) -> TestExtraEnv: env = ( - pants_env.get_subset(test_subsystem.extra_env_vars) + complete_env.get_subset(test_subsystem.extra_env_vars) if test_subsystem.extra_env_vars else FrozenDict({}) ) diff --git a/src/python/pants/core/register.py b/src/python/pants/core/register.py index 55f702f4105..019164ed8be 100644 --- a/src/python/pants/core/register.py +++ b/src/python/pants/core/register.py @@ -15,7 +15,6 @@ external_tool, filter_empty_sources, pants_bin, - pants_environment, source_files, stripped_source_files, subprocess_environment, @@ -43,7 +42,6 @@ def rules(): *stripped_source_files.rules(), *archive.rules(), *external_tool.rules(), - *pants_environment.rules(), *subprocess_environment.rules(), *source_root.rules(), *target_type_rules(), diff --git a/src/python/pants/core/target_types_test.py b/src/python/pants/core/target_types_test.py index 3fa25309e6a..f40cdb506c2 100644 --- a/src/python/pants/core/target_types_test.py +++ b/src/python/pants/core/target_types_test.py @@ -165,7 +165,9 @@ def test_archive() -> None: ], target_types=[ArchiveTarget, Files, RelocatedFiles, PexBinary], ) - rule_runner.set_options(["--backend-packages=pants.backend.python"]) + rule_runner.set_options( + ["--backend-packages=pants.backend.python"], env_inherit={"PATH", "PYENV_ROOT", "HOME"} + ) rule_runner.create_file("resources/d1.json", "{'k': 1}") rule_runner.create_file("resources/d2.json", "{'k': 2}") diff --git a/src/python/pants/core/util_rules/pants_environment.py b/src/python/pants/core/util_rules/pants_environment.py index 691582e7a9a..8bb6cee1920 100644 --- a/src/python/pants/core/util_rules/pants_environment.py +++ b/src/python/pants/core/util_rules/pants_environment.py @@ -6,8 +6,7 @@ from dataclasses import dataclass from typing import Dict, Mapping, Optional, Sequence -from pants.engine.internals.session import SessionValues -from pants.engine.rules import collect_rules, rule +from pants.base.deprecated import deprecated from pants.util.frozendict import FrozenDict from pants.util.meta import frozen_after_init @@ -25,7 +24,11 @@ class PantsEnvironment: env: FrozenDict[str, str] - def __init__(self, env: Optional[Mapping[str, str]] = None): + @deprecated( + "2.5.0.dev0", + hint_message="Request a subset Environment (using EnvironmentRequest) or the CompleteEnvironment.", + ) + def __init__(self, env: Optional[Mapping[str, str]] = None) -> None: """Initialize a `PantsEnvironment` with the current contents of the environment. Explicitly specify the env argument to create a mock environment for testing. @@ -73,12 +76,3 @@ def check_and_set(name: str, value: Optional[str]): ) return FrozenDict(env_var_subset) - - -@rule -async def pants_environment(session_values: SessionValues) -> PantsEnvironment: - return session_values[PantsEnvironment] - - -def rules(): - return collect_rules() diff --git a/src/python/pants/core/util_rules/subprocess_environment.py b/src/python/pants/core/util_rules/subprocess_environment.py index 545987fbdb2..42be1be531d 100644 --- a/src/python/pants/core/util_rules/subprocess_environment.py +++ b/src/python/pants/core/util_rules/subprocess_environment.py @@ -4,8 +4,8 @@ from dataclasses import dataclass from typing import Tuple -from pants.core.util_rules.pants_environment import PantsEnvironment -from pants.engine.rules import collect_rules, rule +from pants.engine.environment import Environment, EnvironmentRequest +from pants.engine.rules import Get, collect_rules, rule from pants.option.subsystem import Subsystem from pants.util.frozendict import FrozenDict @@ -67,12 +67,15 @@ class SubprocessEnvironmentVars: @rule -def get_subprocess_environment( - subproc_env: SubprocessEnvironment, pants_env: PantsEnvironment +async def get_subprocess_environment( + subproc_env: SubprocessEnvironment, ) -> SubprocessEnvironmentVars: return SubprocessEnvironmentVars( - pants_env.get_subset( - subproc_env.env_vars_to_pass_to_subprocesses, allowed=SETTABLE_ENV_VARS + await Get( + Environment, + EnvironmentRequest( + subproc_env.env_vars_to_pass_to_subprocesses, allowed=SETTABLE_ENV_VARS + ), ) ) diff --git a/src/python/pants/engine/desktop.py b/src/python/pants/engine/desktop.py index 1bf2c0c945c..89ea9526878 100644 --- a/src/python/pants/engine/desktop.py +++ b/src/python/pants/engine/desktop.py @@ -6,6 +6,7 @@ from pathlib import PurePath from typing import Iterable, Tuple +from pants.engine.environment import CompleteEnvironment from pants.engine.platform import Platform from pants.engine.process import BinaryPathRequest, BinaryPaths, InteractiveProcess from pants.engine.rules import Get, collect_rules, rule @@ -31,7 +32,11 @@ class OpenFiles: @rule -async def find_open_program(request: OpenFilesRequest, plat: Platform) -> OpenFiles: +async def find_open_program( + request: OpenFilesRequest, + plat: Platform, + complete_env: CompleteEnvironment, +) -> OpenFiles: open_program_name = "open" if plat == Platform.darwin else "xdg-open" open_program_paths = await Get( BinaryPaths, @@ -63,7 +68,7 @@ async def find_open_program(request: OpenFilesRequest, plat: Platform) -> OpenFi # to the various XDG_* environment variables, DISPLAY and other X11 variables are # required. Instead of attempting to track all of these we just export the full user # environment since this is not a cached process. - hermetic_env=False, + env=complete_env, ) for f in request.files ] diff --git a/src/python/pants/engine/environment.py b/src/python/pants/engine/environment.py new file mode 100644 index 00000000000..085e1ed2177 --- /dev/null +++ b/src/python/pants/engine/environment.py @@ -0,0 +1,116 @@ +# Copyright 2020 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). + +import logging +import re +from dataclasses import dataclass +from typing import Dict, Optional, Sequence, cast + +from pants.core.util_rules.pants_environment import PantsEnvironment +from pants.engine.internals.session import SessionValues +from pants.engine.rules import collect_rules, rule +from pants.util.frozendict import FrozenDict +from pants.util.meta import frozen_after_init +from pants.util.ordered_set import FrozenOrderedSet + +logger = logging.getLogger(__name__) + +name_value_re = re.compile(r"([A-Za-z_]\w*)=(.*)") +shorthand_re = re.compile(r"([A-Za-z_]\w*)") + + +class CompleteEnvironment(FrozenDict): + """CompleteEnvironment contains all environment variables from the current Pants process. + + Accesses to `os.environ` cannot be accurately tracked, so @rules that need access to the + environment should request this type instead. + """ + + def get_subset( + self, requested: Sequence[str], *, allowed: Optional[Sequence[str]] = None + ) -> FrozenDict[str, str]: + """Extract a subset of named env vars. + + Given a list of extra environment variable specifiers as strings, filter the contents of + the pants environment to only those variables. + + Each variable can be specified either as a name or as a name=value pair. + In the former case, the value for that name is taken from this env. In the latter + case the specified value overrides the value in this env. + + If `allowed` is specified, the requested variable names must be in that list, or an error + will be raised. + """ + allowed_set = None if allowed is None else set(allowed) + env_var_subset: Dict[str, str] = {} + + def check_and_set(name: str, value: Optional[str]): + if allowed_set is not None and name not in allowed_set: + raise ValueError( + f"{name} is not in the list of variable names that are allowed to be set. " + f"Must be one of {','.join(sorted(allowed_set))}." + ) + if value is not None: + env_var_subset[name] = value + + for env_var in requested: + name_value_match = name_value_re.match(env_var) + if name_value_match: + check_and_set(name_value_match[1], name_value_match[2]) + elif shorthand_re.match(env_var): + check_and_set(env_var, self.get(env_var)) + else: + raise ValueError( + f"An invalid variable was requested via the --test-extra-env-var " + f"mechanism: {env_var}" + ) + + return FrozenDict(env_var_subset) + + +@frozen_after_init +@dataclass(unsafe_hash=True) +class EnvironmentRequest: + """Requests a subset of the variables set in the environment. + + Requesting only the relevant subset of the environment reduces invalidation caused by unrelated + changes. + """ + + requested: FrozenOrderedSet[str] + allowed: Optional[FrozenOrderedSet[str]] + + def __init__(self, requested: Sequence[str], allowed: Optional[Sequence[str]] = None): + self.requested = FrozenOrderedSet(requested) + self.allowed = None if allowed is None else FrozenOrderedSet(allowed) + + +class Environment(FrozenDict[str, str]): + """A subset of the variables set in the environment.""" + + +@rule +def pants_environment(session_values: SessionValues) -> PantsEnvironment: + # TODO: The @deprecated decorator seems to obscure type information. + return cast(PantsEnvironment, PantsEnvironment(session_values[CompleteEnvironment])) + + +@rule +def complete_environment(session_values: SessionValues) -> CompleteEnvironment: + return session_values[CompleteEnvironment] + + +@rule +def environment_subset(session_values: SessionValues, request: EnvironmentRequest) -> Environment: + return Environment( + session_values[CompleteEnvironment] + .get_subset( + requested=tuple(request.requested), + allowed=(None if request.allowed is None else tuple(request.allowed)), + ) + .items() + ) + + +def rules(): + return collect_rules() diff --git a/src/python/pants/core/util_rules/pants_environment_test.py b/src/python/pants/engine/environment_test.py similarity index 79% rename from src/python/pants/core/util_rules/pants_environment_test.py rename to src/python/pants/engine/environment_test.py index e22c370fc35..1451a0966ef 100644 --- a/src/python/pants/core/util_rules/pants_environment_test.py +++ b/src/python/pants/engine/environment_test.py @@ -5,7 +5,7 @@ import pytest -from pants.core.util_rules.pants_environment import PantsEnvironment +from pants.engine.environment import CompleteEnvironment @pytest.mark.parametrize( @@ -21,15 +21,15 @@ (['A=has a " in it'], {"A": 'has a " in it'}), ], ) -def test_pants_environment(input_strs: List[str], expected: Dict[str, str]) -> None: - pants_env = PantsEnvironment({"A": "a", "B": "b", "C": "c"}) +def test_complete_environment(input_strs: List[str], expected: Dict[str, str]) -> None: + pants_env = CompleteEnvironment({"A": "a", "B": "b", "C": "c"}) subset = pants_env.get_subset(input_strs) assert dict(subset) == expected def test_invalid_variable() -> None: - pants_env = PantsEnvironment() + pants_env = CompleteEnvironment() with pytest.raises(ValueError) as exc: pants_env.get_subset(["3INVALID=doesn't matter"]) diff --git a/src/python/pants/engine/process.py b/src/python/pants/engine/process.py index 001e6cd7f4c..8b4bb6e760c 100644 --- a/src/python/pants/engine/process.py +++ b/src/python/pants/engine/process.py @@ -292,7 +292,6 @@ class InteractiveProcess: env: FrozenDict[str, str] input_digest: Digest run_in_workspace: bool - hermetic_env: bool forward_signals_to_process: bool def __init__( @@ -302,7 +301,7 @@ def __init__( env: Mapping[str, str] | None = None, input_digest: Digest = EMPTY_DIGEST, run_in_workspace: bool = False, - hermetic_env: bool = True, + hermetic_env: bool | None = None, forward_signals_to_process: bool = True, ) -> None: """Request to run a subprocess in the foreground, similar to subprocess.run(). @@ -320,8 +319,16 @@ def __init__( self.env = FrozenDict(env or {}) self.input_digest = input_digest self.run_in_workspace = run_in_workspace - self.hermetic_env = hermetic_env self.forward_signals_to_process = forward_signals_to_process + + deprecated_conditional( + predicate=lambda: hermetic_env is not None, + removal_version="2.5.0.dev0", + entity_description="The hermetic_env flag", + hint_message=( + "@rules should request and pass either a CompleteEnvironment or Environment as the `env`." + ), + ) self.__post_init__() def __post_init__(self): @@ -333,7 +340,11 @@ def __post_init__(self): @classmethod def from_process( - cls, process: Process, *, hermetic_env: bool = True, forward_signals_to_process: bool = True + cls, + process: Process, + *, + hermetic_env: bool | None = None, + forward_signals_to_process: bool = True, ) -> InteractiveProcess: return InteractiveProcess( argv=process.argv, diff --git a/src/python/pants/init/engine_initializer.py b/src/python/pants/init/engine_initializer.py index c510c24a632..d037d3443dc 100644 --- a/src/python/pants/init/engine_initializer.py +++ b/src/python/pants/init/engine_initializer.py @@ -13,8 +13,9 @@ from pants.base.exiter import PANTS_SUCCEEDED_EXIT_CODE from pants.base.specs import Specs from pants.build_graph.build_configuration import BuildConfiguration -from pants.engine import desktop, fs, platform, process +from pants.engine import desktop, environment, fs, platform, process from pants.engine.console import Console +from pants.engine.environment import CompleteEnvironment from pants.engine.fs import PathGlobs, Snapshot, Workspace from pants.engine.goal import Goal from pants.engine.internals import build_files, graph, options_parsing @@ -165,8 +166,9 @@ def _make_goal_map_from_rules(rules): def setup_graph( options_bootstrapper: OptionsBootstrapper, build_configuration: BuildConfiguration, + env: CompleteEnvironment, executor: Optional[PyExecutor] = None, - execution_options: Optional[ExecutionOptions] = None, + local_only: bool = False, ) -> GraphScheduler: native = Native() build_root = get_buildroot() @@ -176,7 +178,7 @@ def setup_graph( executor = executor or PyExecutor( *GlobalOptions.compute_executor_arguments(bootstrap_options) ) - execution_options = execution_options or ExecutionOptions.from_options(options) + execution_options = ExecutionOptions.from_options(options, env, local_only=local_only) return EngineInitializer.setup_graph_extended( build_configuration, execution_options, @@ -247,6 +249,7 @@ def build_root_singleton() -> BuildRoot: *collect_rules(locals()), *build_files.rules(), *fs.rules(), + *environment.rules(), *desktop.rules(), *graph.rules(), *options_parsing.rules(), diff --git a/src/python/pants/init/options_initializer.py b/src/python/pants/init/options_initializer.py index 82e177c6601..0a414c72ed1 100644 --- a/src/python/pants/init/options_initializer.py +++ b/src/python/pants/init/options_initializer.py @@ -10,6 +10,7 @@ import pkg_resources from pants.build_graph.build_configuration import BuildConfiguration +from pants.engine.environment import CompleteEnvironment from pants.engine.internals.native_engine import PyExecutor from pants.help.flag_error_help_printer import FlagErrorHelpPrinter from pants.init.bootstrap_scheduler import BootstrapScheduler @@ -21,7 +22,6 @@ from pants.init.plugin_resolver import PluginResolver from pants.init.plugin_resolver import rules as plugin_resolver_rules from pants.option.errors import UnknownFlagsError -from pants.option.global_options import DEFAULT_EXECUTION_OPTIONS from pants.option.options import Options from pants.option.options_bootstrapper import OptionsBootstrapper @@ -31,6 +31,7 @@ def _initialize_build_configuration( plugin_resolver: PluginResolver, options_bootstrapper: OptionsBootstrapper, + env: CompleteEnvironment, ) -> BuildConfiguration: """Initialize a BuildConfiguration for the given OptionsBootstrapper. @@ -40,7 +41,7 @@ def _initialize_build_configuration( """ bootstrap_options = options_bootstrapper.get_bootstrap_options().for_global_scope() - working_set = plugin_resolver.resolve(options_bootstrapper) + working_set = plugin_resolver.resolve(options_bootstrapper, env) # Add any extra paths to python path (e.g., for loading extra source backends). for path in bootstrap_options.pythonpath: @@ -58,6 +59,7 @@ def _initialize_build_configuration( def create_bootstrap_scheduler( options_bootstrapper: OptionsBootstrapper, + env: CompleteEnvironment, executor: Optional[PyExecutor] = None, ) -> BootstrapScheduler: bc_builder = BuildConfiguration.Builder() @@ -72,9 +74,10 @@ def create_bootstrap_scheduler( options_bootstrapper, bc_builder.create(), executor=executor, - # TODO: We use the default execution options to avoid invoking remote execution auth - # plugins. They should be loaded via rules using the bootstrap Scheduler in the future. - execution_options=DEFAULT_EXECUTION_OPTIONS, + env=env, + # TODO: We set local_only to avoid invoking remote execution auth plugins. They should + # be loaded via rules using the bootstrap Scheduler in the future. + local_only=True, ).scheduler ) @@ -95,31 +98,34 @@ class OptionsInitializer: def __init__( self, options_bootstrapper: OptionsBootstrapper, + env: CompleteEnvironment, executor: Optional[PyExecutor] = None, ) -> None: self._bootstrap_scheduler = create_bootstrap_scheduler( - options_bootstrapper, executor=executor + options_bootstrapper, env, executor=executor ) self._plugin_resolver = PluginResolver(self._bootstrap_scheduler) def build_config_and_options( - self, options_bootstrapper: OptionsBootstrapper, *, raise_: bool + self, options_bootstrapper: OptionsBootstrapper, env: CompleteEnvironment, *, raise_: bool ) -> Tuple[BuildConfiguration, Options]: - build_config = _initialize_build_configuration(self._plugin_resolver, options_bootstrapper) - with self.handle_unknown_flags(options_bootstrapper, raise_=raise_): + build_config = _initialize_build_configuration( + self._plugin_resolver, options_bootstrapper, env + ) + with self.handle_unknown_flags(options_bootstrapper, env, raise_=raise_): options = options_bootstrapper.full_options(build_config) return build_config, options @contextmanager def handle_unknown_flags( - self, options_bootstrapper: OptionsBootstrapper, *, raise_: bool + self, options_bootstrapper: OptionsBootstrapper, env: CompleteEnvironment, *, raise_: bool ) -> Iterator[None]: """If there are any unknown flags, print "Did you mean?" and possibly error.""" try: yield except UnknownFlagsError as err: build_config = _initialize_build_configuration( - self._plugin_resolver, options_bootstrapper + self._plugin_resolver, options_bootstrapper, env ) # We need an options instance in order to get "did you mean" suggestions, but we know # there are bad flags in the args, so we generate options with no flags. diff --git a/src/python/pants/init/plugin_resolver.py b/src/python/pants/init/plugin_resolver.py index f4d77329f9f..782f94f02a8 100644 --- a/src/python/pants/init/plugin_resolver.py +++ b/src/python/pants/init/plugin_resolver.py @@ -16,8 +16,8 @@ VenvPex, VenvPexProcess, ) -from pants.core.util_rules.pants_environment import PantsEnvironment from pants.engine.collection import DeduplicatedCollection +from pants.engine.environment import CompleteEnvironment from pants.engine.internals.session import SessionValues from pants.engine.process import ProcessCacheScope, ProcessResult from pants.engine.rules import Get, QueryRule, collect_rules, rule @@ -100,16 +100,18 @@ def __init__( ) def resolve( - self, options_bootstrapper: OptionsBootstrapper, working_set: Optional[WorkingSet] = None + self, + options_bootstrapper: OptionsBootstrapper, + env: CompleteEnvironment, + working_set: Optional[WorkingSet] = None, ) -> WorkingSet: """Resolves any configured plugins and adds them to the global working set. :param working_set: The working set to add the resolved plugins to instead of the global working set (for testing). - :type: :class:`pkg_resources.WorkingSet` """ working_set = working_set or global_working_set - for resolved_plugin_location in self._resolve_plugins(options_bootstrapper): + for resolved_plugin_location in self._resolve_plugins(options_bootstrapper, env): site.addsitedir( resolved_plugin_location ) # Activate any .pth files plugin wheels may have. @@ -117,14 +119,16 @@ def resolve( return working_set def _resolve_plugins( - self, options_bootstrapper: OptionsBootstrapper + self, + options_bootstrapper: OptionsBootstrapper, + env: CompleteEnvironment, ) -> ResolvedPluginDistributions: session = self._scheduler.scheduler.new_session( "plugin_resolver", session_values=SessionValues( { OptionsBootstrapper: options_bootstrapper, - PantsEnvironment: PantsEnvironment(dict(options_bootstrapper.env_tuples)), + CompleteEnvironment: env, } ), ) diff --git a/src/python/pants/option/global_options.py b/src/python/pants/option/global_options.py index aac16ef2e03..ba3b350acf9 100644 --- a/src/python/pants/option/global_options.py +++ b/src/python/pants/option/global_options.py @@ -21,6 +21,7 @@ get_pants_cachedir, pants_version, ) +from pants.engine.environment import CompleteEnvironment from pants.option.custom_types import dir_option from pants.option.errors import OptionsError from pants.option.option_value_container import OptionValueContainer @@ -131,7 +132,9 @@ class ExecutionOptions: remote_execution_overall_deadline_secs: int @classmethod - def from_options(cls, options: Options) -> ExecutionOptions: + def from_options( + cls, options: Options, env: CompleteEnvironment, local_only: bool = False + ) -> ExecutionOptions: bootstrap_options = options.bootstrap_option_values() assert bootstrap_options is not None # Possibly change some remoting options. @@ -153,8 +156,10 @@ def from_options(cls, options: Options) -> ExecutionOptions: token_header = {"authorization": f"Bearer {oauth_token}"} remote_execution_headers.update(token_header) remote_store_headers.update(token_header) - if bootstrap_options.remote_auth_plugin and ( - remote_execution or remote_cache_read or remote_cache_write + if ( + not local_only + and bootstrap_options.remote_auth_plugin + and (remote_execution or remote_cache_read or remote_cache_write) ): if ":" not in bootstrap_options.remote_auth_plugin: raise OptionsError( @@ -171,6 +176,7 @@ def from_options(cls, options: Options) -> ExecutionOptions: initial_execution_headers=remote_execution_headers, initial_store_headers=remote_store_headers, options=options, + env=dict(env), ), ) if not auth_plugin_result.is_available: diff --git a/src/python/pants/option/global_options_test.py b/src/python/pants/option/global_options_test.py index d3781397301..73c1ab439da 100644 --- a/src/python/pants/option/global_options_test.py +++ b/src/python/pants/option/global_options_test.py @@ -10,6 +10,7 @@ import pytest from pants.base.build_environment import get_buildroot +from pants.engine.environment import CompleteEnvironment from pants.init.options_initializer import OptionsInitializer from pants.option.errors import OptionsError from pants.option.global_options import ExecutionOptions, GlobalOptions @@ -39,8 +40,11 @@ def create_execution_options( if plugin: args.append(f"--remote-auth-plugin={plugin}") ob = create_options_bootstrapper(args) - _build_config, options = OptionsInitializer(ob).build_config_and_options(ob, raise_=False) - return ExecutionOptions.from_options(options) + env = CompleteEnvironment({}) + _build_config, options = OptionsInitializer(ob, env).build_config_and_options( + ob, env, raise_=False + ) + return ExecutionOptions.from_options(options, env) def test_execution_options_remote_oauth_bearer_token_path() -> None: @@ -103,7 +107,7 @@ def compute_exec_options(state: str) -> ExecutionOptions: f"""\ from pants.option.global_options import AuthPluginState, AuthPluginResult - def auth_func(initial_execution_headers, initial_store_headers, options): + def auth_func(initial_execution_headers, initial_store_headers, options, **kwargs): return AuthPluginResult( state=AuthPluginState.{state}, execution_headers={{ diff --git a/src/python/pants/option/options_bootstrapper.py b/src/python/pants/option/options_bootstrapper.py index 530dd583742..d43d0f4836c 100644 --- a/src/python/pants/option/options_bootstrapper.py +++ b/src/python/pants/option/options_bootstrapper.py @@ -106,8 +106,8 @@ def create( ) -> OptionsBootstrapper: """Parses the minimum amount of configuration necessary to create an OptionsBootstrapper. - :param env: An environment dictionary, or None to use `os.environ`. - :param args: An args array, or None to use `sys.argv`. + :param env: An environment dictionary. + :param args: An args array. :param allow_pantsrc: True to allow pantsrc files to be used. Unless tests are expecting to consume pantsrc files, they should pass False in order to avoid reading files from absolute paths. Production usecases should pass True to allow options values to make the diff --git a/src/python/pants/pantsd/pants_daemon.py b/src/python/pants/pantsd/pants_daemon.py index e877c1cd9e9..f76accfaecc 100644 --- a/src/python/pants/pantsd/pants_daemon.py +++ b/src/python/pants/pantsd/pants_daemon.py @@ -15,6 +15,7 @@ from pants.base.build_environment import get_buildroot from pants.base.exception_sink import ExceptionSink from pants.bin.daemon_pants_runner import DaemonPantsRunner +from pants.engine.environment import CompleteEnvironment from pants.engine.internals.native import Native from pants.engine.internals.native_engine import PyExecutor from pants.init.engine_initializer import GraphScheduler @@ -29,6 +30,7 @@ from pants.pantsd.service.pants_service import PantsServices from pants.pantsd.service.scheduler_service import SchedulerService from pants.pantsd.service.store_gc_service import StoreGCService +from pants.util.contextutil import argv_as, hermetic_environment_as from pants.util.logging import LogLevel from pants.util.strutil import ensure_text @@ -45,7 +47,9 @@ class RuntimeFailure(Exception): """Represents a pantsd failure at runtime, usually from an underlying service failure.""" @classmethod - def create(cls, options_bootstrapper: OptionsBootstrapper) -> PantsDaemon: + def create( + cls, options_bootstrapper: OptionsBootstrapper, env: CompleteEnvironment + ) -> PantsDaemon: # Any warnings that would be triggered here are re-triggered later per-run of Pants, so we # silence them. with warnings.catch_warnings(record=True): @@ -55,7 +59,7 @@ def create(cls, options_bootstrapper: OptionsBootstrapper) -> PantsDaemon: native = Native() executor = PyExecutor(*GlobalOptions.compute_executor_arguments(bootstrap_options_values)) - core = PantsDaemonCore(options_bootstrapper, executor, cls._setup_services) + core = PantsDaemonCore(options_bootstrapper, env, executor, cls._setup_services) server = native.new_nailgun_server( executor, @@ -163,19 +167,21 @@ def run_sync(self): os.environ.pop("PYTHONPATH") global_bootstrap_options = self._bootstrap_options.for_global_scope() + # Set the process name in ps output to 'pantsd' vs './pants compile src/etc:: -ldebug'. + set_process_title(f"pantsd [{self._build_root}]") - # Switch log output to the daemon's log stream from here forward. + # Switch log output to the daemon's log stream, and empty `env` and `argv` to encourage all + # further usage of those variables to happen via engine APIs and options. self._close_stdio() - with initialize_stdio(global_bootstrap_options): + with initialize_stdio(global_bootstrap_options), argv_as( + tuple() + ), hermetic_environment_as(): # Install signal and panic handling. ExceptionSink.install( log_location=init_workdir(global_bootstrap_options), pantsd_instance=True ) self._native.set_panic_handler() - # Set the process name in ps output to 'pantsd' vs './pants compile src/etc:: -ldebug'. - set_process_title(f"pantsd [{self._build_root}]") - self._initialize_metadata() # Check periodically whether the core is valid, and exit if it is not. @@ -194,5 +200,6 @@ def launch_new_pantsd_instance(): options_bootstrapper = OptionsBootstrapper.create( env=os.environ, args=sys.argv, allow_pantsrc=True ) - daemon = PantsDaemon.create(options_bootstrapper) + env = CompleteEnvironment(os.environ) + daemon = PantsDaemon.create(options_bootstrapper, env) daemon.run_sync() diff --git a/src/python/pants/pantsd/pants_daemon_core.py b/src/python/pants/pantsd/pants_daemon_core.py index c5c39271488..bc59f99c9fe 100644 --- a/src/python/pants/pantsd/pants_daemon_core.py +++ b/src/python/pants/pantsd/pants_daemon_core.py @@ -7,6 +7,7 @@ from typing_extensions import Protocol +from pants.engine.environment import CompleteEnvironment from pants.engine.internals.native_engine import PyExecutor from pants.init.engine_initializer import EngineInitializer, GraphScheduler from pants.init.options_initializer import OptionsInitializer @@ -39,10 +40,11 @@ class PantsDaemonCore: def __init__( self, options_bootstrapper: OptionsBootstrapper, + env: CompleteEnvironment, executor: PyExecutor, services_constructor: PantsServicesConstructor, ): - self._options_initializer = OptionsInitializer(options_bootstrapper, executor=executor) + self._options_initializer = OptionsInitializer(options_bootstrapper, env, executor=executor) self._executor = executor self._services_constructor = services_constructor self._lifecycle_lock = threading.RLock() @@ -68,7 +70,10 @@ def is_valid(self) -> bool: return self._services.are_all_alive() def _initialize( - self, options_fingerprint: str, options_bootstrapper: OptionsBootstrapper + self, + options_fingerprint: str, + options_bootstrapper: OptionsBootstrapper, + env: CompleteEnvironment, ) -> None: """(Re-)Initialize the scheduler. @@ -82,10 +87,10 @@ def _initialize( if self._services: self._services.shutdown() build_config, options = self._options_initializer.build_config_and_options( - options_bootstrapper, raise_=True + options_bootstrapper, env, raise_=True ) self._scheduler = EngineInitializer.setup_graph( - options_bootstrapper, build_config, executor=self._executor + options_bootstrapper, build_config, env, executor=self._executor ) bootstrap_options_values = options.bootstrap_option_values() assert bootstrap_options_values is not None @@ -99,7 +104,7 @@ def _initialize( raise e def prepare( - self, options_bootstrapper: OptionsBootstrapper + self, options_bootstrapper: OptionsBootstrapper, env: CompleteEnvironment ) -> Tuple[GraphScheduler, OptionsInitializer]: """Get a scheduler for the given options_bootstrapper. @@ -121,6 +126,6 @@ def prepare( # The fingerprint mismatches, either because this is the first run (and there is no # fingerprint) or because relevant options have changed. Create a new scheduler # and services. - self._initialize(options_fingerprint, options_bootstrapper) + self._initialize(options_fingerprint, options_bootstrapper, env) assert self._scheduler is not None return self._scheduler, self._options_initializer diff --git a/src/python/pants/python/python_setup.py b/src/python/pants/python/python_setup.py index 316327a366b..16636fb544f 100644 --- a/src/python/pants/python/python_setup.py +++ b/src/python/pants/python/python_setup.py @@ -1,20 +1,22 @@ # Copyright 2014 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). +from __future__ import annotations + import logging import multiprocessing import os -import subprocess from enum import Enum from pathlib import Path -from typing import Callable, Iterable, List, Optional, Tuple, cast +from typing import Iterable, List, Optional, Tuple, cast from pex.variables import Variables from pants.base.build_environment import get_buildroot +from pants.engine.environment import Environment from pants.option.custom_types import file_option from pants.option.subsystem import Subsystem -from pants.util.memo import memoized_property +from pants.util.memo import memoized_method logger = logging.getLogger(__name__) @@ -175,9 +177,9 @@ def resolve_all_constraints(self) -> ResolveAllConstraintsOption: def resolve_all_constraints_was_set_explicitly(self) -> bool: return not self.options.is_default("resolve_all_constraints") - @memoized_property - def interpreter_search_paths(self): - return self.expand_interpreter_search_paths(self.options.interpreter_search_paths) + @memoized_method + def interpreter_search_paths(self, env: Environment): + return self.expand_interpreter_search_paths(self.options.interpreter_search_paths, env) @property def resolver_version(self) -> ResolverVersion: @@ -219,14 +221,12 @@ def compatibilities_or_constraints( ) @classmethod - def expand_interpreter_search_paths(cls, interpreter_search_paths, pyenv_root_func=None): + def expand_interpreter_search_paths(cls, interpreter_search_paths, env: Environment): special_strings = { "": cls.get_pex_python_paths, - "": cls.get_environment_paths, - "": lambda: cls.get_pyenv_paths(pyenv_root_func=pyenv_root_func), - "": lambda: cls.get_pyenv_paths( - pyenv_root_func=pyenv_root_func, pyenv_local=True - ), + "": lambda: cls.get_environment_paths(env), + "": lambda: cls.get_pyenv_paths(env), + "": lambda: cls.get_pyenv_paths(env, pyenv_local=True), } expanded = [] from_pexrc = None @@ -248,9 +248,9 @@ def expand_interpreter_search_paths(cls, interpreter_search_paths, pyenv_root_fu return expanded @staticmethod - def get_environment_paths(): + def get_environment_paths(env: Environment): """Returns a list of paths specified by the PATH env var.""" - pathstr = os.getenv("PATH") + pathstr = env.get("PATH") if pathstr: return pathstr.split(os.pathsep) return [] @@ -270,19 +270,15 @@ def get_pex_python_paths(): return [] @staticmethod - def get_pyenv_paths( - *, pyenv_root_func: Optional[Callable] = None, pyenv_local: bool = False - ) -> List[str]: + def get_pyenv_paths(env: Environment, *, pyenv_local: bool = False) -> List[str]: """Returns a list of paths to Python interpreters managed by pyenv. - :param pyenv_root_func: A no-arg function that returns the pyenv root. Defaults to - running `pyenv root`, but can be overridden for testing. + :param env: The environment to use to look up pyenv. :param bool pyenv_local: If True, only use the interpreter specified by '.python-version' file under `build_root`. """ - pyenv_root_func = pyenv_root_func or get_pyenv_root - pyenv_root = pyenv_root_func() - if pyenv_root is None: + pyenv_root = get_pyenv_root(env) + if not pyenv_root: return [] versions_dir = Path(pyenv_root, "versions") @@ -312,8 +308,12 @@ def get_pyenv_paths( return paths -def get_pyenv_root(): - try: - return subprocess.check_output(["pyenv", "root"]).decode().strip() - except (OSError, subprocess.CalledProcessError): - return None +def get_pyenv_root(env: Environment) -> str | None: + """See https://github.com/pyenv/pyenv#environment-variables.""" + from_env = env.get("PYENV_ROOT") + if from_env: + return from_env + home_from_env = env.get("HOME") + if home_from_env: + return os.path.join(home_from_env, ".cache") + return None diff --git a/src/python/pants/python/python_setup_test.py b/src/python/pants/python/python_setup_test.py index 222d93cd8ee..8faf0c4a21f 100644 --- a/src/python/pants/python/python_setup_test.py +++ b/src/python/pants/python/python_setup_test.py @@ -7,7 +7,8 @@ import pytest from pants.base.build_environment import get_pants_cachedir -from pants.python.python_setup import PythonSetup +from pants.engine.environment import Environment +from pants.python.python_setup import PythonSetup, get_pyenv_root from pants.testutil.rule_runner import RuleRunner from pants.util.contextutil import environment_as, temporary_dir from pants.util.dirutil import safe_mkdir_for @@ -49,8 +50,7 @@ def fake_pyenv_root(fake_versions, fake_local_version): def test_get_environment_paths() -> None: - with environment_as(PATH="foo/bar:baz:/qux/quux"): - paths = PythonSetup.get_environment_paths() + paths = PythonSetup.get_environment_paths(Environment({"PATH": "foo/bar:baz:/qux/quux"})) assert ["foo/bar", "baz", "/qux/quux"] == paths @@ -65,6 +65,16 @@ def rule_runner() -> RuleRunner: return RuleRunner() +def test_get_pyenv_root() -> None: + home = "/♡" + default_root = f"{home}/.cache" + explicit_root = f"{home}/explicit" + + assert explicit_root == get_pyenv_root(Environment({"PYENV_ROOT": explicit_root})) + assert default_root == get_pyenv_root(Environment({"HOME": home})) + assert get_pyenv_root(Environment({})) is None + + def test_get_pyenv_paths(rule_runner: RuleRunner) -> None: local_pyenv_version = "3.5.5" all_pyenv_versions = ["2.7.14", local_pyenv_version] @@ -74,9 +84,9 @@ def test_get_pyenv_paths(rule_runner: RuleRunner) -> None: expected_paths, expected_local_paths, ): - paths = PythonSetup.get_pyenv_paths(pyenv_root_func=lambda: pyenv_root) + paths = PythonSetup.get_pyenv_paths(Environment({"PYENV_ROOT": pyenv_root})) local_paths = PythonSetup.get_pyenv_paths( - pyenv_root_func=lambda: pyenv_root, pyenv_local=True + Environment({"PYENV_ROOT": pyenv_root}), pyenv_local=True ) assert expected_paths == paths assert expected_local_paths == local_paths @@ -86,26 +96,27 @@ def test_expand_interpreter_search_paths(rule_runner: RuleRunner) -> None: local_pyenv_version = "3.5.5" all_pyenv_versions = ["2.7.14", local_pyenv_version] rule_runner.create_file(".python-version", local_pyenv_version + "\n") - with environment_as(PATH="/env/path1:/env/path2"): - with setup_pexrc_with_pex_python_path(["/pexrc/path1:/pexrc/path2"]): - with fake_pyenv_root(all_pyenv_versions, local_pyenv_version) as ( - pyenv_root, - expected_pyenv_paths, - expected_pyenv_local_paths, - ): - paths = [ - "/foo", - "", - "/bar", - "", - "/baz", - "", - "", - "/qux", - ] - expanded_paths = PythonSetup.expand_interpreter_search_paths( - paths, pyenv_root_func=lambda: pyenv_root - ) + with setup_pexrc_with_pex_python_path(["/pexrc/path1:/pexrc/path2"]): + with fake_pyenv_root(all_pyenv_versions, local_pyenv_version) as ( + pyenv_root, + expected_pyenv_paths, + expected_pyenv_local_paths, + ): + paths = [ + "/foo", + "", + "/bar", + "", + "/baz", + "", + "", + "/qux", + ] + env = Environment({"PATH": "/env/path1:/env/path2", "PYENV_ROOT": pyenv_root}) + expanded_paths = PythonSetup.expand_interpreter_search_paths( + paths, + env, + ) expected = [ "/foo", diff --git a/src/python/pants/testutil/rule_runner.py b/src/python/pants/testutil/rule_runner.py index ccbd43829a5..2a1f87a96d3 100644 --- a/src/python/pants/testutil/rule_runner.py +++ b/src/python/pants/testutil/rule_runner.py @@ -20,10 +20,9 @@ from pants.base.specs_parser import SpecsParser from pants.build_graph.build_configuration import BuildConfiguration from pants.build_graph.build_file_aliases import BuildFileAliases -from pants.core.util_rules import pants_environment -from pants.core.util_rules.pants_environment import PantsEnvironment from pants.engine.addresses import Address from pants.engine.console import Console +from pants.engine.environment import CompleteEnvironment from pants.engine.fs import PathGlobs, PathGlobsAndRoot, Snapshot, Workspace from pants.engine.goal import Goal from pants.engine.internals.native import Native @@ -103,7 +102,6 @@ def __init__( all_rules = ( *(rules or ()), *source_root.rules(), - *pants_environment.rules(), QueryRule(WrappedTarget, [Address]), QueryRule(UnionMembership, []), ) @@ -117,6 +115,7 @@ def __init__( build_config_builder.register_target_types(target_types or ()) self.build_config = build_config_builder.create() + self.environment = CompleteEnvironment({}) self.options_bootstrapper = create_options_bootstrapper() options = self.options_bootstrapper.full_options(self.build_config) global_options = self.options_bootstrapper.bootstrap_options.for_global_scope() @@ -140,7 +139,7 @@ def __init__( build_root=self.build_root, build_configuration=self.build_config, executor=_EXECUTOR, - execution_options=ExecutionOptions.from_options(options), + execution_options=ExecutionOptions.from_options(options, self.environment), ca_certs_path=ca_certs_path, native_engine_visualize_to=None, ).new_session( @@ -148,7 +147,7 @@ def __init__( session_values=SessionValues( { OptionsBootstrapper: self.options_bootstrapper, - PantsEnvironment: PantsEnvironment(), + CompleteEnvironment: self.environment, } ), ) @@ -191,9 +190,10 @@ def run_goal_rule( global_args: Iterable[str] | None = None, args: Iterable[str] | None = None, env: Mapping[str, str] | None = None, + env_inherit: set[str] | None = None, ) -> GoalRuleResult: merged_args = (*(global_args or []), goal.name, *(args or [])) - self.set_options(merged_args, env=env) + self.set_options(merged_args, env=env, env_inherit=env_inherit) raw_specs = self.options_bootstrapper.full_options_for_scopes( [*GlobalOptions.known_scope_infos(), *goal.subsystem_cls.known_scope_infos()] @@ -216,23 +216,37 @@ def run_goal_rule( console.flush() return GoalRuleResult(exit_code, stdout.getvalue(), stderr.getvalue()) - def set_options(self, args: Iterable[str], *, env: Mapping[str, str] | None = None) -> None: + def set_options( + self, + args: Iterable[str], + *, + env: Mapping[str, str] | None = None, + env_inherit: set[str] | None = None, + ) -> None: """Update the engine session with new options and/or environment variables. - The environment variables will be used to set the `PantsEnvironment`, which is the + The environment variables will be used to set the `CompleteEnvironment`, which is the environment variables captured by the parent Pants process. Some rules use this to be able to read arbitrary env vars. Any options that start with `PANTS_` will also be used to set options. + Environment variables listed in `env_inherit` and not in `env` will be inherited from the test + runner's environment (os.environ) + This will override any previously configured values. """ + env = { + **{k: os.environ[k] for k in (env_inherit or set()) if k in os.environ}, + **(env or {}), + } self.options_bootstrapper = create_options_bootstrapper(args=args, env=env) + self.environment = CompleteEnvironment(env) self.scheduler = self.scheduler.scheduler.new_session( build_id="buildid_for_test", session_values=SessionValues( { OptionsBootstrapper: self.options_bootstrapper, - PantsEnvironment: PantsEnvironment(env), + CompleteEnvironment: self.environment, } ), ) diff --git a/src/rust/engine/src/externs/interface.rs b/src/rust/engine/src/externs/interface.rs index 4559569d8e4..b35cc9958a3 100644 --- a/src/rust/engine/src/externs/interface.rs +++ b/src/rust/engine/src/externs/interface.rs @@ -1759,19 +1759,11 @@ fn run_local_interactive_process( let run_in_workspace: bool = externs::getattr(&value, "run_in_workspace").unwrap(); let input_digest_value: Value = externs::getattr(&value, "input_digest").unwrap(); let input_digest: Digest = nodes::lift_directory_digest(&input_digest_value)?; - let hermetic_env: bool = externs::getattr(&value, "hermetic_env").unwrap(); let env = externs::getattr_from_frozendict(&value, "env"); let code = block_in_place_and_wait(py, || { scheduler - .run_local_interactive_process( - session, - input_digest, - argv, - env, - hermetic_env, - run_in_workspace, - ) + .run_local_interactive_process(session, input_digest, argv, env, run_in_workspace) .boxed_local() })?; diff --git a/src/rust/engine/src/scheduler.rs b/src/rust/engine/src/scheduler.rs index 509261057e5..138bef9513c 100644 --- a/src/rust/engine/src/scheduler.rs +++ b/src/rust/engine/src/scheduler.rs @@ -188,7 +188,6 @@ impl Scheduler { input_digest: Digest, argv: Vec, env: BTreeMap, - hermetic_env: bool, run_in_workspace: bool, ) -> Result { let maybe_tempdir = if run_in_workspace { @@ -236,9 +235,7 @@ impl Scheduler { command.current_dir(tempdir.path()); } - if hermetic_env { - command.env_clear(); - } + command.env_clear(); command.envs(env); command.kill_on_drop(true); diff --git a/tests/python/pants_test/init/test_options_initializer.py b/tests/python/pants_test/init/test_options_initializer.py index a2e55dec96e..97a3ae338fb 100644 --- a/tests/python/pants_test/init/test_options_initializer.py +++ b/tests/python/pants_test/init/test_options_initializer.py @@ -4,6 +4,7 @@ import unittest from pants.base.exceptions import BuildConfigurationError +from pants.engine.environment import CompleteEnvironment from pants.init.options_initializer import OptionsInitializer from pants.option.errors import OptionsError from pants.option.options_bootstrapper import OptionsBootstrapper @@ -17,9 +18,10 @@ def test_invalid_version(self): allow_pantsrc=False, ) + env = CompleteEnvironment({}) with self.assertRaises(BuildConfigurationError): - OptionsInitializer(options_bootstrapper).build_config_and_options( - options_bootstrapper, raise_=True + OptionsInitializer(options_bootstrapper, env).build_config_and_options( + options_bootstrapper, env, raise_=True ) def test_global_options_validation(self): @@ -27,6 +29,7 @@ def test_global_options_validation(self): ob = OptionsBootstrapper.create( env={}, args=["--backend-packages=[]", "--remote-execution"], allow_pantsrc=False ) + env = CompleteEnvironment({}) with self.assertRaises(OptionsError) as exc: - OptionsInitializer(ob).build_config_and_options(ob, raise_=True) + OptionsInitializer(ob, env).build_config_and_options(ob, env, raise_=True) self.assertIn("The `--remote-execution` option requires", str(exc.exception)) diff --git a/tests/python/pants_test/init/test_plugin_resolver.py b/tests/python/pants_test/init/test_plugin_resolver.py index b201ecf734b..09628223993 100644 --- a/tests/python/pants_test/init/test_plugin_resolver.py +++ b/tests/python/pants_test/init/test_plugin_resolver.py @@ -21,6 +21,7 @@ PexRequirements, ) from pants.core.util_rules import archive, external_tool +from pants.engine.environment import CompleteEnvironment from pants.engine.fs import CreateDigest, Digest, FileContent, MergeDigests, Snapshot from pants.engine.internals.scheduler import ExecutionError from pants.engine.process import Process, ProcessResult @@ -42,7 +43,7 @@ @pytest.fixture def rule_runner() -> RuleRunner: - return RuleRunner( + rule_runner = RuleRunner( rules=[ *pex.rules(), *external_tool.rules(), @@ -52,14 +53,22 @@ def rule_runner() -> RuleRunner: QueryRule(ProcessResult, [Process]), ] ) + rule_runner.set_options( + ["--backend-packages=pants.backend.python"], + env_inherit={"PATH", "PYENV_ROOT", "HOME"}, + ) + return rule_runner -def _create_pex(rule_runner: RuleRunner) -> Pex: - rule_runner.set_options(["--backend-packages=pants.backend.python"]) +def _create_pex( + rule_runner: RuleRunner, + interpreter_constraints: PexInterpreterConstraints, +) -> Pex: request = PexRequest( output_filename="setup-py-runner.pex", internal_only=True, requirements=PexRequirements(["setuptools==44.0.0", "wheel==0.34.2"]), + interpreter_constraints=interpreter_constraints, ) return rule_runner.request(Pex, [request]) @@ -67,11 +76,12 @@ def _create_pex(rule_runner: RuleRunner) -> Pex: def _run_setup_py( rule_runner: RuleRunner, plugin: str, + interpreter_constraints: PexInterpreterConstraints, version: Optional[str], setup_py_args: Iterable[str], install_dir: str, ) -> None: - pex_obj = _create_pex(rule_runner) + pex_obj = _create_pex(rule_runner, interpreter_constraints) setup_py_file = FileContent( "setup.py", dedent( @@ -92,7 +102,7 @@ def _run_setup_py( # works. process = Process( argv=("./setup-py-runner.pex", "setup.py", *setup_py_args), - env={"PATH": os.getenv("PATH", "")}, + env={k: os.environ[k] for k in ["PATH", "HOME", "PYENV_ROOT"] if k in os.environ}, input_digest=merged_digest, description="Run setup.py", output_directories=("dist/",), @@ -117,6 +127,12 @@ def provide_chroot(existing): with temporary_dir() as new_chroot: yield new_chroot, True + interpreter_constraints = ( + PexInterpreterConstraints([f"=={interpreter.identity.version_str}"]) + if interpreter + else PexInterpreterConstraints([">=3.7"]) + ) + with provide_chroot(chroot) as (root_dir, create_artifacts): env: Dict[str, str] = {} repo_dir = None @@ -135,29 +151,34 @@ def provide_chroot(existing): plugin_list.append(f"{plugin}=={version}" if version else plugin) if create_artifacts: setup_py_args = ["sdist" if sdist else "bdist_wheel", "--dist-dir", "dist/"] - _run_setup_py(rule_runner, plugin, version, setup_py_args, repo_dir) + _run_setup_py( + rule_runner, + plugin, + interpreter_constraints, + version, + setup_py_args, + repo_dir, + ) env["PANTS_PLUGINS"] = f"[{','.join(map(repr, plugin_list))}]" - env["PANTS_PLUGIN_CACHE_DIR"] = os.path.join(root_dir, "plugin-cache") configpath = os.path.join(root_dir, "pants.toml") if create_artifacts: touch(configpath) args = [f"--pants-config-files=['{configpath}']"] - interpreter_constraints = ( - PexInterpreterConstraints([f"=={interpreter.identity.version_str}"]) - if interpreter - else None - ) - options_bootstrapper = OptionsBootstrapper.create(env=env, args=args, allow_pantsrc=False) - bootstrap_scheduler = create_bootstrap_scheduler(options_bootstrapper) + complete_env = CompleteEnvironment( + {**{k: os.environ[k] for k in ["PATH", "HOME", "PYENV_ROOT"] if k in os.environ}, **env} + ) + bootstrap_scheduler = create_bootstrap_scheduler(options_bootstrapper, complete_env) plugin_resolver = PluginResolver( bootstrap_scheduler, interpreter_constraints=interpreter_constraints ) cache_dir = options_bootstrapper.bootstrap_options.for_global_scope().named_caches_dir - working_set = plugin_resolver.resolve(options_bootstrapper, WorkingSet(entries=[])) + working_set = plugin_resolver.resolve( + options_bootstrapper, complete_env, WorkingSet(entries=[]) + ) for dist in working_set: assert ( Path(os.path.realpath(cache_dir)) in Path(os.path.realpath(dist.location)).parents diff --git a/tests/python/pants_test/pantsd/test_pants_daemon_core.py b/tests/python/pants_test/pantsd/test_pants_daemon_core.py index fb834502edc..b9cad7b4984 100644 --- a/tests/python/pants_test/pantsd/test_pants_daemon_core.py +++ b/tests/python/pants_test/pantsd/test_pants_daemon_core.py @@ -1,6 +1,7 @@ # Copyright 2020 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). +from pants.engine.environment import CompleteEnvironment from pants.engine.internals.native_engine import PyExecutor from pants.pantsd.pants_daemon_core import PantsDaemonCore from pants.pantsd.service.pants_service import PantsServices @@ -12,13 +13,16 @@ def test_prepare_scheduler(): def create_services(bootstrap_options, legacy_graph_scheduler): return PantsServices() - core = PantsDaemonCore(create_options_bootstrapper([]), PyExecutor(2, 4), create_services) + env = CompleteEnvironment({}) + core = PantsDaemonCore(create_options_bootstrapper([]), env, PyExecutor(2, 4), create_services) first_scheduler, first_options_initializer = core.prepare( - create_options_bootstrapper(["-ldebug"]) + create_options_bootstrapper(["-ldebug"]), + env, ) second_scheduler, second_options_initializer = core.prepare( - create_options_bootstrapper(["-lwarn"]) + create_options_bootstrapper(["-lwarn"]), + env, ) assert first_scheduler is not second_scheduler assert first_options_initializer is second_options_initializer