Constrain abstract specs rather than concatenating strings in the "when" context manager (#26700)

Using the Spec.constrain method doesn't work since it might
trigger a repository lookup which could break our directives
and triggers a circular import error.

To fix that we introduce a function to merge abstract anonymous
specs, based only on package names, which does not perform any
lookup in the repository.

6 files changed, 73 insertions, 11 deletions

diff --git a/lib/spack/docs/packaging_guide.rst b/lib/spack/docs/packaging_guide.rst
index 2f42336d15..b9fbb18cf7 100644
--- a/lib/spack/docs/packaging_guide.rst
+++ b/lib/spack/docs/packaging_guide.rst
@@ -2824,7 +2824,7 @@ is equivalent to:
 
    depends_on('elpa+openmp', when='+openmp+elpa')
 
-Constraints from nested context managers are also added together, but they are rarely
+Constraints from nested context managers are also combined together, but they are rarely
 needed or recommended.
 
 .. _install-method:
diff --git a/lib/spack/spack/build_systems/cuda.py b/lib/spack/spack/build_systems/cuda.py
index 29ca46edff..1feeacd1c0 100644
--- a/lib/spack/spack/build_systems/cuda.py
+++ b/lib/spack/spack/build_systems/cuda.py
@@ -88,7 +88,7 @@ class CudaPackage(PackageBase):
 
     # Linux x86_64 compiler conflicts from here:
     # https://gist.github.com/ax3l/9489132
-    with when('~allow-unsupported-compilers'):
+    with when('^cuda~allow-unsupported-compilers'):
         # GCC
         # According to
         # https://github.com/spack/spack/pull/25054#issuecomment-886531664
diff --git a/lib/spack/spack/directives.py b/lib/spack/spack/directives.py
index 91deedbb5a..863001a601 100644
--- a/lib/spack/spack/directives.py
+++ b/lib/spack/spack/directives.py
@@ -89,6 +89,9 @@ def make_when_spec(value):
            value indicating when a directive should be applied.
 
     """
+    if isinstance(value, spack.spec.Spec):
+        return value
+
     # Unsatisfiable conditions are discarded by the caller, and never
     # added to the package class
     if value is False:
@@ -248,10 +251,16 @@ class DirectiveMeta(type):
                         msg = msg.format(decorated_function.__name__)
                         raise DirectiveError(msg)
 
-                    when_spec_from_context = ' '.join(
+                    when_constraints = [
+                        spack.spec.Spec(x) for x in
                         DirectiveMeta._when_constraints_from_context
+                    ]
+                    if kwargs.get('when'):
+                        when_constraints.append(spack.spec.Spec(kwargs['when']))
+                    when_spec = spack.spec.merge_abstract_anonymous_specs(
+                        *when_constraints
                     )
-                    when_spec = kwargs.get('when', '') + ' ' + when_spec_from_context
+
                     kwargs['when'] = when_spec
 
                 # If any of the arguments are executors returned by a
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index ff71662a03..8b13374430 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -4433,6 +4433,34 @@ class Spec(object):
         return _spec_from_dict, (self.to_dict(hash=ht.build_hash),)
 
 
+def merge_abstract_anonymous_specs(*abstract_specs):
+    """Merge the abstracts specs passed as input and return the result.
+
+    The root specs must be anonymous, and it's duty of the caller to ensure that.
+
+    This function merge the abstract specs based on package names. In particular
+    it doesn't try to resolve virtual dependencies.
+
+    Args:
+        *abstract_specs (list of Specs): abstract specs to be merged
+    """
+    merged_spec = spack.spec.Spec()
+    for current_spec_constraint in abstract_specs:
+        merged_spec.constrain(current_spec_constraint, deps=False)
+
+        for name in merged_spec.common_dependencies(current_spec_constraint):
+            merged_spec[name].constrain(
+                current_spec_constraint[name], deps=False
+            )
+
+        # Update with additional constraints from other spec
+        for name in current_spec_constraint.dep_difference(merged_spec):
+            edge = current_spec_constraint.get_dependency(name)
+            merged_spec._add_dependency(edge.spec.copy(), edge.deptypes)
+
+    return merged_spec
+
+
 def _spec_from_old_dict(data):
     """Construct a spec from JSON/YAML using the format version 1.
     Note: Version 1 format has no notion of a build_spec, and names are
diff --git a/lib/spack/spack/test/directives.py b/lib/spack/spack/test/directives.py
index a40eff6b71..31038ebb47 100644
--- a/lib/spack/spack/test/directives.py
+++ b/lib/spack/spack/test/directives.py
@@ -2,9 +2,10 @@
 # Spack Project Developers. See the top-level COPYRIGHT file for details.
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
+import pytest
 
 import spack.repo
-from spack.spec import Spec
+import spack.spec
 
 
 def test_false_directives_do_not_exist(mock_packages):
@@ -24,21 +25,29 @@ def test_true_directives_exist(mock_packages):
     cls = spack.repo.path.get_pkg_class('when-directives-true')
 
     assert cls.dependencies
-    assert Spec() in cls.dependencies['extendee']
-    assert Spec() in cls.dependencies['b']
+    assert spack.spec.Spec() in cls.dependencies['extendee']
+    assert spack.spec.Spec() in cls.dependencies['b']
 
     assert cls.resources
-    assert Spec() in cls.resources
+    assert spack.spec.Spec() in cls.resources
 
     assert cls.patches
-    assert Spec() in cls.patches
+    assert spack.spec.Spec() in cls.patches
 
 
 def test_constraints_from_context(mock_packages):
     pkg_cls = spack.repo.path.get_pkg_class('with-constraint-met')
 
     assert pkg_cls.dependencies
-    assert Spec('@1.0') in pkg_cls.dependencies['b']
+    assert spack.spec.Spec('@1.0') in pkg_cls.dependencies['b']
 
     assert pkg_cls.conflicts
-    assert (Spec('@1.0'), None) in pkg_cls.conflicts['%gcc']
+    assert (spack.spec.Spec('+foo@1.0'), None) in pkg_cls.conflicts['%gcc']
+
+
+@pytest.mark.regression('26656')
+def test_constraints_from_context_are_merged(mock_packages):
+    pkg_cls = spack.repo.path.get_pkg_class('with-constraint-met')
+
+    assert pkg_cls.dependencies
+    assert spack.spec.Spec('@0.14:15 ^b@3.8:4.0') in pkg_cls.dependencies['c']
diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py
index 97351b0ce2..a3952397cf 100644
--- a/lib/spack/spack/test/spec_semantics.py
+++ b/lib/spack/spack/test/spec_semantics.py
@@ -1212,3 +1212,19 @@ def test_spec_dict_hashless_dep():
                 }
             }
         )
+
+
+@pytest.mark.parametrize('specs,expected', [
+    # Anonymous specs without dependencies
+    (['+baz', '+bar'], '+baz+bar'),
+    (['@2.0:', '@:5.1', '+bar'], '@2.0:5.1 +bar'),
+    # Anonymous specs with dependencies
+    (['^mpich@3.2', '^mpich@:4.0+foo'], '^mpich@3.2 +foo'),
+    # Mix a real package with a virtual one. This test
+    # should fail if we start using the repository
+    (['^mpich@3.2', '^mpi+foo'], '^mpich@3.2 ^mpi+foo'),
+])
+def test_merge_abstract_anonymous_specs(specs, expected):
+    specs = [Spec(x) for x in specs]
+    result = spack.spec.merge_abstract_anonymous_specs(*specs)
+    assert result == Spec(expected)