From a0e8ad7a8bc6d903a0eacf456088ff2581afa6da Mon Sep 17 00:00:00 2001
From: Massimiliano Culpo <massimiliano.culpo@gmail.com>
Date: Thu, 22 Oct 2020 15:26:44 +0200
Subject: [PATCH] concretizer: ensure upfront that variants are valid

---
 lib/spack/spack/solver/asp.py      |  9 ++++++++-
 lib/spack/spack/spec.py            | 30 +++++++++++++++++++++---------
 lib/spack/spack/test/concretize.py |  4 ++--
 3 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index 264ab9e54b..f41a0c117a 100644
--- a/lib/spack/spack/solver/asp.py
+++ b/lib/spack/spack/solver/asp.py
@@ -207,7 +207,7 @@ def check_packages_exist(specs):
                 tty.debug(msg)
 
             if not check_passed:
-                raise spack.error.SpecError(str(s.name))
+                raise spack.repo.UnknownPackageError(str(s.fullname))
 
 
 class Result(object):
@@ -1753,5 +1753,12 @@ def solve(specs, dump=(), models=0, timers=False, stats=False, tests=False):
     if "asp" in dump:
         driver.out = sys.stdout
 
+    # Check upfront that the variants are admissible
+    for root in specs:
+        for s in root.traverse():
+            if s.virtual:
+                continue
+            spack.spec.Spec.ensure_valid_variants(s)
+
     setup = SpackSolverSetup()
     return driver.solve(setup, specs, dump, models, timers, stats, tests)
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index cacad483f6..127091d8da 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -2858,17 +2858,29 @@ def validate_or_raise(self):
 
             # Ensure correctness of variants (if the spec is not virtual)
             if not spec.virtual:
-                pkg_cls = spec.package_class
-                pkg_variants = pkg_cls.variants
-                # reserved names are variants that may be set on any package
-                # but are not necessarily recorded by the package's class
-                not_existing = set(spec.variants) - (
-                    set(pkg_variants) | set(spack.directives.reserved_names))
-                if not_existing:
-                    raise vt.UnknownVariantError(spec, not_existing)
-
+                Spec.ensure_valid_variants(spec)
                 vt.substitute_abstract_variants(spec)
 
+    @staticmethod
+    def ensure_valid_variants(spec):
+        """Ensures that the variant attached to a spec are valid.
+
+        Args:
+            spec (Spec): spec to be analyzed
+
+        Raises:
+            UnknownVariantError: on the first unknown variant found
+        """
+        pkg_cls = spec.package_class
+        pkg_variants = pkg_cls.variants
+        # reserved names are variants that may be set on any package
+        # but are not necessarily recorded by the package's class
+        not_existing = set(spec.variants) - (
+            set(pkg_variants) | set(spack.directives.reserved_names)
+        )
+        if not_existing:
+            raise vt.UnknownVariantError(spec, not_existing)
+
     def constrain(self, other, deps=True):
         """Merge the constraints of other with self.
 
diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py
index 7eed82568f..91d2874e48 100644
--- a/lib/spack/spack/test/concretize.py
+++ b/lib/spack/spack/test/concretize.py
@@ -649,7 +649,7 @@ def test_compiler_version_matches_any_entry_in_compilers_yaml(self):
         assert str(s.compiler.version) == '4.5.0'
 
     def test_concretize_anonymous(self):
-        with pytest.raises(spack.error.SpecError):
+        with pytest.raises(spack.error.SpackError):
             s = Spec('+variant')
             s.concretize()
 
@@ -657,6 +657,6 @@ def test_concretize_anonymous(self):
         'mpileaks ^%gcc', 'mpileaks ^cflags=-g'
     ])
     def test_concretize_anonymous_dep(self, spec_str):
-        with pytest.raises(spack.error.SpecError):
+        with pytest.raises(spack.error.SpackError):
             s = Spec(spec_str)
             s.concretize()