Feature/better version ranges (#14912)

* better version ranges

* tests passing better version ranges

* Fully implement intersection operator

* Add OR support, ensure prerelease support

* Bring back None on incompatible

* Improve limit condition logic

* Use display versions for conditions that change due to prereleases

* Fix bug with * range and 0.0.0 pres, add pres tests

* Improve verison range comparison

* Add handling for spcial non-void check cases

---------

Co-authored-by: Rubén Rincón Blanco <rubenrb@jfrog.com>

conans/client/graph/graph_builder.py

@@ -106,7 +106,11 @@ def _conflicting_version(require, node,
         if version_range:
             # TODO: Check user/channel conflicts first
             if prev_version_range is not None:
-                pass  # Do nothing, evaluate current as it were a fixed one
+                # It it is not conflicting, but range can be incompatible, restrict range
+                restricted_version_range = version_range.intersection(prev_version_range)
+                if restricted_version_range is None:
+                    raise GraphConflictError(node, require, prev_node, prev_require, base_previous)
+                require.ref.version = restricted_version_range.version()
             else:
                 if version_range.contains(prev_ref.version, resolve_prereleases):
                     require.ref = prev_ref

conans/model/version_range.py

@@ -1,11 +1,64 @@
-from collections import namedtuple
+from functools import total_ordering
 from typing import Optional
 
 from conans.errors import ConanException
 from conans.model.recipe_ref import Version
 
 
-_Condition = namedtuple("_Condition", ["operator", "version"])
+@total_ordering
+class _Condition:
+    def __init__(self, operator, version):
+        self.operator = operator
+        self.display_version = version
+
+        value = str(version)
+        if (operator == ">=" or operator == "<") and "-" not in value:
+            value += "-"
+        self.version = Version(value)
+
+    def __str__(self):
+        return f"{self.operator}{self.display_version}"
+
+    def __repr__(self):
+        return self.__str__()
+
+    def __hash__(self):
+        return hash((self.operator, self.version))
+
+    def __lt__(self, other):
+        # Notice that this is done on the modified version, might contain extra prereleases
+        if self.version < other.version:
+            return True
+        elif self.version == other.version:
+            if self.operator == "<":
+                if other.operator == "<":
+                    return self.display_version.pre is not None
+                else:
+                    return True
+            elif self.operator == "<=":
+                if other.operator == "<":
+                    return False
+                else:
+                    return self.display_version.pre is None
+            elif self.operator == ">":
+                if other.operator == ">":
+                    return self.display_version.pre is None
+                else:
+                    return False
+            else:
+                if other.operator == ">":
+                    return True
+                # There's a possibility of getting here while validating if a range is non-void
+                # by comparing >= & <= for lower limit <= upper limit
+                elif other.operator == "<=":
+                    return True
+                else:
+                    return self.display_version.pre is not None
+        return False
+
+    def __eq__(self, other):
+        return (self.display_version == other.display_version and
+                self.operator == other.operator)
 
 
 class _ConditionSet:
@@ -45,8 +98,6 @@ def _parse_expression(expression):
             index = 1 if len(v.main) > 1 else 0
             return [_Condition(">=", v), _Condition("<", v.upper_bound(index))]
         elif operator == "^":  # caret major
-            if "-" not in version:
-                version += "-"
             v = Version(version)
 
             def first_non_zero(main):
@@ -58,8 +109,6 @@ def first_non_zero(main):
             initial_index = first_non_zero(v.main)
             return [_Condition(">=", v), _Condition("<", v.upper_bound(initial_index))]
         else:
-            if (operator == ">=" or operator == "<") and "-" not in version:
-                version += "-"
             return [_Condition(operator, Version(version))]
 
     def _valid(self, version, conf_resolve_prepreleases):
@@ -135,3 +184,34 @@ def contains(self, version: Version, resolve_prerelease: Optional[bool]):
                 return True
         return False
 
+    def intersection(self, other):
+        conditions = []
+
+        def _calculate_limits(operator, lhs, rhs):
+            limits = ([c for c in lhs.conditions if operator in c.operator]
+                      + [c for c in rhs.conditions if operator in c.operator])
+            if limits:
+                return sorted(limits, reverse=operator == ">")[0]
+
+        for lhs_conditions in self.condition_sets:
+            for rhs_conditions in other.condition_sets:
+                internal_conditions = []
+                lower_limit = _calculate_limits(">", lhs_conditions, rhs_conditions)
+                upper_limit = _calculate_limits("<", lhs_conditions, rhs_conditions)
+                if lower_limit:
+                    internal_conditions.append(lower_limit)
+                if upper_limit:
+                    internal_conditions.append(upper_limit)
+                if internal_conditions and (not lower_limit or not upper_limit or lower_limit <= upper_limit):
+                    conditions.append(internal_conditions)
+
+        if not conditions:
+            return None
+        expression = ' || '.join(' '.join(str(c) for c in cs) for cs in conditions)
+        result = VersionRange(expression)
+        # TODO: Direct definition of conditions not reparsing
+        # result.condition_sets = self.condition_sets + other.condition_sets
+        return result
+
+    def version(self):
+        return Version(f"[{self._expression}]")

conans/test/integration/graph/core/test_version_ranges.py

@@ -322,9 +322,10 @@ def test_two_ranges_overriden(self):
         self._check_node(libb, "libb/0.1#123", dependents=[app], deps=[liba])
         self._check_node(app, "app/0.1", deps=[libb, liba])
 
-    def test_two_ranges_overriden_conflict(self):
+    def test_two_ranges_overriden_no_conflict(self):
         # app -> libb/0.1 -(range >0)-> liba/0.1
         #   \ ---------liba/[<0.3>]-------------/
+        # Conan learned to solve this conflict in 2.0.14
         self.recipe_cache("liba/0.1")
         self.recipe_cache("liba/0.2")
         self.recipe_cache("liba/0.3")
@@ -333,16 +334,16 @@ def test_two_ranges_overriden_conflict(self):
                                            .with_requirement("liba/[<0.3]"))
         deps_graph = self.build_consumer(consumer, install=False)
 
-        assert type(deps_graph.error) == GraphConflictError
+        # This is no longer a conflict, and Conan knows that liba/2.0 is a valid joint solution
 
         self.assertEqual(3, len(deps_graph.nodes))
         app = deps_graph.root
         libb = app.dependencies[0].dst
         liba = libb.dependencies[0].dst
 
-        self._check_node(liba, "liba/0.3#123", dependents=[libb], deps=[])
+        self._check_node(liba, "liba/0.2#123", dependents=[libb, app], deps=[])
         self._check_node(libb, "libb/0.1#123", dependents=[app], deps=[liba])
-        self._check_node(app, "app/0.1", deps=[libb])
+        self._check_node(app, "app/0.1", deps=[libb, liba])
 
 
 def test_mixed_user_channel():

conans/test/unittests/model/version/test_version_range.py

@@ -20,15 +20,15 @@
     ['1.0.0',   [[["=", "1.0.0"]]],                   ["1.0.0"],          ["2", "1.0.1"]],
     ['=1.0.0',  [[["=", "1.0.0"]]],                   ["1.0.0"],          ["2", "1.0.1"]],
     # Any
-    ['*',       [[[">=", "0.0.0"]]],                  ["1.0", "a.b"],          []],
-    ['',        [[[">=", "0.0.0"]]],                  ["1.0", "a.b"],          []],
+    ['*',       [[[">=", "0.0.0-"]]],                  ["1.0", "a.b"],          []],
+    ['',        [[[">=", "0.0.0-"]]],                  ["1.0", "a.b"],          []],
     # Unions
     ['1.0.0 || 2.1.3',  [[["=", "1.0.0"]], [["=", "2.1.3"]]],  ["1.0.0", "2.1.3"],  ["2", "1.0.1"]],
     ['>1 <2.0 || ^3.2 ',  [[['>', '1'], ['<', '2.0-']],
                            [['>=', '3.2-'], ['<', '4.0-']]],   ["1.5", "3.3"],  ["2.1", "0.1", "5"]],
     # pre-releases
-    ['',                             [[[">=", "0.0.0"]]],    ["1.0"],                ["1.0-pre.1"]],
-    ['*, include_prerelease=True',   [[[">=", "0.0.0"]]],    ["1.0", "1.0-pre.1"],   []],
+    ['',                             [[[">=", "0.0.0-"]]],    ["1.0"],                ["1.0-pre.1"]],
+    ['*, include_prerelease=True',   [[[">=", "0.0.0-"]]],    ["1.0", "1.0-pre.1", "0.0.0", "0.0.0-pre.1"],   []],
     ['>1- <2.0',                     [[['>', '1-'], ['<', '2.0-']]],
                                      ["1.0", "1.1", "1.9"],   ["1-pre.1", "1.5.1-pre1", "2.1-pre1"]],
     ['>1- <2.0 || ^3.2 ',  [[['>', '1-'], ['<', '2.0-']], [['>=', '3.2-'], ['<', '4.0-']]],
@@ -46,14 +46,14 @@ def test_range(version_range, conditions, versions_in, versions_out):
     r = VersionRange(version_range)
     for condition_set, expected_condition_set in zip(r.condition_sets, conditions):
         for condition, expected_condition in zip(condition_set.conditions, expected_condition_set):
-            assert condition.operator == expected_condition[0]
-            assert condition.version == expected_condition[1]
+            assert condition.operator == expected_condition[0], f"Expected {r} condition operator to be {expected_condition[0]}, but got {condition.operator}"
+            assert condition.version == expected_condition[1], f"Expected {r} condition version to be {expected_condition[1]}, but got {condition.version}"
 
     for v in versions_in:
-        assert r.contains(Version(v), None)
+        assert r.contains(Version(v), None), f"[{r}] must contain {v}"
 
     for v in versions_out:
-        assert not r.contains(Version(v), None)
+        assert not r.contains(Version(v), None), f"[{r}] must not contain {v}"
 
 
 @pytest.mark.parametrize("version_range, resolve_prereleases, versions_in, versions_out", [

conans/test/unittests/model/version/test_version_range_intersection.py

@@ -0,0 +1,82 @@
+import pytest
+
+from conans.model.version_range import VersionRange
+
+values = [
+    # single lower limits bounds
+    ['>1.0', ">1.0", ">1.0"],
+    ['>=1.0', ">1.0", ">1.0"],
+    ['>1.0', ">1.1", ">1.1"],
+    ['>1.0', ">=1.1", ">=1.1"],
+    ['>=1.0', ">=1.1", ">=1.1"],
+    # single upper limits bounds
+    ['<2.0', "<2.0", "<2.0"],
+    ['<=1.0', "<1.0", "<1.0"],
+    ['<2.0', "<2.1", "<2.0"],
+    ['<2.0', "<=1.1", "<=1.1"],
+    ['<=1.0', "<=1.1", "<=1.0"],
+    # One lower limit, one upper
+    ['>=1.0', "<2.0", ">=1.0 <2.0"],
+    ['>=1', '<=1', ">=1 <=1"],
+    [">=1", "<=1-", ">=1 <=1-"],
+    [">=1-", "<=1", ">=1- <=1"],
+    # Two lower, one upper
+    ['>=1.0', ">1.0 <2.0", ">1.0 <2.0"],
+    ['>=1.0', ">1.1 <2.0", ">1.1 <2.0"],
+    ['>1.0', ">1.1 <=2.0", ">1.1 <=2.0"],
+    ['>1.0', ">=1.1 <=2.0", ">=1.1 <=2.0"],
+    # one lower, two upper
+    ['<3.0', ">1.0 <2.0", ">1.0 <2.0"],
+    ['<=2.0', ">1.1 <2.0", ">1.1 <2.0"],
+    ['<1.9', ">1.1 <=2.0", ">1.1 <1.9"],
+    ['<=1.9', ">=1.1 <=2.0", ">=1.1 <=1.9"],
+    # two lower, two upper
+    ['>0.1 <3.0', ">1.0 <2.0", ">1.0 <2.0"],
+    ['>1.2 <=2.0', ">1.1 <2.0", ">1.2 <2.0"],
+    ['>0.1 <1.9', ">1.1 <=2.0", ">1.1 <1.9"],
+    ['>=1.3 <=1.9', ">=1.1 <=2.0", ">=1.3 <=1.9"],
+    ['>=1.0 <=5.0', ">2 <2.5", ">2 <2.5"],
+    # equal limits
+    ['>=1.0 <3.0', ">0.0 <=1.0", ">=1.0 <=1.0"],
+    # prereleases
+    ['>1.0', ">1.0-", ">1.0"],
+    ['>=1.0- <3.0', ">=1.0 <3.0-", ">=1.0 <3.0-"],
+    ['>=1.0 <=3.0-', "<3", ">=1.0 <3"],
+    # OR
+    ['>=1.0 <2.0 || >=2.1 <3', ">=2.3", ">=2.3 <3"],
+    ['>=1.3 <=1.9 || >2.1', ">=1.1 <=2.0 || >=2.1 <2.6", ">=1.3 <=1.9 || >2.1 <2.6"],
+    ['>=1.3 <=1.9 || >=2.2', ">=1.8- <2.3 || >=2.1 <2.6", ">=1.8- <=1.9 || >=2.2 <2.3 || >=2.2 <2.6"],
+]
+
+
+@pytest.mark.parametrize("range1, range2, result", values)
+def test_range_intersection(range1, range2, result):
+    r1 = VersionRange(range1)
+    r2 = VersionRange(range2)
+    inter = r1.intersection(r2)
+    result = f"[{result}]"
+    assert inter.version() == result
+    inter = r2.intersection(r1)  # Test reverse order, result should be the same
+    assert inter.version() == result
+
+
+incompatible_values = [
+    ['>1.0', "<1.0"],
+    ['>=1.0', "<1.0"],
+    ['>1.0', "<=1.0"],
+    ['>1.0 <2.0', ">2.0"],
+    ['>1.0 <2.0', "<1.0"],
+    ['>1.0 <2.0', ">3.0 <4.0"],
+    ['<1.0', ">3.0 <4.0"],
+    ['>=1.0 <2 || >2 <3', ">4 <5"]
+]
+
+
+@pytest.mark.parametrize("range1, range2", incompatible_values)
+def test_range_intersection_incompatible(range1, range2):
+    r1 = VersionRange(range1)
+    r2 = VersionRange(range2)
+    inter = r1.intersection(r2)
+    assert inter is None
+    inter = r2.intersection(r1)  # Test reverse order, result should be the same
+    assert inter is None