concretizer: unify logic for spec conditionals

This builds on #20638 by unifying all the places in the concretizer where
things are conditional on specs. Previously, we duplicated a common spec
conditional pattern for dependencies, virtual providers, conflicts, and
externals. That was introduced in #20423 and refined in #20507, and
roughly looked as follows.

Given some directives in a package like:

```python
depends_on("foo@1.0+bar", when="@2.0+variant")
provides("mpi@2:", when="@1.9:")
```

We handled the `@2.0+variant` and `@1.9:` parts by generating generated
`dependency_condition()`, `required_dependency_condition()`, and
`imposed_dependency_condition()` facts to trigger rules like this:

```prolog
dependency_conditions_hold(ID, Parent, Dependency) :-
  attr(Name, Arg1)             : required_dependency_condition(ID, Name, Arg1);
  attr(Name, Arg1, Arg2)       : required_dependency_condition(ID, Name, Arg1, Arg2);
  attr(Name, Arg1, Arg2, Arg3) : required_dependency_condition(ID, Name, Arg1, Arg2, Arg3);
  dependency_condition(ID, Parent, Dependency);
  node(Parent).
```

And we handled `foo@1.0+bar` and `mpi@2:` parts ("imposed constraints")
like this:

```prolog
attr(Name, Arg1, Arg2) :-
  dependency_conditions_hold(ID, Package, Dependency),
  imposed_dependency_condition(ID, Name, Arg1, Arg2).

attr(Name, Arg1, Arg2, Arg3) :-
  dependency_conditions_hold(ID, Package, Dependency),
  imposed_dependency_condition(ID, Name, Arg1, Arg2, Arg3).
```

These rules were repeated with different input predicates for
requirements (e.g., `required_dependency_condition`) and imposed
constraints (e.g., `imposed_dependency_condition`) throughout
`concretize.lp`. In #20638 it got to be a bit confusing, because we used
the same `dependency_condition_holds` predicate to impose constraints on
conditional dependencies and virtual providers. So, even though the
pattern was repeated, some of the conditional rules were conjoined in a
weird way.

Instead of repeating this pattern everywhere, we now have *one* set of
consolidated rules for conditions:

```prolog
condition_holds(ID) :-
  condition(ID);
  attr(Name, A1)         : condition_requirement(ID, Name, A1);
  attr(Name, A1, A2)     : condition_requirement(ID, Name, A1, A2);
  attr(Name, A1, A2, A3) : condition_requirement(ID, Name, A1, A2, A3).

attr(Name, A1)         :- condition_holds(ID), imposed_constraint(ID, Name, A1).
attr(Name, A1, A2)     :- condition_holds(ID), imposed_constraint(ID, Name, A1, A2).
attr(Name, A1, A2, A3) :- condition_holds(ID), imposed_constraint(ID, Name, A1, A2, A3).
```

this allows us to use `condition(ID)` and `condition_holds(ID)` to
encapsulate the conditional logic on specs in all the scenarios where we
need it. Instead of defining predicates for the requirements and imposed
constraints, we generate the condition inputs with generic facts, and
define predicates to associate the condition ID with a particular
scenario. So, now, the generated facts for a condition look like this:

```prolog
condition(121).
condition_requirement(121,"node","cairo").
condition_requirement(121,"variant_value","cairo","fc","True").
imposed_constraint(121,"version_satisfies","fontconfig","2.10.91:").
dependency_condition(121,"cairo","fontconfig").
dependency_type(121,"build").
dependency_type(121,"link").
```

The requirements and imposed constraints are generic, and we associate
them with their meaning via the id. Here, `dependency_condition(121,
"cairo", "fontconfig")` tells us that condition 121 has to do with the
dependency of `cairo` on `fontconfig`, and the conditional dependency
rules just become:

```prolog
dependency_holds(Package, Dependency, Type) :-
  dependency_condition(ID, Package, Dependency),
  dependency_type(ID, Type),
  condition_holds(ID).
```

Dependencies, virtuals, conflicts, and externals all now use similar
patterns, and the logic for generating condition facts is common to all
of them on the python side, as well. The more specific routines like
`package_dependencies_rules` just call `self.condition(...)` to get an id
and generate requirements and imposed constraints, then they generate
their extra facts with the returned id, like this:

```python
    def package_dependencies_rules(self, pkg, tests):
        """Translate 'depends_on' directives into ASP logic."""
        for _, conditions in sorted(pkg.dependencies.items()):
            for cond, dep in sorted(conditions.items()):
                condition_id = self.condition(cond, dep.spec, pkg.name)  # create a condition and get its id
                self.gen.fact(fn.dependency_condition(  # associate specifics about the dependency w/the id
                    condition_id, pkg.name, dep.spec.name
                ))
        # etc.
```

- [x] unify generation and logic for conditions
- [x] use unified logic for dependencies
- [x] use unified logic for virtuals
- [x] use unified logic for conflicts
- [x] use unified logic for externals

LocalWords:  concretizer mpi attr Arg concretize lp cairo fc fontconfig
LocalWords:  virtuals def pkg cond dep fn refactor github py

lib/spack/spack/solver/asp.py

@@ -483,28 +483,12 @@ def target_ranges(self, spec, single_target_fn):
 
     def conflict_rules(self, pkg):
         for trigger, constraints in pkg.conflicts.items():
+            trigger_id = self.condition(spack.spec.Spec(trigger), name=pkg.name)
+            self.gen.fact(fn.conflict_trigger(trigger_id))
+
             for constraint, _ in constraints:
-                constraint_body = spack.spec.Spec(pkg.name)
+                constraint_id = self.condition(constraint, name=pkg.name)
-                constraint_body.constrain(constraint)
+                self.gen.fact(fn.conflict(pkg.name, trigger_id, constraint_id))
-                constraint_body.constrain(trigger)
-
-                clauses = []
-                for s in constraint_body.traverse():
-                    clauses += self.spec_clauses(s, body=True)
-
-                # TODO: find a better way to generate clauses for integrity
-                # TODO: constraints, instead of generating them for the body
-                # TODO: of a rule and filter unwanted functions.
-                to_be_filtered = ['node_compiler_hard']
-                clauses = [x for x in clauses if x.name not in to_be_filtered]
-
-                # Emit facts based on clauses
-                condition_id = next(self._condition_id_counter)
-                self.gen.fact(fn.conflict(condition_id, pkg.name))
-                for clause in clauses:
-                    self.gen.fact(fn.conflict_condition(
-                        condition_id, clause.name, *clause.args
-                    ))
                 self.gen.newline()
 
     def available_compilers(self):
@@ -642,50 +626,44 @@ def pkg_rules(self, pkg, tests):
             )
         )
 
-    def _condition_facts(
+    def condition(self, required_spec, imposed_spec=None, name=None):
-            self, pkg_name, cond_spec, dep_spec,
-            cond_fn, require_fn, impose_fn
-    ):
         """Generate facts for a dependency or virtual provider condition.
 
         Arguments:
-            pkg_name (str): name of the package that triggers the
+            required_spec (Spec): the spec that triggers this condition
-                condition (e.g., the dependent or the provider)
+            imposed_spec (optional, Spec): the sepc with constraints that
-            cond_spec (Spec): the dependency spec representing the
+                are imposed when this condition is triggered
-                condition that needs to be True (can be anonymous)
+            name (optional, str): name for `required_spec` (required if
-            dep_spec (Spec): the sepc of the dependency or provider
+                required_spec is anonymous, ignored if not)
-                to be depended on/provided if the condition holds.
-            cond_fn (AspFunction): function to use to declare the condition;
-                will be called with the cond id, pkg_name, an dep_spec.name
-            require_fn (AspFunction): function to use to declare the conditions
-                required of the dependent/provider to trigger
-            impose_fn (AspFunction): function to use for constraints imposed
-                on the dependency/virtual
 
         Returns:
             (int): id of the condition created by this function
         """
+        named_cond = required_spec.copy()
+        named_cond.name = named_cond.name or name
+        assert named_cond.name, "must provide name for anonymous condtions!"
+
         condition_id = next(self._condition_id_counter)
-        named_cond = cond_spec.copy()
+        self.gen.fact(fn.condition(condition_id))
-        named_cond.name = named_cond.name or pkg_name
 
-        self.gen.fact(cond_fn(condition_id, pkg_name, dep_spec.name))
+        # requirements trigger the condition
-
+        requirements = self.checked_spec_clauses(
-        # conditions that trigger the condition
+            named_cond, body=True, required_from=name)
-        conditions = self.checked_spec_clauses(
+        for pred in requirements:
-            named_cond, body=True, required_from=pkg_name
+            self.gen.fact(
+                fn.condition_requirement(condition_id, pred.name, *pred.args)
             )
-        for pred in conditions:
-            self.gen.fact(require_fn(condition_id, pred.name, *pred.args))
 
+        if imposed_spec:
             imposed_constraints = self.checked_spec_clauses(
-            dep_spec, required_from=pkg_name
+                imposed_spec, body=False, required_from=name)
-        )
             for pred in imposed_constraints:
                 # imposed "node"-like conditions are no-ops
                 if pred.name in ("node", "virtual_node"):
                     continue
-            self.gen.fact(impose_fn(condition_id, pred.name, *pred.args))
+                self.gen.fact(
+                    fn.imposed_constraint(condition_id, pred.name, *pred.args)
+                )
 
         return condition_id
 
@@ -695,25 +673,20 @@ def package_provider_rules(self, pkg):
 
         for provided, whens in pkg.provided.items():
             for when in whens:
-                self._condition_facts(
+                condition_id = self.condition(when, provided, pkg.name)
-                    pkg.name, when, provided,
+                self.gen.fact(fn.provider_condition(
-                    fn.provider_condition,
+                    condition_id, when.name, provided.name
-                    fn.required_provider_condition,
+                ))
-                    fn.imposed_dependency_condition
-                )
-
             self.gen.newline()
 
     def package_dependencies_rules(self, pkg, tests):
         """Translate 'depends_on' directives into ASP logic."""
         for _, conditions in sorted(pkg.dependencies.items()):
             for cond, dep in sorted(conditions.items()):
-                condition_id = self._condition_facts(
+                condition_id = self.condition(cond, dep.spec, pkg.name)
-                    pkg.name, cond, dep.spec,
+                self.gen.fact(fn.dependency_condition(
-                    fn.dependency_condition,
+                    condition_id, pkg.name, dep.spec.name
-                    fn.required_dependency_condition,
+                ))
-                    fn.imposed_dependency_condition
-                )
 
                 for t in sorted(dep.type):
                     # Skip test dependencies if they're not requested at all
@@ -794,24 +767,13 @@ def external_packages(self):
                     pkg_name, str(version), weight, id
                 ))
 
+            # Declare external conditions with a local index into packages.yaml
             for local_idx, spec in enumerate(external_specs):
-                condition_id = next(self._condition_id_counter)
+                condition_id = self.condition(spec)
-
-                # Declare the global ID associated with this external spec
-                self.gen.fact(fn.external_spec(condition_id, pkg_name))
-
-                # Local index into packages.yaml
                 self.gen.fact(
-                    fn.external_spec_index(condition_id, pkg_name, local_idx))
+                    fn.possible_external(condition_id, pkg_name, local_idx)
-
-                # Add conditions to be satisfied for this external
-                self.possible_versions[spec.name].add(spec.version)
-                clauses = self.spec_clauses(spec, body=True)
-                for clause in clauses:
-                    self.gen.fact(
-                        fn.external_spec_condition(
-                            condition_id, clause.name, *clause.args)
                 )
+                self.possible_versions[spec.name].add(spec.version)
                 self.gen.newline()
 
     def preferred_variants(self, pkg_name):
@@ -1474,7 +1436,7 @@ def node_flag_source(self, pkg, source):
     def no_flags(self, pkg, flag_type):
         self._specs[pkg].compiler_flags[flag_type] = []
 
-    def external_spec_selected(self, condition_id, pkg, idx):
+    def external_spec_selected(self, pkg, idx):
         """This means that the external spec and index idx
         has been selected for this package.
         """

lib/spack/spack/solver/concretize.lp

@@ -31,16 +31,54 @@ version_satisfies(Package, Constraint)
 #defined preferred_version_declared/3.
 #defined version_satisfies/3.
 
+%-----------------------------------------------------------------------------
+% Spec conditions and imposed constraints
+%
+% Given Spack directives like these:
+%    depends_on("foo@1.0+bar", when="@2.0+variant")
+%    provides("mpi@2:", when="@1.9:")
+%
+% The conditions are `@2.0+variant` and `@1.9:`, and the imposed constraints
+% are `@1.0+bar` on `foo` and `@2:` on `mpi`.
+%-----------------------------------------------------------------------------
+% conditions are specified with `condition_requirement` and hold when
+% corresponding spec attributes hold.
+condition_holds(ID) :-
+  condition(ID);
+  attr(Name, A1)         : condition_requirement(ID, Name, A1);
+  attr(Name, A1, A2)     : condition_requirement(ID, Name, A1, A2);
+  attr(Name, A1, A2, A3) : condition_requirement(ID, Name, A1, A2, A3).
+
+% conditions that hold impose constraints on other specs
+attr(Name, A1)         :- condition_holds(ID), imposed_constraint(ID, Name, A1).
+attr(Name, A1, A2)     :- condition_holds(ID), imposed_constraint(ID, Name, A1, A2).
+attr(Name, A1, A2, A3) :- condition_holds(ID), imposed_constraint(ID, Name, A1, A2, A3).
+
+#defined condition/1.
+#defined condition_requirement/3.
+#defined condition_requirement/4.
+#defined condition_requirement/5.
+#defined imposed_constraint/3.
+#defined imposed_constraint/4.
+#defined imposed_constraint/5.
+
 %-----------------------------------------------------------------------------
 % Dependency semantics
 %-----------------------------------------------------------------------------
 % Dependencies of any type imply that one package "depends on" another
 depends_on(Package, Dependency) :- depends_on(Package, Dependency, _).
 
+% a dependency holds if its condition holds
+dependency_holds(Package, Dependency, Type) :-
+  dependency_condition(ID, Package, Dependency),
+  dependency_type(ID, Type),
+  condition_holds(ID).
+
 % declared dependencies are real if they're not virtual AND
-% the package is not an external
+% the package is not an external.
+% They're only triggered if the associated dependnecy condition holds.
 depends_on(Package, Dependency, Type)
- :- dependency_conditions(Package, Dependency, Type),
+ :- dependency_holds(Package, Dependency, Type),
     not virtual(Dependency),
     not external(Package).
 
@@ -64,74 +102,19 @@ path(Parent, Child) :- depends_on(Parent, Child).
 path(Parent, Descendant) :- path(Parent, A), depends_on(A, Descendant).
 :- path(A, B), path(B, A).
 
-%-----------------------------------------------------------------------------
-% Conditional dependencies
-%
-% This takes care of `when=SPEC` in `depends_on("foo@1.0+bar", when="SPEC")`.
-%-----------------------------------------------------------------------------
-% if any individual condition below is true, trigger the dependency.
-dependency_conditions(Package, Dependency, Type) :-
-  dependency_conditions_hold(ID, Package, Dependency),
-  dependency_type(ID, Type).
-
 #defined dependency_type/2.
-
-% collect all the dependency conditions into a single conditional rule
-% distinguishing between Parent and Package (Arg1) is needed to account
-% for conditions like:
-%
-% depends_on('patchelf@0.9', when='@1.0:1.1 ^python@:2')
-%
-% that include dependencies
-dependency_conditions_hold(ID, Parent, Dependency) :-
-  attr(Name, Arg1)             : required_dependency_condition(ID, Name, Arg1);
-  attr(Name, Arg1, Arg2)       : required_dependency_condition(ID, Name, Arg1, Arg2);
-  attr(Name, Arg1, Arg2, Arg3) : required_dependency_condition(ID, Name, Arg1, Arg2, Arg3);
-  dependency_condition(ID, Parent, Dependency);
-  % There must be at least a dependency type declared,
-  % otherwise the dependency doesn't hold
-  dependency_type(ID, _);
-  node(Parent);
-  not external(Parent).
-
 #defined dependency_condition/3.
-#defined required_dependency_condition/3.
-#defined required_dependency_condition/4.
-#defined required_dependency_condition/5.
-
-%-----------------------------------------------------------------------------
-% Imposed constraints on dependencies
-%
-% This handles the `@1.0+bar` in `depends_on("foo@1.0+bar", when="SPEC")`, or
-% the `mpi@2:` in `provides("mpi@2:", when="@1.9:")`.
-%-----------------------------------------------------------------------------
-% NOTE: `attr(Name, Arg1)` is omitted here b/c the only single-arg attribute is
-% NOTE: `node()`, which is handled above under "Dependency Semantics"
-
-attr(Name, Arg1, Arg2) :-
-  dependency_conditions_hold(ID, Package, Dependency),
-  imposed_dependency_condition(ID, Name, Arg1, Arg2).
-
-attr(Name, Arg1, Arg2, Arg3) :-
-  dependency_conditions_hold(ID, Package, Dependency),
-  imposed_dependency_condition(ID, Name, Arg1, Arg2, Arg3).
-
-#defined imposed_dependency_condition/4.
-#defined imposed_dependency_condition/5.
 
 %-----------------------------------------------------------------------------
 % Conflicts
 %-----------------------------------------------------------------------------
-:- not external(Package)        : conflict_condition(ID, "node", Package);
+:- node(Package),
-   attr(Name, Arg1)             : conflict_condition(ID, Name, Arg1);
+   not external(Package),
-   attr(Name, Arg1, Arg2)       : conflict_condition(ID, Name, Arg1, Arg2);
+   conflict(Package, TriggerID, ConstraintID),
-   attr(Name, Arg1, Arg2, Arg3) : conflict_condition(ID, Name, Arg1, Arg2, Arg3);
+   condition_holds(TriggerID),
-   conflict(ID, Package).
+   condition_holds(ConstraintID).
 
-#defined conflict/2.
+#defined conflict/3.
-#defined conflict_condition/3.
-#defined conflict_condition/4.
-#defined conflict_condition/5.
 
 %-----------------------------------------------------------------------------
 % Virtual dependencies
@@ -140,10 +123,15 @@ attr(Name, Arg1, Arg2, Arg3) :-
 % if a package depends on a virtual, it's not external and we have a
 % provider for that virtual then it depends on the provider
 depends_on(Package, Provider, Type)
-  :- dependency_conditions(Package, Virtual, Type),
+  :- dependency_holds(Package, Virtual, Type),
      provides_virtual(Provider, Virtual),
      not external(Package).
 
+% dependencies on virtuals also imply that the virtual is a virtual node
+virtual_node(Virtual)
+  :- dependency_holds(Package, Virtual, Type),
+     virtual(Virtual), not external(Package).
+
 % if there's a virtual node, we must select one provider
 1 { provides_virtual(Package, Virtual) : possible_provider(Package, Virtual) } 1
  :- virtual_node(Virtual).
@@ -153,32 +141,21 @@ virtual_node(Virtual) :- virtual_root(Virtual).
 1 { root(Package) : provides_virtual(Package, Virtual) } 1
   :- virtual_root(Virtual).
 
-% all virtual providers come from provider conditions like this
-dependency_conditions_hold(ID, Provider, Virtual) :-
-  attr(Name, Arg1)             : required_provider_condition(ID, Name, Arg1);
-  attr(Name, Arg1, Arg2)       : required_provider_condition(ID, Name, Arg1, Arg2);
-  attr(Name, Arg1, Arg2, Arg3) : required_provider_condition(ID, Name, Arg1, Arg2, Arg3);
-  virtual(Virtual);
-  provider_condition(ID, Provider, Virtual).
-
 % The provider provides the virtual if some provider condition holds.
 provides_virtual(Provider, Virtual) :-
    provider_condition(ID, Provider, Virtual),
-   dependency_conditions_hold(ID, Provider, Virtual),
+   condition_holds(ID),
    virtual(Virtual).
 
 % a node that provides a virtual is a provider
 provider(Package, Virtual)
   :- node(Package), provides_virtual(Package, Virtual).
 
-% dependencies on virtuals also imply that the virtual is a virtual node
-virtual_node(Virtual)
-  :- dependency_conditions(Package, Virtual, Type),
-     virtual(Virtual), not external(Package).
-
 % for any virtual, there can be at most one provider in the DAG
 0 { node(Package) : provides_virtual(Package, Virtual) } 1 :- virtual(Virtual).
 
+#defined possible_provider/2.
+
 %-----------------------------------------------------------------------------
 % Virtual dependency weights
 %-----------------------------------------------------------------------------
@@ -302,32 +279,29 @@ external(Package) :- external_only(Package), node(Package).
 % a package is a real_node if it is not external
 real_node(Package) :- node(Package), not external(Package).
 
-% if an external version is selected, the package is external and
+% a package is external if we are using an external spec for it
-% we are using the corresponding spec
+external(Package) :- external_spec_selected(Package, _).
-external(Package) :-
+
-    version(Package, Version), version_weight(Package, Weight),
+% we can't use the weight for an external version if we don't use the
-    external_version_declared(Package, Version, Weight, ID).
+% corresponding external spec.
+:- version(Package, Version),
+   version_weight(Package, Weight),
+   external_version_declared(Package, Version, Weight, ID),
+   not external(Package).
 
 % determine if an external spec has been selected
-external_spec_selected(ID, Package, LocalIndex) :-
+external_spec_selected(Package, LocalIndex) :-
-    version(Package, Version), version_weight(Package, Weight),
+    external_conditions_hold(Package, LocalIndex),
-    external_spec_index(ID, Package, LocalIndex),
-    external_version_declared(Package, Version, Weight, LocalIndex),
-    external_spec_conditions_hold(ID, Package).
-
-% determine if all the conditions on an external spec hold. If they do
-% the spec can be selected.
-external_spec_conditions_hold(ID, Package) :-
-  attr(Name, Arg1)             : external_spec_condition(ID, Name, Arg1);
-  attr(Name, Arg1, Arg2)       : external_spec_condition(ID, Name, Arg1, Arg2);
-  attr(Name, Arg1, Arg2, Arg3) : external_spec_condition(ID, Name, Arg1, Arg2, Arg3);
-  external_spec(ID, Package);
     node(Package).
 
+external_conditions_hold(Package, LocalIndex) :-
+    possible_external(ID, Package, LocalIndex), condition_holds(ID).
+
 % it cannot happen that a spec is external, but none of the external specs
 % conditions hold.
-:- external(Package), not external_spec_conditions_hold(_, Package).
+:- external(Package), not external_conditions_hold(Package, _).
 
+#defined possible_external/3.
 #defined external_spec_index/3.
 #defined external_spec_condition/3.
 #defined external_spec_condition/4.

lib/spack/spack/solver/display.lp

@@ -24,4 +24,4 @@
 #show compiler_weight/2.
 #show node_target_match/2.
 #show node_target_weight/2.
-#show external_spec_selected/3.
+#show external_spec_selected/2.