Fix --elide-unused-requires-dist: don't expose partial extras.

A combination of `extra` environment marker evaluation and trimming of
any requires-dists that are not locked is used to fully prune the locked
graph of unused requires-dist entries.

CHANGES.md

@@ -1,5 +1,22 @@
 # Release Notes
 
+## 2.25.2
+
+This release fixes the `--elide-unused-requires-dist` lock option once
+again. The fix in 2.25.1 could lead to locked requirements having only
+a partial graph of extras which would allow a subsequent subset of those
+partial extras to silently resolve an incomplete set of dependencies.
+
+In addition, the Pex REPL for PEXes without entry points or else when
+forced with `PEX_INTERPRETER=1` is now fixed such that readline support
+always works. Previously, the yellow foreground color applied to the PS1
+and PS2 prompts would interfere with the tracked cursor position in some
+Pythons; so the yellow foreground color for those prompts is now
+dropped.
+
+* Fix `--elide-unused-requires-dist`: don't expose partial extras. (#2618)
+* Fix Pex REPL prompt. (#2617)
+
 ## 2.25.1
 
 This is a hotfix release that fixes a bug in the implementation of the

pex/resolve/lockfile/model.py

@@ -121,7 +121,7 @@ def extract_requirement(req):
             overridden=SortedTuple(overridden),
             locked_resolves=SortedTuple(
                 (
-                    requires_dist.remove_unused_requires_dist(locked_resolve)
+                    requires_dist.remove_unused_requires_dist(resolve_requirements, locked_resolve)
                     if elide_unused_requires_dist
                     else locked_resolve
                 )

pex/resolve/lockfile/requires_dist.py

@@ -3,36 +3,163 @@
 
 from __future__ import absolute_import
 
-from pex.resolve.locked_resolve import LockedResolve
+import operator
+from collections import defaultdict, deque
+
+from pex.dist_metadata import Requirement
+from pex.exceptions import production_assert
+from pex.orderedset import OrderedSet
+from pex.pep_503 import ProjectName
+from pex.resolve.locked_resolve import LockedRequirement, LockedResolve
 from pex.sorted_tuple import SortedTuple
-from pex.typing import TYPE_CHECKING
+from pex.third_party.packaging.markers import Marker, Variable
+from pex.typing import TYPE_CHECKING, cast
 
 if TYPE_CHECKING:
+    from typing import Callable, DefaultDict, Dict, Iterable, List, Optional, Tuple, Union
+
     import attr  # vendor:skip
+
+    EvalExtra = Callable[[ProjectName], bool]
 else:
     from pex.third_party import attr
 
 
-def remove_unused_requires_dist(locked_resolve):
-    # type: (LockedResolve) -> LockedResolve
+_OPERATORS = {
+    "in": lambda lhs, rhs: lhs in rhs,
+    "not in": lambda lhs, rhs: lhs not in rhs,
+    "<": operator.lt,
+    "<=": operator.le,
+    "==": operator.eq,
+    "!=": operator.ne,
+    ">=": operator.ge,
+    ">": operator.gt,
+}
+
+
+class _Op(object):
+    def __init__(self, lhs):
+        self.lhs = lhs  # type: EvalExtra
+        self.rhs = None  # type: Optional[EvalExtra]
+
+
+class _And(_Op):
+    def __call__(self, extra):
+        # type: (ProjectName) -> bool
+        production_assert(self.rhs is not None)
+        return self.lhs(extra) and cast("EvalExtra", self.rhs)(extra)
+
+
+class _Or(_Op):
+    def __call__(self, extra):
+        # type: (ProjectName) -> bool
+        production_assert(self.rhs is not None)
+        return self.lhs(extra) or cast("EvalExtra", self.rhs)(extra)
+
+
+def _parse_extra_item(
+    stack,  # type: List[EvalExtra]
+    item,  # type: Union[str, List, Tuple]
+    marker,  # type: Marker
+):
+    # type: (...) -> None
+
+    if item == "and":
+        stack.append(_And(stack.pop()))
+    elif item == "or":
+        stack.append(_Or(stack.pop()))
+    elif isinstance(item, list):
+        for element in item:
+            _parse_extra_item(stack, element, marker)
+    elif isinstance(item, tuple):
+        lhs, op, rhs = item
+        if isinstance(lhs, Variable) and "extra" == str(lhs):
+            check = lambda extra: _OPERATORS[str(op)](extra, ProjectName(str(rhs)))
+        elif isinstance(rhs, Variable) and "extra" == str(rhs):
+            check = lambda extra: _OPERATORS[str(op)](extra, ProjectName(str(lhs)))
+        else:
+            # Any other condition could potentially be true.
+            check = lambda _: True
+        if stack:
+            production_assert(isinstance(stack[-1], _Op))
+            cast(_Op, stack[-1]).rhs = check
+        else:
+            stack.append(check)
+    else:
+        raise ValueError("Marker is invalid: {marker}".format(marker=marker))
+
 
-    locked_projects = {
-        locked_req.pin.project_name for locked_req in locked_resolve.locked_requirements
+def _parse_extra_check(marker):
+    # type: (Marker) -> EvalExtra
+    checks = []  # type: List[EvalExtra]
+    for item in marker._markers:
+        _parse_extra_item(checks, item, marker)
+    production_assert(len(checks) == 1)
+    return checks[0]
+
+
+_EXTRA_CHECKS = {}  # type: Dict[str, EvalExtra]
+
+
+def _parse_marker_for_extra_check(marker):
+    # type: (Marker) -> EvalExtra
+    maker_str = str(marker)
+    eval_extra = _EXTRA_CHECKS.get(maker_str)
+    if not eval_extra:
+        eval_extra = _parse_extra_check(marker)
+        _EXTRA_CHECKS[maker_str] = eval_extra
+    return eval_extra
+
+
+def _evaluate_for_extras(
+    marker,  # type: Optional[Marker]
+    extras,  # type: Iterable[str]
+):
+    # type: (...) -> bool
+    if not marker:
+        return True
+    eval_extra = _parse_marker_for_extra_check(marker)
+    return any(eval_extra(ProjectName(extra)) for extra in (extras or [""]))
+
+
+def remove_unused_requires_dist(
+    resolve_requirements,  # type: Iterable[Requirement]
+    locked_resolve,  # type: LockedResolve
+):
+    # type: (...) -> LockedResolve
+
+    locked_req_by_project_name = {
+        locked_req.pin.project_name: locked_req for locked_req in locked_resolve.locked_requirements
     }
+    requires_dist_by_locked_req = defaultdict(
+        OrderedSet
+    )  # type: DefaultDict[LockedRequirement, OrderedSet[Requirement]]
+    seen = set()
+    requirements = deque(resolve_requirements)
+    while requirements:
+        requirement = requirements.popleft()
+        if requirement in seen:
+            continue
+
+        seen.add(requirement)
+        locked_req = locked_req_by_project_name.get(requirement.project_name)
+        if not locked_req:
+            continue
+
+        for dep in locked_req.requires_dists:
+            if dep.project_name in locked_req_by_project_name and _evaluate_for_extras(
+                dep.marker, requirement.extras
+            ):
+                requires_dist_by_locked_req[locked_req].add(dep)
+                requirements.append(dep)
+
     return attr.evolve(
         locked_resolve,
         locked_requirements=SortedTuple(
             attr.evolve(
                 locked_requirement,
                 requires_dists=SortedTuple(
-                    (
-                        requires_dist
-                        for requires_dist in locked_requirement.requires_dists
-                        # Otherwise, the requirement markers were never selected in the resolve
-                        # process; so the requirement was not locked.
-                        if requires_dist.project_name in locked_projects
-                    ),
-                    key=str,
+                    requires_dist_by_locked_req[locked_requirement], key=str
                 ),
             )
             for locked_requirement in locked_resolve.locked_requirements

pex/version.py

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

tests/resolve/lockfile/test_requires_dist.py

@@ -46,7 +46,7 @@ def test_remove_unused_requires_dist_noop():
         locked_req("baz", "1.0"),
     )
     assert locked_resolve_with_no_extras == requires_dist.remove_unused_requires_dist(
-        locked_resolve_with_no_extras
+        resolve_requirements=[req("foo")], locked_resolve=locked_resolve_with_no_extras
     )
 
 
@@ -58,7 +58,8 @@ def test_remove_unused_requires_dist_simple():
         locked_req("bar", "1.0"),
         locked_req("spam", "1.0"),
     ) == requires_dist.remove_unused_requires_dist(
-        locked_resolve(
+        resolve_requirements=[req("foo")],
+        locked_resolve=locked_resolve(
             locked_req("foo", "1.0", "bar", "baz; extra == 'tests'", "spam"),
             locked_req("bar", "1.0"),
             locked_req("spam", "1.0"),
@@ -74,7 +75,8 @@ def test_remove_unused_requires_dist_mixed_extras():
         locked_req("bar", "1.0"),
         locked_req("spam", "1.0"),
     ) == requires_dist.remove_unused_requires_dist(
-        locked_resolve(
+        resolve_requirements=[req("foo[extra1]")],
+        locked_resolve=locked_resolve(
             locked_req("foo", "1.0", "bar; extra == 'extra1'", "baz; extra == 'tests'", "spam"),
             locked_req("bar", "1.0"),
             locked_req("spam", "1.0"),
@@ -97,7 +99,8 @@ def test_remove_unused_requires_dist_mixed_markers():
         locked_req("baz", "1.0"),
         locked_req("spam", "1.0"),
     ) == requires_dist.remove_unused_requires_dist(
-        locked_resolve(
+        resolve_requirements=[req("foo[extra1]")],
+        locked_resolve=locked_resolve(
             locked_req(
                 "foo",
                 "1.0",
@@ -124,7 +127,8 @@ def test_remove_unused_requires_dist_mixed_markers():
         locked_req("bar", "1.0"),
         locked_req("spam", "1.0"),
     ) == requires_dist.remove_unused_requires_dist(
-        locked_resolve(
+        resolve_requirements=[req("foo[extra1]")],
+        locked_resolve=locked_resolve(
             locked_req(
                 "foo",
                 "1.0",
@@ -151,7 +155,8 @@ def test_remove_unused_requires_dist_complex_markers():
         locked_req("bar", "1.0"),
         locked_req("spam", "1.0"),
     ) == requires_dist.remove_unused_requires_dist(
-        locked_resolve(
+        resolve_requirements=[req("foo")],
+        locked_resolve=locked_resolve(
             locked_req(
                 "foo",
                 "1.0",
@@ -173,7 +178,8 @@ def test_remove_unused_requires_dist_not_present_due_to_other_markers():
         locked_req("bar", "1.0"),
         locked_req("spam", "1.0"),
     ) == requires_dist.remove_unused_requires_dist(
-        locked_resolve(
+        resolve_requirements=[req("foo")],
+        locked_resolve=locked_resolve(
             locked_req(
                 "foo",
                 "1.0",