From 49ac3471cf97f5308c648a78a9fd7b0c3a083e90 Mon Sep 17 00:00:00 2001
From: Todd Gamblin <tgamblin@llnl.gov>
Date: Mon, 28 Dec 2020 10:12:14 -0800
Subject: [PATCH] concretizer: consolidate handling of virtuals into
 spec_clauses

---
 lib/spack/spack/solver/asp.py | 46 +++++++++++++++--------------------
 1 file changed, 20 insertions(+), 26 deletions(-)

diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index fdb3ece52c..7f7ceeafc0 100644
--- a/lib/spack/spack/solver/asp.py
+++ b/lib/spack/spack/solver/asp.py
@@ -925,6 +925,7 @@ def spec_clauses(self, spec, body=False, transitive=True):
         # TODO: do this with consistent suffixes.
         class Head(object):
             node = fn.node
+            virtual_node = fn.virtual_node
             node_platform = fn.node_platform_set
             node_os = fn.node_os_set
             node_target = fn.node_target_set
@@ -935,6 +936,7 @@ class Head(object):
 
         class Body(object):
             node = fn.node
+            virtual_node = fn.virtual_node
             node_platform = fn.node_platform
             node_os = fn.node_os
             node_target = fn.node_target
@@ -946,7 +948,9 @@ class Body(object):
         f = Body if body else Head
 
         if spec.name:
-            clauses.append(f.node(spec.name))
+            clauses.append(
+                f.node(spec.name) if not spec.virtual
+                else f.virtual_node(spec.name))
 
         clauses.extend(self.spec_versions(spec))
 
@@ -973,7 +977,8 @@ class Body(object):
                     continue
 
                 # validate variant value
-                if vname not in spack.directives.reserved_names:
+                reserved_names = spack.directives.reserved_names
+                if (not spec.virtual and vname not in reserved_names):
                     variant_def = spec.package.variants[vname]
                     variant_def.validate_or_raise(variant, spec.package)
 
@@ -1014,11 +1019,7 @@ class Body(object):
         # add all clauses from dependencies
         if transitive:
             for dep in spec.traverse(root=False):
-                if dep.virtual:
-                    clauses.extend(self.virtual_spec_clauses(dep))
-                else:
-                    clauses.extend(
-                        self.spec_clauses(dep, body, transitive=False))
+                clauses.extend(self.spec_clauses(dep, body, transitive=False))
 
         return clauses
 
@@ -1392,17 +1393,12 @@ def setup(self, driver, specs, tests=False):
 
         self.gen.h1('Spec Constraints')
         for spec in sorted(specs):
-            if not spec.virtual:
-                self.gen.fact(fn.root(spec.name))
-            else:
-                self.gen.fact(fn.virtual_root(spec.name))
-
             self.gen.h2('Spec: %s' % str(spec))
-            if spec.virtual:
-                clauses = self.virtual_spec_clauses(spec)
-            else:
-                clauses = self.spec_clauses(spec)
-            for clause in clauses:
+            self.gen.fact(
+                fn.virtual_root(spec.name) if spec.virtual
+                else fn.root(spec.name)
+            )
+            for clause in self.spec_clauses(spec):
                 self.gen.fact(clause)
 
         self.gen.h1("Variant Values defined in specs")
@@ -1420,15 +1416,6 @@ def setup(self, driver, specs, tests=False):
         self.gen.h1("Target Constraints")
         self.define_target_constraints()
 
-    def virtual_spec_clauses(self, dep):
-        assert dep.virtual
-        self.virtual_constraints.add(str(dep))
-        clauses = [
-            fn.virtual_node(dep.name),
-            fn.single_provider_for(str(dep.name), str(dep.versions))
-        ]
-        return clauses
-
 
 class SpecBuilder(object):
     """Class with actions to rebuild a spec from ASP results."""
@@ -1590,6 +1577,13 @@ def build_specs(self, function_tuples):
                 continue
 
             assert action and callable(action)
+
+            # ignore predicates on virtual packages, as they're used for
+            # solving but don't construct anything
+            pkg = args[0]
+            if spack.repo.path.is_virtual(pkg):
+                continue
+
             action(*args)
 
         # namespace assignment is done after the fact, as it is not