concretizer: move remove_node transform into spec_clauses

The `remove_node` transformation is used in almost all calls to `condition()`,
but it's removing `attr("node", ...)` and `attr("virtual_node", ...)` attributes
that we could avoid adding in the first place. Some uses cases need the `node()`
clause, others do not.

- [x] Add a `node` argument to `spec_clauses` that defaults to `True`.
- [x] Remove `remove_node` transform.
- [x] Update calls to `spec_clauses` to use `node=False` where we would have
      used `remove_node`.
- [x] Remove the now unused `impose()` function (missed in #46729)

This is part of a larger effort to simplify the use of transformations
in the concretizer, but it's an easy first step.

Signed-off-by: Todd Gamblin <tgamblin@llnl.gov>

lib/spack/spack/solver/asp.py

@@ -265,11 +265,6 @@ def specify(spec):
     return spack.spec.Spec(spec)
 
 
-def remove_node(spec: spack.spec.Spec, facts: List[AspFunction]) -> List[AspFunction]:
-    """Transformation that removes all "node" and "virtual_node" from the input list of facts."""
-    return list(filter(lambda x: x.args[0] not in ("node", "virtual_node"), facts))
-
-
 def _create_counter(specs: List[spack.spec.Spec], tests: bool):
     strategy = spack.config.CONFIG.get("concretizer:duplicates:strategy", "none")
     if strategy == "full":
@@ -1521,6 +1516,7 @@ def _get_condition_id(
             return result[0]
 
         cond_id = next(self._id_counter)
+
         requirements = self.spec_clauses(named_cond, body=body, context=context)
         if context.transform:
             requirements = context.transform(named_cond, requirements)
@@ -1559,7 +1555,6 @@ def condition(
 
         if not context:
             context = ConditionContext()
-            context.transform_imposed = remove_node
 
         if imposed_spec:
             imposed_name = imposed_spec.name or imposed_name
@@ -1594,14 +1589,6 @@ def condition(
 
             return condition_id
 
-    def impose(self, condition_id, imposed_spec, node=True, body=False):
-        imposed_constraints = self.spec_clauses(imposed_spec, body=body)
-        for pred in imposed_constraints:
-            # imposed "node"-like conditions are no-ops
-            if not node and pred.args[0] in ("node", "virtual_node"):
-                continue
-            self.gen.fact(fn.imposed_constraint(condition_id, *pred.args))
-
     def package_provider_rules(self, pkg):
         for vpkg_name in pkg.provided_virtual_names():
             if vpkg_name not in self.possible_virtuals:
@@ -1659,7 +1646,7 @@ def track_dependencies(input_spec, requirements):
                     return requirements + [fn.attr("track_dependencies", input_spec.name)]
 
                 def dependency_holds(input_spec, requirements):
-                    return remove_node(input_spec, requirements) + [
+                    return requirements + [
                         fn.attr(
                             "dependency_holds", pkg.name, input_spec.name, dt.flag_to_string(t)
                         )
@@ -1717,13 +1704,13 @@ def package_splice_rules(self, pkg):
                 splice_node = fn.node(AspVar("NID"), cond.name)
                 when_spec_attrs = [
                     fn.attr(c.args[0], splice_node, *(c.args[2:]))
-                    for c in self.spec_clauses(cond, body=True, required_from=None)
-                    if c.args[0] != "node"
+                    for c in self.spec_clauses(cond, body=True, required_from=None, node=False)
                 ]
                 splice_spec_hash_attrs = [
                     fn.hash_attr(hash_var, *(c.args))
-                    for c in self.spec_clauses(spec_to_splice, body=True, required_from=None)
-                    if c.args[0] != "node"
+                    for c in self.spec_clauses(
+                        spec_to_splice, body=True, required_from=None, node=False
+                    )
                 ]
                 if match_variants is None:
                     variant_constraints = []
@@ -1845,10 +1832,6 @@ def emit_facts_from_requirement_rules(self, rules: List[RequirementRule]):
                     context.source = ConstraintOrigin.append_type_suffix(
                         pkg_name, ConstraintOrigin.REQUIRE
                     )
-                    if not virtual:
-                        context.transform_imposed = remove_node
-                    # else: for virtuals we want to emit "node" and
-                    # "virtual_node" in imposed specs
 
                     member_id = self.condition(
                         required_spec=when_spec,
@@ -2022,6 +2005,7 @@ def spec_clauses(
         self,
         spec: spack.spec.Spec,
         *,
+        node: bool = True,
         body: bool = False,
         transitive: bool = True,
         expand_hashes: bool = False,
@@ -2039,6 +2023,7 @@ def spec_clauses(
         try:
             clauses = self._spec_clauses(
                 spec,
+                node=node,
                 body=body,
                 transitive=transitive,
                 expand_hashes=expand_hashes,
@@ -2056,6 +2041,7 @@ def _spec_clauses(
         self,
         spec: spack.spec.Spec,
         *,
+        node: bool = True,
         body: bool = False,
         transitive: bool = True,
         expand_hashes: bool = False,
@@ -2066,6 +2052,7 @@ def _spec_clauses(
 
         Arguments:
             spec: the spec to analyze
+            node: if True, emit node(PackageName, ...) and virtual_node(PackageaName, ...) facts
             body: if True, generate clauses to be used in rule bodies (final values) instead
                 of rule heads (setters).
             transitive: if False, don't generate clauses from dependencies (default True)
@@ -2085,8 +2072,10 @@ def _spec_clauses(
 
         f: Union[Type[_Head], Type[_Body]] = _Body if body else _Head
 
-        if spec.name:
+        # only generate this if caller asked for node facts -- not needed for most conditions
+        if node and spec.name:
             clauses.append(f.node(spec.name) if not spec.virtual else f.virtual_node(spec.name))
+
         if spec.namespace:
             clauses.append(f.namespace(spec.name, spec.namespace))
 
@@ -2244,6 +2233,7 @@ def _spec_clauses(
                     clauses.extend(
                         self._spec_clauses(
                             dep,
+                            node=node,
                             body=body,
                             expand_hashes=expand_hashes,
                             concrete_build_deps=concrete_build_deps,
@@ -2628,7 +2618,7 @@ def concrete_specs(self):
             # this indicates that there is a spec like this installed
             self.gen.fact(fn.installed_hash(spec.name, h))
             # indirection layer between hash constraints and imposition to allow for splicing
-            for pred in self.spec_clauses(spec, body=True, required_from=None):
+            for pred in self.spec_clauses(spec, body=True, required_from=None, node=False):
                 self.gen.fact(fn.hash_attr(h, *pred.args))
             self.gen.newline()
             # Declare as possible parts of specs that are not in package.py
@@ -3238,7 +3228,7 @@ def depends_on(
         node_variable = "node(ID, Package)"
         when_spec.name = placeholder
 
-        body_clauses = self._setup.spec_clauses(when_spec, body=True)
+        body_clauses = self._setup.spec_clauses(when_spec, body=True, node=False)
         body_str = (
             f"  {f',{os.linesep}  '.join(str(x) for x in body_clauses)},\n"
             f"  not external({node_variable}),\n"
@@ -3326,7 +3316,7 @@ def propagate(self, constraint_str: str, *, when: str):
         node_variable = "node(ID, Package)"
         when_spec.name = placeholder
 
-        body_clauses = self._setup.spec_clauses(when_spec, body=True)
+        body_clauses = self._setup.spec_clauses(when_spec, body=True, node=False)
         body_str = (
             f"  {f',{os.linesep}  '.join(str(x) for x in body_clauses)},\n"
             f"  not external({node_variable}),\n"
@@ -3337,7 +3327,7 @@ def propagate(self, constraint_str: str, *, when: str):
         assert constraint_spec.name is None, "only anonymous constraint specs are accepted"
 
         constraint_spec.name = placeholder
-        constraint_clauses = self._setup.spec_clauses(constraint_spec, body=False)
+        constraint_clauses = self._setup.spec_clauses(constraint_spec, body=False, node=False)
         for clause in constraint_clauses:
             if clause.args[0] == "node_compiler_version_satisfies":
                 self._setup.compiler_version_constraints.add(constraint_spec.compiler)

var/spack/repos/builtin/packages/ectrans/package.py

@@ -58,6 +58,9 @@ class Ectrans(CMakePackage):
     depends_on("fiat~mpi", when="~mpi")
     depends_on("fiat+mpi", when="+mpi")
 
+    # https://github.com/ecmwf-ifs/ectrans/issues/194
+    conflicts("%oneapi@2025:", when="@1.3.1:1.5.1")
+
     def cmake_args(self):
         args = [
             self.define_from_variant("ENABLE_MPI", "mpi"),

var/spack/repos/builtin/packages/mapl/package.py

@@ -38,6 +38,7 @@ class Mapl(CMakePackage):
     version("develop", branch="develop")
     version("main", branch="main")
 
+    version("2.51.2", sha256="f6df2be24d0c113af3d0424b674d970621660bf11e59a699373f014a14d0716e")
     version("2.51.1", sha256="337dba3980de1d5e603361ecf8f001c5bf99d0addecbeb5c207f3604183ca623")
     version("2.51.0", sha256="56213d845f5287e599213aab1dea60bf6b64c29cd8093313639304b270c45676")
     version("2.50.3", sha256="506f73d511b6a63645bbf953bf04f663da06f5069cb559340786e9fe8eeb170f")

var/spack/repos/builtin/packages/python/package.py

@@ -59,7 +59,9 @@ class Python(Package):
 
     license("0BSD")
 
+    version("3.13.1", sha256="1513925a9f255ef0793dbf2f78bb4533c9f184bdd0ad19763fd7f47a400a7c55")
     version("3.13.0", sha256="12445c7b3db3126c41190bfdc1c8239c39c719404e844babbd015a1bc3fafcd4")
+    version("3.12.8", sha256="5978435c479a376648cb02854df3b892ace9ed7d32b1fead652712bee9d03a45")
     version("3.12.5", sha256="38dc4e2c261d49c661196066edbfb70fdb16be4a79cc8220c224dfeb5636d405")
     version("3.12.4", sha256="01b3c1c082196f3b33168d344a9c85fb07bfe0e7ecfe77fee4443420d1ce2ad9")
     version("3.12.3", sha256="a6b9459f45a6ebbbc1af44f5762623fa355a0c87208ed417628b379d762dddb0")

var/spack/repos/builtin/packages/rccl/package.py

@@ -111,7 +111,7 @@ class Rccl(CMakePackage):
     ]:
         depends_on(f"rocm-core@{ver}", when=f"@{ver}")
 
-    depends_on("googletest@1.11.0:", when="@5.3:")
+    depends_on("googletest@1.11.0:", type="test", when="@5.3:")
 
     @classmethod
     def determine_version(cls, lib):
@@ -138,10 +138,10 @@ def cmake_args(self):
             args.append(self.define_from_variant("AMDGPU_TARGETS", "amdgpu_target"))
 
         if self.spec.satisfies("^cmake@3.21.0:3.21.2"):
-            args.append(self.define("__skip_rocmclang", "ON"))
+            args.append(self.define("__skip_rocmclang", True))
 
         if self.spec.satisfies("@5.3.0:"):
-            args.append(self.define("BUILD_TESTS", "ON"))
+            args.append(self.define("BUILD_TESTS", self.run_tests))
         return args
 
     def test_unit(self):

var/spack/repos/builtin/packages/snakemake/package.py

@@ -158,6 +158,9 @@ class Snakemake(PythonPackage):
     )
     depends_on("py-requests", when="+http", type=("build", "run"))
 
+    # snakemake.common.tests requires pytest
+    skip_modules = ["snakemake.common.tests"]
+
     def test_run(self):
         """Test if snakemake runs with the version option"""
         Executable(self.prefix.bin.snakemake)("--version")