concretizer: handle constraints on dependencies, adjust optimization

This needs more thought, as I am pretty sure the weights are not correct.
Or, at least, I'm not convinced that they do what we want in all cases.
See note in concretize.lp.

3 files changed, 49 insertions, 10 deletions

diff --git a/lib/spack/spack/cmd/solve.py b/lib/spack/spack/cmd/solve.py
index 0cd11f5507..3f4787fdad 100644
--- a/lib/spack/spack/cmd/solve.py
+++ b/lib/spack/spack/cmd/solve.py
@@ -16,6 +16,7 @@ import spack.cmd
 import spack.cmd.common.arguments as arguments
 import spack.package
 import spack.solver.asp as asp
+from spack.util.string import plural
 
 description = "concretize a specs using an ASP solver"
 section = 'developer'
@@ -103,7 +104,7 @@ def solve(parser, args):
         opt, i, answer = best
         if not args.yaml:
             tty.msg("Best of %d answers." % (i + 1))
-            tty.msg("Optimization %s" % opt)
+            tty.msg("Optimization: %s" % opt)
 
         # iterate over roots from command line
         for spec in specs:
diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index 080a685584..500d34845b 100644
--- a/lib/spack/spack/solver/asp.py
+++ b/lib/spack/spack/solver/asp.py
@@ -346,6 +346,10 @@ class AspGenerator(object):
         # dependencies
         for name, conditions in pkg.dependencies.items():
             for cond, dep in conditions.items():
+                named_cond = cond.copy()
+                if not named_cond.name:
+                    named_cond.name = pkg.name
+
                 if cond == spack.spec.Spec():
                     for t in dep.type:
                         self.fact(
@@ -354,9 +358,6 @@ class AspGenerator(object):
                             )
                         )
                 else:
-                    named_cond = cond.copy()
-                    if not named_cond.name:
-                        named_cond.name = pkg.name
                     for t in dep.type:
                         self.rule(
                             fn.declared_dependency(
@@ -367,6 +368,16 @@ class AspGenerator(object):
                             )
                         )
 
+                # add constraints on the dependency from dep spec.
+                for clause in self.spec_clauses(dep.spec):
+                    self.rule(
+                        clause,
+                        self._and(
+                            fn.depends_on(dep.pkg.name, dep.spec.name),
+                            *self.spec_clauses(named_cond, body=True)
+                        )
+                    )
+
         # virtual preferences
         self.virtual_preferences(
             pkg.name,
diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp
index 97ed4ea82e..2b8baadc5d 100644
--- a/lib/spack/spack/solver/concretize.lp
+++ b/lib/spack/spack/solver/concretize.lp
@@ -17,7 +17,6 @@ version_declared(P, V) :- version_declared(P, V, _).
 version_possible(P, V) :- version_declared(P, V), not version_conflict(P, V).
 
 version_weight(P, V, N) :- version(P, V), version_declared(P, V, N).
-#minimize{ N@3,P,V : version_weight(P, V, N) }.
 
 #defined version_conflict/2.
 
@@ -54,9 +53,6 @@ provider_weight(D, 100)
        not pkg_provider_preference(P, V, D, _),
        not default_provider_preference(V, D, _).
 
-% pick most preferred virtual providers
-#minimize{ N@2,D : provider_weight(D, N) }.
-
 % all nodes must be reachable from some root
 needed(D) :- root(D), node(D).
 needed(D) :- root(P), depends_on(P, D).
@@ -100,8 +96,6 @@ variant_not_default(P, V, X, 0)
        variant_default_value(P, V, X),
        node(P).
 
-#minimize { N@1,P,V,X : variant_not_default(P, V, X, N) }.
-
 % suppress wranings about this atom being unset.  It's only set if some
 % spec or some package sets it, and without this, clingo will give
 % warnings like 'info: atom does not occur in any rule head'.
@@ -187,3 +181,36 @@ node_compiler_set(D, C)
 node_compiler_version_set(D, C, V)
   :- node(D), compiler(C), depends_on(P, D), node_compiler(D, C),
      node_compiler_version_set(P, C, V).
+
+
+%-----------------------------------------------------------------------------
+% How to optimize the spec (high to low priority)
+%-----------------------------------------------------------------------------
+% weight root preferences higher
+%
+% TODO: how best to deal with this issue?  It's not clear how best to
+% weight all the constraints. Without this root preference, `spack solve
+% hdf5` will pick mpich instead of openmpi, even if openmpi is the
+% preferred provider, because openmpi has a version constraint on hwloc.
+% It ends up choosing between settling for an old version of hwloc, or
+% picking the second-best provider.  This workaround weights root
+% preferences higher so that hdf5's prefs are more important, but it's
+% not clear this is a general solution.  It would be nice to weight by
+% distance to root, but that seems to slow down the solve a lot.
+%
+% One option is to make preferences hard constraints.  Or maybe we need
+% to look more closely at where a constraint came from and factor that
+% into our weights.  e.g., a non-default variant resulting from a version
+% constraint counts like a version constraint.  Needs more thought later.
+%
+root(D, 2) :- root(D), node(D).
+root(D, 1) :- not root(D), node(D).
+
+% pick most preferred virtual providers
+#minimize{ N*R@3,D : provider_weight(D, N), root(P, R) }.
+
+% prefer default variants
+#minimize { N*R@2,P,V,X : variant_not_default(P, V, X, N), root(P, R) }.
+
+% prefer more recent versions.
+#minimize{ N@1,P,V : version_weight(P, V, N) }.