feat: combine and validator constraints

Fromager's `Constraints` class now supports combining multiple
constraints and performs stricter validation of constraints. The class
now refuses constraints without a version specifier, extras, or an URL.
It packaging's new `SpecifierSet.is_unsatisfiable()` feature to detect
constraints that are not satisfiable.

Example:
The constraints `foo>=1.0`, `foo<2.0`, and `foo!=1.1.0` are combined
into to `foo<2.0,>=1.0,!=1.1.0`. A combination of `foo<1.0` and
`foo>=2.0` would result in a `InvalidConstraintError` instead.

See: #1096
Signed-off-by: Christian Heimes <cheimes@redhat.com>

Author: tiran

pyproject.toml

@@ -34,7 +34,7 @@ dependencies = [
     "click>=8.1.7",
     "elfdeps>=0.2.0",
     "license-expression",
-    "packaging",
+    "packaging>=26.1",
     "packageurl-python",
     "psutil",
     "pydantic>=2.12",

src/fromager/constraints.py

@@ -11,6 +11,10 @@
 logger = logging.getLogger(__name__)
 
 
+class InvalidConstraintError(ValueError):
+    pass
+
+
 class Constraints:
     def __init__(self) -> None:
         # mapping of canonical names to requirements
@@ -21,22 +25,46 @@ def __iter__(self) -> Generator[NormalizedName, None, None]:
         yield from self._data
 
     def add_constraint(self, unparsed: str) -> None:
-        """Add new constraint, must not conflict with any existing constraints"""
+        """Add new constraint, must not conflict with any existing constraints
+
+        .. versionchanged: 0.83.0
+           Non-conflicting constraints are now combined. Constraints with
+           conflicts now raise :exc:`InvalidConstraintError`. Inputs without a
+           version specifier or with extras or url are also refused.
+        """
         req = Requirement(unparsed)
         canon_name = canonicalize_name(req.name)
         previous = self._data.get(canon_name)
 
+        # validator properties: must have a specifier, must not have extras or URL
+        if req.extras:
+            raise InvalidConstraintError(f"Constraint {unparsed!r} has extras")
+        if req.url:
+            raise InvalidConstraintError(f"Constraint '{unparsed!r} has an url")
+        if not req.specifier:
+            raise InvalidConstraintError(f"Constraint {unparsed!r} has no specifiers")
+
+        # verify that incoming constraint is okay by itself
+        if req.specifier.is_unsatisfiable():
+            raise InvalidConstraintError(f"Constraint {unparsed!r} is unsatisfiable")
+
         if not requirements_file.evaluate_marker(req, req):
             logger.debug(f"Constraint {req} does not match environment")
             return
 
         if previous is not None:
-            raise KeyError(
-                f"{canon_name}: new constraint '{req}' conflicts with '{previous}'"
-            )
-        if requirements_file.evaluate_marker(req, req):
+            logger.debug("combining constraints %s and %s", previous, req)
+            new_specifier = req.specifier & previous.specifier
+            if new_specifier.is_unsatisfiable():
+                raise InvalidConstraintError(
+                    f"Combined specifier '{new_specifier}' is not satisfiable "
+                    f"(existing: {previous}, new: {req})"
+                )
+            req.specifier = new_specifier
+        else:
             logger.debug(f"adding constraint {req}")
-            self._data[canon_name] = req
+
+        self._data[canon_name] = req
 
     def load_constraints_file(self, constraints_file: str | pathlib.Path) -> None:
         """Load constraints from a constraints file"""

tests/test_constraints.py

@@ -6,19 +6,19 @@
 from packaging.requirements import Requirement
 from packaging.version import Version
 
-from fromager import constraints
+from fromager.constraints import Constraints, InvalidConstraintError
 
 
 def test_constraint_is_satisfied_by() -> None:
-    c = constraints.Constraints()
+    c = Constraints()
     c.add_constraint("foo<=1.1")
     assert c.is_satisfied_by("foo", Version("1.1"))
     assert c.is_satisfied_by("foo", Version("1.0"))
     assert c.is_satisfied_by("bar", Version("2.0"))
 
 
 def test_constraint_canonical_name() -> None:
-    c = constraints.Constraints()
+    c = Constraints()
     c.add_constraint("flash_attn<=1.1")
     assert c.is_satisfied_by("flash_attn", Version("1.1"))
     assert c.is_satisfied_by("flash-attn", Version("1.1"))
@@ -27,7 +27,7 @@ def test_constraint_canonical_name() -> None:
 
 
 def test_constraint_not_is_satisfied_by() -> None:
-    c = constraints.Constraints()
+    c = Constraints()
     c.add_constraint("foo<=1.1")
     c.add_constraint("bar>=2.0")
     assert not c.is_satisfied_by("foo", Version("1.2"))
@@ -36,28 +36,24 @@ def test_constraint_not_is_satisfied_by() -> None:
 
 
 def test_add_constraint_conflict() -> None:
-    c = constraints.Constraints()
+    c = Constraints()
     c.add_constraint("foo<=1.1")
     c.add_constraint("flit_core==2.0rc3")
 
-    # Exact duplicate should raise error (same package, same marker)
-    with pytest.raises(KeyError):
-        c.add_constraint("foo<=1.1")
-
-    # Different version, same marker (no marker) should raise error
-    with pytest.raises(KeyError):
+    # Conflicting version, same marker (no marker) should raise error
+    with pytest.raises(InvalidConstraintError):
         c.add_constraint("foo>1.1")
 
-    # Different version for flit_core should raise error
-    with pytest.raises(KeyError):
+    # Conflicting version for flit_core should raise error
+    with pytest.raises(InvalidConstraintError):
         c.add_constraint("flit_core>2.0.0")
 
     # Normalized name conflict should raise error
-    with pytest.raises(KeyError):
+    with pytest.raises(InvalidConstraintError):
         c.add_constraint("flit-core>2.0.0")
 
     # Different, but equivalent markers should raise KeyError
-    with pytest.raises(KeyError):
+    with pytest.raises(InvalidConstraintError):
         # arm64 -> macos; aarch64 -> linux
         for arch in ["x86_64", "arm64", "aarch64"]:
             c.add_constraint(
@@ -72,7 +68,7 @@ def test_add_constraint_conflict() -> None:
     c.add_constraint("baz==1.1; platform_machine == 'ppc64le'")
 
     # But same package with same marker should raise error
-    with pytest.raises(KeyError):
+    with pytest.raises(InvalidConstraintError):
         c.add_constraint("foo==1.2; platform_machine != 'ppc64le'")
 
     # Verify multiple constraints for same package are stored
@@ -98,7 +94,7 @@ def test_add_constraint_conflict() -> None:
 
 
 def test_allow_prerelease() -> None:
-    c = constraints.Constraints()
+    c = Constraints()
     c.add_constraint("foo>=1.1")
     assert not c.allow_prerelease("foo")
     c.add_constraint("bar>=1.1a0")
@@ -109,15 +105,40 @@ def test_allow_prerelease() -> None:
 
 def test_load_non_existant_constraints_file(tmp_path: pathlib.Path) -> None:
     non_existant_file = tmp_path / "non_existant.txt"
-    c = constraints.Constraints()
+    c = Constraints()
     with pytest.raises(FileNotFoundError):
         c.load_constraints_file(non_existant_file)
 
 
 def test_load_constraints_file(tmp_path: pathlib.Path) -> None:
     constraint_file = tmp_path / "constraint.txt"
-    constraint_file.write_text("egg\ntorch==3.1.0 # comment\n")
-    c = constraints.Constraints()
+    constraint_file.write_text("egg==1.0\ntorch==3.1.0 # comment\n")
+    c = Constraints()
     c.load_constraints_file(constraint_file)
     assert list(c) == ["egg", "torch"]  # type: ignore
     assert c.get_constraint("torch") == Requirement("torch==3.1.0")
+
+
+def test_invalid_constraints() -> None:
+    c = Constraints()
+    with pytest.raises(InvalidConstraintError, match=r".*no specifier"):
+        c.add_constraint("foo")
+    with pytest.raises(InvalidConstraintError, match=r".*has extras"):
+        c.add_constraint("foo[extra]>=1.0")
+    with pytest.raises(InvalidConstraintError, match=r".*has an url"):
+        c.add_constraint("foo@https://foo.test")
+
+
+def test_unsatisfiable() -> None:
+    c = Constraints()
+    with pytest.raises(InvalidConstraintError):
+        c.add_constraint("foo<1.0,>2.0")
+
+
+def test_combine_constraints() -> None:
+    c = Constraints()
+    c.add_constraint("foo>=1.0")
+    c.add_constraint("foo<2.0")
+    assert c.get_constraint("foo") == Requirement("foo<2.0,>=1.0")
+    c.add_constraint("foo!=1.1.0")
+    assert c.get_constraint("foo") == Requirement("foo<2.0,>=1.0,!=1.1.0")