Spec.ensure_external_path_if_external, Spec.inject_patches_variant -> spack.solver.asp (#48988)

y

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' "

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:

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.