diff --git a/src/python/pants/backend/go/subsystems/golang.py b/src/python/pants/backend/go/subsystems/golang.py index 3a70ad766db..6591a743aad 100644 --- a/src/python/pants/backend/go/subsystems/golang.py +++ b/src/python/pants/backend/go/subsystems/golang.py @@ -6,6 +6,7 @@ import logging import os +from pants.core.util_rules.asdf import AsdfPathString from pants.option.option_types import BoolOption, StrListOption, StrOption from pants.option.subsystem import Subsystem from pants.util.memo import memoized_property @@ -29,7 +30,7 @@ class EnvironmentAware(Subsystem.EnvironmentAware): _go_search_paths = StrListOption( default=[""], help=softwrap( - """ + f""" A list of paths to search for Go. Specify absolute paths to directories with the `go` binary, e.g. `/usr/bin`. @@ -38,9 +39,8 @@ class EnvironmentAware(Subsystem.EnvironmentAware): The following special strings are supported: * ``, the contents of the PATH environment variable - * ``, all Go versions currently configured by ASDF \ - `(asdf shell, ${HOME}/.tool-versions)`, with a fallback to all installed versions - * ``, the ASDF interpreter with the version in BUILD_ROOT/.tool-versions + * `{AsdfPathString.STANDARD}`, {AsdfPathString.STANDARD.description("Go")} + * `{AsdfPathString.LOCAL}`, {AsdfPathString.LOCAL.description("binary")} """ ), ) diff --git a/src/python/pants/backend/go/util_rules/go_bootstrap.py b/src/python/pants/backend/go/util_rules/go_bootstrap.py index ccba276241a..2e15b7baabd 100644 --- a/src/python/pants/backend/go/util_rules/go_bootstrap.py +++ b/src/python/pants/backend/go/util_rules/go_bootstrap.py @@ -3,18 +3,18 @@ from __future__ import annotations import logging -import os from dataclasses import dataclass -from typing import Iterable +from typing import Collection from pants.backend.go.subsystems.golang import GolangSubsystem -from pants.core.util_rules import asdf -from pants.core.util_rules.asdf import AsdfToolPathsRequest, AsdfToolPathsResult -from pants.core.util_rules.environments import EnvironmentTarget, LocalEnvironmentTarget -from pants.engine.env_vars import EnvironmentVars, EnvironmentVarsRequest +from pants.core.util_rules import asdf, search_paths +from pants.core.util_rules.asdf import AsdfPathString, AsdfToolPathsResult +from pants.core.util_rules.environments import EnvironmentTarget +from pants.core.util_rules.search_paths import ValidatedSearchPaths, ValidateSearchPathsRequest +from pants.engine.env_vars import PathEnvironmentVariable from pants.engine.internals.selectors import Get from pants.engine.rules import collect_rules, rule -from pants.util.strutil import softwrap +from pants.util.ordered_set import FrozenOrderedSet logger = logging.getLogger(__name__) @@ -25,101 +25,50 @@ class GoBootstrap: async def _go_search_paths( - env_tgt: EnvironmentTarget, golang_subsystem: GolangSubsystem, paths: Iterable[str] + env_tgt: EnvironmentTarget, golang_subsystem: GolangSubsystem, paths: Collection[str] ) -> tuple[str, ...]: - - resolve_standard, resolve_local = "" in paths, "" in paths - - if resolve_standard or resolve_local: - # AsdfToolPathsResult is uncacheable, so only request it if absolutely necessary. - asdf_result = await Get( - AsdfToolPathsResult, - AsdfToolPathsRequest( - env_tgt=env_tgt, - tool_name=golang_subsystem.asdf_tool_name, - tool_description="Go distribution", - resolve_standard=resolve_standard, - resolve_local=resolve_local, - paths_option_name="[golang].go_search_paths", - bin_relpath=golang_subsystem.asdf_bin_relpath, - ), - ) - asdf_standard_tool_paths = asdf_result.standard_tool_paths - asdf_local_tool_paths = asdf_result.local_tool_paths - else: - asdf_standard_tool_paths = () - asdf_local_tool_paths = () - + asdf_result = await AsdfToolPathsResult.get_un_cachable_search_paths( + paths, + env_tgt=env_tgt, + tool_name=golang_subsystem.asdf_tool_name, + tool_description="Go distribution", + paths_option_name="[golang].go_search_paths", + bin_relpath=golang_subsystem.asdf_bin_relpath, + ) special_strings = { - "": lambda: asdf_standard_tool_paths, - "": lambda: asdf_local_tool_paths, + AsdfPathString.STANDARD.value: asdf_result.standard_tool_paths, + AsdfPathString.LOCAL.value: asdf_result.local_tool_paths, } + path_variables = await Get(PathEnvironmentVariable, {}) expanded: list[str] = [] for s in paths: if s == "": - expanded.extend(await _environment_paths()) + expanded.extend(path_variables) elif s in special_strings: - special_paths = special_strings[s]() + special_paths = special_strings[s] expanded.extend(special_paths) else: expanded.append(s) return tuple(expanded) -async def _environment_paths() -> list[str]: - """Returns a list of paths specified by the PATH env var.""" - env = await Get(EnvironmentVars, EnvironmentVarsRequest(("PATH",))) - path = env.get("PATH") - if path: - return path.split(os.pathsep) - return [] - - -def _error_if_not_compatible_with_asdf( - env_tgt: EnvironmentTarget, - _search_paths: Iterable[str], -) -> None: - """Raises an exception if any special search path strings any are invalid for the environment. - - If the environment is non-local and there are invalid tokens for those environments, raise - `ValueError`. - """ - - env = env_tgt.val - - if env is None or isinstance(env, LocalEnvironmentTarget): - return - - not_allowed = {"", ""} - - any_not_allowed = set(_search_paths) & not_allowed - if any_not_allowed: - env_type = type(env) - raise ValueError( - softwrap( - f"`[python-bootstrap].search_paths` is configured to use local Go discovery " - f"tools, which do not work in {env_type.__name__} runtime environments. To fix " - f"this, set the value of `golang_go_search_paths` in the `{env.alias}` " - f"defined at `{env.address}` to contain only hardcoded paths or the `` " - "special string." - ) - ) - - return - - @rule async def resolve_go_bootstrap( golang_subsystem: GolangSubsystem, golang_env_aware: GolangSubsystem.EnvironmentAware ) -> GoBootstrap: - - _error_if_not_compatible_with_asdf( - golang_env_aware.env_tgt, golang_env_aware.raw_go_search_paths - ) - paths = await _go_search_paths( - golang_env_aware.env_tgt, golang_subsystem, golang_env_aware.raw_go_search_paths + search_paths = await Get( + ValidatedSearchPaths, + ValidateSearchPathsRequest( + env_tgt=golang_env_aware.env_tgt, + search_paths=tuple(golang_env_aware.raw_go_search_paths), + option_origin=f"[{GolangSubsystem.options_scope}].go_search_paths", + environment_key="golang_go_search_paths", + is_default=golang_env_aware._is_default("_go_search_paths"), + local_only=FrozenOrderedSet((AsdfPathString.STANDARD, AsdfPathString.LOCAL)), + ), ) + paths = await _go_search_paths(golang_env_aware.env_tgt, golang_subsystem, search_paths) return GoBootstrap(go_search_paths=paths) @@ -146,4 +95,5 @@ def rules(): return ( *collect_rules(), *asdf.rules(), + *search_paths.rules(), ) diff --git a/src/python/pants/core/subsystems/python_bootstrap.py b/src/python/pants/core/subsystems/python_bootstrap.py index e3d405f7105..e06be7a69bb 100644 --- a/src/python/pants/core/subsystems/python_bootstrap.py +++ b/src/python/pants/core/subsystems/python_bootstrap.py @@ -7,18 +7,20 @@ import os from dataclasses import dataclass from pathlib import Path -from typing import Iterable, Sequence +from typing import Collection from pex.variables import Variables from pants.base.build_environment import get_buildroot -from pants.core.util_rules import asdf -from pants.core.util_rules.asdf import AsdfToolPathsRequest, AsdfToolPathsResult +from pants.core.util_rules import asdf, search_paths +from pants.core.util_rules.asdf import AsdfPathString, AsdfToolPathsResult from pants.core.util_rules.environments import EnvironmentTarget, LocalEnvironmentTarget -from pants.engine.env_vars import EnvironmentVars, EnvironmentVarsRequest +from pants.core.util_rules.search_paths import ValidatedSearchPaths, ValidateSearchPathsRequest +from pants.engine.env_vars import EnvironmentVars, EnvironmentVarsRequest, PathEnvironmentVariable from pants.engine.rules import Get, _uncacheable_rule, collect_rules, rule from pants.option.option_types import StrListOption from pants.option.subsystem import Subsystem +from pants.util.ordered_set import FrozenOrderedSet from pants.util.strutil import help_text, softwrap logger = logging.getLogger(__name__) @@ -40,10 +42,10 @@ class EnvironmentAware(Subsystem.EnvironmentAware): search_path = StrListOption( default=["", ""], help=softwrap( - """ + f""" A list of paths to search for Python interpreters. - Which interpeters are actually used from these paths is context-specific: + Which interpreters are actually used from these paths is context-specific: the Python backend selects interpreters using options on the `python` subsystem, in particular, the `[python].interpreter_constraints` option. @@ -59,9 +61,8 @@ class EnvironmentAware(Subsystem.EnvironmentAware): When the environment is a `local_environment` target: - * ``, all Python versions currently configured by ASDF \ - `(asdf shell, ${HOME}/.tool-versions)`, with a fallback to all installed versions - * ``, the ASDF interpreter with the version in BUILD_ROOT/.tool-versions + * `{AsdfPathString.STANDARD}`, {AsdfPathString.STANDARD.description("Python")} + * `{AsdfPathString.LOCAL}`, {AsdfPathString.LOCAL.description("interpreter")} * ``, all Python versions under $(pyenv root)/versions * ``, the Pyenv interpreter with the version in BUILD_ROOT/.python-version * ``, paths in the PEX_PYTHON_PATH variable in /etc/pexrc or ~/.pexrc @@ -94,7 +95,7 @@ class PythonBootstrap: @dataclass(frozen=True) class _ExpandInterpreterSearchPathsRequest: - interpreter_search_paths: Sequence[str] + interpreter_search_paths: Collection[str] env_tgt: EnvironmentTarget @@ -111,43 +112,29 @@ class _SearchPaths: @rule async def _expand_interpreter_search_paths( - request: _ExpandInterpreterSearchPathsRequest, + request: _ExpandInterpreterSearchPathsRequest, path_env: PathEnvironmentVariable ) -> _SearchPaths: interpreter_search_paths, env_tgt = (request.interpreter_search_paths, request.env_tgt) - env = await Get(EnvironmentVars, EnvironmentVarsRequest(("PATH",))) - - has_asdf_standard_path_token, has_asdf_local_path_token = _contains_asdf_path_tokens( - interpreter_search_paths + asdf_paths = await AsdfToolPathsResult.get_un_cachable_search_paths( + interpreter_search_paths, + env_tgt=env_tgt, + tool_name="python", + tool_description="Python interpreters", + paths_option_name="[python-bootstrap].search_path", ) - if has_asdf_standard_path_token or has_asdf_local_path_token: - # `AsdfToolPathsResult` is uncacheable, so don't request it unless we actually need it. - asdf_paths = await Get( - AsdfToolPathsResult, - AsdfToolPathsRequest( - env_tgt=env_tgt, - tool_name="python", - tool_description="Python interpreters", - resolve_standard=has_asdf_standard_path_token, - resolve_local=has_asdf_local_path_token, - paths_option_name="[python-bootstrap].search_path", - ), - ) - - asdf_standard_tool_paths, asdf_local_tool_paths = ( - asdf_paths.standard_tool_paths, - asdf_paths.local_tool_paths, - ) - else: - asdf_local_tool_paths, asdf_standard_tool_paths = (), () + asdf_standard_tool_paths, asdf_local_tool_paths = ( + asdf_paths.standard_tool_paths, + asdf_paths.local_tool_paths, + ) special_strings = { "": _get_pex_python_paths, - "": lambda: _get_environment_paths(env), - "": lambda: asdf_standard_tool_paths, - "": lambda: asdf_local_tool_paths, + "": lambda: path_env, + AsdfPathString.STANDARD: lambda: asdf_standard_tool_paths, + AsdfPathString.LOCAL: lambda: asdf_local_tool_paths, } expanded: list[str] = [] @@ -180,14 +167,6 @@ async def _expand_interpreter_search_paths( return _SearchPaths(tuple(expanded)) -def _get_environment_paths(env: EnvironmentVars): - """Returns a list of paths specified by the PATH env var.""" - pathstr = env.get("PATH") - if pathstr: - return pathstr.split(os.pathsep) - return [] - - def _get_pex_python_paths(): """Returns a list of paths to Python interpreters as defined in a pexrc file. @@ -201,18 +180,6 @@ def _get_pex_python_paths(): return [] -def _contains_asdf_path_tokens(interpreter_search_paths: Iterable[str]) -> tuple[bool, bool]: - """Returns tuple of whether the path list contains standard or local ASDF path tokens.""" - standard_path_token = False - local_path_token = False - for interpreter_search_path in interpreter_search_paths: - if interpreter_search_path == "": - standard_path_token = True - elif interpreter_search_path == "": - local_path_token = True - return standard_path_token, local_path_token - - @_uncacheable_rule async def _get_pyenv_paths(request: _PyEnvPathsRequest) -> _SearchPaths: """Returns a tuple of paths to Python interpreters managed by pyenv. @@ -275,63 +242,28 @@ def _get_pyenv_root(env: EnvironmentVars) -> str | None: return None -def _preprocessed_interpreter_search_paths( - env_tgt: EnvironmentTarget, - _search_paths: Iterable[str], - is_default: bool, -) -> tuple[str, ...]: - """Checks for special search path strings, and errors if any are invalid for the environment. - - This will return: - * The search paths, unaltered, for local/undefined environments, OR - * The search paths, with invalid tokens removed, if the provided value was unaltered from the - default value in the options system - (see `PythonBootstrapSubsystem.EnvironmentAware.search_paths`) - * The search paths unaltered, if the search paths are all valid tokens for this environment - - If the environment is non-local and there are invalid tokens for those environments, raise - `ValueError`. - """ - - env = env_tgt.val - search_paths = tuple(_search_paths) - - if env is None or isinstance(env, LocalEnvironmentTarget): - return search_paths - - not_allowed = {"", "", "", "", ""} - - if is_default: - # Strip out the not-allowed special strings from search_paths. - # An error will occur on the off chance the non-local environment expects pyenv - # but there's nothing we can do here to detect it. - return tuple(path for path in search_paths if path not in not_allowed) - - any_not_allowed = set(search_paths) & not_allowed - if any_not_allowed: - env_type = type(env) - raise ValueError( - softwrap( - f"`[python-bootstrap].search_paths` is configured to use local Python discovery " - f"tools, which do not work in {env_type.__name__} runtime environments. To fix " - f"this, set the value of `python_bootstrap_search_path` in the `{env.alias}` " - f"defined at `{env.address}` to contain only hardcoded paths or the `` " - "special string." - ) - ) - - return search_paths - - @rule async def python_bootstrap( python_bootstrap_subsystem: PythonBootstrapSubsystem.EnvironmentAware, ) -> PythonBootstrap: - - interpreter_search_paths = _preprocessed_interpreter_search_paths( - python_bootstrap_subsystem.env_tgt, - python_bootstrap_subsystem.search_path, - python_bootstrap_subsystem._is_default("search_path"), + interpreter_search_paths = await Get( + ValidatedSearchPaths, + ValidateSearchPathsRequest( + env_tgt=python_bootstrap_subsystem.env_tgt, + search_paths=tuple(python_bootstrap_subsystem.search_path), + option_origin=f"[{PythonBootstrapSubsystem.options_scope}].search_path", + environment_key="python_bootstrap_search_path", + is_default=python_bootstrap_subsystem._is_default("search_path"), + local_only=FrozenOrderedSet( + ( + "", + "", + AsdfPathString.STANDARD, + AsdfPathString.LOCAL, + "", + ) + ), + ), ) interpreter_names = python_bootstrap_subsystem.names @@ -353,4 +285,5 @@ def rules(): return ( *collect_rules(), *asdf.rules(), + *search_paths.rules(), ) diff --git a/src/python/pants/core/subsystems/python_bootstrap_test.py b/src/python/pants/core/subsystems/python_bootstrap_test.py index 5db55d5466e..cca74e2e9f6 100644 --- a/src/python/pants/core/subsystems/python_bootstrap_test.py +++ b/src/python/pants/core/subsystems/python_bootstrap_test.py @@ -12,10 +12,8 @@ from pants.base.build_environment import get_pants_cachedir from pants.core.subsystems.python_bootstrap import ( _ExpandInterpreterSearchPathsRequest, - _get_environment_paths, _get_pex_python_paths, _get_pyenv_root, - _preprocessed_interpreter_search_paths, _PyEnvPathsRequest, _SearchPaths, ) @@ -23,13 +21,7 @@ from pants.core.util_rules import asdf from pants.core.util_rules.asdf import AsdfToolPathsRequest, AsdfToolPathsResult from pants.core.util_rules.asdf_test import fake_asdf_root -from pants.core.util_rules.environments import ( - DockerEnvironmentTarget, - DockerImageField, - EnvironmentTarget, - LocalEnvironmentTarget, - RemoteEnvironmentTarget, -) +from pants.core.util_rules.environments import EnvironmentTarget, LocalEnvironmentTarget from pants.engine.addresses import Address from pants.engine.env_vars import CompleteEnvironmentVars, EnvironmentVars from pants.engine.rules import QueryRule @@ -95,11 +87,6 @@ def materialize_indices(sequence: Sequence[_T], indices: Iterable[int]) -> List[ return [sequence[i] for i in indices] -def test_get_environment_paths() -> None: - paths = _get_environment_paths(EnvironmentVars({"PATH": "foo/bar:baz:/qux/quux"})) - assert ["foo/bar", "baz", "/qux/quux"] == paths - - def test_get_pex_python_paths() -> None: with setup_pexrc_with_pex_python_path(["foo/bar", "baz", "/qux/quux"]): paths = _get_pex_python_paths() @@ -229,77 +216,3 @@ def test_expand_interpreter_search_paths(rule_runner: RuleRunner) -> None: "/qux", ) assert expected == expanded_paths.paths - - -@pytest.mark.parametrize( - ("env_tgt_type", "search_paths", "is_default", "expected"), - ( - (LocalEnvironmentTarget, ("",), False, ("",)), - (LocalEnvironmentTarget, ("",), False, ("",)), - ( - LocalEnvironmentTarget, - ("", "/home/derryn/pythons"), - False, - ("", "/home/derryn/pythons"), - ), - (DockerEnvironmentTarget, ("", ""), True, ("",)), - (DockerEnvironmentTarget, ("", ""), False, ValueError), - (DockerEnvironmentTarget, ("", ""), False, ValueError), - ( - DockerEnvironmentTarget, - ("", "/home/derryn/pythons"), - False, - ValueError, - ), # Contains a banned special-string - (DockerEnvironmentTarget, ("",), False, ValueError), - (DockerEnvironmentTarget, ("",), False, ValueError), - (DockerEnvironmentTarget, ("",), False, ValueError), - (DockerEnvironmentTarget, ("",), False, ("",)), - ( - DockerEnvironmentTarget, - ("", "/home/derryn/pythons"), - False, - ("", "/home/derryn/pythons"), - ), - (RemoteEnvironmentTarget, ("", ""), True, ("",)), - (RemoteEnvironmentTarget, ("", ""), False, ValueError), - (RemoteEnvironmentTarget, ("", ""), False, ValueError), - ( - RemoteEnvironmentTarget, - ("", "/home/derryn/pythons"), - False, - ValueError, - ), # Contains a banned special-string - (RemoteEnvironmentTarget, ("",), False, ValueError), - (RemoteEnvironmentTarget, ("",), False, ValueError), - (RemoteEnvironmentTarget, ("",), False, ValueError), - (RemoteEnvironmentTarget, ("",), False, ("",)), - ( - RemoteEnvironmentTarget, - ("", "/home/derryn/pythons"), - False, - ("", "/home/derryn/pythons"), - ), - ), -) -def test_preprocessed_interpreter_search_paths( - env_tgt_type: type[LocalEnvironmentTarget] - | type[DockerEnvironmentTarget] - | type[RemoteEnvironmentTarget], - search_paths: Iterable[str], - is_default: bool, - expected: tuple[str] | type[ValueError], -): - - extra_kwargs: dict = {} - if env_tgt_type is DockerEnvironmentTarget: - extra_kwargs = { - DockerImageField.alias: "my_img", - } - - env_tgt = EnvironmentTarget(env_tgt_type(extra_kwargs, address=Address("flem"))) - if expected is not ValueError: - assert expected == _preprocessed_interpreter_search_paths(env_tgt, search_paths, is_default) - else: - with pytest.raises(ValueError): - _preprocessed_interpreter_search_paths(env_tgt, search_paths, is_default) diff --git a/src/python/pants/core/util_rules/asdf.py b/src/python/pants/core/util_rules/asdf.py index 9775782c198..9296c5bdfa7 100644 --- a/src/python/pants/core/util_rules/asdf.py +++ b/src/python/pants/core/util_rules/asdf.py @@ -6,7 +6,9 @@ import re from collections import OrderedDict from dataclasses import dataclass +from enum import Enum from pathlib import Path, PurePath +from typing import Collection from pants.base.build_environment import get_buildroot from pants.base.build_root import BuildRoot @@ -19,6 +21,27 @@ logger = logging.getLogger(__name__) +class AsdfPathString(str, Enum): + STANDARD = "" + LOCAL = "" + + @staticmethod + def contains_strings(search_paths: Collection[str]) -> tuple[bool, bool]: + return AsdfPathString.STANDARD in search_paths, AsdfPathString.LOCAL in search_paths + + def description(self, tool: str) -> str: + if self is self.STANDARD: + return softwrap( + f""" + all {tool} versions currently configured by ASDF `(asdf shell, ${{HOME}}/.tool-versions)`, + with a fallback to all installed versions + """ + ) + if self is self.LOCAL: + return f"the ASDF {tool} with the version in BUILD_ROOT/.tool-versions" + raise NotImplementedError(f"{self} has no description.") + + @dataclass(frozen=True) class AsdfToolPathsRequest: env_tgt: EnvironmentTarget @@ -36,6 +59,35 @@ class AsdfToolPathsResult: standard_tool_paths: tuple[str, ...] = () local_tool_paths: tuple[str, ...] = () + @classmethod + async def get_un_cachable_search_paths( + cls, + search_paths: Collection[str], + env_tgt: EnvironmentTarget, + tool_name: str, + tool_description: str, + paths_option_name: str, + bin_relpath: str = "bin", + ) -> AsdfToolPathsResult: + + resolve_standard, resolve_local = AsdfPathString.contains_strings(search_paths) + + if resolve_standard or resolve_local: + # AsdfToolPathsResult is not cacheable, so only request it if absolutely necessary. + return await Get( + AsdfToolPathsResult, + AsdfToolPathsRequest( + env_tgt=env_tgt, + tool_name=tool_name, + tool_description=tool_description, + resolve_standard=resolve_standard, + resolve_local=resolve_local, + paths_option_name=paths_option_name, + bin_relpath=bin_relpath, + ), + ) + return AsdfToolPathsResult(tool_name) + async def _resolve_asdf_tool_paths( env_tgt: EnvironmentTarget, diff --git a/src/python/pants/core/util_rules/search_paths.py b/src/python/pants/core/util_rules/search_paths.py new file mode 100644 index 00000000000..e6eacd88e41 --- /dev/null +++ b/src/python/pants/core/util_rules/search_paths.py @@ -0,0 +1,72 @@ +# Copyright 2023 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). +from __future__ import annotations + +from dataclasses import dataclass +from typing import Iterable + +from pants.core.util_rules.environments import EnvironmentTarget, LocalEnvironmentTarget +from pants.engine.rules import Rule, collect_rules, rule +from pants.util.logging import LogLevel +from pants.util.ordered_set import FrozenOrderedSet +from pants.util.strutil import softwrap + + +@dataclass(frozen=True) +class ValidateSearchPathsRequest: + env_tgt: EnvironmentTarget + search_paths: tuple[str, ...] + option_origin: str + environment_key: str + is_default: bool + local_only: FrozenOrderedSet[str] + + +class ValidatedSearchPaths(FrozenOrderedSet): + """Search paths that are valid for the current target environment.""" + + +@rule(level=LogLevel.DEBUG) +async def validate_search_paths(request: ValidateSearchPathsRequest) -> ValidatedSearchPaths: + """Checks for special search path strings, and errors if any are invalid for the environment. + + This will return: + * The search paths, unaltered, for local/undefined environments, OR + * The search paths, with invalid tokens removed, if the provided value was unaltered from the + default value in the options system. + * The search paths unaltered, if the search paths are all valid tokens for this environment + + If the environment is non-local and there are invalid tokens for those environments, raise + `ValueError`. + """ + + env = request.env_tgt.val + search_paths = request.search_paths + + if env is None or isinstance(env, LocalEnvironmentTarget): + return ValidatedSearchPaths(search_paths) + + if request.is_default: + # Strip out the not-allowed special strings from search_paths. + # An error will occur on the off chance the non-local environment expects local_only tokens, + # but there's nothing we can do here to detect it. + return ValidatedSearchPaths(path for path in search_paths if path not in request.local_only) + + any_not_allowed = set(search_paths) & request.local_only + if any_not_allowed: + env_type = type(env) + raise ValueError( + softwrap( + f"`{request.option_origin}` is configured to use local discovery " + f"tools, which do not work in {env_type.__name__} runtime environments. To fix " + f"this, set the value of `{request.environment_key}` in the `{env.alias}` " + f"defined at `{env.address}` to contain only hardcoded paths or the `` " + "special string." + ) + ) + + return ValidatedSearchPaths(search_paths) + + +def rules() -> Iterable[Rule]: + return collect_rules() diff --git a/src/python/pants/core/util_rules/search_paths_test.py b/src/python/pants/core/util_rules/search_paths_test.py new file mode 100644 index 00000000000..2393f73313a --- /dev/null +++ b/src/python/pants/core/util_rules/search_paths_test.py @@ -0,0 +1,116 @@ +# Copyright 2023 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). +from __future__ import annotations + +import pytest + +from pants.build_graph.address import Address +from pants.core.util_rules.asdf import AsdfPathString +from pants.core.util_rules.environments import ( + DockerEnvironmentTarget, + DockerImageField, + EnvironmentTarget, + LocalEnvironmentTarget, + RemoteEnvironmentTarget, +) +from pants.core.util_rules.search_paths import ValidateSearchPathsRequest, validate_search_paths +from pants.testutil.rule_runner import run_rule_with_mocks +from pants.util.ordered_set import FrozenOrderedSet + + +@pytest.mark.parametrize( + ("env_tgt_type", "search_paths", "is_default", "expected"), + ( + (LocalEnvironmentTarget, ("",), False, ("",)), + (LocalEnvironmentTarget, ("",), False, ("",)), + ( + LocalEnvironmentTarget, + ("", "/home/derryn/pythons"), + False, + ("", "/home/derryn/pythons"), + ), + (DockerEnvironmentTarget, ("", ""), True, ("",)), + (DockerEnvironmentTarget, ("", ""), False, ValueError), + (DockerEnvironmentTarget, ("", ""), False, ValueError), + ( + DockerEnvironmentTarget, + ("", "/home/derryn/pythons"), + False, + ValueError, + ), # Contains a banned special-string + (DockerEnvironmentTarget, ("",), False, ValueError), + (DockerEnvironmentTarget, ("",), False, ValueError), + (DockerEnvironmentTarget, ("",), False, ValueError), + (DockerEnvironmentTarget, ("",), False, ("",)), + ( + DockerEnvironmentTarget, + ("", "/home/derryn/pythons"), + False, + ("", "/home/derryn/pythons"), + ), + (RemoteEnvironmentTarget, ("", ""), True, ("",)), + (RemoteEnvironmentTarget, ("", ""), False, ValueError), + (RemoteEnvironmentTarget, ("", ""), False, ValueError), + ( + RemoteEnvironmentTarget, + ("", "/home/derryn/pythons"), + False, + ValueError, + ), # Contains a banned special-string + (RemoteEnvironmentTarget, ("",), False, ValueError), + (RemoteEnvironmentTarget, ("",), False, ValueError), + (RemoteEnvironmentTarget, ("",), False, ValueError), + (RemoteEnvironmentTarget, ("",), False, ("",)), + ( + RemoteEnvironmentTarget, + ("", "/home/derryn/pythons"), + False, + ("", "/home/derryn/pythons"), + ), + ), +) +def test_validated_search_paths( + env_tgt_type: type[LocalEnvironmentTarget] + | type[DockerEnvironmentTarget] + | type[RemoteEnvironmentTarget], + search_paths: tuple[str], + is_default: bool, + expected: tuple[str] | type[ValueError], +): + extra_kwargs: dict = {} + if env_tgt_type is DockerEnvironmentTarget: + extra_kwargs = { + DockerImageField.alias: "my_img", + } + env_tgt = EnvironmentTarget(env_tgt_type(extra_kwargs, address=Address("flem"))) + local_only = FrozenOrderedSet( + { + "", + "", + AsdfPathString.STANDARD, + AsdfPathString.LOCAL, + "", + } + ) + + if expected is not ValueError: + assert expected == tuple( + run_rule_with_mocks( + validate_search_paths, + rule_args=[ + ValidateSearchPathsRequest( + env_tgt, search_paths, "[mock].opt", "mock_opt", is_default, local_only + ) + ], + ) + ) + else: + with pytest.raises(ValueError): + run_rule_with_mocks( + validate_search_paths, + rule_args=[ + ValidateSearchPathsRequest( + env_tgt, search_paths, "[mock].opt", "mock_opt", is_default, local_only + ) + ], + ) diff --git a/src/python/pants/engine/env_vars.py b/src/python/pants/engine/env_vars.py index 157a809b1a2..58a54e783bb 100644 --- a/src/python/pants/engine/env_vars.py +++ b/src/python/pants/engine/env_vars.py @@ -89,3 +89,7 @@ class EnvironmentVars(FrozenDict[str, str]): `CompleteEnvironmentVars`, as it represents a filtered/relevant subset of the environment, rather than the entire unfiltered environment. """ + + +class PathEnvironmentVariable(FrozenOrderedSet): + """The PATH environment variable entries, split on `os.pathsep`.""" diff --git a/src/python/pants/engine/internals/platform_rules.py b/src/python/pants/engine/internals/platform_rules.py index 11089c60e54..ac4e5c9065f 100644 --- a/src/python/pants/engine/internals/platform_rules.py +++ b/src/python/pants/engine/internals/platform_rules.py @@ -3,6 +3,8 @@ from __future__ import annotations +import os + from pants.core.util_rules.environments import ( DockerImageField, DockerPlatformField, @@ -10,7 +12,12 @@ EnvironmentTarget, RemotePlatformField, ) -from pants.engine.env_vars import CompleteEnvironmentVars, EnvironmentVars, EnvironmentVarsRequest +from pants.engine.env_vars import ( + CompleteEnvironmentVars, + EnvironmentVars, + EnvironmentVarsRequest, + PathEnvironmentVariable, +) from pants.engine.internals.session import SessionValues from pants.engine.platform import Platform from pants.engine.process import Process, ProcessResult @@ -94,5 +101,12 @@ def environment_vars_subset( ) +@rule +async def environment_path_variable() -> PathEnvironmentVariable: + env = await Get(EnvironmentVars, EnvironmentVarsRequest(("PATH",))) + path = env.get("PATH", None) + return PathEnvironmentVariable(path.split(os.pathsep) if path else ()) + + def rules(): return collect_rules() diff --git a/src/python/pants/engine/internals/platform_rules_test.py b/src/python/pants/engine/internals/platform_rules_test.py index 4019eb7d991..a2ebe022ced 100644 --- a/src/python/pants/engine/internals/platform_rules_test.py +++ b/src/python/pants/engine/internals/platform_rules_test.py @@ -19,9 +19,13 @@ RemoteEnvironmentTarget, RemotePlatformField, ) -from pants.engine.env_vars import CompleteEnvironmentVars +from pants.engine.env_vars import CompleteEnvironmentVars, EnvironmentVars, EnvironmentVarsRequest from pants.engine.environment import EnvironmentName -from pants.engine.internals.platform_rules import complete_environment_vars, current_platform +from pants.engine.internals.platform_rules import ( + complete_environment_vars, + current_platform, + environment_path_variable, +) from pants.engine.internals.session import SessionValues from pants.engine.platform import Platform from pants.engine.process import Process, ProcessResult @@ -152,6 +156,33 @@ def mock_env_process(process: Process) -> ProcessResult: ) +@pytest.mark.parametrize( + ("env", "expected_entries"), + [ + pytest.param( + {"PATH": "foo/bar:baz:/qux/quux"}, ["foo/bar", "baz", "/qux/quux"], id="populated_PATH" + ), + pytest.param({"PATH": ""}, [], id="empty_PATH"), + pytest.param({}, [], id="unset_PATH"), + ], +) +def test_get_environment_paths(env: dict[str, str], expected_entries: list[str]) -> None: + def mock_environment_vars_subset(_req: EnvironmentVarsRequest) -> EnvironmentVars: + return EnvironmentVars(env) + + paths = run_rule_with_mocks( + environment_path_variable, + mock_gets=[ + MockGet( + output_type=EnvironmentVars, + input_types=(CompleteEnvironmentVars, EnvironmentVarsRequest), + mock=mock_environment_vars_subset, + ) + ], + ) + assert list(paths) == expected_entries + + def test_docker_complete_env_vars() -> None: rule_runner = RuleRunner( rules=[QueryRule(CompleteEnvironmentVars, [EnvironmentName])], diff --git a/src/python/pants/option/subsystem.py b/src/python/pants/option/subsystem.py index 582d92cb01e..2a1a559f3dd 100644 --- a/src/python/pants/option/subsystem.py +++ b/src/python/pants/option/subsystem.py @@ -135,7 +135,8 @@ def _is_default(self, __name: str) -> bool: assert isinstance(v, OptionsInfo) return ( - self.options.is_default(__name) + # vars beginning with `_` are exposed as option names with the leading `_` stripped + self.options.is_default(__name.lstrip("_")) and resolve_environment_sensitive_option(v.flag_names[0], self) is None )