diff --git a/stackinator/builder.py b/stackinator/builder.py index 89a14dde..a9021bc8 100644 --- a/stackinator/builder.py +++ b/stackinator/builder.py @@ -229,6 +229,16 @@ def generate(self, recipe): dest.parent.mkdir(parents=True, exist_ok=True) dest.write_bytes(content) + # Spack 1.2 makes the new (parallel) installer the default. + # Stackinator 6 the behaviour of the old installer, so pin config:installer to + # "old" for spack >= 1.2. + if spack_version >= (1, 2): + config_yaml_path = config_path / "config.yaml" + config_doc = yaml.safe_load(config_yaml_path.read_text()) if config_yaml_path.exists() else None + config_doc = config_doc or {} + config_doc.setdefault("config", {})["installer"] = "old" + config_yaml_path.write_text(yaml.dump(config_doc, default_flow_style=False)) + # generate top level makefiles makefile_template = jinja_env.get_template("Makefile") diff --git a/stackinator/mirror.py b/stackinator/mirror.py index 689c8413..3e8c7c1c 100644 --- a/stackinator/mirror.py +++ b/stackinator/mirror.py @@ -8,6 +8,7 @@ import magic from . import schema, root_logger +from .spack_util import Version # GPG keys may be presented either ASCII-armored (these headers) or as binary @@ -27,15 +28,14 @@ ) -def _supports_concretization_cache(spack_version: str) -> bool: - """Whether the given "major.minor" spack version supports the concretizer cache. +def _supports_concretization_cache(spack_version: Version) -> bool: + """Whether the given spack version supports the concretizer cache. The concretizer:concretization_cache config key was introduced in spack 1.1; spack 1.0 rejects it (its concretizer schema forbids unknown keys). """ - major_minor = tuple(int(x) for x in spack_version.split(".")) - return major_minor >= (1, 1) + return spack_version >= (1, 1) class MirrorError(RuntimeError): @@ -88,7 +88,7 @@ def __init__( self, system_config_root: pathlib.Path, mount_path: pathlib.Path, - spack_version: str, + spack_version: Version, mirror_file: Optional[pathlib.Path] = None, cmdline_cache: Optional[pathlib.Path] = None, ): @@ -96,7 +96,7 @@ def __init__( Mirrors are supplied with the --mirror command line option (mirror_file). mount_path is the recipe mount point (used to make a build cache mount-specific). - spack_version is the best-effort "major.minor" spack version, used to gate the + spack_version is the best-effort spack Version, used to gate the concretizer cache (only emitted for spack >= 1.1). cmdline_cache is an optional legacy cache.yaml passed on the command line (--cache). diff --git a/stackinator/recipe.py b/stackinator/recipe.py index 8b905be7..41309373 100644 --- a/stackinator/recipe.py +++ b/stackinator/recipe.py @@ -340,28 +340,28 @@ def config(self, config_path): def with_modules(self) -> bool: return self.modules is not None - # Make a best-effort determination of the "major.minor" version of spack being + # Make a best-effort determination of the major.minor version of spack being # used, inferred from the spack commit in config.yaml. This is only a hint: the # commit can be an arbitrary branch/tag/sha, so when the version cannot be pinned - # we default to the latest supported version ("1.1"). Returns a "major.minor" - # string (e.g. "1.0", "1.1"). + # we default to the most recent version expected by default with this tool + # ("1.1"). Returns a spack_util.Version. def find_spack_version(self, develop): - # the latest supported version, used when the version cannot be determined - # (an explicit --develop, the default branch, or an unrecognised commit). - default = "1.1" + # the most recent version expected to be used by default with this tool, + # used when the version cannot be determined from an unrecognised commit. + default = spack_util.Version(1, 1) + # the develop and main branches of spack are now at 1.2. if develop: - return default + return spack_util.Version(1, 2) commit = self.config["spack"]["commit"] if commit is None or commit in ("develop", "main"): - return default + return spack_util.Version(1, 2) - # match a release branch/tag (releases/v1.0, v1.1, v1.1.2) or a bare "1.0", - # and extract the major.minor version. - match = re.search(r"v?(\d+)\.(\d+)(?:\.\d+)?", commit) - if match: - return f"{match.group(1)}.{match.group(2)}" + # match a release branch/tag (releases/v1.0, v1.1, v1.1.2) or a bare "1.0". + version = spack_util.Version.parse(commit) + if version is not None: + return version return default diff --git a/stackinator/spack_util.py b/stackinator/spack_util.py index ba30e440..36118236 100644 --- a/stackinator/spack_util.py +++ b/stackinator/spack_util.py @@ -1,3 +1,35 @@ +import re +from typing import NamedTuple, Optional + + +class Version(NamedTuple): + """A minimal "major.minor" spack version that supports comparison. + + Implemented as a NamedTuple so that the comparison operators (==, <, >=, ...) + come for free via tuple ordering, e.g. ``Version(1, 2) > Version(1, 1)``. + Because it is a tuple it also compares directly against plain tuples, which is + convenient in Jinja templates: ``{% if spack_version >= (1, 2) %}``. + """ + + major: int + minor: int + + @classmethod + def parse(cls, text: str) -> Optional["Version"]: + """Extract a "major.minor" Version from text, or None if none is present. + + Matches a release branch/tag (releases/v1.0, v1.1, v1.1.2) or a bare + "1.0", ignoring any trailing patch component. + """ + match = re.search(r"v?(\d+)\.(\d+)(?:\.\d+)?", text) + if match is None: + return None + return cls(int(match.group(1)), int(match.group(2))) + + def __str__(self) -> str: + return f"{self.major}.{self.minor}" + + def is_repo(path): """ Returns True if path contains a spack package repo, where the definition of diff --git a/stackinator/templates/Makefile b/stackinator/templates/Makefile index 8ba9108f..1cd261bf 100644 --- a/stackinator/templates/Makefile +++ b/stackinator/templates/Makefile @@ -35,11 +35,11 @@ mirror-setup: spack-setup{% if pre_install_hook %} pre-install{% endif %} {% if cache %} @echo "Pulling and trusting keys from configured buildcaches." - $(SANDBOX) $(SPACK) buildcache keys --install --trust + $(SANDBOX) $(SPACK) buildcache keys --install --trust{% if spack_version >= (1, 2) %} --yes-to-all{% endif +%} {% endif %} @echo "Adding mirror gpg keys." {% for key_path in gpg_keys %} - $(SANDBOX) $(SPACK) gpg trust {{ key_path }} + $(SANDBOX) $(SPACK) gpg trust{% if spack_version >= (1, 2) %} --yes-to-all{% endif %} {{ key_path }} {% endfor %} @echo "Current mirror list:" $(SANDBOX) $(SPACK) mirror list diff --git a/unittests/test_mirrors.py b/unittests/test_mirrors.py index 47ec1dd1..050a7cd4 100644 --- a/unittests/test_mirrors.py +++ b/unittests/test_mirrors.py @@ -4,6 +4,8 @@ import stackinator.mirror as mirror import yaml +from stackinator.spack_util import Version + @pytest.fixture def test_path(): @@ -36,7 +38,7 @@ def mirror_ok(systems_path): def test_mirror_init(clean_root, mount_path, systems_path, mirror_ok): """Check that the three kinds of mirror are resolved into separate members.""" - mirrors_obj = mirror.Mirrors(clean_root, mount_path, "1.1", mirror_file=mirror_ok) + mirrors_obj = mirror.Mirrors(clean_root, mount_path, Version(1, 1), mirror_file=mirror_ok) with (systems_path / "../test-gpg-pub.asc").open("rb") as pub_key_file: pub_key_b64 = base64.b64encode(pub_key_file.read()).decode() @@ -79,20 +81,20 @@ def test_system_mirrors_yaml_rejected(systems_path, mount_path): # mirror-ok contains a mirrors.yaml; passing it as the system config root must raise. with pytest.raises(mirror.MirrorError): - mirror.Mirrors(systems_path / "mirror-ok", mount_path, "1.1") + mirror.Mirrors(systems_path / "mirror-ok", mount_path, Version(1, 1)) def test_missing_mirror_file(clean_root, mount_path): """A --mirror file that does not exist raises MirrorError.""" with pytest.raises(mirror.MirrorError): - mirror.Mirrors(clean_root, mount_path, "1.1", mirror_file=clean_root / "does-not-exist.yaml") + mirror.Mirrors(clean_root, mount_path, Version(1, 1), mirror_file=clean_root / "does-not-exist.yaml") def test_no_mirror_file(clean_root, mount_path): """With no --mirror file there are no mirrors configured.""" - mirrors_obj = mirror.Mirrors(clean_root, mount_path, "1.1") + mirrors_obj = mirror.Mirrors(clean_root, mount_path, Version(1, 1)) assert mirrors_obj.buildcache is None assert mirrors_obj.bootstrap is None @@ -105,7 +107,11 @@ def test_command_line_cache(clean_root, mount_path, systems_path, mirror_ok): """Check that adding a cache from the command line works.""" mirrors = mirror.Mirrors( - clean_root, mount_path, "1.1", mirror_file=mirror_ok, cmdline_cache=systems_path / "mirror-ok/cache.yaml" + clean_root, + mount_path, + Version(1, 1), + mirror_file=mirror_ok, + cmdline_cache=systems_path / "mirror-ok/cache.yaml", ) # the command line cache overrides any build cache defined in the mirror file, @@ -127,7 +133,11 @@ def test_keyless_command_line_cache(tmp_path, clean_root, mount_path, systems_pa """A cache.yaml without a key configures a read-only (fetch-only) build cache.""" mirrors = mirror.Mirrors( - clean_root, mount_path, "1.1", mirror_file=mirror_ok, cmdline_cache=systems_path / "mirror-ok/cache-nokey.yaml" + clean_root, + mount_path, + Version(1, 1), + mirror_file=mirror_ok, + cmdline_cache=systems_path / "mirror-ok/cache-nokey.yaml", ) # the cache exists (so it is fetched from), but has no signing key ... @@ -158,7 +168,7 @@ def test_readonly_buildcache(tmp_path, clean_root, mount_path, systems_path): """A buildcache in the mirror file without a private_key is read-only (fetch-only).""" mirrors = mirror.Mirrors( - clean_root, mount_path, "1.1", mirror_file=systems_path / "mirror-readonly-cache/mirrors.yaml" + clean_root, mount_path, Version(1, 1), mirror_file=systems_path / "mirror-readonly-cache/mirrors.yaml" ) # the cache exists (so it is fetched from), but has no signing key ... @@ -192,7 +202,7 @@ def test_config_files(tmp_path, clean_root, mount_path, mirror_ok): so this also exercises relative key paths resolving against the mirror file's dir. """ - mirrors_obj = mirror.Mirrors(clean_root, mount_path, "1.1", mirror_file=mirror_ok) + mirrors_obj = mirror.Mirrors(clean_root, mount_path, Version(1, 1), mirror_file=mirror_ok) files = mirrors_obj.config_files(tmp_path) expected = { @@ -236,7 +246,7 @@ def test_spack_mirrors_yaml(tmp_path, clean_root, mount_path, mirror_ok): } } - mirrors_obj = mirror.Mirrors(clean_root, mount_path, "1.1", mirror_file=mirror_ok) + mirrors_obj = mirror.Mirrors(clean_root, mount_path, Version(1, 1), mirror_file=mirror_ok) files = mirrors_obj.config_files(tmp_path) data = yaml.safe_load(files[tmp_path / "mirrors.yaml"]) @@ -250,7 +260,7 @@ def test_mount_specific_buildcache(tmp_path, clean_root, mount_path, mirror_ok): cache is namespaced per-mount-point to avoid relocation issues / collisions. """ - mirrors_obj = mirror.Mirrors(clean_root, mount_path, "1.1", mirror_file=mirror_ok) + mirrors_obj = mirror.Mirrors(clean_root, mount_path, Version(1, 1), mirror_file=mirror_ok) # mirror-ok's buildcache is mount_specific: false by default; enable it. mirrors_obj.buildcache["mount_specific"] = True @@ -269,7 +279,7 @@ def test_mount_specific_buildcache(tmp_path, clean_root, mount_path, mirror_ok): def test_mount_specific_disabled(tmp_path, clean_root, mount_path, mirror_ok): """A buildcache with mount_specific false is unchanged, even when a mount point is set.""" - mirrors_obj = mirror.Mirrors(clean_root, mount_path, "1.1", mirror_file=mirror_ok) + mirrors_obj = mirror.Mirrors(clean_root, mount_path, Version(1, 1), mirror_file=mirror_ok) # confirm the fixture leaves the flag off assert mirrors_obj.buildcache["mount_specific"] is False @@ -301,7 +311,7 @@ def test_remote_bootstrap_configs(tmp_path, clean_root, mount_path, mirror_ok): }, } - mirrors_obj = mirror.Mirrors(clean_root, mount_path, "1.1", mirror_file=mirror_ok) + mirrors_obj = mirror.Mirrors(clean_root, mount_path, Version(1, 1), mirror_file=mirror_ok) files = mirrors_obj.config_files(tmp_path) bs_data = yaml.safe_load(files[tmp_path / "bootstrap.yaml"]) @@ -327,7 +337,7 @@ def test_local_bootstrap_configs(tmp_path, clean_root, mount_path): mirror_file.write_text(f"bootstrap:\n url: {boot}\n") config_root = tmp_path / "config" - mirrors_obj = mirror.Mirrors(clean_root, mount_path, "1.1", mirror_file=mirror_file) + mirrors_obj = mirror.Mirrors(clean_root, mount_path, Version(1, 1), mirror_file=mirror_file) files = mirrors_obj.config_files(config_root) bs_data = yaml.safe_load(files[config_root / "bootstrap.yaml"]) @@ -357,13 +367,13 @@ def test_local_bootstrap_missing_metadata(tmp_path, clean_root, mount_path): mirror_file.write_text(f"bootstrap:\n url: {boot}\n") with pytest.raises(mirror.MirrorError): - mirror.Mirrors(clean_root, mount_path, "1.1", mirror_file=mirror_file) + mirror.Mirrors(clean_root, mount_path, Version(1, 1), mirror_file=mirror_file) def test_keys(tmp_path, clean_root, mount_path, systems_path, mirror_ok): """Check that gpg keys are decoded, relocated and reported consistently.""" - mirrors_obj = mirror.Mirrors(clean_root, mount_path, "1.1", mirror_file=mirror_ok) + mirrors_obj = mirror.Mirrors(clean_root, mount_path, Version(1, 1), mirror_file=mirror_ok) files = mirrors_obj.config_files(tmp_path) # the buildcache is the only mirror with keys (sources are checksum-verified) @@ -383,7 +393,7 @@ def test_keys(tmp_path, clean_root, mount_path, systems_path, mirror_ok): def test_local_caches_config(tmp_path, clean_root, mount_path, mirror_ok): """The source cache is emitted to config.yaml; the concretizer cache to concretizer.yaml.""" - mirrors_obj = mirror.Mirrors(clean_root, mount_path, "1.1", mirror_file=mirror_ok) + mirrors_obj = mirror.Mirrors(clean_root, mount_path, Version(1, 1), mirror_file=mirror_ok) files = mirrors_obj.config_files(tmp_path) config_data = yaml.safe_load(files[tmp_path / "config.yaml"]) @@ -401,7 +411,7 @@ def test_concretizer_cache_only(tmp_path, clean_root, mount_path): mirror_file = tmp_path / "mirrors.yaml" mirror_file.write_text("concretizer:\n path: /scratch/only-concretizer\n") - mirrors_obj = mirror.Mirrors(clean_root, mount_path, "1.1", mirror_file=mirror_file) + mirrors_obj = mirror.Mirrors(clean_root, mount_path, Version(1, 1), mirror_file=mirror_file) files = mirrors_obj.config_files(tmp_path) assert mirrors_obj.source_cache is None @@ -418,7 +428,7 @@ def test_concretizer_cache_skipped_on_spack_1_0(tmp_path, clean_root, mount_path mirror_file = tmp_path / "mirrors.yaml" mirror_file.write_text("concretizer:\n path: /scratch/only-concretizer\n") - mirrors_obj = mirror.Mirrors(clean_root, mount_path, "1.0", mirror_file=mirror_file) + mirrors_obj = mirror.Mirrors(clean_root, mount_path, Version(1, 0), mirror_file=mirror_file) files = mirrors_obj.config_files(tmp_path) # the cache is still recorded as requested, but no concretizer.yaml is written @@ -431,7 +441,7 @@ def test_local_caches_absent(tmp_path, clean_root, mount_path, systems_path): # mirror-no-sourcecache has neither a sourcecache nor a concretizer entry mirrors_obj = mirror.Mirrors( - clean_root, mount_path, "1.1", mirror_file=systems_path / "mirror-no-sourcecache/mirrors.yaml" + clean_root, mount_path, Version(1, 1), mirror_file=systems_path / "mirror-no-sourcecache/mirrors.yaml" ) files = mirrors_obj.config_files(tmp_path) @@ -449,7 +459,9 @@ def test_s3_fetch_and_push_connections(tmp_path, clean_root, mount_path, systems buildcache and sourcemirror entries accept the same spack connection config. """ - mirrors_obj = mirror.Mirrors(clean_root, mount_path, "1.1", mirror_file=systems_path / "mirror-s3/mirrors.yaml") + mirrors_obj = mirror.Mirrors( + clean_root, mount_path, Version(1, 1), mirror_file=systems_path / "mirror-s3/mirrors.yaml" + ) files = mirrors_obj.config_files(tmp_path) data = yaml.safe_load(files[tmp_path / "mirrors.yaml"]) @@ -492,4 +504,4 @@ def test_bad_config(clean_root, mount_path, systems_path, system_name): """Check that MirrorError is raised at construction for bad keys or a bad cache path.""" with pytest.raises(mirror.MirrorError): - mirror.Mirrors(clean_root, mount_path, "1.1", mirror_file=systems_path / system_name / "mirrors.yaml") + mirror.Mirrors(clean_root, mount_path, Version(1, 1), mirror_file=systems_path / system_name / "mirrors.yaml") diff --git a/unittests/test_recipe.py b/unittests/test_recipe.py index 7aa1fabe..e8f09211 100644 --- a/unittests/test_recipe.py +++ b/unittests/test_recipe.py @@ -1,6 +1,7 @@ import pytest from stackinator.recipe import Recipe +from stackinator.spack_util import Version def make_recipe(commit): @@ -14,26 +15,27 @@ def make_recipe(commit): "commit, expected", [ # release branches and tags pin the major.minor version - ("releases/v1.0", "1.0"), - ("releases/v1.1", "1.1"), - ("releases/v2.0", "2.0"), - ("v1.0.0", "1.0"), - ("v1.1.3", "1.1"), - ("1.0", "1.0"), - # the version cannot be determined -> default to the latest supported (1.1) - ("develop", "1.1"), - ("main", "1.1"), - (None, "1.1"), - ("a3f9c1e8b2d4f6a7c9e1b3d5f7a9c1e3b5d7f9a1", "1.1"), + ("releases/v1.0", Version(1, 0)), + ("releases/v1.1", Version(1, 1)), + ("releases/v2.0", Version(2, 0)), + ("v1.0.0", Version(1, 0)), + ("v1.1.3", Version(1, 1)), + ("1.0", Version(1, 0)), + # the develop and main branches of spack are now at 1.2 + ("develop", Version(1, 2)), + ("main", Version(1, 2)), + (None, Version(1, 2)), + # an unrecognised commit -> default to the most recent expected version (1.1) + ("a3f9c1e8b2d4f6a7c9e1b3d5f7a9c1e3b5d7f9a1", Version(1, 1)), ], ) def test_find_spack_version(commit, expected): - """find_spack_version infers major.minor from the commit, defaulting to 1.1.""" + """find_spack_version infers a Version from the commit, defaulting to 1.1.""" recipe = make_recipe(commit) assert recipe.find_spack_version(develop=False) == expected def test_find_spack_version_develop_flag(): - """The --develop flag forces the latest supported version regardless of commit.""" + """The --develop flag targets the develop branch of spack, now at 1.2.""" recipe = make_recipe("releases/v1.0") - assert recipe.find_spack_version(develop=True) == "1.1" + assert recipe.find_spack_version(develop=True) == Version(1, 2)