Better error message when setting unknown variants during concretization (#13128)

fixes #13124

3 files changed, 17 insertions, 8 deletions

diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index 6a16a308db..c51f463850 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -2661,7 +2661,7 @@ class Spec(object):
                 not_existing = set(spec.variants) - (
                     set(pkg_variants) | set(spack.directives.reserved_names))
                 if not_existing:
-                    raise UnknownVariantError(spec.name, not_existing)
+                    raise UnknownVariantError(spec, not_existing)
 
                 substitute_abstract_variants(spec)
 
diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py
index a183742e65..9d8b9de647 100644
--- a/lib/spack/spack/test/spec_semantics.py
+++ b/lib/spack/spack/test/spec_semantics.py
@@ -9,7 +9,7 @@ import pytest
 from spack.spec import Spec, UnsatisfiableSpecError, SpecError
 from spack.spec import substitute_abstract_variants
 from spack.spec import SpecFormatSigilError, SpecFormatStringError
-from spack.variant import InvalidVariantValueError
+from spack.variant import InvalidVariantValueError, UnknownVariantError
 from spack.variant import MultipleValuesInExclusiveVariantError
 
 import spack.architecture
@@ -981,3 +981,9 @@ class TestSpecSematics(object):
     def test_target_constraints(self, spec, constraint, expected_result):
         s = Spec(spec)
         assert s.satisfies(constraint) is expected_result
+
+    @pytest.mark.regression('13124')
+    def test_error_message_unknown_variant(self):
+        s = Spec('mpileaks +unknown')
+        with pytest.raises(UnknownVariantError, match=r'package has no such'):
+            s.concretize()
diff --git a/lib/spack/spack/variant.py b/lib/spack/spack/variant.py
index 7eea243b06..bdee9d3552 100644
--- a/lib/spack/spack/variant.py
+++ b/lib/spack/spack/variant.py
@@ -600,7 +600,9 @@ def substitute_abstract_variants(spec):
     for name, v in spec.variants.items():
         if name in spack.directives.reserved_names:
             continue
-        pkg_variant = spec.package_class.variants[name]
+        pkg_variant = spec.package_class.variants.get(name, None)
+        if not pkg_variant:
+            raise UnknownVariantError(spec, [name])
         new_variant = pkg_variant.make_variant(v._original_value)
         pkg_variant.validate_or_raise(new_variant, spec.package_class)
         spec.variants.substitute(new_variant)
@@ -778,12 +780,13 @@ class DuplicateVariantError(error.SpecError):
 
 class UnknownVariantError(error.SpecError):
     """Raised when an unknown variant occurs in a spec."""
-
-    def __init__(self, pkg, variants):
+    def __init__(self, spec, variants):
         self.unknown_variants = variants
-        super(UnknownVariantError, self).__init__(
-            'Package {0} has no variant {1}!'.format(pkg, comma_or(variants))
-        )
+        variant_str = 'variant' if len(variants) == 1 else 'variants'
+        msg = ('trying to set {0} "{1}" in package "{2}", but the package'
+               ' has no such {0} [happened during concretization of {3}]')
+        msg = msg.format(variant_str, comma_or(variants), spec.name, spec.root)
+        super(UnknownVariantError, self).__init__(msg)
 
 
 class InconsistentValidationError(error.SpecError):