From 1a65ae2d5bc0543b45bc7d3be1950f399088dd19 Mon Sep 17 00:00:00 2001 From: Jonas Rauber Date: Fri, 26 Jul 2024 02:17:49 -0700 Subject: [PATCH] make `pyenv` backend respect patch version constraints (#21139) fixes #20175 fix inspired by #19462 --------- Co-authored-by: Jonas Rauber Co-authored-by: Huon Wilson --- docs/notes/2.23.x.md | 10 ++-- .../backend/python/providers/pyenv/rules.py | 51 +++++++++++++++---- .../providers/pyenv/rules_integration_test.py | 30 +++++++++-- 3 files changed, 73 insertions(+), 18 deletions(-) diff --git a/docs/notes/2.23.x.md b/docs/notes/2.23.x.md index e510a5bbf9a..1406ee25aa5 100644 --- a/docs/notes/2.23.x.md +++ b/docs/notes/2.23.x.md @@ -19,11 +19,11 @@ The deprecations for the `--changed-dependees` option and the `dependees` goal h ### Test goal A new option `--experimental-report-test-result-info` is added to the `[test]` config section. Enabling this option will -produce a file on disk with information that would tell you more about the test results. For now, it reports only -the source of the results. The tests might have been executed locally or remotely, but they might have been retrieved -from the local or remote cache, or be memoized. +produce a file on disk with information that would tell you more about the test results. For now, it reports only +the source of the results. The tests might have been executed locally or remotely, but they might have been retrieved +from the local or remote cache, or be memoized. -Knowing where the test results come from may be useful when evaluating the efficiency of the caching strategy and +Knowing where the test results come from may be useful when evaluating the efficiency of the caching strategy and the nature of the changes in the source code that may lead to frequent cache invalidations. ### Remote caching/execution @@ -112,6 +112,8 @@ When using the `vcs_version` target, force `setuptools_scm` git operations to ru When building function-as-a-service targets like `python_google_cloud_function`, `python_aws_lambda_function`, and `python_aws_lambda_layer`, the `complete_platforms` field may now be specified as either a `file` target or a `resource` target. +[The `pants.backend.python.providers.experimental.pyenv` backend](https://www.pantsbuild.org/2.23/reference/subsystems/pyenv-python-provider) now respects the patch version of the interpeter constraints. + #### Terraform The default version of terraform has been updated from 1.7.1 to 1.9.0. diff --git a/src/python/pants/backend/python/providers/pyenv/rules.py b/src/python/pants/backend/python/providers/pyenv/rules.py index 4bcf60bdb49..1460f9fd85b 100644 --- a/src/python/pants/backend/python/providers/pyenv/rules.py +++ b/src/python/pants/backend/python/providers/pyenv/rules.py @@ -216,6 +216,11 @@ class PyenvPythonProvider(PythonProvider): pass +def _major_minor_patch_to_int(major_minor_patch: str) -> tuple[int, int, int]: + major, minor, patch = map(int, major_minor_patch.split(".", maxsplit=2)) + return (major, minor, patch) + + @rule async def get_python( request: PyenvPythonProvider, @@ -229,26 +234,52 @@ async def get_python( Get(RunRequest, PyenvInstallInfoRequest()), ) - python_to_use = request.interpreter_constraints.minimum_python_version( + # Determine the lowest major/minor version supported according to the interpreter constraints. + major_minor_to_use_str = request.interpreter_constraints.minimum_python_version( python_setup.interpreter_versions_universe ) - if python_to_use is None: + if major_minor_to_use_str is None: raise ValueError( f"Couldn't determine a compatible Interpreter Constraint from {python_setup.interpreter_versions_universe}" ) - which_python_result = await Get( + # Find the highest patch version given the major/minor version that is known to our version of pyenv. + pyenv_latest_known_result = await Get( ProcessResult, Process( - [pyenv.exe, "latest", "--known", python_to_use], + [pyenv.exe, "latest", "--known", major_minor_to_use_str], input_digest=pyenv.digest, - description=f"Choose specific version for Python {python_to_use}", + description=f"Choose specific version for Python {major_minor_to_use_str}", env={"PATH": env_vars.get("PATH", "")}, - # Caching the result is OK, since if the user really needs a different patch, - # they should list a more precise IC. ), ) - specific_python = which_python_result.stdout.decode().strip() + major_to_use, minor_to_use, latest_known_patch = _major_minor_patch_to_int( + pyenv_latest_known_result.stdout.decode().strip() + ) + + # Pick the highest patch version given the major/minor version that is supported according to + # the interpreter constraints and known to our version of pyenv. + # We assume pyenv knows every patch version smaller or equal the its latest known patch + # version, to avoid calling it for each patch version separately. + supported_triplets = request.interpreter_constraints.enumerate_python_versions( + python_setup.interpreter_versions_universe + ) + try: + major_minor_patch_to_use = max( + (major, minor, patch) + for (major, minor, patch) in supported_triplets + if major == major_to_use and minor == minor_to_use and patch <= latest_known_patch + ) + except ValueError: + raise ValueError( + f"Couldn't find a Python {major_minor_to_use_str} version that" + f" is compatible with the interpreter constraints {request.interpreter_constraints}" + f" and known to pyenv {pyenv_subsystem.version}" + f" (latest known version {major_to_use}.{minor_to_use}.{latest_known_patch})." + " Suggestion: consider upgrading pyenv or adjusting your interpreter constraints." + ) from None + + major_minor_patch_to_use_str = ".".join(map(str, major_minor_patch_to_use)) # NB: We don't cache this process at any level for two reasons: # 1. Several tools (including pex) refer to Python at an absolute path, so a named cache is @@ -263,10 +294,10 @@ async def get_python( result = await Get( ProcessResult, Process( - pyenv_install.args + (specific_python,), + pyenv_install.args + (major_minor_patch_to_use_str,), level=LogLevel.DEBUG, input_digest=pyenv_install.digest, - description=f"Install Python {python_to_use}", + description=f"Install Python {major_minor_patch_to_use_str}", append_only_caches=pyenv_install.append_only_caches, env=pyenv_install.extra_env, # Don't cache, we want this to always be run so that we can assume for the rest of the diff --git a/src/python/pants/backend/python/providers/pyenv/rules_integration_test.py b/src/python/pants/backend/python/providers/pyenv/rules_integration_test.py index 9e6d855df33..56c901d589a 100644 --- a/src/python/pants/backend/python/providers/pyenv/rules_integration_test.py +++ b/src/python/pants/backend/python/providers/pyenv/rules_integration_test.py @@ -16,6 +16,7 @@ from pants.backend.python.target_types import PythonSourcesGeneratorTarget from pants.build_graph.address import Address from pants.core.goals.run import RunRequest +from pants.engine.internals.scheduler import ExecutionError from pants.engine.process import InteractiveProcess from pants.engine.rules import QueryRule from pants.engine.target import Target @@ -62,8 +63,11 @@ def run_run_request( return mocked_console[1].get_stdout().strip() -@pytest.mark.parametrize("py_version", ["2.7", "3.9"]) -def test_using_pyenv(rule_runner, py_version): +@pytest.mark.parametrize( + "interpreter_constraints, expected_version_substring", + [("2.7.*", "2.7"), ("3.9.*", "3.9"), ("3.10.4", "3.10.4")], +) +def test_using_pyenv(rule_runner, interpreter_constraints, expected_version_substring): rule_runner.write_files( { "src/app.py": dedent( @@ -76,7 +80,7 @@ def test_using_pyenv(rule_runner, py_version): print(sys.version.replace("\\n", " ")) """ ), - "src/BUILD": f"python_sources(interpreter_constraints=['=={py_version}.*'])", + "src/BUILD": f"python_sources(interpreter_constraints=['=={interpreter_constraints}'])", } ) @@ -87,7 +91,7 @@ def test_using_pyenv(rule_runner, py_version): ) prefix_dir, version = stdout.splitlines() assert prefix_dir.startswith(f"{named_caches_dir}/pyenv") - assert py_version in version + assert expected_version_substring in version def test_venv_pex_reconstruction(rule_runner): @@ -121,3 +125,21 @@ def test_venv_pex_reconstruction(rule_runner): shutil.rmtree(venv_location) stdout2 = run_run_request(rule_runner, target) assert stdout1 == stdout2 + + +def test_using_pyenv_with_incompatible_interpreter_constraints(rule_runner): + rule_runner.write_files( + { + "src/app.py": "", + # 3.7.17 was the final release in the 3.7 series: + # https://peps.python.org/pep-0537/#schedule-last-security-only-release + "src/BUILD": "python_sources(interpreter_constraints=['==3.7.20'])", + } + ) + + target = rule_runner.get_target(Address("src", relative_file_path="app.py")) + with pytest.raises( + ExecutionError, + match=r"(?si)couldn't find a Python 3.7 .* compatible with .* CPython==3.7.20 .* pyenv 2.3.13 .*latest known version 3.7.1[0-9].* Suggestion: .*", + ): + run_run_request(rule_runner, target)