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)