From 79ba0c50c1afefba99387578c0dd9237b3c8cd58 Mon Sep 17 00:00:00 2001 From: Greg Becker Date: Fri, 8 Apr 2022 02:01:04 -0700 Subject: concretize.lp: enforce target compatibility through DAG (#29694) Spack currently allows dependencies to be concretized for an architecture incompatible with the root. This commit adds rules to make this situation impossible by design. --- lib/spack/spack/solver/concretize.lp | 34 ++++++++++++++++++++++++++++++---- lib/spack/spack/test/concretize.py | 14 ++++++++++---- 2 files changed, 40 insertions(+), 8 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp index 13902d0892..36426bd836 100644 --- a/lib/spack/spack/solver/concretize.lp +++ b/lib/spack/spack/solver/concretize.lp @@ -615,17 +615,43 @@ node_os(Package, OS) :- node_os_set(Package, OS), node(Package). % Each node has only one target chosen among the known targets 1 { node_target(Package, Target) : target(Target) } 1 :- node(Package), error("Each node must have exactly one target"). -% If a node must satisfy a target constraint the choice is reduced among the targets -% that satisfy that constraint -1 { node_target(Package, Target) : target_satisfies(Constraint, Target) } 1 - :- node_target_satisfies(Package, Constraint), error("Each node must have exactly one target"). +% If a node must satisfy a target constraint, enforce it +:- node_target(Package, Target), + node_target_satisfies(Package, Constraint), + not target_satisfies(Constraint, Target), + error("Node targets must satisfy node target constraints"). + % If a node has a target and the target satisfies a constraint, then the target % associated with the node satisfies the same constraint node_target_satisfies(Package, Constraint) :- node_target(Package, Target), target_satisfies(Constraint, Target). +% If a node has a target, all of its dependencies must be compatible with that target +:- depends_on(Package, Dependency), + node_target(Package, Target), + not node_target_compatible(Dependency, Target), + error("Dependency node targets must be compatible with dependent targets"). + +% Intermediate step for performance reasons +% When the integrity constraint above was formulated including this logic +% we suffered a substantial performance penalty +node_target_compatible(Package, Target) + :- node_target(Package, MyTarget), + target_compatible(Target, MyTarget). + +% target_compatible(T1, T2) means code for T2 can run on T1 +% This order is dependent -> dependency in the node DAG, which +% is contravariant with the target DAG. +target_compatible(Target, Target) :- target(Target). +target_compatible(Child, Parent) :- target_parent(Child, Parent). +target_compatible(Descendent, Ancestor) + :- target_parent(Target, Ancestor), + target_compatible(Descendent, Target), + target(Target). + #defined target_satisfies/2. +#defined target_parent/2. % The target weight is either the default target weight % or a more specific per-package weight if set diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py index 92ff38ea34..4a348828eb 100644 --- a/lib/spack/spack/test/concretize.py +++ b/lib/spack/spack/test/concretize.py @@ -768,7 +768,7 @@ class TestConcretize(object): ]) def test_compiler_conflicts_in_package_py(self, spec_str, expected_str): if spack.config.get('config:concretizer') == 'original': - pytest.skip('Original concretizer cannot work around conflicts') + pytest.xfail('Original concretizer cannot work around conflicts') s = Spec(spec_str).concretized() assert s.satisfies(expected_str) @@ -1103,6 +1103,12 @@ class TestConcretize(object): with pytest.raises(spack.error.SpackError): Spec('impossible-concretization').concretized() + def test_target_compatibility(self): + if spack.config.get('config:concretizer') == 'original': + pytest.xfail('Known failure of the original concretizer') + with pytest.raises(spack.error.SpackError): + Spec('libdwarf target=x86_64 ^libelf target=x86_64_v2').concretized() + @pytest.mark.regression('20040') def test_variant_not_default(self): s = Spec('ecp-viz-sdk').concretized() @@ -1369,7 +1375,7 @@ class TestConcretize(object): self, mutable_database, spec_str, expect_installed, config ): if spack.config.get('config:concretizer') == 'original': - pytest.skip('Original concretizer cannot reuse specs') + pytest.xfail('Original concretizer cannot reuse specs') # Test the internal consistency of solve + DAG reconstruction # when reused specs are added to the mix. This prevents things @@ -1383,7 +1389,7 @@ class TestConcretize(object): @pytest.mark.regression('26721,19736') def test_sticky_variant_in_package(self): if spack.config.get('config:concretizer') == 'original': - pytest.skip('Original concretizer cannot use sticky variants') + pytest.xfail('Original concretizer cannot use sticky variants') # Here we test that a sticky variant cannot be changed from its default value # by the ASP solver if not set explicitly. The package used in the test needs @@ -1400,7 +1406,7 @@ class TestConcretize(object): def test_do_not_invent_new_concrete_versions_unless_necessary(self): if spack.config.get('config:concretizer') == 'original': - pytest.skip( + pytest.xfail( "Original concretizer doesn't resolve concrete versions to known ones" ) -- cgit v1.2.3-60-g2f50