Requirements and preferences should not define (non-git) versions (#37687)
Ensure that requirements `packages:*:require:@x` and preferences `packages:*:version:[x]` fail concretization when no version defined in the package satisfies `x`. This always holds except for git versions -- they are defined on the fly.
This commit is contained in:
		| @@ -1759,13 +1759,30 @@ def key_fn(item): | ||||
|             # All the preferred version from packages.yaml, versions in external | ||||
|             # specs will be computed later | ||||
|             version_preferences = packages_yaml.get(pkg_name, {}).get("version", []) | ||||
|             for idx, v in enumerate(version_preferences): | ||||
|                 # v can be a string so force it into an actual version for comparisons | ||||
|                 ver = vn.Version(v) | ||||
|             version_defs = [] | ||||
|             pkg_class = spack.repo.path.get_pkg_class(pkg_name) | ||||
|             for vstr in version_preferences: | ||||
|                 v = vn.ver(vstr) | ||||
|                 if isinstance(v, vn.GitVersion): | ||||
|                     version_defs.append(v) | ||||
|                 else: | ||||
|                     satisfying_versions = list(x for x in pkg_class.versions if x.satisfies(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 | ||||
|                     # from greatest to least | ||||
|                     version_defs.extend(sorted(satisfying_versions, reverse=True)) | ||||
| 
 | ||||
|             for weight, vdef in enumerate(llnl.util.lang.dedupe(version_defs)): | ||||
|                 self.declared_versions[pkg_name].append( | ||||
|                     DeclaredVersion(version=ver, idx=idx, origin=Provenance.PACKAGES_YAML) | ||||
|                     DeclaredVersion(version=vdef, idx=weight, origin=Provenance.PACKAGES_YAML) | ||||
|                 ) | ||||
|                 self.possible_versions[pkg_name].add(ver) | ||||
|                 self.possible_versions[pkg_name].add(vdef) | ||||
| 
 | ||||
|     def add_concrete_versions_from_specs(self, specs, origin): | ||||
|         """Add concrete versions to possible versions from lists of CLI/dev specs.""" | ||||
| @@ -2296,6 +2313,11 @@ def _get_versioned_specs_from_pkg_requirements(): | ||||
| 
 | ||||
| 
 | ||||
| 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 | ||||
|     otherwise. | ||||
|     """ | ||||
|     if isinstance(section, str): | ||||
|         spec = spack.spec.Spec(section) | ||||
|         if not spec.name: | ||||
| @@ -2324,7 +2346,30 @@ def _specs_from_requires(pkg_name, section): | ||||
|                 spec.name = pkg_name | ||||
|             extracted_specs.append(spec) | ||||
| 
 | ||||
|     version_specs = [x for x in extracted_specs if x.versions.concrete] | ||||
|     version_specs = [] | ||||
|     for spec in extracted_specs: | ||||
|         if spec.versions.concrete: | ||||
|             # Note: this includes git versions | ||||
|             version_specs.append(spec) | ||||
|             continue | ||||
| 
 | ||||
|         # Prefer spec's name if it exists, in case the spec is | ||||
|         # 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)) | ||||
|         if not satisfying_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 | ||||
|         # will end up here | ||||
|         ordered_satisfying_versions = sorted(satisfying_versions, reverse=True) | ||||
|         vspecs = list(spack.spec.Spec("@{0}".format(x)) for x in ordered_satisfying_versions) | ||||
|         version_specs.extend(vspecs) | ||||
| 
 | ||||
|     for spec in version_specs: | ||||
|         spec.attach_git_version_lookup() | ||||
|     return version_specs | ||||
|   | ||||
| @@ -906,7 +906,7 @@ def test_env_config_precedence(environment_from_manifest): | ||||
|   mpileaks: | ||||
|     version: ["2.2"] | ||||
|   libelf: | ||||
|     version: ["0.8.11"] | ||||
|     version: ["0.8.10"] | ||||
| """ | ||||
|         ) | ||||
| 
 | ||||
|   | ||||
| @@ -152,7 +152,9 @@ def test_preferred_versions(self): | ||||
|         assert spec.version == Version("2.2") | ||||
| 
 | ||||
|     def test_preferred_versions_mixed_version_types(self): | ||||
|         update_packages("mixedversions", "version", ["2.0"]) | ||||
|         if spack.config.get("config:concretizer") == "original": | ||||
|             pytest.skip("This behavior is not enforced for the old concretizer") | ||||
|         update_packages("mixedversions", "version", ["=2.0"]) | ||||
|         spec = concretize("mixedversions") | ||||
|         assert spec.version == Version("2.0") | ||||
| 
 | ||||
| @@ -228,6 +230,29 @@ def test_preferred(self): | ||||
|         spec.concretize() | ||||
|         assert spec.version == Version("3.5.0") | ||||
| 
 | ||||
|     def test_preferred_undefined_raises(self): | ||||
|         """Preference should not specify an undefined version""" | ||||
|         if spack.config.get("config:concretizer") == "original": | ||||
|             pytest.xfail("This behavior is not enforced for the old concretizer") | ||||
| 
 | ||||
|         update_packages("python", "version", ["3.5.0.1"]) | ||||
|         spec = Spec("python") | ||||
|         with pytest.raises(spack.config.ConfigError): | ||||
|             spec.concretize() | ||||
| 
 | ||||
|     def test_preferred_truncated(self): | ||||
|         """Versions without "=" are treated as version ranges: if there is | ||||
|         a satisfying version defined in the package.py, we should use that | ||||
|         (don't define a new version). | ||||
|         """ | ||||
|         if spack.config.get("config:concretizer") == "original": | ||||
|             pytest.skip("This behavior is not enforced for the old concretizer") | ||||
| 
 | ||||
|         update_packages("python", "version", ["3.5"]) | ||||
|         spec = Spec("python") | ||||
|         spec.concretize() | ||||
|         assert spec.satisfies("@3.5.1") | ||||
| 
 | ||||
|     def test_develop(self): | ||||
|         """Test concretization with develop-like versions""" | ||||
|         spec = Spec("develop-test") | ||||
|   | ||||
| @@ -66,6 +66,28 @@ class V(Package): | ||||
| ) | ||||
| 
 | ||||
| 
 | ||||
| _pkgt = ( | ||||
|     "t", | ||||
|     """\ | ||||
| class T(Package): | ||||
|     version('2.1') | ||||
|     version('2.0') | ||||
| 
 | ||||
|     depends_on('u', when='@2.1:') | ||||
| """, | ||||
| ) | ||||
| 
 | ||||
| 
 | ||||
| _pkgu = ( | ||||
|     "u", | ||||
|     """\ | ||||
| class U(Package): | ||||
|     version('1.1') | ||||
|     version('1.0') | ||||
| """, | ||||
| ) | ||||
| 
 | ||||
| 
 | ||||
| @pytest.fixture | ||||
| def create_test_repo(tmpdir, mutable_config): | ||||
|     repo_path = str(tmpdir) | ||||
| @@ -79,7 +101,7 @@ def create_test_repo(tmpdir, mutable_config): | ||||
|         ) | ||||
| 
 | ||||
|     packages_dir = tmpdir.join("packages") | ||||
|     for pkg_name, pkg_str in [_pkgx, _pkgy, _pkgv]: | ||||
|     for pkg_name, pkg_str in [_pkgx, _pkgy, _pkgv, _pkgt, _pkgu]: | ||||
|         pkg_dir = packages_dir.ensure(pkg_name, dir=True) | ||||
|         pkg_file = pkg_dir.join("package.py") | ||||
|         with open(str(pkg_file), "w") as f: | ||||
| @@ -144,6 +166,45 @@ def test_requirement_isnt_optional(concretize_scope, test_repo): | ||||
|         Spec("x@1.1").concretize() | ||||
| 
 | ||||
| 
 | ||||
| def test_require_undefined_version(concretize_scope, test_repo): | ||||
|     """If a requirement specifies a numbered version that isn't in | ||||
|     the associated package.py and isn't part of a Git hash | ||||
|     equivalence (hash=number), then Spack should raise an error | ||||
|     (it is assumed this is a typo, and raising the error here | ||||
|     avoids a likely error when Spack attempts to fetch the version). | ||||
|     """ | ||||
|     if spack.config.get("config:concretizer") == "original": | ||||
|         pytest.skip("Original concretizer does not support configuration requirements") | ||||
| 
 | ||||
|     conf_str = """\ | ||||
| packages: | ||||
|   x: | ||||
|     require: "@1.2" | ||||
| """ | ||||
|     update_packages_config(conf_str) | ||||
|     with pytest.raises(spack.config.ConfigError): | ||||
|         Spec("x").concretize() | ||||
| 
 | ||||
| 
 | ||||
| def test_require_truncated(concretize_scope, test_repo): | ||||
|     """A requirement specifies a version range, with satisfying | ||||
|     versions defined in the package.py. Make sure we choose one | ||||
|     of the defined versions (vs. allowing the requirement to | ||||
|     define a new version). | ||||
|     """ | ||||
|     if spack.config.get("config:concretizer") == "original": | ||||
|         pytest.skip("Original concretizer does not support configuration requirements") | ||||
| 
 | ||||
|     conf_str = """\ | ||||
| packages: | ||||
|   x: | ||||
|     require: "@1" | ||||
| """ | ||||
|     update_packages_config(conf_str) | ||||
|     xspec = Spec("x").concretized() | ||||
|     assert xspec.satisfies("@1.1") | ||||
| 
 | ||||
| 
 | ||||
| def test_git_user_supplied_reference_satisfaction( | ||||
|     concretize_scope, test_repo, mock_git_version_info, monkeypatch | ||||
| ): | ||||
| @@ -220,6 +281,40 @@ def test_requirement_adds_new_version( | ||||
|     assert s1.version.ref == a_commit_hash | ||||
| 
 | ||||
| 
 | ||||
| def test_requirement_adds_version_satisfies( | ||||
|     concretize_scope, test_repo, mock_git_version_info, monkeypatch | ||||
| ): | ||||
|     """Make sure that new versions added by requirements are factored into | ||||
|     conditions. In this case create a new version that satisfies a | ||||
|     depends_on condition and make sure it is triggered (i.e. the | ||||
|     dependency is added). | ||||
|     """ | ||||
|     if spack.config.get("config:concretizer") == "original": | ||||
|         pytest.skip("Original concretizer does not support configuration" " requirements") | ||||
| 
 | ||||
|     repo_path, filename, commits = mock_git_version_info | ||||
|     monkeypatch.setattr( | ||||
|         spack.package_base.PackageBase, "git", path_to_file_url(repo_path), raising=False | ||||
|     ) | ||||
| 
 | ||||
|     # Sanity check: early version of T does not include U | ||||
|     s0 = Spec("t@2.0").concretized() | ||||
|     assert not ("u" in s0) | ||||
| 
 | ||||
|     conf_str = """\ | ||||
| packages: | ||||
|   t: | ||||
|     require: "@{0}=2.2" | ||||
| """.format( | ||||
|         commits[0] | ||||
|     ) | ||||
|     update_packages_config(conf_str) | ||||
| 
 | ||||
|     s1 = Spec("t").concretized() | ||||
|     assert "u" in s1 | ||||
|     assert s1.satisfies("@2.2") | ||||
| 
 | ||||
| 
 | ||||
| def test_requirement_adds_git_hash_version( | ||||
|     concretize_scope, test_repo, mock_git_version_info, monkeypatch | ||||
| ): | ||||
|   | ||||
		Reference in New Issue
	
	Block a user
	 Peter Scheibel
					Peter Scheibel