diff --git a/lib/spack/spack/cmd/solve.py b/lib/spack/spack/cmd/solve.py index 1c92591c303..accdeb6d05d 100644 --- a/lib/spack/spack/cmd/solve.py +++ b/lib/spack/spack/cmd/solve.py @@ -97,7 +97,7 @@ def setup_parser(subparser): def _process_result(result, show, required_format, kwargs): result.raise_if_unsat() - opt, _, _ = min(result.answers) + opt, *_ = min(result.answers) if ("opt" in show) and (not required_format): tty.msg("Best of %d considered solutions." % result.nmodels) tty.msg("Optimization Criteria:") diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index 1dc3a09c6f5..22ec984c501 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -13,7 +13,7 @@ import re import types import warnings -from typing import Tuple +from typing import Tuple, Union import archspec.cpu @@ -240,7 +240,11 @@ def _id(thing): class AspFunction(AspObject): def __init__(self, name, args=None): self.name = name - self.args = () if args is None else tuple(args) + + def simplify(arg): + return arg if isinstance(arg, (str, bool, int)) else str(arg) + + self.args = () if args is None else tuple(simplify(arg) for arg in args) def _cmp_key(self): return (self.name, self.args) @@ -275,7 +279,7 @@ def argify(arg): elif isinstance(arg, int): return clingo.Number(arg) else: - return clingo.String(str(arg)) + return clingo.String(arg) return clingo.Function(self.name, [argify(arg) for arg in self.args], positive=positive) @@ -502,7 +506,8 @@ def _compute_specs_from_answer_set(self): self._concrete_specs, self._unsolved_specs = [], [] self._concrete_specs_by_input = {} best = min(self.answers) - opt, _, answer = best + + opt, _, answer, _ = best for input_spec in self.abstract_specs: key = input_spec.name if input_spec.virtual: @@ -773,7 +778,7 @@ def on_model(model): answers = builder.build_specs(spec_attrs) # add best spec to the results - result.answers.append((list(min_cost), 0, answers)) + result.answers.append((list(min_cost), 0, answers, spec_attrs)) # get optimization criteria criteria = extract_functions(best_model, "opt_criterion") @@ -1417,23 +1422,23 @@ def spec_clauses(self, *args, **kwargs): def _spec_clauses( self, spec, - body=False, - transitive=True, - expand_hashes=False, - concrete_build_deps=False, + body: bool = False, + transitive: bool = True, + expand_hashes: bool = False, + concrete_build_deps: bool = False, + deptype: Union[str, Tuple[str, ...]] = "all", ): """Return a list of clauses for a spec mandates are true. Arguments: - spec (spack.spec.Spec): the spec to analyze - body (bool): if True, generate clauses to be used in rule bodies + spec: the spec to analyze + body: if True, generate clauses to be used in rule bodies (final values) instead of rule heads (setters). - transitive (bool): if False, don't generate clauses from - dependencies (default True) - expand_hashes (bool): if True, descend into hashes of concrete specs - (default False) - concrete_build_deps (bool): if False, do not include pure build deps + transitive: if False, don't generate clauses from dependencies. + expand_hashes: If transitive and True, descend into hashes of concrete specs. + concrete_build_deps: if False, do not include pure build deps of concrete specs (as they have no effect on runtime constraints) + deptype: dependency types to follow when transitive (default "all"). Normally, if called with ``transitive=True``, ``spec_clauses()`` just generates hashes for the dependency requirements of concrete specs. If ``expand_hashes`` @@ -1561,7 +1566,7 @@ class Body(object): # add all clauses from dependencies if transitive: # TODO: Eventually distinguish 2 deps on the same pkg (build and link) - for dspec in spec.edges_to_dependencies(): + for dspec in spec.edges_to_dependencies(deptype=deptype): dep = dspec.spec if spec.concrete: @@ -1593,6 +1598,7 @@ class Body(object): body=body, expand_hashes=expand_hashes, concrete_build_deps=concrete_build_deps, + deptype=deptype, ) ) diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index ebaefbcf27a..30c06269013 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -2928,7 +2928,7 @@ def _new_concretize(self, tests=False): result.raise_if_unsat() # take the best answer - opt, i, answer = min(result.answers) + opt, i, answer, _ = min(result.answers) name = self.name # TODO: Consolidate this code with similar code in solve.py if self.virtual: diff --git a/lib/spack/spack/test/solver/asp.py b/lib/spack/spack/test/solver/asp.py new file mode 100644 index 00000000000..623ca4f8587 --- /dev/null +++ b/lib/spack/spack/test/solver/asp.py @@ -0,0 +1,72 @@ +# Copyright 2013-2022 Lawrence Livermore National Security, LLC and other +# Spack Project Developers. See the top-level COPYRIGHT file for details. +# +# SPDX-License-Identifier: (Apache-2.0 OR MIT) + +import itertools +import pytest + +import spack.spec +import spack.solver.asp as asp +import spack.store + + +pytestmark = [ + pytest.mark.skipif( + spack.config.get("config:concretizer") == "original", reason="requires new concretizer" + ), + pytest.mark.usefixtures("mutable_config", "mock_packages"), +] + + +@pytest.fixture +def reusable_specs(mock_packages): + reusable_specs = [] + for spec in ["mpich", "openmpi", "zmpi"]: + reusable_specs.extend(s for s in spack.spec.Spec(spec).concretized().traverse(root=True)) + return list(sorted(set(reusable_specs))) + + +@pytest.mark.parametrize( + "root,reuse", + itertools.product( + ("mpileaks ^mpich", "mpileaks ^openmpi", "mpileaks ^zmpi", "patch"), + (True, False), + ), +) +def test_all_facts_in_solve(database, root, reuse, reusable_specs): + reusable_specs = reusable_specs if reuse else [] + + solver = spack.solver.asp.Solver() + setup = spack.solver.asp.SpackSolverSetup() + result, _, _ = solver.driver.solve(setup, [spack.spec.Spec(root)], reuse=reusable_specs) + + *_, result_attrs = result.answers[0] + result_attrs = set(result_attrs) + + def remove_hashes(attrs): + return [] + + for spec in result.specs: + # check only link and run deps if reusing. + deptype = ("link", "run") if reuse else "all" + + # get all facts about the spec and filter out just the "attr" ones. + attrs = setup.spec_clauses(spec, deptype=deptype, body=True, expand_hashes=True) + + # only consider attr() functions, not other displayed atoms + # don't consider any DAG/package hashes, as they are added after solving + attrs = set(attr for attr in attrs if attr.name == "attr" and "hash" not in attr.args[0]) + + # make sure all facts from the solver are in the actual solution. + diff = attrs - result_attrs + + # this is a current bug in the solver: we don't manage dependency patches + # properly, and with reuse it can grab something w/o the right patch. + # See https://github.com/spack/spack/issues/32497 + # TODO: Remove this XFAIL when #32497 is fixed. + patches = [a for a in diff if a.args[0] == "variant_value" and a.args[2] == "patches"] + if diff and not (diff - set(patches)): + pytest.xfail("Bug in new concretizer with patch constraints. See #32497.") + + assert not diff diff --git a/var/spack/repos/builtin.mock/packages/openmpi/package.py b/var/spack/repos/builtin.mock/packages/openmpi/package.py index ea4cb42f7a1..a2fb5fdc90c 100644 --- a/var/spack/repos/builtin.mock/packages/openmpi/package.py +++ b/var/spack/repos/builtin.mock/packages/openmpi/package.py @@ -12,4 +12,6 @@ class Openmpi(Package): variant("internal-hwloc", default=False) variant("fabrics", values=any_combination_of("psm", "mxm")) + provides("mpi") + depends_on("hwloc", when="~internal-hwloc")