Support both & and | for metasets

news/68.feature.rst

@@ -0,0 +1 @@
+Use logic OR to combine metasets derived from different parents.

src/passa/internals/markers.py

@@ -2,7 +2,7 @@
 
 from __future__ import absolute_import, unicode_literals
 
-from packaging.markers import Marker
+from packaging.markers import Marker as _Marker
 from .specifiers import PySpecs, gen_marker
 import six
 import distlib.markers
@@ -16,9 +16,9 @@
     from backports.functools_lru_cache import lru_cache
 
 
-def _ensure_marker(marker):
-    if not isinstance(marker, Marker):
-        return Marker(str(marker))
+def _ensure_marker(marker, cls=_Marker):
+    if not isinstance(marker, cls):
+        return cls(str(marker))
     return marker
 
 
@@ -231,7 +231,7 @@ def parse_marker_dict(marker_dict):
                 specs.add(parse_marker_dict(side))
             else:
                 # This is the easiest way to go from a string to a PySpec instance
-                specs.add(PySpecs.from_marker(Marker(side)))
+                specs.add(PySpecs.from_marker(_Marker(side)))
             sides.add(specs)
         if op == "and":
             # When we are "and"-ing things together, it probably makes the most sense
@@ -241,11 +241,69 @@ def parse_marker_dict(marker_dict):
             sides = reduce(lambda x, y: x | y, sides)
             if not sides:
                 return sides
-            return PySpecs.from_marker(Marker(str(sides)))
+            return PySpecs.from_marker(_Marker(str(sides)))
         # Actually when we "or" things as well we can also just turn them into a reduced
         # set using this logic now
         return reduce(lambda x, y: x & y, sides)
     else:
         # At the tip of the tree we are dealing with strings all around and they just need
         # to be smashed together
-        return PySpecs.from_marker(Marker(format_string.format(**marker_dict)))
+        return PySpecs.from_marker(_Marker(format_string.format(**marker_dict)))
+
+
+class Marker(_Marker):
+    """A customized marker class with support of '&' and '|' operators."""
+    def __init__(self, marker=None):
+        if not marker:
+            self._markers = []
+        else:
+            super(Marker, self).__init__(marker)
+
+    def __and__(self, other):
+        other = _ensure_marker(other, Marker)
+        if self == other:
+            marker_string = str(self)
+        elif not self:
+            marker_string = str(other)
+        elif not other:
+            marker_string = str(self)
+        else:
+            lhs, rhs = str(self), str(other)
+            # When combining markers with 'and', wrap the part with parentheses
+            # if there exist 'or's.
+            if 'or' in self._markers:
+                lhs = '({})'.format(str(self))
+            if 'or' in other._markers:
+                rhs = '({})'.format(str(other))
+            marker_string = "{} and {}".format(lhs, rhs)
+        return type(self)(marker_string)
+
+    def __or__(self, other):
+        other = _ensure_marker(other, Marker)
+        if self == other:
+            marker_string = str(self)
+        elif not self:
+            marker_string = str(other)
+        elif not other:
+            marker_string = str(self)
+        else:
+            # Just join parts with 'or' since it has the lowest priority.
+            marker_string = "{} or {}".format(str(self), str(other))
+        return type(self)(marker_string)
+
+    def __bool__(self):
+        return bool(self._markers)
+
+    def __nonzero__(self):
+        return self.__bool__()
+
+    def __hash__(self):
+        return hash(tuple(self._markers))
+
+    def __eq__(self, other):
+        if not isinstance(other, _Marker):
+            return False
+        return str(self) == str(other)
+
+    def __lt__(self, other):
+        return hash(self) < hash(other)

src/passa/internals/specifiers.py

@@ -88,7 +88,6 @@ def _get_specs(specset):
     return result
 
 
-@lru_cache(maxsize=128)
 def _group_by_op(specs):
     specs = [_get_specs(x) for x in list(specs)]
     flattened = [(op, version) for spec in specs for op, version in spec]
@@ -381,6 +380,13 @@ def get_version(v):
                     ranges.add((min_,))
                 else:
                     ranges.add((min_, max_))
+        # Now add the holes between ranges to excludes
+        sorted_ranges = sorted(ranges, key=lambda x: x[0])
+        for k in range(len(sorted_ranges) - 1):
+            lower = ALL_PYTHON_VERSIONS.index(sorted_ranges[k][-1])
+            upper = ALL_PYTHON_VERSIONS.index(sorted_ranges[k + 1][0])
+            if lower < upper - 1:
+                excludes.update({ALL_PYTHON_VERSIONS[i] for i in range(lower + 1, upper)})
         return ranges, excludes
 
     def create_specset_from_ranges(self, specset=None, ranges=None, excludes=None):
@@ -427,24 +433,29 @@ def create_specset_from_ranges(self, specset=None, ranges=None, excludes=None):
         return new_specset
 
     @lru_cache(maxsize=128)
-    def __and__(self, other):
+    def __or__(self, other):
         # Unintuitive perhaps, but this is for "x or y" and needs to handle the
-        # widest possible range encapsulated by the two using the intersection
+        # widest possible range encapsulated by the two using the union
         if not isinstance(other, PySpecs):
             other = PySpecs(other)
         if self == other:
             return self
         new_specset = SpecifierSet()
-        # In order to do an "or" propertly we need to intersect the "good" versions
-        intersection = self.get_version_includes() | other.get_version_includes()
-        intersection_specset = self.get_specset_from_versions(intersection)
+        if not self.specifierset:
+            union = other.get_version_includes()
+        elif not other.specifierset:
+            union = self.get_version_includes()
+        else:
+            # In order to do an "or" propertly we need to intersect the "good" versions
+            union = self.get_version_includes() | other.get_version_includes()
+        union_specset = self.get_specset_from_versions(union)
         # And then we need to union the "bad" versions
         excludes = self.get_version_excludes() & other.get_version_excludes()
-        new_specset = self.create_specset_from_ranges(ranges=intersection_specset, excludes=excludes)
+        new_specset = self.create_specset_from_ranges(ranges=union_specset, excludes=excludes)
         return PySpecs(new_specset)
 
     @lru_cache(maxsize=128)
-    def __or__(self, specset):
+    def __and__(self, specset):
         if not isinstance(specset, PySpecs):
             specset = PySpecs(specset)
         if str(self) == str(specset):

src/passa/models/metadata.py

@@ -3,18 +3,16 @@
 from __future__ import absolute_import, unicode_literals
 
 import copy
-import itertools
 import operator
 
-import packaging.markers
 import packaging.specifiers
 import vistir
 import vistir.misc
 
 from six.moves import reduce
 
 from ..internals.markers import (
-    get_without_extra, get_without_pyversion, get_contained_pyversions
+    get_without_extra, get_without_pyversion, get_contained_pyversions, Marker
 )
 from ..internals.specifiers import PySpecs
 
@@ -32,57 +30,34 @@ class MetaSet(object):
     includes a marker, and a specifier set of Python versions required.
     """
     def __init__(self):
-        self.markerset = frozenset()
+        self.marker = Marker()
         self.pyspecset = PySpecs()
 
     def __repr__(self):
-        return "MetaSet(markerset={0!r}, pyspecset={1!r})".format(
-            ",".join(sorted(self.markerset)), str(self.pyspecset),
+        return "MetaSet(marker={0!r}, pyspecset={1!r})".format(
+            str(self.marker), str(self.pyspecset),
         )
 
     def __key(self):
-        return (tuple(self.markerset), hash(tuple(self.pyspecset)))
+        return (self.marker, hash(tuple(self.pyspecset)))
 
     def __hash__(self):
         return hash(self.__key())
 
     def __eq__(self, other):
         return self.__key() == other.__key()
 
-    def __len__(self):
-        return len(self.markerset) + len(self.pyspecset)
-
-    def __iter__(self):
-        return itertools.chain(self.markerset, self.pyspecset)
-
     def __lt__(self, other):
         return operator.lt(self.__key(), other.__key())
 
-    def __le__(self, other):
-        return operator.le(self.__key(), other.__key())
-
-    def __ge__(self, other):
-        return operator.ge(self.__key(), other.__key())
-
-    def __gt__(self, other):
-        return operator.gt(self.__key(), other.__key())
-
     def __str__(self):
-        self.markerset = frozenset(filter(None, self.markerset))
-        return " and ".join([mkr_part for mkr_part in dedup_markers(itertools.chain(
-            # Make sure to always use the same quotes so we can dedup properly.
-            (
-                "{0}".format(ms) if " or " in ms else ms
-                for ms in (str(m).replace('"', "'") for m in self.markerset)
-                if ms
-            ),
-            (
-                "{0}".format(str(self.pyspecset)) if self.pyspecset else "",
-            ))) if mkr_part
-        ])
+        marker = self.marker
+        if self.pyspecset:
+            marker = marker & self.pyspecset
+        return str(marker)
 
     def __bool__(self):
-        return bool(self.markerset or self.pyspecset)
+        return bool(self.marker or self.pyspecset)
 
     def __nonzero__(self):  # Python 2.
         return self.__bool__()
@@ -91,7 +66,7 @@ def __nonzero__(self):  # Python 2.
     def from_tuple(cls, pair):
         marker, specset = pair
         pyspecs = PySpecs(specset)
-        markerset = set()
+        new_marker = Marker()
         if marker:
             # Returns a PySpec instance or None
             marker_pyversions = get_contained_pyversions(marker)
@@ -100,24 +75,39 @@ def from_tuple(cls, pair):
             # The remainder of the marker, if there is any
             cleaned_marker = get_without_pyversion(marker)
             if cleaned_marker:
-                markerset.add(str(cleaned_marker))
+                new_marker = new_marker & cleaned_marker
         metaset = cls()
-        metaset.markerset = frozenset(markerset)
+        metaset.marker = marker
         metaset.pyspecset = pyspecs
         return metaset
 
     def __or__(self, other):
         if not isinstance(other, type(self)):
             other = self.from_tuple(other)
         metaset = MetaSet()
-        markerset = set()
+        marker = Marker()
         specset = PySpecs()
         for meta in (self, other):
-            if meta.markerset:
-                markerset |= set(meta.markerset)
+            if meta.marker:
+                marker = marker | meta.marker
             if meta.pyspecset:
                 specset = specset | meta.pyspecset
-        metaset.markerset = frozenset(markerset)
+        metaset.marker = marker
+        metaset.pyspecset = specset
+        return metaset
+
+    def __and__(self, other):
+        if not isinstance(other, type(self)):
+            other = self.from_tuple(other)
+        metaset = MetaSet()
+        marker = Marker()
+        specset = PySpecs()
+        for meta in (self, other):
+            if meta.marker:
+                marker = marker & meta.marker
+            if meta.pyspecset:
+                specset = specset & meta.pyspecset
+        metaset.marker = marker
         metaset.pyspecset = specset
         return metaset
 
@@ -144,7 +134,8 @@ def _build_metasets(dependencies, pythons, key, trace, all_metasets):
             packaging.specifiers.SpecifierSet(python),
         )
         for parent_metaset in parent_metasets:
-            child_metaset = parent_metaset | metaset
+            # Use 'and' to connect markers inherited from parent.
+            child_metaset = parent_metaset & metaset
             metasets.add(child_metaset)
     return list(metasets)
 
@@ -178,11 +169,12 @@ def _format_metasets(metasets):
     # If there is an unconditional route, this needs to be unconditional.
     if not metasets:
         return ""
+    # Use 'or' to combine markers from different parent.
     combined_metaset = str(MetaSet() | reduce(lambda x, y: x | y, metasets))
     if not combined_metaset:
         return ""
     # This extra str(Marker()) call helps simplify the expression.
-    metaset_string = str(packaging.markers.Marker(combined_metaset))
+    metaset_string = str(Marker(combined_metaset))
     return metaset_string
 
 

tests/unit/test_specifiers.py

@@ -2,7 +2,7 @@
 
 from packaging.specifiers import SpecifierSet
 
-from passa.internals.specifiers import cleanup_pyspecs
+from passa.internals.specifiers import cleanup_pyspecs, PySpecs
 
 
 @pytest.mark.parametrize("spec, cleaned", [
@@ -28,3 +28,39 @@
 def test_cleanup_pyspecs(spec, cleaned):
     cleaned_specifierset = frozenset(s for s in cleaned)
     assert cleanup_pyspecs(SpecifierSet(spec)) == cleaned_specifierset
+
+
+@pytest.mark.parametrize("left, right, result", [
+    ("", ">=3.6", ">=3.6"),
+    (">=2.7,!=3.0,!=3.1,!=3.2,!=3.3", ">=3.6", "!=3.0,!=3.1,!=3.2,!=3.3,>=3.6"),
+    ("<3.0", ">=3.3", "<3.0,>=3.3"),
+    ("==3.7", ">=3.6", "==3.7,>=3.6")
+])
+def test_pyspecs_and_operator(left, right, result):
+    left = PySpecs(SpecifierSet(left))
+    right = PySpecs(SpecifierSet(right))
+    rv = left & right
+    assert str(rv.specifierset) == result
+
+
+@pytest.mark.parametrize("left, right, result", [
+    ("", ">=3.6", ">=3.6"),
+    (">=2.7,!=3.0,!=3.1,!=3.2,!=3.3", ">=3.6", "!=3.0,!=3.1,!=3.2,!=3.3,>=2.7"),
+    ("==2.7", ">=3.4", "!=3.0,!=3.1,!=3.2,!=3.3,>=2.7"),
+    (">=2.6,<3.0", ">=3.3,<3.6", "!=3.0,!=3.1,!=3.2,<3.6,>=2.6"),
+    ("~=3.0", ">=2.7", ">=2.7")
+])
+def test_pyspecs_or_operator(left, right, result):
+    left = PySpecs(SpecifierSet(left))
+    right = PySpecs(SpecifierSet(right))
+    rv = left | right
+    assert str(rv.specifierset) == result
+
+
+@pytest.mark.parametrize("spec, marker", [
+    ("", ""),
+    (">=3.3,<3.7", 'python_version < "3.7" and python_version >= "3.3"')
+])
+def test_pyspecs_to_marker(spec, marker):
+    pyspec = PySpecs(SpecifierSet(spec))
+    assert str(pyspec) == marker