Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[internal] Preprocesses python bootstrap search paths to check for invalid settings #17041

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 60 additions & 2 deletions src/python/pants/core/subsystems/python_bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
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.environments import EnvironmentTarget, LocalEnvironmentTarget
from pants.engine.env_vars import EnvironmentVars
from pants.engine.rules import Get, collect_rules, rule
from pants.option.option_types import StrListOption
Expand All @@ -35,7 +36,7 @@ class PythonBootstrapSubsystem(Subsystem):
"""
)

class EnvironmentAware:
class EnvironmentAware(Subsystem.EnvironmentAware):
search_path = StrListOption(
default=["<PYENV>", "<PATH>"],
help=softwrap(
Expand All @@ -52,7 +53,12 @@ class EnvironmentAware:

The following special strings are supported:

For all runtime environment types:

* `<PATH>`, the contents of the PATH env var

When the environment is a `local_environment` target:

* `<ASDF>`, all Python versions currently configured by ASDF \
`(asdf shell, ${HOME}/.tool-versions)`, with a fallback to all installed versions
* `<ASDF_LOCAL>`, the ASDF interpreter with the version in BUILD_ROOT/.tool-versions
Expand Down Expand Up @@ -214,12 +220,64 @@ 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 = {"<PYENV>", "<PYENV_LOCAL>", "<ASDF>", "<ASDF_LOCAL>", "<PEXRC>"}

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 `<PATH>` "
"special string."
)
)

return search_paths


@rule
async def python_bootstrap(
python_bootstrap_subsystem: PythonBootstrapSubsystem.EnvironmentAware,
) -> PythonBootstrap:

interpreter_search_paths = python_bootstrap_subsystem.search_path
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_names = python_bootstrap_subsystem.names

has_standard_path_token, has_local_path_token = _contains_asdf_path_tokens(
Expand Down
87 changes: 87 additions & 0 deletions src/python/pants/core/subsystems/python_bootstrap_test.py
Original file line number Diff line number Diff line change
@@ -1,22 +1,35 @@
# Copyright 2018 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import annotations

import os
from contextlib import contextmanager
from typing import Iterable, List, Sequence, TypeVar

import pytest

from pants.base.build_environment import get_pants_cachedir
from pants.core.subsystems.python_bootstrap import (
PythonBootstrap,
_expand_interpreter_search_paths,
_get_environment_paths,
_get_pex_python_paths,
_get_pyenv_paths,
_preprocessed_interpreter_search_paths,
get_pyenv_root,
)
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, get_asdf_paths
from pants.core.util_rules.environments import (
DockerEnvironmentTarget,
DockerImageField,
EnvironmentTarget,
LocalEnvironmentTarget,
RemoteEnvironmentTarget,
)
from pants.engine.addresses import Address
from pants.engine.env_vars import EnvironmentVars
from pants.engine.rules import QueryRule
from pants.testutil.rule_runner import RuleRunner
Expand Down Expand Up @@ -192,3 +205,77 @@ def test_expand_interpreter_search_paths() -> None:
"/qux",
)
assert expected == expanded_paths


@pytest.mark.parametrize(
("env_tgt_type", "search_paths", "is_default", "expected"),
(
(LocalEnvironmentTarget, ("<PYENV>",), False, ("<PYENV>",)),
(LocalEnvironmentTarget, ("<ASDF>",), False, ("<ASDF>",)),
(
LocalEnvironmentTarget,
("<ASDF_LOCAL>", "/home/derryn/pythons"),
False,
("<ASDF_LOCAL>", "/home/derryn/pythons"),
),
(DockerEnvironmentTarget, ("<PYENV>", "<PATH>"), True, ("<PATH>",)),
(DockerEnvironmentTarget, ("<PYENV>", "<PATH>"), False, ValueError),
(DockerEnvironmentTarget, ("<PYENV>", "<PATH>"), False, ValueError),
(
DockerEnvironmentTarget,
("<ASDF>", "/home/derryn/pythons"),
False,
ValueError,
), # Contains a banned special-string
(DockerEnvironmentTarget, ("<ASDF_LOCAL>",), False, ValueError),
(DockerEnvironmentTarget, ("<PYENV_LOCAL>",), False, ValueError),
(DockerEnvironmentTarget, ("<PEXRC>",), False, ValueError),
(DockerEnvironmentTarget, ("<PATH>",), False, ("<PATH>",)),
(
DockerEnvironmentTarget,
("<PATH>", "/home/derryn/pythons"),
False,
("<PATH>", "/home/derryn/pythons"),
),
(RemoteEnvironmentTarget, ("<PYENV>", "<PATH>"), True, ("<PATH>",)),
(RemoteEnvironmentTarget, ("<PYENV>", "<PATH>"), False, ValueError),
(RemoteEnvironmentTarget, ("<PYENV>", "<PATH>"), False, ValueError),
(
RemoteEnvironmentTarget,
("<ASDF>", "/home/derryn/pythons"),
False,
ValueError,
), # Contains a banned special-string
(RemoteEnvironmentTarget, ("<ASDF_LOCAL>",), False, ValueError),
(RemoteEnvironmentTarget, ("<PYENV_LOCAL>",), False, ValueError),
(RemoteEnvironmentTarget, ("<PEXRC>",), False, ValueError),
(RemoteEnvironmentTarget, ("<PATH>",), False, ("<PATH>",)),
(
RemoteEnvironmentTarget,
("<PATH>", "/home/derryn/pythons"),
False,
("<PATH>", "/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)
12 changes: 12 additions & 0 deletions src/python/pants/option/subsystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,18 @@ def __getattribute__(self, __name: str) -> Any:
# We should just return the default at this point.
return default

def _is_default(self, __name: str) -> bool:
"""Returns true if the value of the named option is unchanged from the default."""
from pants.core.util_rules.environments import resolve_environment_sensitive_option

v = getattr(type(self), __name)
assert isinstance(v, OptionsInfo)

return (
self.options.is_default(__name)
and resolve_environment_sensitive_option(v.flag_names[0], self) is None
)

@memoized_classmethod
def rules(cls: Any) -> Iterable[Rule]:
from pants.core.util_rules.environments import add_option_fields_for
Expand Down