Configuration: Allow requirements for virtual packages (#32369)

Extend the semantics of package requirements to
allow using them also under a virtual package
attribute in packages.yaml

These requirements are enforced whenever that
virtual spec is present in the DAG.

4 files changed, 149 insertions, 22 deletions

diff --git a/lib/spack/docs/build_settings.rst b/lib/spack/docs/build_settings.rst
index 6438c0735e..654473de9c 100644
--- a/lib/spack/docs/build_settings.rst
+++ b/lib/spack/docs/build_settings.rst
@@ -396,6 +396,16 @@ choose between a set of options using ``any_of`` or ``one_of``:
   ``mpich`` already includes a conflict, so this is redundant but
   still demonstrates the concept).
 
+.. note::
+
+   For ``any_of`` and ``one_of``, the order of specs indicates a
+   preference: items that appear earlier in the list are preferred
+   (note that these preferences can be ignored in favor of others).
+
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+Setting default requirements
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
 You can also set default requirements for all packages under ``all``
 like this:
 
@@ -422,13 +432,33 @@ under ``all`` are disregarded. For example, with a configuration like this:
 Spack requires ``cmake`` to use ``gcc`` and all other nodes (including cmake dependencies)
 to use ``clang``.
 
-Other notes about ``requires``:
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+Setting requirements on virtual specs
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+A requirement on a virtual spec applies whenever that virtual is present in the DAG. This
+can be useful for fixing which virtual provider you want to use:
+
+.. code-block:: yaml
+
+   packages:
+     mpi:
+       require: 'mvapich2 %gcc'
+
+With the configuration above the only allowed ``mpi`` provider is ``mvapich2 %gcc``.
+
+Requirements on the virtual spec and on the specific provider are both applied, if present. For
+instance with a configuration like:
+
+.. code-block:: yaml
+
+   packages:
+     mpi:
+       require: 'mvapich2 %gcc'
+     mvapich2:
+       require: '~cuda'
 
-* You cannot specify requirements for virtual packages (e.g. you can
-  specify requirements for ``openmpi`` but not ``mpi``).
-* For ``any_of`` and ``one_of``, the order of specs indicates a
-  preference: items that appear earlier in the list are preferred
-  (note that these preferences can be ignored in favor of others).
+you will use ``mvapich2~cuda %gcc`` as an ``mpi`` provider.
 
 .. _package_permissions:
 
diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index b1d5f284ce..7e489f4537 100644
--- a/lib/spack/spack/solver/asp.py
+++ b/lib/spack/spack/solver/asp.py
@@ -945,6 +945,13 @@ class SpackSolverSetup(object):
         requirements = config.get(pkg_name, {}).get("require", []) or config.get("all", {}).get(
             "require", []
         )
+        rules = self._rules_from_requirements(pkg_name, requirements)
+        self.emit_facts_from_requirement_rules(rules, virtual=False)
+
+    def _rules_from_requirements(self, pkg_name, requirements):
+        """Manipulate requirements from packages.yaml, and return a list of tuples
+        with a uniform structure (name, policy, requirements).
+        """
         if isinstance(requirements, string_types):
             rules = [(pkg_name, "one_of", [requirements])]
         else:
@@ -953,17 +960,7 @@ class SpackSolverSetup(object):
                 for policy in ("one_of", "any_of"):
                     if policy in requirement:
                         rules.append((pkg_name, policy, requirement[policy]))
-
-        for requirement_grp_id, (pkg_name, policy, requirement_grp) in enumerate(rules):
-            self.gen.fact(fn.requirement_group(pkg_name, requirement_grp_id))
-            self.gen.fact(fn.requirement_policy(pkg_name, requirement_grp_id, policy))
-            for requirement_weight, spec_str in enumerate(requirement_grp):
-                spec = spack.spec.Spec(spec_str)
-                if not spec.name:
-                    spec.name = pkg_name
-                member_id = self.condition(spec, imposed_spec=spec, name=pkg_name)
-                self.gen.fact(fn.requirement_group_member(member_id, pkg_name, requirement_grp_id))
-                self.gen.fact(fn.requirement_has_weight(member_id, requirement_weight))
+        return rules
 
     def pkg_rules(self, pkg, tests):
         pkg = packagize(pkg)
@@ -1057,7 +1054,7 @@ class SpackSolverSetup(object):
 
         self.package_requirement_rules(pkg)
 
-    def condition(self, required_spec, imposed_spec=None, name=None, msg=None):
+    def condition(self, required_spec, imposed_spec=None, name=None, msg=None, node=False):
         """Generate facts for a dependency or virtual provider condition.
 
         Arguments:
@@ -1067,6 +1064,8 @@ class SpackSolverSetup(object):
             name (str or None): name for `required_spec` (required if
                 required_spec is anonymous, ignored if not)
             msg (str or None): description of the condition
+            node (bool): if False does not emit "node" or "virtual_node" requirements
+                from the imposed spec
         Returns:
             int: id of the condition created by this function
         """
@@ -1083,7 +1082,7 @@ class SpackSolverSetup(object):
             self.gen.fact(fn.condition_requirement(condition_id, pred.name, *pred.args))
 
         if imposed_spec:
-            self.impose(condition_id, imposed_spec, node=False, name=name)
+            self.impose(condition_id, imposed_spec, node=node, name=name)
 
         return condition_id
 
@@ -1161,6 +1160,37 @@ class SpackSolverSetup(object):
             lambda v, p, i: self.gen.fact(fn.default_provider_preference(v, p, i)),
         )
 
+    def provider_requirements(self):
+        self.gen.h2("Requirements on virtual providers")
+        msg = (
+            "Internal Error: possible_virtuals is not populated. Please report to the spack"
+            " maintainers"
+        )
+        packages_yaml = spack.config.config.get("packages")
+        assert self.possible_virtuals is not None, msg
+        for virtual_str in sorted(self.possible_virtuals):
+            requirements = packages_yaml.get(virtual_str, {}).get("require", [])
+            rules = self._rules_from_requirements(virtual_str, requirements)
+            self.emit_facts_from_requirement_rules(rules, virtual=True)
+
+    def emit_facts_from_requirement_rules(self, rules, virtual=False):
+        """Generate facts to enforce requirements from packages.yaml."""
+        for requirement_grp_id, (pkg_name, policy, requirement_grp) in enumerate(rules):
+            self.gen.fact(fn.requirement_group(pkg_name, requirement_grp_id))
+            self.gen.fact(fn.requirement_policy(pkg_name, requirement_grp_id, policy))
+            for requirement_weight, spec_str in enumerate(requirement_grp):
+                spec = spack.spec.Spec(spec_str)
+                if not spec.name:
+                    spec.name = pkg_name
+                when_spec = spec
+                if virtual:
+                    when_spec = spack.spec.Spec(pkg_name)
+                member_id = self.condition(
+                    required_spec=when_spec, imposed_spec=spec, name=pkg_name, node=virtual
+                )
+                self.gen.fact(fn.requirement_group_member(member_id, pkg_name, requirement_grp_id))
+                self.gen.fact(fn.requirement_has_weight(member_id, requirement_weight))
+
     def external_packages(self):
         """Facts on external packages, as read from packages.yaml"""
         # Read packages.yaml and normalize it, so that it
@@ -1930,6 +1960,7 @@ class SpackSolverSetup(object):
 
         self.virtual_providers()
         self.provider_defaults()
+        self.provider_requirements()
         self.external_packages()
         self.flag_defaults()
 
diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp
index 9884530306..fb58589b7a 100644
--- a/lib/spack/spack/solver/concretize.lp
+++ b/lib/spack/spack/solver/concretize.lp
@@ -504,9 +504,12 @@ error(2, "Attempted to use external for '{0}' which does not satisfy any configu
 % Config required semantics
 %-----------------------------------------------------------------------------
 
+activate_requirement_rules(Package) :- node(Package).
+activate_requirement_rules(Package) :- virtual_node(Package).
+
 requirement_group_satisfied(Package, X) :-
   1 { condition_holds(Y) : requirement_group_member(Y, Package, X) } 1,
-  node(Package),
+  activate_requirement_rules(Package),
   requirement_policy(Package, X, "one_of"),
   requirement_group(Package, X).
 
@@ -519,7 +522,7 @@ requirement_weight(Package, W) :-
 
 requirement_group_satisfied(Package, X) :-
   1 { condition_holds(Y) : requirement_group_member(Y, Package, X) } ,
-  node(Package),
+  activate_requirement_rules(Package),
   requirement_policy(Package, X, "any_of"),
   requirement_group(Package, X).
 
@@ -535,7 +538,7 @@ requirement_weight(Package, W) :-
   requirement_group_satisfied(Package, X).
 
 error(2, "Cannot satisfy requirement group for package '{0}'", Package) :-
-  node(Package),
+  activate_requirement_rules(Package),
   requirement_group(Package, X),
   not requirement_group_satisfied(Package, X).
 
diff --git a/lib/spack/spack/test/concretize_requirements.py b/lib/spack/spack/test/concretize_requirements.py
index 329c881cf9..50cc6baa9a 100644
--- a/lib/spack/spack/test/concretize_requirements.py
+++ b/lib/spack/spack/test/concretize_requirements.py
@@ -349,3 +349,66 @@ packages:
     assert spec.satisfies(specific_exp)
     for s in spec.traverse(root=False):
         assert s.satisfies(generic_exp)
+
+
+@pytest.mark.parametrize("mpi_requirement", ["mpich", "mpich2", "zmpi"])
+def test_requirements_on_virtual(mpi_requirement, concretize_scope, mock_packages):
+    if spack.config.get("config:concretizer") == "original":
+        pytest.skip("Original concretizer does not support configuration" " requirements")
+    conf_str = """\
+packages:
+  mpi:
+    require: "{}"
+""".format(
+        mpi_requirement
+    )
+    update_packages_config(conf_str)
+
+    spec = Spec("callpath").concretized()
+    assert "mpi" in spec
+    assert mpi_requirement in spec
+
+
+@pytest.mark.parametrize(
+    "mpi_requirement,specific_requirement",
+    [
+        ("mpich", "@3.0.3"),
+        ("mpich2", "%clang"),
+        ("zmpi", "%gcc"),
+    ],
+)
+def test_requirements_on_virtual_and_on_package(
+    mpi_requirement, specific_requirement, concretize_scope, mock_packages
+):
+    if spack.config.get("config:concretizer") == "original":
+        pytest.skip("Original concretizer does not support configuration" " requirements")
+    conf_str = """\
+packages:
+  mpi:
+    require: "{0}"
+  {0}:
+    require: "{1}"
+""".format(
+        mpi_requirement, specific_requirement
+    )
+    update_packages_config(conf_str)
+
+    spec = Spec("callpath").concretized()
+    assert "mpi" in spec
+    assert mpi_requirement in spec
+    assert spec["mpi"].satisfies(specific_requirement)
+
+
+def test_incompatible_virtual_requirements_raise(concretize_scope, mock_packages):
+    if spack.config.get("config:concretizer") == "original":
+        pytest.skip("Original concretizer does not support configuration" " requirements")
+    conf_str = """\
+    packages:
+      mpi:
+        require: "mpich"
+    """
+    update_packages_config(conf_str)
+
+    spec = Spec("callpath ^zmpi")
+    with pytest.raises(UnsatisfiableSpecError):
+        spec.concretize()