Spec.satisfies should be commutative when strict=False (#35598)

The call:
```
x.satisfies(y[, strict=False])
```
is commutative, and tests non-empty intersection, whereas:
```
x.satsifies(y, strict=True)
```
is not commutative, and tests set-inclusion.

There are 2 fast paths. When strict=False both self and other need to 
be concrete, when strict=True we can optimize when other is concrete.

2 files changed, 36 insertions, 10 deletions

diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index 94e3230285..45d236c630 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -3451,25 +3451,38 @@ class Spec(object):
             return spec_like
         return Spec(spec_like)
 
-    def satisfies(self, other, deps=True, strict=False, strict_deps=False):
+    def satisfies(self, other, deps=True, strict=False):
         """Determine if this spec satisfies all constraints of another.
 
-        There are two senses for satisfies:
+        There are two senses for satisfies, depending on the ``strict``
+        argument.
 
-          * `loose` (default): the absence of a constraint in self
-            implies that it *could* be satisfied by other, so we only
-            check that there are no conflicts with other for
-            constraints that this spec actually has.
+          * ``strict=False``: the left-hand side and right-hand side have
+            non-empty intersection. For example ``zlib`` satisfies
+            ``zlib@1.2.3`` and ``zlib@1.2.3`` satisfies ``zlib``. In this
+            sense satisfies is a commutative operation: ``x.satisfies(y)``
+            if and only if ``y.satisfies(x)``.
 
-          * `strict`: strict means that we *must* meet all the
-            constraints specified on other.
+          * ``strict=True``: the left-hand side is a subset of the right-hand
+            side. For example ``zlib@1.2.3`` satisfies ``zlib``, but ``zlib``
+            does not satisfy ``zlib@1.2.3``. In this sense satisfies is not
+            commutative: the left-hand side should be at least as constrained
+            as the right-hand side.
         """
 
         other = self._autospec(other)
 
-        # The only way to satisfy a concrete spec is to match its hash exactly.
+        # Optimizations for right-hand side concrete:
+        # 1. For subset (strict=True) tests this means the left-hand side must
+        # be the same singleton with identical hash. Notice that package hashes
+        # can be different for otherwise indistinguishable concrete Spec objects.
+        # 2. For non-empty intersection (strict=False) we only have a fast path
+        # when the left-hand side is also concrete.
         if other.concrete:
-            return self.concrete and self.dag_hash() == other.dag_hash()
+            if strict:
+                return self.concrete and self.dag_hash() == other.dag_hash()
+            elif self.concrete:
+                return self.dag_hash() == other.dag_hash()
 
         # If the names are different, we need to consider virtuals
         if self.name != other.name and self.name and other.name:
diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py
index d60df890ba..dd086fd632 100644
--- a/lib/spack/spack/test/spec_semantics.py
+++ b/lib/spack/spack/test/spec_semantics.py
@@ -1293,3 +1293,16 @@ def test_package_hash_affects_dunder_and_dag_hash(mock_packages, default_mock_co
     assert hash(a1) != hash(a2)
     assert a1.dag_hash() != a2.dag_hash()
     assert a1.process_hash() != a2.process_hash()
+
+
+def test_satisfies_is_commutative_with_concrete_specs(mock_packages, default_mock_concretization):
+    a1 = default_mock_concretization("a@1.0")
+    a2 = Spec("a@1.0")
+
+    # strict=False means non-empty intersection, which is commutative.
+    assert a1.satisfies(a2)
+    assert a2.satisfies(a1)
+
+    # strict=True means set inclusion, which is not commutative.
+    assert a1.satisfies(a2, strict=True)
+    assert not a2.satisfies(a1, strict=True)