diff --git a/lib/spack/spack/patch.py b/lib/spack/spack/patch.py index a909bca1056..4579f122d67 100644 --- a/lib/spack/spack/patch.py +++ b/lib/spack/spack/patch.py @@ -83,6 +83,7 @@ def __init__( level: int, working_dir: str, reverse: bool = False, + ordering_key: Optional[Tuple[str, int]] = None, ) -> None: """Initialize a new Patch instance. @@ -92,6 +93,7 @@ def __init__( level: patch level working_dir: relative path *within* the stage to change to reverse: reverse the patch + ordering_key: key used to ensure patches are applied in a consistent order """ # validate level (must be an integer >= 0) if not isinstance(level, int) or not level >= 0: @@ -105,6 +107,13 @@ def __init__( self.working_dir = working_dir self.reverse = reverse + # The ordering key is passed when executing package.py directives, and is only relevant + # after a solve to build concrete specs with consistently ordered patches. For concrete + # specs read from a file, we add patches in the order of its patches variants and the + # ordering_key is irrelevant. In that case, use a default value so we don't need to branch + # on whether ordering_key is None where it's used, just to make static analysis happy. + self.ordering_key: Tuple[str, int] = ordering_key or ("", 0) + def apply(self, stage: "spack.stage.Stage") -> None: """Apply a patch to source in a stage. @@ -202,9 +211,8 @@ def __init__( msg += "package %s.%s does not exist." % (pkg.namespace, pkg.name) raise ValueError(msg) - super().__init__(pkg, abs_path, level, working_dir, reverse) + super().__init__(pkg, abs_path, level, working_dir, reverse, ordering_key) self.path = abs_path - self.ordering_key = ordering_key @property def sha256(self) -> str: @@ -266,13 +274,11 @@ def __init__( archive_sha256: sha256 sum of the *archive*, if the patch is compressed (only required for compressed URL patches) """ - super().__init__(pkg, url, level, working_dir, reverse) + super().__init__(pkg, url, level, working_dir, reverse, ordering_key) self.url = url self._stage: Optional["spack.stage.Stage"] = None - self.ordering_key = ordering_key - if allowed_archive(self.url) and not archive_sha256: raise spack.error.PatchDirectiveError( "Compressed patches require 'archive_sha256' " diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index da924ff11f7..b44c56d1fd0 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -36,6 +36,7 @@ import spack.error import spack.package_base import spack.package_prefs +import spack.patch import spack.platforms import spack.repo import spack.solver.splicing @@ -43,6 +44,7 @@ import spack.store import spack.util.crypto import spack.util.libc +import spack.util.module_cmd as md import spack.util.path import spack.util.timer import spack.variant as vt @@ -3702,11 +3704,11 @@ def build_specs(self, function_tuples): roots = [spec.root for spec in self._specs.values()] roots = dict((id(r), r) for r in roots) for root in roots.values(): - spack.spec.Spec.inject_patches_variant(root) + _inject_patches_variant(root) # Add external paths to specs with just external modules for s in self._specs.values(): - spack.spec.Spec.ensure_external_path_if_external(s) + _ensure_external_path_if_external(s) for s in self._specs.values(): _develop_specs_from_env(s, ev.active_environment()) @@ -3778,6 +3780,92 @@ def execute_explicit_splices(self): return specs +def _inject_patches_variant(root: spack.spec.Spec) -> None: + # This dictionary will store object IDs rather than Specs as keys + # since the Spec __hash__ will change as patches are added to them + spec_to_patches: Dict[int, Set[spack.patch.Patch]] = {} + for s in root.traverse(): + # After concretizing, assign namespaces to anything left. + # Note that this doesn't count as a "change". The repository + # configuration is constant throughout a spack run, and + # normalize and concretize evaluate Packages using Repo.get(), + # which respects precedence. So, a namespace assignment isn't + # changing how a package name would have been interpreted and + # we can do it as late as possible to allow as much + # compatibility across repositories as possible. + if s.namespace is None: + s.namespace = spack.repo.PATH.repo_for_pkg(s.name).namespace + + if s.concrete: + continue + + # Add any patches from the package to the spec. + node_patches = { + patch + for cond, patch_list in spack.repo.PATH.get_pkg_class(s.fullname).patches.items() + if s.satisfies(cond) + for patch in patch_list + } + if node_patches: + spec_to_patches[id(s)] = node_patches + + # Also record all patches required on dependencies by depends_on(..., patch=...) + for dspec in root.traverse_edges(deptype=dt.ALL, cover="edges", root=False): + if dspec.spec.concrete: + continue + + pkg_deps = spack.repo.PATH.get_pkg_class(dspec.parent.fullname).dependencies + + edge_patches: List[spack.patch.Patch] = [] + for cond, deps_by_name in pkg_deps.items(): + if not dspec.parent.satisfies(cond): + continue + + dependency = deps_by_name.get(dspec.spec.name) + if not dependency: + continue + + for pcond, patch_list in dependency.patches.items(): + if dspec.spec.satisfies(pcond): + edge_patches.extend(patch_list) + + if edge_patches: + spec_to_patches.setdefault(id(dspec.spec), set()).update(edge_patches) + + for spec in root.traverse(): + if id(spec) not in spec_to_patches: + continue + + patches = list(spec_to_patches[id(spec)]) + variant: vt.MultiValuedVariant = spec.variants.setdefault( + "patches", vt.MultiValuedVariant("patches", ()) + ) + variant.value = tuple(p.sha256 for p in patches) + # FIXME: Monkey patches variant to store patches order + ordered_hashes = [(*p.ordering_key, p.sha256) for p in patches if p.ordering_key] + ordered_hashes.sort() + tty.debug( + f"Ordered hashes [{spec.name}]: " + + ", ".join("/".join(str(e) for e in t) for t in ordered_hashes) + ) + setattr( + variant, "_patches_in_order_of_appearance", [sha256 for _, _, sha256 in ordered_hashes] + ) + + +def _ensure_external_path_if_external(spec: spack.spec.Spec) -> None: + if not spec.external_modules or spec.external_path: + return + + # Get the path from the module the package can override the default + # (this is mostly needed for Cray) + pkg_cls = spack.repo.PATH.get_pkg_class(spec.name) + package = pkg_cls(spec) + spec.external_path = getattr(package, "external_prefix", None) or md.path_from_modules( + spec.external_modules + ) + + def _develop_specs_from_env(spec, env): dev_info = env.dev_specs.get(spec.name, {}) if env else {} if not dev_info: diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index ee75b6fd097..1910b5d0143 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -99,7 +99,6 @@ import spack.traverse import spack.util.executable import spack.util.hash -import spack.util.module_cmd as md import spack.util.prefix import spack.util.spack_json as sjson import spack.util.spack_yaml as syaml @@ -2845,94 +2844,6 @@ def _patches_assigned(self): return True - @staticmethod - def inject_patches_variant(root): - # This dictionary will store object IDs rather than Specs as keys - # since the Spec __hash__ will change as patches are added to them - spec_to_patches = {} - for s in root.traverse(): - # After concretizing, assign namespaces to anything left. - # Note that this doesn't count as a "change". The repository - # configuration is constant throughout a spack run, and - # normalize and concretize evaluate Packages using Repo.get(), - # which respects precedence. So, a namespace assignment isn't - # changing how a package name would have been interpreted and - # we can do it as late as possible to allow as much - # compatibility across repositories as possible. - if s.namespace is None: - s.namespace = spack.repo.PATH.repo_for_pkg(s.name).namespace - - if s.concrete: - continue - - # Add any patches from the package to the spec. - patches = set() - for cond, patch_list in spack.repo.PATH.get_pkg_class(s.fullname).patches.items(): - if s.satisfies(cond): - for patch in patch_list: - patches.add(patch) - if patches: - spec_to_patches[id(s)] = patches - - # Also record all patches required on dependencies by - # depends_on(..., patch=...) - for dspec in root.traverse_edges(deptype=all, cover="edges", root=False): - if dspec.spec.concrete: - continue - - pkg_deps = spack.repo.PATH.get_pkg_class(dspec.parent.fullname).dependencies - - patches = [] - for cond, deps_by_name in pkg_deps.items(): - if not dspec.parent.satisfies(cond): - continue - - dependency = deps_by_name.get(dspec.spec.name) - if not dependency: - continue - - for pcond, patch_list in dependency.patches.items(): - if dspec.spec.satisfies(pcond): - patches.extend(patch_list) - - if patches: - all_patches = spec_to_patches.setdefault(id(dspec.spec), set()) - for patch in patches: - all_patches.add(patch) - - for spec in root.traverse(): - if id(spec) not in spec_to_patches: - continue - - patches = list(lang.dedupe(spec_to_patches[id(spec)])) - mvar = spec.variants.setdefault("patches", vt.MultiValuedVariant("patches", ())) - mvar.value = tuple(p.sha256 for p in patches) - # FIXME: Monkey patches mvar to store patches order - full_order_keys = list(tuple(p.ordering_key) + (p.sha256,) for p in patches) - ordered_hashes = sorted(full_order_keys) - tty.debug( - "Ordered hashes [{0}]: ".format(spec.name) - + ", ".join("/".join(str(e) for e in t) for t in ordered_hashes) - ) - mvar._patches_in_order_of_appearance = list(t[-1] for t in ordered_hashes) - - @staticmethod - def ensure_external_path_if_external(external_spec): - if external_spec.external_modules and not external_spec.external_path: - compiler = spack.compilers.compiler_for_spec( - external_spec.compiler, external_spec.architecture - ) - for mod in compiler.modules: - md.load_module(mod) - - # Get the path from the module the package can override the default - # (this is mostly needed for Cray) - pkg_cls = spack.repo.PATH.get_pkg_class(external_spec.name) - package = pkg_cls(external_spec) - external_spec.external_path = getattr( - package, "external_prefix", md.path_from_modules(external_spec.external_modules) - ) - @staticmethod def ensure_no_deprecated(root): """Raise if a deprecated spec is in the dag.