Allow plugins to use range requirements by applying constraints to pl…

…ugin resolution (#14058)

As discussed in #14053, currently if a plugin uses a range requirement (even if it matches the requirement used by Pants itself), a different version might be chosen than what is already installed in the virtualenv for Pants, triggering a conflict when we try to add it to the working set (i.e. `sys.path`).

This change uses the current working set to constrain the plugin resolve, so that plugins with range requirements will choose matching versions from those ranges (if possible). To do that, it consumes #14064 to specify the working set as constraints in the `PluginResolver`.

Fixes #14053.

[ci skip-rust]

src/python/pants/init/plugin_resolver.py

@@ -1,13 +1,15 @@
 # Copyright 2015 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
+from __future__ import annotations
+
 import logging
 import site
 import sys
 from dataclasses import dataclass
-from typing import Optional, TypeVar, cast
+from typing import Optional, cast
 
-from pkg_resources import WorkingSet
+from pkg_resources import Requirement, WorkingSet
 from pkg_resources import working_set as global_working_set
 
 from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints
@@ -21,20 +23,19 @@
 from pants.init.bootstrap_scheduler import BootstrapScheduler
 from pants.option.global_options import GlobalOptions
 from pants.option.options_bootstrapper import OptionsBootstrapper
-from pants.option.subsystem import Subsystem
 from pants.util.logging import LogLevel
 
 logger = logging.getLogger(__name__)
 
 
-S = TypeVar("S", bound=Subsystem)
-
-
 @dataclass(frozen=True)
 class PluginsRequest:
     # Interpreter constraints to resolve for, or None to resolve for the interpreter that Pants is
     # running under.
-    interpreter_constraints: Optional[InterpreterConstraints] = None
+    interpreter_constraints: InterpreterConstraints | None
+    # Requirement constraints to resolve with. If plugins will be loaded into the global working_set
+    # (i.e., onto the `sys.path`), then these should be the current contents of the working_set.
+    constraints: tuple[Requirement, ...]
 
 
 class ResolvedPluginDistributions(DeduplicatedCollection[str]):
@@ -53,6 +54,7 @@ async def resolve_plugins(
     """
     requirements = PexRequirements(
         req_strings=sorted(global_options.options.plugins),
+        constraints_strings=(str(constraint) for constraint in request.constraints),
     )
     if not requirements:
         return ResolvedPluginDistributions()
@@ -100,27 +102,22 @@ async def resolve_plugins(
 
 
 class PluginResolver:
-    def __init__(
-        self,
-        scheduler: BootstrapScheduler,
-        interpreter_constraints: Optional[InterpreterConstraints] = None,
-    ) -> None:
+    def __init__(self, scheduler: BootstrapScheduler) -> None:
         self._scheduler = scheduler
-        self._request = PluginsRequest(interpreter_constraints)
 
     def resolve(
         self,
         options_bootstrapper: OptionsBootstrapper,
         env: CompleteEnvironment,
+        interpreter_constraints: Optional[InterpreterConstraints] = None,
         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).
-        """
+        """Resolves any configured plugins and adds them to the working_set."""
         working_set = working_set or global_working_set
-        for resolved_plugin_location in self._resolve_plugins(options_bootstrapper, env):
+        request = PluginsRequest(
+            interpreter_constraints, tuple(dist.as_requirement() for dist in working_set)
+        )
+        for resolved_plugin_location in self._resolve_plugins(options_bootstrapper, env, request):
             site.addsitedir(
                 resolved_plugin_location
             )  # Activate any .pth files plugin wheels may have.
@@ -131,6 +128,7 @@ def _resolve_plugins(
         self,
         options_bootstrapper: OptionsBootstrapper,
         env: CompleteEnvironment,
+        request: PluginsRequest,
     ) -> ResolvedPluginDistributions:
         session = self._scheduler.scheduler.new_session(
             "plugin_resolver",
@@ -143,7 +141,7 @@ def _resolve_plugins(
         )
         return cast(
             ResolvedPluginDistributions,
-            session.product_request(ResolvedPluginDistributions, [self._request])[0],
+            session.product_request(ResolvedPluginDistributions, [request])[0],
         )
 
 

tests/python/pants_test/init/test_plugin_resolver.py

@@ -1,17 +1,20 @@
 # Copyright 2015 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
+from __future__ import annotations
+
 import os
 import shutil
 import sys
 from contextlib import contextmanager
+from dataclasses import dataclass
 from pathlib import Path, PurePath
 from textwrap import dedent
-from typing import Dict, Iterable, Optional
+from typing import Dict, Sequence
 
 import pytest
 from pex.interpreter import PythonInterpreter
-from pkg_resources import Requirement, WorkingSet
+from pkg_resources import Distribution, Requirement, WorkingSet
 
 from pants.backend.python.util_rules import pex
 from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints
@@ -72,18 +75,20 @@ def _run_setup_py(
     rule_runner: RuleRunner,
     plugin: str,
     interpreter_constraints: InterpreterConstraints,
-    version: Optional[str],
-    setup_py_args: Iterable[str],
+    version: str | None,
+    install_requires: Sequence[str] | None,
+    setup_py_args: Sequence[str],
     install_dir: str,
 ) -> None:
     pex_obj = _create_pex(rule_runner, interpreter_constraints)
+    install_requires_str = f", install_requires={install_requires!r}" if install_requires else ""
     setup_py_file = FileContent(
         "setup.py",
         dedent(
             f"""
                 from setuptools import setup
 
-                setup(name="{plugin}", version="{version or DEFAULT_VERSION}")
+                setup(name="{plugin}", version="{version or DEFAULT_VERSION}"{install_requires_str})
             """
         ).encode(),
     )
@@ -108,9 +113,23 @@ def _run_setup_py(
         shutil.copy(PurePath(rule_runner.build_root, "output", path), install_dir)
 
 
+@dataclass
+class Plugin:
+    name: str
+    version: str | None = None
+    install_requires: list[str] | None = None
+
+
 @contextmanager
 def plugin_resolution(
-    rule_runner: RuleRunner, *, interpreter=None, chroot=None, plugins=None, sdist=True
+    rule_runner: RuleRunner,
+    *,
+    interpreter: PythonInterpreter | None = None,
+    chroot: str | None = None,
+    plugins: Sequence[Plugin] = (),
+    sdist: bool = True,
+    working_set_entries: Sequence[Distribution] = (),
+    use_pypi: bool = False,
 ):
     @contextmanager
     def provide_chroot(existing):
@@ -135,22 +154,22 @@ def provide_chroot(existing):
             repo_dir = os.path.join(root_dir, "repo")
             env.update(
                 PANTS_PYTHON_REPOS_REPOS=f"['file://{repo_dir}']",
-                PANTS_PYTHON_REPOS_INDEXES="[]",
                 PANTS_PYTHON_RESOLVER_CACHE_TTL="1",
             )
+            if not use_pypi:
+                env.update(PANTS_PYTHON_REPOS_INDEXES="[]")
             plugin_list = []
             for plugin in plugins:
-                version = None
-                if isinstance(plugin, tuple):
-                    plugin, version = plugin
-                plugin_list.append(f"{plugin}=={version}" if version else plugin)
+                version = plugin.version
+                plugin_list.append(f"{plugin.name}=={version}" if version else plugin.name)
                 if create_artifacts:
                     setup_py_args = ["sdist" if sdist else "bdist_wheel", "--dist-dir", "dist/"]
                     _run_setup_py(
                         rule_runner,
-                        plugin,
+                        plugin.name,
                         artifact_interpreter_constraints,
                         version,
+                        plugin.install_requires,
                         setup_py_args,
                         repo_dir,
                     )
@@ -166,13 +185,17 @@ def provide_chroot(existing):
             {**{k: os.environ[k] for k in ["PATH", "HOME", "PYENV_ROOT"] if k in os.environ}, **env}
         )
         bootstrap_scheduler = create_bootstrap_scheduler(options_bootstrapper)
-        plugin_resolver = PluginResolver(
-            bootstrap_scheduler, interpreter_constraints=interpreter_constraints
-        )
+        plugin_resolver = PluginResolver(bootstrap_scheduler)
         cache_dir = options_bootstrapper.bootstrap_options.for_global_scope().named_caches_dir
 
+        input_working_set = WorkingSet(entries=[])
+        for dist in working_set_entries:
+            input_working_set.add(dist)
         working_set = plugin_resolver.resolve(
-            options_bootstrapper, complete_env, WorkingSet(entries=[])
+            options_bootstrapper,
+            complete_env,
+            interpreter_constraints,
+            input_working_set,
         )
         for dist in working_set:
             assert (
@@ -196,7 +219,9 @@ def test_plugins_bdist(rule_runner: RuleRunner) -> None:
 
 
 def _do_test_plugins(rule_runner: RuleRunner, sdist: bool) -> None:
-    with plugin_resolution(rule_runner, plugins=[("jake", "1.2.3"), "jane"], sdist=sdist) as (
+    with plugin_resolution(
+        rule_runner, plugins=[Plugin("jake", "1.2.3"), Plugin("jane")], sdist=sdist
+    ) as (
         working_set,
         _,
         _,
@@ -220,24 +245,42 @@ def test_exact_requirements_bdist(rule_runner: RuleRunner) -> None:
 
 def _do_test_exact_requirements(rule_runner: RuleRunner, sdist: bool) -> None:
     with plugin_resolution(
-        rule_runner, plugins=[("jake", "1.2.3"), ("jane", "3.4.5")], sdist=sdist
+        rule_runner, plugins=[Plugin("jake", "1.2.3"), Plugin("jane", "3.4.5")], sdist=sdist
     ) as results:
         working_set, chroot, repo_dir = results
 
-        # Kill the repo source dir and re-resolve.  If the PluginResolver truly detects exact
+        # Kill the repo source dir and re-resolve. If the PluginResolver truly detects exact
         # requirements it should skip any resolves and load directly from the still intact
         # cache.
         safe_rmtree(repo_dir)
 
         with plugin_resolution(
-            rule_runner, chroot=chroot, plugins=[("jake", "1.2.3"), ("jane", "3.4.5")]
+            rule_runner, chroot=chroot, plugins=[Plugin("jake", "1.2.3"), Plugin("jane", "3.4.5")]
         ) as results2:
 
             working_set2, _, _ = results2
 
             assert list(working_set) == list(working_set2)
 
 
+def test_range_deps(rule_runner: RuleRunner) -> None:
+    # Test that when a plugin has a range dependency, specifying a working set constrains
+    # to a particular version, where otherwise we would get the highest released (2.27.0 in
+    # this case).
+    with plugin_resolution(
+        rule_runner,
+        plugins=[Plugin("jane", "3.4.5", ["requests>=2.25.1,<2.28.0"])],
+        working_set_entries=[Distribution(project_name="requests", version="2.26.0")],
+        # Because we're resolving real distributions, we enable access to pypi.
+        use_pypi=True,
+    ) as (
+        working_set,
+        _,
+        _,
+    ):
+        assert "2.26.0" == working_set.find(Requirement.parse("requests")).version
+
+
 @skip_unless_python36_and_python37_present
 def test_exact_requirements_interpreter_change_sdist(rule_runner: RuleRunner) -> None:
     _do_test_exact_requirements_interpreter_change(rule_runner, True)
@@ -255,7 +298,7 @@ def _do_test_exact_requirements_interpreter_change(rule_runner: RuleRunner, sdis
     with plugin_resolution(
         rule_runner,
         interpreter=python36,
-        plugins=[("jake", "1.2.3"), ("jane", "3.4.5")],
+        plugins=[Plugin("jake", "1.2.3"), Plugin("jane", "3.4.5")],
         sdist=sdist,
     ) as results:
 
@@ -267,7 +310,7 @@ def _do_test_exact_requirements_interpreter_change(rule_runner: RuleRunner, sdis
                 rule_runner,
                 interpreter=python37,
                 chroot=chroot,
-                plugins=[("jake", "1.2.3"), ("jane", "3.4.5")],
+                plugins=[Plugin("jake", "1.2.3"), Plugin("jane", "3.4.5")],
             ):
                 pytest.fail(
                     "Plugin re-resolution is expected for an incompatible interpreter and it is "
@@ -280,7 +323,7 @@ def _do_test_exact_requirements_interpreter_change(rule_runner: RuleRunner, sdis
             rule_runner,
             interpreter=python36,
             chroot=chroot,
-            plugins=[("jake", "1.2.3"), ("jane", "3.4.5")],
+            plugins=[Plugin("jake", "1.2.3"), Plugin("jane", "3.4.5")],
         ) as results2:
 
             working_set2, _, _ = results2