From 646c2f42c40aa6cfb44e29abe9281c38ae149753 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Tue, 22 Oct 2024 11:57:17 +0200 Subject: [PATCH] Fixup binary cache reuse Signed-off-by: Massimiliano Culpo --- lib/spack/spack/binary_distribution.py | 14 +++-- lib/spack/spack/environment/environment.py | 4 +- lib/spack/spack/provider_index.py | 4 +- lib/spack/spack/spec.py | 64 ++++++++++++++++++---- 4 files changed, 65 insertions(+), 21 deletions(-) diff --git a/lib/spack/spack/binary_distribution.py b/lib/spack/spack/binary_distribution.py index 6bf995f77c1..bb2b07c3912 100644 --- a/lib/spack/spack/binary_distribution.py +++ b/lib/spack/spack/binary_distribution.py @@ -629,10 +629,11 @@ def tarball_directory_name(spec): Return name of the tarball directory according to the convention -//-/ """ - if not spec.compiler_as_nodes(): + if spec.original_spec_format() < 5: + compiler = spec.annotations.compiler_node_attribute + assert compiler is not None, "a compiler spec is expected" return spec.format_path( - f"{spec.architecture}/{spec.compiler_annotation.name}" - f"-{spec.compiler_annotation.version}/{spec.name}-{spec.version}" + f"{spec.architecture}/{compiler.name}-{compiler.version}/{spec.name}-{spec.version}" ) return spec.format_path(f"{spec.architecture.platform}/{spec.name}-{spec.version}") @@ -643,10 +644,11 @@ def tarball_name(spec, ext): Return the name of the tarfile according to the convention --- """ - if not spec.compiler_as_nodes(): + if spec.original_spec_format() < 5: + compiler = spec.annotations.compiler_node_attribute + assert compiler is not None, "a compiler spec is expected" spec_formatted = ( - f"{spec.architecture}-{spec.compiler_annotation.name}" - f"-{spec.compiler_annotation.version}-{spec.name}" + f"{spec.architecture}-{compiler.name}-{compiler.version}-{spec.name}" f"-{spec.version}-{spec.dag_hash()}" ) else: diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index beab4ee6d0e..bfaa574cbce 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -23,7 +23,6 @@ import spack import spack.caches -import spack.compilers.config import spack.concretize import spack.config import spack.deptypes as dt @@ -136,7 +135,7 @@ def default_manifest_yaml(): valid_environment_name_re = r"^\w[\w-]*$" #: version of the lockfile format. Must increase monotonically. -lockfile_format_version = 5 +lockfile_format_version = 6 READER_CLS = { @@ -145,6 +144,7 @@ def default_manifest_yaml(): 3: spack.spec.SpecfileV2, 4: spack.spec.SpecfileV3, 5: spack.spec.SpecfileV4, + 6: spack.spec.SpecfileV5, } diff --git a/lib/spack/spack/provider_index.py b/lib/spack/spack/provider_index.py index 3a6431bba0b..20595867a41 100644 --- a/lib/spack/spack/provider_index.py +++ b/lib/spack/spack/provider_index.py @@ -240,8 +240,8 @@ def from_json(stream, repository): index.providers = _transform( providers, lambda vpkg, plist: ( - spack.spec.SpecfileV4.from_node_dict(vpkg), - set(spack.spec.SpecfileV4.from_node_dict(p) for p in plist), + spack.spec.SpecfileV5.from_node_dict(vpkg), + set(spack.spec.SpecfileV5.from_node_dict(p) for p in plist), ), ) return index diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 9a46f6488e5..026e547cca9 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -1426,6 +1426,26 @@ def tree( return out +class SpecAnnotations: + def __init__(self) -> None: + self.original_spec_format = SPECFILE_FORMAT_VERSION + self.compiler_node_attribute: Optional["spack.spec.Spec"] = None + + def with_spec_format(self, spec_format: int) -> "SpecAnnotations": + self.original_spec_format = spec_format + return self + + def with_compiler(self, compiler: "spack.spec.Spec") -> "SpecAnnotations": + self.compiler_node_attribute = compiler + return self + + def __repr__(self) -> str: + result = f"SpecAnnotations().with_spec_format({self.original_spec_format})" + if self.compiler_node_attribute: + result += f"with_compiler({str(self.compiler_node_attribute)})" + return result + + @lang.lazy_lexicographic_ordering(set_hash=False) class Spec: compiler = DeprecatedCompilerSpec() @@ -1494,6 +1514,7 @@ def __init__(self, spec_like=None, *, external_path=None, external_modules=None) # is deployed "as built." # Build spec should be the actual build spec unless marked dirty. self._build_spec = None + self.annotations = SpecAnnotations() if isinstance(spec_like, str): spack.spec_parser.parse_one_or_raise(spec_like, self) @@ -2399,6 +2420,12 @@ def to_node_dict(self, hash=ht.dag_hash): "name": self.build_spec.name, hash.name: self.build_spec._cached_hash(hash), } + + # Annotations + d["annotations"] = {"original_specfile_version": self.annotations.original_spec_format} + if self.annotations.original_spec_format < 5: + d["annotations"]["compiler"] = str(self.annotations.compiler_node_attribute) + return d def to_dict(self, hash=ht.dag_hash): @@ -3509,6 +3536,7 @@ def _dup(self, other: "Spec", deps: Union[bool, dt.DepTypes, dt.DepFlag] = True) self.name = other.name self.versions = other.versions.copy() self.architecture = other.architecture.copy() if other.architecture else None + if hasattr(other, "compiler_annotation"): self.compiler_annotation = other.compiler_annotation @@ -3534,6 +3562,7 @@ def _dup(self, other: "Spec", deps: Union[bool, dt.DepTypes, dt.DepFlag] = True) self.external_modules = other.external_modules self.extra_attributes = other.extra_attributes self.namespace = other.namespace + self.annotations = other.annotations # If we copy dependencies, preserve DAG structure in the new spec if deps: @@ -4445,9 +4474,9 @@ def attach_git_version_lookup(self): if isinstance(v, vn.GitVersion) and v._ref_version is None: v.attach_lookup(spack.version.git_ref_lookup.GitRefLookup(self.fullname)) - def compiler_as_nodes(self) -> bool: - """Returns True if compiler are treated as nodes""" - return not hasattr(self, "compiler_annotation") + def original_spec_format(self) -> int: + """Returns the spec format originally used for this spec.""" + return self.annotations.original_spec_format class VariantMap(lang.HashableMap): @@ -4774,10 +4803,6 @@ def from_node_dict(cls, node): if "arch" in node: spec.architecture = ArchSpec.from_dict(node) - if "compiler" in node: - # Annotate the compiler spec, might be used later - spec.compiler_annotation = cls.legacy_compiler(node) - propagated_names = node.get("propagate", []) for name, values in node.get("parameters", {}).items(): propagate = name in propagated_names @@ -4816,6 +4841,17 @@ def from_node_dict(cls, node): # FIXME: Monkey patches mvar to store patches order mvar._patches_in_order_of_appearance = patches + # Annotate the compiler spec, might be used later + if "annotations" not in node: + # Specfile v4 and earlier + spec.annotations.with_spec_format(cls.SPEC_VERSION) + if "compiler" in node: + spec.annotations.with_compiler(cls.legacy_compiler(node)) + else: + spec.annotations.with_spec_format(node["annotations"]["original_specfile_version"]) + if "compiler" in node["annotations"]: + spec.annotations.with_compiler(Spec(f"{node['annotations']['compiler']}")) + # Don't read dependencies here; from_dict() is used by # from_yaml() and from_json() to read the root *and* each dependency # spec. @@ -4825,9 +4861,7 @@ def from_node_dict(cls, node): @classmethod def legacy_compiler(cls, node): d = node["compiler"] - return Spec( - f"{d['name']}@{vn.VersionList.from_dict(d)}", external_path="/dev-null/", concrete=True - ) + return Spec(f"{d['name']}@{vn.VersionList.from_dict(d)}") @classmethod def _load(cls, data): @@ -4895,6 +4929,8 @@ def read_specfile_dep_specs(cls, deps, hash_type=ht.dag_hash.name): class SpecfileV1(SpecfileReaderBase): + SPEC_VERSION = 1 + @classmethod def load(cls, data): """Construct a spec from JSON/YAML using the format version 1. @@ -4964,6 +5000,8 @@ def read_specfile_dep_specs(cls, deps, hash_type=ht.dag_hash.name): class SpecfileV2(SpecfileReaderBase): + SPEC_VERSION = 2 + @classmethod def load(cls, data): result = cls._load(data) @@ -5018,10 +5056,12 @@ def extract_build_spec_info_from_node_dict(cls, node, hash_type=ht.dag_hash.name class SpecfileV3(SpecfileV2): - pass + SPEC_VERSION = 3 class SpecfileV4(SpecfileV2): + SPEC_VERSION = 4 + @classmethod def extract_info_from_dep(cls, elt, hash): dep_hash = elt[hash.name] @@ -5036,6 +5076,8 @@ def load(cls, data): class SpecfileV5(SpecfileV4): + SPEC_VERSION = 5 + @classmethod def legacy_compiler(cls, node): raise RuntimeError("The 'compiler' option is unexpected in specfiles at v5 or greater")