From bc24a8f2906203beb69a5477d43d7cda3cfdb4eb Mon Sep 17 00:00:00 2001 From: Harmen Stoppels Date: Tue, 21 Feb 2023 14:30:47 +0100 Subject: 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. --- lib/spack/spack/spec.py | 33 +++++++++++++++++++++++---------- lib/spack/spack/test/spec_semantics.py | 13 +++++++++++++ 2 files changed, 36 insertions(+), 10 deletions(-) (limited to 'lib') 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) -- cgit v1.2.3-60-g2f50