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.

2 files changed, 40 insertions, 8 deletions

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"
             )