Bugfix: allow preferred new versions from externals (#37747)

lib/spack/spack/solver/asp.py

@@ -861,9 +861,9 @@ class SpackSolverSetup(object):
     def __init__(self, tests=False):
         self.gen = None  # set by setup()
 
-        self.declared_versions = {}
+        self.declared_versions = collections.defaultdict(list)
-        self.possible_versions = {}
+        self.possible_versions = collections.defaultdict(set)
-        self.deprecated_versions = {}
+        self.deprecated_versions = collections.defaultdict(set)
 
         self.possible_virtuals = None
         self.possible_compilers = []
@@ -1722,10 +1722,6 @@ class Body(object):
 
     def build_version_dict(self, possible_pkgs):
         """Declare any versions in specs not declared in packages."""
-        self.declared_versions = collections.defaultdict(list)
-        self.possible_versions = collections.defaultdict(set)
-        self.deprecated_versions = collections.defaultdict(set)
-
         packages_yaml = spack.config.get("packages")
         packages_yaml = _normalize_packages_yaml(packages_yaml)
         for pkg_name in possible_pkgs:
@@ -1766,12 +1762,7 @@ def key_fn(item):
                 if isinstance(v, vn.GitVersion):
                     version_defs.append(v)
                 else:
-                    satisfying_versions = list(x for x in pkg_class.versions if x.satisfies(v))
+                    satisfying_versions = self._check_for_defined_matching_versions(pkg_class, v)
-                    if not satisfying_versions:
-                        raise spack.config.ConfigError(
-                            "Preference for version {0} does not match any version "
-                            " defined in {1}".format(str(v), pkg_name)
-                        )
                     # Amongst all defined versions satisfying this specific
                     # preference, the highest-numbered version is the
                     # most-preferred: therefore sort satisfying versions
@@ -1784,6 +1775,28 @@ def key_fn(item):
                 )
                 self.possible_versions[pkg_name].add(vdef)
 
+    def _check_for_defined_matching_versions(self, pkg_class, v):
+        """Given a version specification (which may be a concrete version,
+        range, etc.), determine if any package.py version declarations
+        or externals define a version which satisfies it.
+
+        This is primarily for determining whether a version request (e.g.
+        version preferences, which should not themselves define versions)
+        refers to a defined version.
+
+        This function raises an exception if no satisfying versions are
+        found.
+        """
+        pkg_name = pkg_class.name
+        satisfying_versions = list(x for x in pkg_class.versions if x.satisfies(v))
+        satisfying_versions.extend(x for x in self.possible_versions[pkg_name] if x.satisfies(v))
+        if not satisfying_versions:
+            raise spack.config.ConfigError(
+                "Preference for version {0} does not match any version"
+                " defined for {1} (in its package.py or any external)".format(str(v), pkg_name)
+            )
+        return satisfying_versions
+
     def add_concrete_versions_from_specs(self, specs, origin):
         """Add concrete versions to possible versions from lists of CLI/dev specs."""
         for s in spack.traverse.traverse_nodes(specs):
@@ -2215,14 +2228,6 @@ def setup(self, driver, specs, reuse=None):
         # get possible compilers
         self.possible_compilers = self.generate_possible_compilers(specs)
 
-        # traverse all specs and packages to build dict of possible versions
-        self.build_version_dict(possible)
-        self.add_concrete_versions_from_specs(specs, Provenance.SPEC)
-        self.add_concrete_versions_from_specs(dev_specs, Provenance.DEV_SPEC)
-
-        req_version_specs = _get_versioned_specs_from_pkg_requirements()
-        self.add_concrete_versions_from_specs(req_version_specs, Provenance.PACKAGE_REQUIREMENT)
-
         self.gen.h1("Concrete input spec definitions")
         self.define_concrete_input_specs(specs, possible)
 
@@ -2250,6 +2255,14 @@ def setup(self, driver, specs, reuse=None):
         self.provider_requirements()
         self.external_packages()
 
+        # traverse all specs and packages to build dict of possible versions
+        self.build_version_dict(possible)
+        self.add_concrete_versions_from_specs(specs, Provenance.SPEC)
+        self.add_concrete_versions_from_specs(dev_specs, Provenance.DEV_SPEC)
+
+        req_version_specs = self._get_versioned_specs_from_pkg_requirements()
+        self.add_concrete_versions_from_specs(req_version_specs, Provenance.PACKAGE_REQUIREMENT)
+
         self.gen.h1("Package Constraints")
         for pkg in sorted(self.pkgs):
             self.gen.h2("Package rules: %s" % pkg)
@@ -2296,8 +2309,7 @@ def literal_specs(self, specs):
         if self.concretize_everything:
             self.gen.fact(fn.concretize_everything())
 
-
+    def _get_versioned_specs_from_pkg_requirements(self):
-def _get_versioned_specs_from_pkg_requirements():
         """If package requirements mention versions that are not mentioned
         elsewhere, then we need to collect those to mark them as possible
         versions.
@@ -2308,11 +2320,10 @@ def _get_versioned_specs_from_pkg_requirements():
             if pkg_name == "all":
                 continue
             if "require" in d:
-            req_version_specs.extend(_specs_from_requires(pkg_name, d["require"]))
+                req_version_specs.extend(self._specs_from_requires(pkg_name, d["require"]))
         return req_version_specs
 
-
+    def _specs_from_requires(self, pkg_name, section):
-def _specs_from_requires(pkg_name, section):
         """Collect specs from requirements which define versions (i.e. those that
         have a concrete version). Requirements can define *new* versions if
         they are included as part of an equivalence (hash=number) but not
@@ -2357,11 +2368,8 @@ def _specs_from_requires(pkg_name, section):
             # requiring a specific implementation inside of a virtual section
             # e.g. packages:mpi:require:openmpi@4.0.1
             pkg_class = spack.repo.path.get_pkg_class(spec.name or pkg_name)
-        satisfying_versions = list(v for v in pkg_class.versions if v.satisfies(spec.versions))
+            satisfying_versions = self._check_for_defined_matching_versions(
-        if not satisfying_versions:
+                pkg_class, spec.versions
-            raise spack.config.ConfigError(
-                "{0} assigns a version that is not defined in"
-                " the associated package.py".format(str(spec))
             )
 
             # Version ranges ("@1.3" without the "=", "@1.2:1.4") and lists

lib/spack/spack/test/concretize_requirements.py

@@ -367,8 +367,11 @@ def test_requirement_adds_multiple_new_versions(
 def test_preference_adds_new_version(
     concretize_scope, test_repo, mock_git_version_info, monkeypatch
 ):
+    """Normally a preference cannot define a new version, but that constraint
+    is ignored if the version is a Git hash-based version.
+    """
     if spack.config.get("config:concretizer") == "original":
-        pytest.skip("Original concretizer does not support configuration requirements")
+        pytest.skip("Original concretizer does not enforce this constraint for preferences")
 
     repo_path, filename, commits = mock_git_version_info
     monkeypatch.setattr(
@@ -391,6 +394,29 @@ def test_preference_adds_new_version(
     assert not s3.satisfies("@2.3")
 
 
+def test_external_adds_new_version_that_is_preferred(concretize_scope, test_repo):
+    """Test that we can use a version, not declared in package recipe, as the
+    preferred version if that version appears in an external spec.
+    """
+    if spack.config.get("config:concretizer") == "original":
+        pytest.skip("Original concretizer does not enforce this constraint for preferences")
+
+    conf_str = """\
+packages:
+  y:
+    version: ["2.7"]
+    externals:
+    - spec: y@2.7 # Not defined in y
+      prefix: /fake/nonexistent/path/
+    buildable: false
+"""
+    update_packages_config(conf_str)
+
+    spec = Spec("x").concretized()
+    assert spec["y"].satisfies("@2.7")
+    assert spack.version.Version("2.7") not in spec["y"].package.versions
+
+
 def test_requirement_is_successfully_applied(concretize_scope, test_repo):
     """If a simple requirement can be satisfied, make sure the
     concretization succeeds and the requirement spec is applied.