Revert "Specs: propagated variants affect == equality (#47376)"

This reverts commit 395c911689.

lib/spack/spack/spec.py

@@ -877,9 +877,8 @@ def constrain(self, other):
                 # Next, if any flags in other propagate, we force them to propagate in our case
                 shared = list(sorted(set(other[flag_type]) - extra_other))
                 for x, y in _shared_subset_pair_iterate(shared, sorted(self[flag_type])):
-                    if y.propagate is True and x.propagate is False:
-                        changed = True
-                        y.propagate = False
+                    if x.propagate:
+                        y.propagate = True
 
         # TODO: what happens if flag groups with a partial (but not complete)
         # intersection specify different behaviors for flag propagation?
@@ -934,7 +933,6 @@ def _cmp_iter(self):
             def flags():
                 for flag in v:
                     yield flag
-                    yield flag.propagate
 
             yield flags
 

lib/spack/spack/test/spec_semantics.py

@@ -231,7 +231,7 @@ class TestSpecSemantics:
             ("mpich+foo", "mpich foo=True", "mpich+foo"),
             ("mpich++foo", "mpich foo=True", "mpich+foo"),
             ("mpich foo=true", "mpich+foo", "mpich+foo"),
-            ("mpich foo==true", "mpich++foo", "mpich++foo"),
+            ("mpich foo==true", "mpich++foo", "mpich+foo"),
             ("mpich~foo", "mpich foo=FALSE", "mpich~foo"),
             ("mpich~~foo", "mpich foo=FALSE", "mpich~foo"),
             ("mpich foo=False", "mpich~foo", "mpich~foo"),
@@ -271,17 +271,17 @@ class TestSpecSemantics:
             ("mpich+foo", "mpich", "mpich+foo"),
             ("mpich~foo", "mpich", "mpich~foo"),
             ("mpich foo=1", "mpich", "mpich foo=1"),
-            ("mpich", "mpich++foo", "mpich++foo"),
+            ("mpich", "mpich++foo", "mpich+foo"),
             ("libelf+debug", "libelf+foo", "libelf+debug+foo"),
             ("libelf+debug", "libelf+debug+foo", "libelf+debug+foo"),
             ("libelf debug=2", "libelf foo=1", "libelf debug=2 foo=1"),
             ("libelf debug=2", "libelf debug=2 foo=1", "libelf debug=2 foo=1"),
             ("libelf+debug", "libelf~foo", "libelf+debug~foo"),
             ("libelf+debug", "libelf+debug~foo", "libelf+debug~foo"),
-            ("libelf++debug", "libelf+debug+foo", "libelf+debug+foo"),
-            ("libelf debug==2", "libelf foo=1", "libelf debug==2 foo=1"),
-            ("libelf debug==2", "libelf debug=2 foo=1", "libelf debug=2 foo=1"),
-            ("libelf++debug", "libelf++debug~foo", "libelf++debug~foo"),
+            ("libelf++debug", "libelf+debug+foo", "libelf++debug++foo"),
+            ("libelf debug==2", "libelf foo=1", "libelf debug==2 foo==1"),
+            ("libelf debug==2", "libelf debug=2 foo=1", "libelf debug==2 foo==1"),
+            ("libelf++debug", "libelf++debug~foo", "libelf++debug~~foo"),
             ("libelf foo=bar,baz", "libelf foo=*", "libelf foo=bar,baz"),
             ("libelf foo=*", "libelf foo=bar,baz", "libelf foo=bar,baz"),
             (
@@ -367,24 +367,19 @@ def test_abstract_specs_can_constrain_each_other(self, lhs, rhs, expected):
                 'mpich cflags="-O3 -g"',
                 'mpich cflags=="-O3"',
                 'mpich cflags="-O3 -g"',
-                'mpich cflags="-O3 -g"',
-                [],
-                [],
-            ),
-            (
                 'mpich cflags=="-O3 -g"',
-                'mpich cflags=="-O3"',
-                'mpich cflags=="-O3 -g"',
-                'mpich cflags=="-O3 -g"',
-                [("cflags", "-O3"), ("cflags", "-g")],
-                [("cflags", "-O3"), ("cflags", "-g")],
+                [("cflags", "-O3")],
+                [("cflags", "-O3")],
             ),
         ],
     )
     def test_constrain_compiler_flags(
         self, lhs, rhs, expected_lhs, expected_rhs, propagated_lhs, propagated_rhs
     ):
-        """Constraining is asymmetric for compiler flags."""
+        """Constraining is asymmetric for compiler flags. Also note that
+        Spec equality does not account for flag propagation, so the checks
+        here are manual.
+        """
         lhs, rhs, expected_lhs, expected_rhs = (
             Spec(lhs),
             Spec(rhs),
@@ -1958,20 +1953,3 @@ def test_old_format_strings_trigger_error(default_mock_concretization):
     s = Spec("pkg-a").concretized()
     with pytest.raises(SpecFormatStringError):
         s.format("${PACKAGE}-${VERSION}-${HASH}")
-
-
-@pytest.mark.regression("47362")
-@pytest.mark.parametrize(
-    "lhs,rhs",
-    [
-        ("hdf5 +mpi", "hdf5++mpi"),
-        ("hdf5 cflags==-g", "hdf5 cflags=-g"),
-        ("hdf5 +mpi ++shared", "hdf5+mpi +shared"),
-        ("hdf5 +mpi cflags==-g", "hdf5++mpi cflag=-g"),
-    ],
-)
-def test_equality_discriminate_on_propagation(lhs, rhs):
-    """Tests that == can discriminate abstract specs based on their 'propagation' status"""
-    s, t = Spec(lhs), Spec(rhs)
-    assert s != t
-    assert len({s, t}) == 2

lib/spack/spack/variant.py

@@ -251,7 +251,7 @@ def implicit_variant_conversion(method):
     def convert(self, other):
         # We don't care if types are different as long as I can convert other to type(self)
         try:
-            other = type(self)(other.name, other._original_value, propagate=other.propagate)
+            other = type(self)(other.name, other._original_value)
         except (error.SpecError, ValueError):
             return False
         return method(self, other)
@@ -379,7 +379,6 @@ def _value_setter(self, value: ValueType) -> None:
     def _cmp_iter(self) -> Iterable:
         yield self.name
         yield from (str(v) for v in self.value_as_tuple)
-        yield self.propagate
 
     def copy(self) -> "AbstractVariant":
         """Returns an instance of a variant equivalent to self
@@ -454,7 +453,6 @@ def constrain(self, other: "AbstractVariant") -> bool:
             values.remove("*")
 
         self._value_setter(",".join(str(v) for v in values))
-        self.propagate = self.propagate and other.propagate
         return old_value != self.value
 
     def __contains__(self, item: Union[str, bool]) -> bool:
@@ -559,7 +557,6 @@ def constrain(self, other: "AbstractVariant") -> bool:
 
         if self.value != other.value:
             raise UnsatisfiableVariantSpecError(other.value, self.value)
-        self.propagate = self.propagate and other.propagate
         return False
 
     def __contains__(self, item: ValueType) -> bool: