Support for using a single global resolve. (#10405)

benjyw · web-flow · commit 45ca5b494020 · 2020-07-20T23:30:03.000-07:00
Introduces a --python-setup-use-all-requirements flag.
If set, instead of resolving just the exact requirements
needed, we resolve the entire lockfile.

This prevents a huge amount of resolving work of all
the various requirement subsets needed by specific tests.

[ci skip-rust-tests]
diff --git a/pants.toml b/pants.toml
@@ -53,6 +53,11 @@ root_patterns = [
   "/build-support/migration-support"
 ]
 
+[python-setup]
+# TODO: Set up an actual lockfile. For now we just "lock" our requirements.txt to itself,
+#  so we can use --python-setup-resolve-all-constraints (which requires a lockfile).
+requirement_constraints = "3rdparty/python/requirements.txt"
+
 [black]
 config = "pyproject.toml"
 
diff --git a/src/python/pants/backend/python/rules/pex_from_targets.py b/src/python/pants/backend/python/rules/pex_from_targets.py
@@ -2,9 +2,12 @@
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
 import dataclasses
+import logging
 from dataclasses import dataclass
 from typing import Iterable, Optional, Tuple
 
+from pkg_resources import Requirement, parse_requirements
+
 from pants.backend.python.rules.pex import (
     PexInterpreterConstraints,
     PexPlatforms,
@@ -21,13 +24,17 @@
     PythonRequirementsField,
 )
 from pants.engine.addresses import Addresses
-from pants.engine.fs import Digest, MergeDigests
+from pants.engine.fs import Digest, DigestContents, MergeDigests, PathGlobs, Snapshot
 from pants.engine.rules import RootRule, rule
 from pants.engine.selectors import Get
 from pants.engine.target import TransitiveTargets
+from pants.option.custom_types import GlobExpansionConjunction
+from pants.option.global_options import GlobMatchErrorBehavior
 from pants.python.python_setup import PythonSetup
 from pants.util.meta import frozen_after_init
 
+logger = logging.getLogger(__name__)
+
 
 @frozen_after_init
 @dataclass(unsafe_hash=True)
@@ -59,7 +66,7 @@ def __init__(
         include_source_files: bool = True,
         additional_sources: Optional[Digest] = None,
         additional_inputs: Optional[Digest] = None,
-        description: Optional[str] = None
+        description: Optional[str] = None,
     ) -> None:
         self.addresses = addresses
         self.output_filename = output_filename
@@ -112,7 +119,7 @@ async def pex_from_targets(request: PexFromTargetsRequest, python_setup: PythonS
         python_setup,
     )
 
-    requirements = PexRequirements.create_from_requirement_fields(
+    exact_reqs = PexRequirements.create_from_requirement_fields(
         (
             tgt[PythonRequirementsField]
             for tgt in all_targets
@@ -121,6 +128,47 @@ async def pex_from_targets(request: PexFromTargetsRequest, python_setup: PythonS
         additional_requirements=request.additional_requirements,
     )
 
+    requirements = exact_reqs
+
+    if python_setup.requirement_constraints:
+        exact_req_projects = {Requirement.parse(req).project_name for req in exact_reqs}
+        constraint_file_snapshot = await Get(
+            Snapshot,
+            PathGlobs(
+                [python_setup.requirement_constraints],
+                glob_match_error_behavior=GlobMatchErrorBehavior.error,
+                conjunction=GlobExpansionConjunction.all_match,
+                description_of_origin="the option `--python-setup-requirement-constraints`",
+            ),
+        )
+        constraints_file_contents = await Get(
+            DigestContents, Digest, constraint_file_snapshot.digest
+        )
+        constraints_file_reqs = set(
+            parse_requirements(next(iter(constraints_file_contents)).content.decode())
+        )
+        constraint_file_projects = {req.project_name for req in constraints_file_reqs}
+        unconstrained_projects = exact_req_projects - constraint_file_projects
+        if unconstrained_projects:
+            logger.warning(
+                f"The constraints file {python_setup.requirement_constraints} does not contain "
+                f"entries for the following requirements: {', '.join(unconstrained_projects)}"
+            )
+
+        if python_setup.options.resolve_all_constraints:
+            if unconstrained_projects:
+                logger.warning(
+                    "Ignoring resolve_all_constraints setting in [python_setup] scope"
+                    "Because constraints file does not cover all requirements."
+                )
+            else:
+                requirements = PexRequirements(str(req) for req in constraints_file_reqs)
+    elif python_setup.options.resolve_all_constraints:
+        raise ValueError(
+            "resolve_all_constraints in the [python-setup] scope is set, so "
+            "requirement_constraints in [python-setup] must also be provided."
+        )
+
     return PexRequest(
         output_filename=request.output_filename,
         requirements=requirements,
diff --git a/src/python/pants/backend/python/rules/pex_from_targets_test.py b/src/python/pants/backend/python/rules/pex_from_targets_test.py
@@ -0,0 +1,112 @@
+# Copyright 2020 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+from textwrap import dedent
+from typing import Optional
+
+from pants.backend.python.rules import pex_from_targets, python_sources
+from pants.backend.python.rules.pex import PexRequest, PexRequirements
+from pants.backend.python.rules.pex_from_targets import PexFromTargetsRequest
+from pants.backend.python.rules.python_sources import StrippedPythonSourcesRequest
+from pants.backend.python.target_types import PythonLibrary, PythonRequirementLibrary
+from pants.build_graph.address import Address
+from pants.build_graph.build_file_aliases import BuildFileAliases
+from pants.engine.addresses import Addresses
+from pants.engine.internals.scheduler import ExecutionError
+from pants.engine.rules import RootRule, SubsystemRule
+from pants.engine.selectors import Params
+from pants.python.python_requirement import PythonRequirement
+from pants.python.python_setup import PythonSetup
+from pants.testutil.option.util import create_options_bootstrapper
+from pants.testutil.test_base import TestBase
+
+
+class PexTest(TestBase):
+    @classmethod
+    def rules(cls):
+        return (
+            *super().rules(),
+            *pex_from_targets.rules(),
+            *python_sources.rules(),
+            RootRule(PexFromTargetsRequest),
+            RootRule(StrippedPythonSourcesRequest),
+            SubsystemRule(PythonSetup),
+        )
+
+    @classmethod
+    def alias_groups(cls):
+        return BuildFileAliases(objects={"python_requirement": PythonRequirement})
+
+    @classmethod
+    def target_types(cls):
+        return [PythonLibrary, PythonRequirementLibrary]
+
+    def test_constraints_validation(self) -> None:
+
+        self.add_to_build_file(
+            "",
+            dedent(
+                """
+                python_requirement_library(name="foo",
+                    requirements=[python_requirement("foo>=0.1.2")])
+                python_requirement_library(name="bar",
+                    requirements=[ python_requirement("bar==5.5.5")])
+                python_requirement_library(name="baz",
+                    requirements=[python_requirement("baz")])
+                python_library(name="tgt", sources=[], dependencies=[":foo", ":bar", ":baz"])
+                """
+            ),
+        )
+        self.create_file(
+            "constraints1.txt",
+            dedent(
+                """
+                foo==1.0.0
+                bar==5.5.5
+                baz==2.2.2
+                qux==3.4.5
+            """
+            ),
+        )
+        self.create_file(
+            "constraints2.txt",
+            dedent(
+                """
+                foo==1.0.0
+                bar==5.5.5
+                qux==3.4.5
+            """
+            ),
+        )
+
+        request = PexFromTargetsRequest(
+            Addresses((Address.parse("//:tgt"),)), output_filename="dummy.pex"
+        )
+
+        def get_pex_request(constraints_file: Optional[str], resolve_all: bool) -> PexRequest:
+            args = [
+                "--backend-packages=pants.backend.python",
+                f"--python-setup-resolve-all-constraints={resolve_all}",
+            ]
+            if constraints_file:
+                args.append(f"--python-setup-requirement-constraints={constraints_file}")
+            return self.request_single_product(
+                PexRequest, Params(request, create_options_bootstrapper(args=args))
+            )
+
+        pex_req1 = get_pex_request("constraints1.txt", False)
+        assert pex_req1.requirements == PexRequirements(["foo>=0.1.2", "bar==5.5.5", "baz"])
+
+        pex_req2 = get_pex_request("constraints1.txt", True)
+        assert pex_req2.requirements == PexRequirements(
+            ["foo==1.0.0", "bar==5.5.5", "baz==2.2.2", "qux==3.4.5"]
+        )
+
+        with self.assertRaises(ExecutionError) as err:
+            get_pex_request(None, True)
+        assert len(err.exception.wrapped_exceptions) == 1
+        assert isinstance(err.exception.wrapped_exceptions[0], ValueError)
+        assert (
+            "resolve_all_constraints in the [python-setup] scope is set, so "
+            "requirement_constraints in [python-setup] must also be provided."
+        ) in str(err.exception)
diff --git a/src/python/pants/engine/fs.py b/src/python/pants/engine/fs.py
@@ -70,7 +70,7 @@ def __init__(
         :param globs: globs to match, e.g. `foo.txt` or `**/*.txt`. To exclude something, prefix it
                       with `!`, e.g. `!ignore.py`.
         :param glob_match_error_behavior: whether to warn or error upon match failures
-        :param conjunction: whether all `include`s must match or only at least one must match
+        :param conjunction: whether all `globs` must match or only at least one must match
         :param description_of_origin: a human-friendly description of where this PathGlobs request
                                       is coming from, used to improve the error message for
                                       unmatched globs. For example, this might be the text string
diff --git a/src/python/pants/python/python_setup.py b/src/python/pants/python/python_setup.py
@@ -50,6 +50,24 @@ def register_options(cls, register):
                 "on the format of constraint files and how constraints are applied in Pex and Pip."
             ),
         )
+        register(
+            "--resolve-all-constraints",
+            advanced=True,
+            default=False,
+            type=bool,
+            help=(
+                "If set, and the requirements of the code being operated on are a subset of the "
+                "constraints file, then the entire constraints file will be used instead of the "
+                "subset. If unset, or any requirement of the code being operated on is not in the "
+                "constraints file, each subset will be independently resolved as needed, which is "
+                "more correct - work is only invalidated if a requirement it actually depends on "
+                "changes - but also a lot slower, due to the extra resolving. "
+                "You may wish to leave this option set for normal work, such as running tests, "
+                "but selectively turn it off via command-line-flag when building deployable "
+                "binaries, so that you only deploy the requirements you actually need for a "
+                "given binary."
+            ),
+        )
         register(
             "--platforms",
             advanced=True,
