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.
This commit is contained in:
		| @@ -3451,25 +3451,38 @@ def _autospec(self, spec_like): | ||||
|             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: | ||||
|   | ||||
| @@ -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) | ||||
|   | ||||
		Reference in New Issue
	
	Block a user
	 Harmen Stoppels
					Harmen Stoppels