Fix resolve check to cover dists satisfying root reqs. (#2610)

CHANGES.md

@@ -1,5 +1,17 @@
 # Release Notes
 
+## 2.24.3
+
+This release fixes a long-standing bug in resolve checking. Previously,
+only resolve dependency chains where checked, but not the resolved
+distributions that satisfied the input root requirements.
+
+In addition, the 2.24.2 release included a wheel with no compression
+(~11MB instead of ~3.5MB). The Pex wheel is now fixed to be compressed.
+
+* Fix resolve check to cover dists satisfying root reqs. (#2610)
+* Fix build process to produce a compressed `.whl`. (#2609)
+
 ## 2.24.2
 
 This release fixes a long-standing bug in "YOLO-mode" foreign platform

pex/resolve/resolvers.py

@@ -92,6 +92,7 @@ def check_resolve(
     resolved_distributions,  # type: Iterable[ResolvedDistribution]
 ):
     # type: (...) -> None
+
     resolved_distributions_by_project_name = (
         OrderedDict()
     )  # type: OrderedDict[ProjectName, List[ResolvedDistribution]]
@@ -102,11 +103,77 @@ def check_resolve(
 
     maybe_unsatisfied = defaultdict(list)  # type: DefaultDict[Target, List[str]]
     unsatisfied = defaultdict(list)  # type: DefaultDict[Target, List[str]]
+
+    def check(
+        target,  # type: Target
+        dist,  # type: Distribution
+        requirement,  # type: Requirement
+        root=False,  # type: bool
+    ):
+        # type: (...) -> None
+
+        required_by = (
+            "{requirement} was requested".format(requirement=requirement)
+            if root
+            else "{dist} requires {requirement}".format(dist=dist, requirement=requirement)
+        )
+        installed_requirement_dists = resolved_distributions_by_project_name.get(
+            requirement.project_name
+        )
+        if not installed_requirement_dists:
+            unsatisfied[target].append(
+                "{required_by} but no version was resolved".format(required_by=required_by)
+            )
+        else:
+            resolved_dists = [
+                installed_requirement_dist.distribution
+                for installed_requirement_dist in installed_requirement_dists
+            ]
+            version_matches = any(
+                (
+                    # We don't attempt a match against a legacy version in order to avoid false
+                    # negatives.
+                    resolved_dist.metadata.version.is_legacy
+                    or requirement.specifier.contains(resolved_dist.version, prereleases=True)
+                )
+                for resolved_dist in resolved_dists
+            )
+            tags_match = any(
+                target.wheel_applies(resolved_dist) for resolved_dist in resolved_dists
+            )
+            if not version_matches or not tags_match:
+                message = (
+                    "{required_by} but {count} incompatible {dists_were} resolved:\n"
+                    "        {dists}".format(
+                        required_by=required_by,
+                        count=len(resolved_dists),
+                        dists_were="dists were" if len(resolved_dists) > 1 else "dist was",
+                        dists="\n        ".join(
+                            os.path.basename(resolved_dist.location)
+                            for resolved_dist in resolved_dists
+                        ),
+                    )
+                )
+                if version_matches and not tags_match and isinstance(target, AbbreviatedPlatform):
+                    # We don't know for sure an abbreviated platform doesn't match a wheels tags
+                    # until we are running on that platform; so just warn for these instead of
+                    # hard erroring.
+                    maybe_unsatisfied[target].append(message)
+                else:
+                    unsatisfied[target].append(message)
+
     for resolved_distribution in itertools.chain.from_iterable(
         resolved_distributions_by_project_name.values()
     ):
-        dist = resolved_distribution.distribution
         target = resolved_distribution.target
+        for root_req in resolved_distribution.direct_requirements:
+            check(
+                target=target,
+                dist=resolved_distribution.distribution,
+                requirement=root_req,
+                root=True,
+            )
+        dist = resolved_distribution.distribution
 
         for requirement in dist.requires():
             if dependency_configuration.excluded_by(requirement):
@@ -116,53 +183,7 @@ def check_resolve(
             )
             if not target.requirement_applies(requirement):
                 continue
-
-            installed_requirement_dists = resolved_distributions_by_project_name.get(
-                requirement.project_name
-            )
-            if not installed_requirement_dists:
-                unsatisfied[target].append(
-                    "{dist} requires {requirement} but no version was resolved".format(
-                        dist=dist.as_requirement(), requirement=requirement
-                    )
-                )
-            else:
-                resolved_dists = [
-                    installed_requirement_dist.distribution
-                    for installed_requirement_dist in installed_requirement_dists
-                ]
-                version_matches = any(
-                    requirement.specifier.contains(resolved_dist.version, prereleases=True)
-                    for resolved_dist in resolved_dists
-                )
-                tags_match = any(
-                    target.wheel_applies(resolved_dist) for resolved_dist in resolved_dists
-                )
-                if not version_matches or not tags_match:
-                    message = (
-                        "{dist} requires {requirement} but {count} incompatible {dists_were} "
-                        "resolved:\n        {dists}".format(
-                            dist=dist,
-                            requirement=requirement,
-                            count=len(resolved_dists),
-                            dists_were="dists were" if len(resolved_dists) > 1 else "dist was",
-                            dists="\n        ".join(
-                                os.path.basename(resolved_dist.location)
-                                for resolved_dist in resolved_dists
-                            ),
-                        )
-                    )
-                    if (
-                        version_matches
-                        and not tags_match
-                        and isinstance(target, AbbreviatedPlatform)
-                    ):
-                        # We don't know for sure an abbreviated platform doesn't match a wheels tags
-                        # until we are running on that platform; so just warn for these instead of
-                        # hard erroring.
-                        maybe_unsatisfied[target].append(message)
-                    else:
-                        unsatisfied[target].append(message)
+            check(target, dist, requirement)
 
     if unsatisfied:
         unsatisfieds = []

pex/resolver.py

@@ -1029,10 +1029,11 @@ def add_installation(install_result):
             except WheelError as e:
                 raise Untranslatable("Failed to install a wheel: {err}".format(err=e))
 
+        installations = list(direct_requirements.adjust(installations))
         if not ignore_errors:
             with TRACER.timed("Checking install"):
                 check_resolve(self._dependency_configuration, installations)
-        return direct_requirements.adjust(installations)
+        return installations
 
 
 def _parse_reqs(

pex/version.py

@@ -1,4 +1,4 @@
 # Copyright 2015 Pex project contributors.
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
-__version__ = "2.24.2"
+__version__ = "2.24.3"

tests/integration/resolve/test_issue_2532.py

@@ -99,6 +99,9 @@ def test_resolved_wheel_tag_platform_mismatch_warns(
             """\
             PEXWarning: The resolved distributions for 1 target may not be compatible:
             1: abbreviated platform cp311-cp311-manylinux_2_28_x86_64 may not be compatible with:
+                cffi==1.16.0 was requested but 2 incompatible dists were resolved:
+                    cffi-1.16.0-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
+                    cffi-1.16.0-cp311-cp311-linux_x86_64.whl
                 cryptography 42.0.8 requires cffi>=1.12; platform_python_implementation != "PyPy" but 2 incompatible dists were resolved:
                     cffi-1.16.0-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
                     cffi-1.16.0-cp311-cp311-linux_x86_64.whl

tests/integration/test_issue_940.py

@@ -51,10 +51,9 @@ def prepare_project(project_dir):
     ) as whl:
         pex_root = os.path.join(str(tmpdir), "pex_root")
         pex_file = os.path.join(str(tmpdir), "pex")
-        results = run_pex_command(
+        run_pex_command(
             args=["-o", pex_file, "--pex-root", pex_root, "--runtime-pex-root", pex_root, whl]
-        )
-        results.assert_success()
+        ).assert_success()
 
         output, returncode = run_simple_pex(pex_file, args=["-c", "import foo"])
         assert returncode == 0, output

tests/integration/test_issue_996.py

@@ -3,9 +3,12 @@
 
 import multiprocessing
 import os
+import re
 import subprocess
+from textwrap import dedent
 
 from pex.interpreter import PythonInterpreter
+from pex.targets import AbbreviatedPlatform
 from pex.typing import TYPE_CHECKING
 from testing import PY39, PY310, IntegResults, ensure_python_interpreter, make_env, run_pex_command
 from testing.pytest.tmp import Tempdir
@@ -20,10 +23,13 @@ def test_resolve_local_platform(tmpdir):
     python310 = ensure_python_interpreter(PY310)
     pex_python_path = os.pathsep.join((python39, python310))
 
+    platform = PythonInterpreter.from_binary(python310).platform
+    target = AbbreviatedPlatform.create(platform)
+
     def create_platform_pex(args):
         # type: (List[str]) -> IntegResults
         return run_pex_command(
-            args=["--platform", str(PythonInterpreter.from_binary(python310).platform)] + args,
+            args=["--platform", str(platform)] + args,
             python=python39,
             env=make_env(PEX_PYTHON_PATH=pex_python_path),
         )
@@ -34,11 +40,25 @@ def create_platform_pex(args):
     # distribution has no dependencies.
     build_args = ["psutil==5.7.0", "-o", pex_file]
     check_args = [python310, pex_file, "-c", "import psutil; print(psutil.cpu_count())"]
-    check_env = make_env(PEX_PYTHON_PATH=python310, PEX_VERBOSE=1)
+    check_env = make_env(PEX_PYTHON_PATH=python310)
 
     # By default, no --platforms are resolved and so the yolo build process will produce a wheel
-    # using Python 3.9, which is not compatible with Python 3.10.
-    create_platform_pex(build_args).assert_success()
+    # using Python 3.9, which is not compatible with Python 3.10. Since we can't be sure of this,
+    # we allow the build but warn it may be incompatible.
+    create_platform_pex(build_args).assert_success(
+        expected_error_re=dedent(
+            r"""
+            .* PEXWarning: The resolved distributions for 1 target may not be compatible:
+            1: {target} may not be compatible with:
+                psutil==5\.7\.0 was requested but 1 incompatible dist was resolved:
+                    psutil-5\.7\.0-cp39-cp39-.+\.whl
+            .*
+            """
+        )
+        .format(target=re.escape(target.render_description()))
+        .strip(),
+        re_flags=re.DOTALL | re.MULTILINE,
+    )
     process = subprocess.Popen(check_args, env=check_env, stderr=subprocess.PIPE)
     _, stderr = process.communicate()
     error = stderr.decode("utf-8")