ASP-based solver: fix issue with conditional requirements and trigger conditions (#42566)
The lack of a rule to avoid enforcing requirements on multi-valued variants, when the condition activating the environment was not met, resulted in multiple optimal solutions. The fix is to prevent imposing a requirement if the when= rule activating it is not met.
This commit is contained in:
		 Massimiliano Culpo
					Massimiliano Culpo
				
			
				
					committed by
					
						 GitHub
						GitHub
					
				
			
			
				
	
			
			
			 GitHub
						GitHub
					
				
			
						parent
						
							f71669175f
						
					
				
				
					commit
					7ff3b17f14
				
			| @@ -698,6 +698,14 @@ requirement_group_satisfied(node(ID, Package), X) :- | |||||||
|   activate_requirement(node(ID, Package), X), |   activate_requirement(node(ID, Package), X), | ||||||
|   requirement_group(Package, X). |   requirement_group(Package, X). | ||||||
|  |  | ||||||
|  | % Do not impose requirements, if the conditional requirement is not active | ||||||
|  | do_not_impose(EffectID, node(ID, Package)) :- | ||||||
|  |   trigger_condition_holds(TriggerID, node(ID, Package)), | ||||||
|  |   pkg_fact(Package, condition_trigger(ConditionID, TriggerID)), | ||||||
|  |   pkg_fact(Package, condition_effect(ConditionID, EffectID)), | ||||||
|  |   requirement_group_member(ConditionID , Package, RequirementID), | ||||||
|  |   not activate_requirement(node(ID, Package), RequirementID). | ||||||
|  |  | ||||||
| % When we have a required provider, we need to ensure that the provider/2 facts respect | % When we have a required provider, we need to ensure that the provider/2 facts respect | ||||||
| % the requirement. This is particularly important for packages that could provide multiple | % the requirement. This is particularly important for packages that could provide multiple | ||||||
| % virtuals independently | % virtuals independently | ||||||
|   | |||||||
| @@ -1133,3 +1133,46 @@ def test_conflict_packages_yaml(packages_yaml, spec_str, concretize_scope, mock_ | |||||||
|     update_packages_config(packages_yaml) |     update_packages_config(packages_yaml) | ||||||
|     with pytest.raises(UnsatisfiableSpecError): |     with pytest.raises(UnsatisfiableSpecError): | ||||||
|         Spec(spec_str).concretized() |         Spec(spec_str).concretized() | ||||||
|  | 
 | ||||||
|  | 
 | ||||||
|  | @pytest.mark.parametrize( | ||||||
|  |     "spec_str,expected,not_expected", | ||||||
|  |     [ | ||||||
|  |         ( | ||||||
|  |             "forward-multi-value +cuda cuda_arch=10 ^dependency-mv~cuda", | ||||||
|  |             ["cuda_arch=10", "^dependency-mv~cuda"], | ||||||
|  |             ["cuda_arch=11", "^dependency-mv cuda_arch=10", "^dependency-mv cuda_arch=11"], | ||||||
|  |         ), | ||||||
|  |         ( | ||||||
|  |             "forward-multi-value +cuda cuda_arch=10 ^dependency-mv+cuda", | ||||||
|  |             ["cuda_arch=10", "^dependency-mv cuda_arch=10"], | ||||||
|  |             ["cuda_arch=11", "^dependency-mv cuda_arch=11"], | ||||||
|  |         ), | ||||||
|  |         ( | ||||||
|  |             "forward-multi-value +cuda cuda_arch=11 ^dependency-mv+cuda", | ||||||
|  |             ["cuda_arch=11", "^dependency-mv cuda_arch=11"], | ||||||
|  |             ["cuda_arch=10", "^dependency-mv cuda_arch=10"], | ||||||
|  |         ), | ||||||
|  |         ( | ||||||
|  |             "forward-multi-value +cuda cuda_arch=10,11 ^dependency-mv+cuda", | ||||||
|  |             ["cuda_arch=10,11", "^dependency-mv cuda_arch=10,11"], | ||||||
|  |             [], | ||||||
|  |         ), | ||||||
|  |     ], | ||||||
|  | ) | ||||||
|  | def test_forward_multi_valued_variant_using_requires( | ||||||
|  |     spec_str, expected, not_expected, config, mock_packages | ||||||
|  | ): | ||||||
|  |     """Tests that a package can forward multivalue variants to dependencies, using | ||||||
|  |     `requires` directives of the form: | ||||||
|  | 
 | ||||||
|  |         for _val in ("shared", "static"): | ||||||
|  |             requires(f"^some-virtual-mv libs={_val}", when=f"libs={_val} ^some-virtual-mv") | ||||||
|  |     """ | ||||||
|  |     s = Spec(spec_str).concretized() | ||||||
|  | 
 | ||||||
|  |     for constraint in expected: | ||||||
|  |         assert s.satisfies(constraint) | ||||||
|  | 
 | ||||||
|  |     for constraint in not_expected: | ||||||
|  |         assert not s.satisfies(constraint) | ||||||
|   | |||||||
| @@ -0,0 +1,18 @@ | |||||||
|  | # Copyright 2013-2024 Lawrence Livermore National Security, LLC and other | ||||||
|  | # Spack Project Developers. See the top-level COPYRIGHT file for details. | ||||||
|  | # | ||||||
|  | # SPDX-License-Identifier: (Apache-2.0 OR MIT) | ||||||
|  | 
 | ||||||
|  | from spack.package import * | ||||||
|  | 
 | ||||||
|  | 
 | ||||||
|  | class DependencyMv(Package): | ||||||
|  |     """Package providing a virtual dependency and with a multivalued variant.""" | ||||||
|  | 
 | ||||||
|  |     homepage = "http://www.example.com" | ||||||
|  |     url = "http://www.example.com/foo-1.0.tar.gz" | ||||||
|  | 
 | ||||||
|  |     version("1.0", md5="0123456789abcdef0123456789abcdef") | ||||||
|  | 
 | ||||||
|  |     variant("cuda", default=False, description="Build with CUDA") | ||||||
|  |     variant("cuda_arch", values=any_combination_of("10", "11"), when="+cuda") | ||||||
| @@ -0,0 +1,23 @@ | |||||||
|  | # Copyright 2013-2024 Lawrence Livermore National Security, LLC and other | ||||||
|  | # Spack Project Developers. See the top-level COPYRIGHT file for details. | ||||||
|  | # | ||||||
|  | # SPDX-License-Identifier: (Apache-2.0 OR MIT) | ||||||
|  | 
 | ||||||
|  | from spack.package import * | ||||||
|  | 
 | ||||||
|  | 
 | ||||||
|  | class ForwardMultiValue(Package): | ||||||
|  |     """A package that forwards the value of a multi-valued variant to a dependency""" | ||||||
|  | 
 | ||||||
|  |     homepage = "http://www.llnl.gov" | ||||||
|  |     url = "http://www.llnl.gov/mpileaks-1.0.tar.gz" | ||||||
|  | 
 | ||||||
|  |     version("1.0", md5="0123456789abcdef0123456789abcdef") | ||||||
|  | 
 | ||||||
|  |     variant("cuda", default=False, description="Build with CUDA") | ||||||
|  |     variant("cuda_arch", values=any_combination_of("10", "11"), when="+cuda") | ||||||
|  | 
 | ||||||
|  |     depends_on("dependency-mv") | ||||||
|  | 
 | ||||||
|  |     requires("^dependency-mv cuda_arch=10", when="+cuda cuda_arch=10 ^dependency-mv+cuda") | ||||||
|  |     requires("^dependency-mv cuda_arch=11", when="+cuda cuda_arch=11 ^dependency-mv+cuda") | ||||||
		Reference in New Issue
	
	Block a user