From d8dc4b141e37a36f6bd9041be10f023bc2fe1770 Mon Sep 17 00:00:00 2001
From: Massimiliano Culpo <massimiliano.culpo@gmail.com>
Date: Wed, 16 Dec 2020 14:03:37 +0100
Subject: concretizer: avoid redundant grounding on dependency types

---
 lib/spack/spack/solver/asp.py        | 25 +++++++++++--------------
 lib/spack/spack/solver/concretize.lp | 26 +++++++++++++-------------
 2 files changed, 24 insertions(+), 27 deletions(-)

diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index 543f604f8d..e07cc92ae6 100644
--- a/lib/spack/spack/solver/asp.py
+++ b/lib/spack/spack/solver/asp.py
@@ -728,7 +728,7 @@ class SpackSolverSetup(object):
 
     def package_dependencies_rules(self, pkg, tests):
         """Translate 'depends_on' directives into ASP logic."""
-        for name, conditions in sorted(pkg.dependencies.items()):
+        for _, conditions in sorted(pkg.dependencies.items()):
             for cond_id, (cond, dep) in enumerate(sorted(conditions.items())):
                 named_cond = cond.copy()
                 named_cond.name = named_cond.name or pkg.name
@@ -751,22 +751,19 @@ class SpackSolverSetup(object):
                         continue
 
                     # there is a declared dependency of type t
+                    self.gen.fact(
+                        fn.declared_dependency(dep.pkg.name, dep.spec.name, cond_id, t)
+                    )
 
-                    # TODO: this ends up being redundant in the output --
-                    # TODO: not sure if we really need it anymore.
-                    # TODO: Look at simplifying the logic in concretize.lp
+                # if it has conditions, declare them.
+                conditions = self.spec_clauses(named_cond, body=True)
+                for cond in conditions:
                     self.gen.fact(
-                        fn.declared_dependency(dep.pkg.name, dep.spec.name, t))
-
-                    # if it has conditions, declare them.
-                    conditions = self.spec_clauses(named_cond, body=True)
-                    for cond in conditions:
-                        self.gen.fact(
-                            fn.dep_cond(
-                                dep.pkg.name, dep.spec.name, t, cond_id,
-                                cond.name, *cond.args
-                            )
+                        fn.dep_cond(
+                            dep.pkg.name, dep.spec.name, cond_id,
+                            cond.name, *cond.args
                         )
+                    )
 
                 # add constraints on the dependency from dep spec.
 
diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp
index 7c4e0d6d02..234243523e 100644
--- a/lib/spack/spack/solver/concretize.lp
+++ b/lib/spack/spack/solver/concretize.lp
@@ -55,32 +55,32 @@ depends_on(Package, Dependency, Type)
     not external(Package).
 
 % if any individual condition below is true, trigger the dependency.
-dependency_conditions(P, D, T) :- dependency_conditions(P, D, T, _).
+dependency_conditions(P, D, T) :-
+  dependency_conditions_hold(P, D, I), declared_dependency(P, D, I, T).
 
 % collect all the dependency condtions into a single conditional rule
-dependency_conditions(P, D, T, I) :-
+dependency_conditions_hold(P, D, I) :-
   node(Package)
-    : dep_cond(P, D, T, I, "node", Package);
+    : dep_cond(P, D, I, "node", Package);
   version(Package, Version)
-    : dep_cond(P, D, T, I, "version", Package, Version);
+    : dep_cond(P, D, I, "version", Package, Version);
   version_satisfies(Package, Constraint)
-    : dep_cond(P, D, T, I, "version_satisfies", Package, Constraint);
+    : dep_cond(P, D, I, "version_satisfies", Package, Constraint);
   node_platform(Package, Platform)
-    : dep_cond(P, D, T, I, "node_platform", Package, Platform);
+    : dep_cond(P, D, I, "node_platform", Package, Platform);
   node_os(Package, OS)
-    : dep_cond(P, D, T, I, "node_os", Package, OS);
+    : dep_cond(P, D, I, "node_os", Package, OS);
   node_target(Package, Target)
-    : dep_cond(P, D, T, I, "node_target", Package, Target);
+    : dep_cond(P, D, I, "node_target", Package, Target);
   variant_value(Package, Variant, Value)
-    : dep_cond(P, D, T, I, "variant_value", Package, Variant, Value);
+    : dep_cond(P, D, I, "variant_value", Package, Variant, Value);
   node_compiler(Package, Compiler)
-    : dep_cond(P, D, T, I, "node_compiler", Package, Compiler);
+    : dep_cond(P, D, I, "node_compiler", Package, Compiler);
   node_compiler_version(Package, Compiler, Version)
-    : dep_cond(P, D, T, I, "node_compiler_version", Package, Compiler, Version);
+    : dep_cond(P, D, I, "node_compiler_version", Package, Compiler, Version);
   node_flag(Package, FlagType, Flag)
-    : dep_cond(P, D, T, I, "node_flag", Package, FlagType, Flag);
+    : dep_cond(P, D, I, "node_flag", Package, FlagType, Flag);
   dependency_condition(P, D, I);
-  declared_dependency(P, D, T);
   node(P).
 
 % if a virtual was required by some root spec, one provider is in the DAG
-- 
cgit v1.2.3-60-g2f50