Fix flags semantic

alalazo · alalazo · commit 3a5b534fa9c8 · 2023-03-04T00:09:55.000+01:00
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
@@ -775,19 +775,22 @@ def __init__(self, spec):
         self.spec = spec
 
     def satisfies(self, other):
-        return all(f in self and set(self[f]) == set(other[f]) for f in other)
+        return all(f in self and self[f] == other[f] for f in other)
 
     def intersects(self, other):
-        if not all(set(self[f]) == set(other[f]) for f in other if (other[f] != [] and f in self)):
-            return False
+        common_types = set(self) & set(other)
+        for flag_type in common_types:
+            if not self[flag_type] or not other[flag_type]:
+                # At least one of the two is empty
+                continue
+
+            if self[flag_type] != other[flag_type]:
+                return False
 
-        # Check that the propagation values match
-        for flag_type in other:
             if not all(
-                other[flag_type][i].propagate == self[flag_type][i].propagate
-                for i in range(len(other[flag_type]))
-                if flag_type in self
+                f1.propagate == f2.propagate for f1, f2 in zip(self[flag_type], other[flag_type])
             ):
+                # At least one propagation flag didn't match
                 return False
         return True
 
diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py
@@ -328,7 +328,9 @@ def test_constraining_abstract_specs_with_empty_intersection(self, lhs, rhs):
             ("mpich", "mpich~~foo", True),
             ("mpich", "mpich foo==1", True),
             # Flags semantics is currently different from other variant
-            # ("mpich", 'mpich cppflags="-O3"', False),
+            ("mpich", 'mpich cflags="-O3"', True),
+            ("mpich cflags=-O3", 'mpich cflags="-O3 -Ofast"', False),
+            ("mpich cflags=-O2", 'mpich cflags="-O3"', False),
             ("multivalue-variant foo=bar", "multivalue-variant +foo", False),
             ("multivalue-variant foo=bar", "multivalue-variant ~foo", False),
             ("multivalue-variant fee=bar", "multivalue-variant fee=baz", False),
@@ -457,22 +459,18 @@ def test_copy_satisfies_transitive(self):
             assert s.satisfies(copy[s.name])
             assert copy[s.name].satisfies(s)
 
-    def test_satisfies_virtual(self):
-        # Don't use check_satisfies: it checks constrain() too, and
-        # you can't constrain a non-virtual by a virtual.
+    def test_intersects_virtual(self):
         assert Spec("mpich").intersects(Spec("mpi"))
         assert Spec("mpich2").intersects(Spec("mpi"))
         assert Spec("zmpi").intersects(Spec("mpi"))
 
-    def test_satisfies_virtual_dep_with_virtual_constraint(self):
-        """Ensure we can satisfy virtual constraints when there are multiple
-        vdep providers in the specs."""
+    def test_intersects_virtual_dep_with_virtual_constraint(self):
         assert Spec("netlib-lapack ^openblas").intersects("netlib-lapack ^openblas")
         assert not Spec("netlib-lapack ^netlib-blas").intersects("netlib-lapack ^openblas")
         assert not Spec("netlib-lapack ^openblas").intersects("netlib-lapack ^netlib-blas")
         assert Spec("netlib-lapack ^netlib-blas").intersects("netlib-lapack ^netlib-blas")
 
-    def test_satisfies_same_spec_with_different_hash(self):
+    def test_intersectable_concrete_specs_must_have_the_same_hash(self):
         """Ensure that concrete specs are matched *exactly* by hash."""
         s1 = Spec("mpileaks").concretized()
         s2 = s1.copy()
@@ -1195,15 +1193,15 @@ def test_package_hash_affects_dunder_and_dag_hash(mock_packages, default_mock_co
     assert a1.process_hash() != a2.process_hash()
 
 
-def test_satisfies_is_commutative_with_concrete_specs(default_mock_concretization):
+def test_intersects_and_satisfies_on_concretized_spec(default_mock_concretization):
+    """Test that a spec obtained by concretizing an abstract spec, satisfies the abstract spec
+    but not vice-versa.
+    """
     a1 = default_mock_concretization("a@1.0")
     a2 = Spec("a@1.0")
 
-    # Spec.intersects is commutative.
     assert a1.intersects(a2)
     assert a2.intersects(a1)
-
-    # Spec.satisfies means set inclusion, which is not commutative.
     assert a1.satisfies(a2)
     assert not a2.satisfies(a1)
 
