zhangyiss/spack
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

ASP-based solver: don't declare deprecated versions unless required (#38181)

Browse Source 
Currently, the concretizer emits facts for all versions known to Spack, including deprecated versions, and has a specific optimization objective to minimize their use.

This commit simplifies how deprecated versions are handled by considering possible versions for a spec only if they appear in a spec literal, or if the `config:deprecated:true` is set directly or through the `--deprecated` flag. The optimization objective has also been removed, in favor of just ordering versions and having deprecated ones last.

This results in:

a) no delayed errors on install, but concretization errors when deprecated versions would be the only option. This is in particular relevant for CI where it's better to get errors early
b) a slight concretization speed-up due to fewer facts
c) a simplification of the logic program.

Co-authored-by: Harmen Stoppels <me@harmenstoppels.nl>
This commit is contained in:
Massimiliano Culpo
2023-09-13 18:15:57 +02:00
committed by GitHub
parent c4e2d24ca9
commit babd29da50
8 changed files with 91 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
lib/spack/spack/cmd/solve.py
View File

@@ -176,17 +176,29 @@ def solve(parser, args):
output = sys.stdout if "asp" in show else None
setup_only = set(show) == {"asp"}
unify = spack.config.get("concretizer:unify")
allow_deprecated = spack.config.get("config:deprecated", False)
if unify != "when_possible":
# set up solver parameters
# Note: reuse and other concretizer prefs are passed as configuration
result = solver.solve(
specs, out=output, timers=args.timers, stats=args.stats, setup_only=setup_only
specs,
out=output,
timers=args.timers,
stats=args.stats,
setup_only=setup_only,
deprecated=allow_deprecated,
)
if not setup_only:
_process_result(result, show, required_format, kwargs)
else:
for idx, result in enumerate(
solver.solve_in_rounds(specs, out=output, timers=args.timers, stats=args.stats)
solver.solve_in_rounds(
specs,
out=output,
timers=args.timers,
stats=args.stats,
deprecated=allow_deprecated,
)
):
if "solutions" in show:
tty.msg("ROUND {0}".format(idx))

5
lib/spack/spack/concretize.py
View File

@@ -744,8 +744,11 @@ def concretize_specs_together(*abstract_specs, **kwargs):
def _concretize_specs_together_new(*abstract_specs, **kwargs):
import spack.solver.asp
allow_deprecated = spack.config.get("config:deprecated", False)
solver = spack.solver.asp.Solver()
result = solver.solve(abstract_specs, tests=kwargs.get("tests", False))
result = solver.solve(
abstract_specs, tests=kwargs.get("tests", False), deprecated=allow_deprecated
)
result.raise_if_unsat()
return [s.copy() for s in result.specs]

5
lib/spack/spack/environment/environment.py
View File

@@ -1395,7 +1395,10 @@ def _concretize_together_where_possible(
result_by_user_spec = {}
solver = spack.solver.asp.Solver()
for result in solver.solve_in_rounds(specs_to_concretize, tests=tests):
allow_deprecated = spack.config.get("config:deprecated", False)
for result in solver.solve_in_rounds(
specs_to_concretize, tests=tests, deprecated=allow_deprecated
):
result_by_user_spec.update(result.specs_by_input)
result = []

88
lib/spack/spack/solver/asp.py
View File

@@ -13,7 +13,7 @@
import re
import types
import warnings
from typing import List, NamedTuple, Tuple, Union
from typing import List, NamedTuple, Optional, Sequence, Tuple, Union
import archspec.cpu
@@ -137,7 +137,15 @@ class RequirementKind(enum.Enum):
PACKAGE = enum.auto()
DeclaredVersion = collections.namedtuple("DeclaredVersion", ["version", "idx", "origin"])
class DeclaredVersion(NamedTuple):
"""Data class to contain information on declared versions used in the solve"""
#: String representation of the version
version: str
#: Unique index assigned to this version
idx: int
#: Provenance of the version
origin: Provenance
# Below numbers are used to map names of criteria to the order
@@ -784,7 +792,7 @@ def fact(self, head):
if choice:
self.assumptions.append(atom)
def solve(self, setup, specs, reuse=None, output=None, control=None):
def solve(self, setup, specs, reuse=None, output=None, control=None, deprecated=False):
"""Set up the input and solve for dependencies of ``specs``.
Arguments:
@@ -795,6 +803,7 @@ def solve(self, setup, specs, reuse=None, output=None, control=None):
the output of this solve.
control (clingo.Control): configuration for the solver. If None,
default values will be used
deprecated: if True, allow deprecated versions in the solve
Return:
A tuple of the solve result, the timer for the different phases of the
@@ -814,7 +823,7 @@ def solve(self, setup, specs, reuse=None, output=None, control=None):
timer.start("setup")
with self.control.backend() as backend:
self.backend = backend
setup.setup(self, specs, reuse=reuse)
setup.setup(self, specs, reuse=reuse, deprecated=deprecated)
timer.stop("setup")
timer.start("load")
@@ -1894,7 +1903,7 @@ class Body:
return clauses
def define_package_versions_and_validate_preferences(
self, possible_pkgs, require_checksum: bool
self, possible_pkgs, *, require_checksum: bool, deprecated: bool
):
"""Declare any versions in specs not declared in packages."""
packages_yaml = spack.config.get("packages")
@@ -1914,13 +1923,15 @@ def define_package_versions_and_validate_preferences(
]
for idx, (v, version_info) in enumerate(package_py_versions):
if version_info.get("deprecated", False):
self.deprecated_versions[pkg_name].add(v)
if not deprecated:
continue
self.possible_versions[pkg_name].add(v)
self.declared_versions[pkg_name].append(
DeclaredVersion(version=v, idx=idx, origin=Provenance.PACKAGE_PY)
)
deprecated = version_info.get("deprecated", False)
if deprecated:
self.deprecated_versions[pkg_name].add(v)
if pkg_name not in packages_yaml or "version" not in packages_yaml[pkg_name]:
continue
@@ -1949,7 +1960,7 @@ def define_package_versions_and_validate_preferences(
)
self.possible_versions[pkg_name].add(vdef)
def define_ad_hoc_versions_from_specs(self, specs, origin, require_checksum: bool):
def define_ad_hoc_versions_from_specs(self, specs, origin, *, require_checksum: bool):
"""Add concrete versions to possible versions from lists of CLI/dev specs."""
for s in traverse.traverse_nodes(specs):
# If there is a concrete version on the CLI *that we know nothing
@@ -2329,7 +2340,14 @@ def define_concrete_input_specs(self, specs, possible):
if spec.concrete:
self._facts_from_concrete_spec(spec, possible)
def setup(self, driver, specs, reuse=None):
def setup(
self,
driver: PyclingoDriver,
specs: Sequence[spack.spec.Spec],
*,
reuse: Optional[List[spack.spec.Spec]] = None,
deprecated: bool = False,
):
"""Generate an ASP program with relevant constraints for specs.
This calls methods on the solve driver to set up the problem with
@@ -2337,9 +2355,10 @@ def setup(self, driver, specs, reuse=None):
specs, as well as constraints from the specs themselves.
Arguments:
driver (PyclingoDriver): driver instance of this solve
specs (list): list of Specs to solve
reuse (None or list): list of concrete specs that can be reused
driver: driver instance of this solve
specs: list of Specs to solve
reuse: list of concrete specs that can be reused
deprecated: if True adds deprecated versions into the solve
"""
self._condition_id_counter = itertools.count()
@@ -2368,7 +2387,7 @@ def setup(self, driver, specs, reuse=None):
# Calculate develop specs
# they will be used in addition to command line specs
# in determining known versions/targets/os
dev_specs = ()
dev_specs: Tuple[spack.spec.Spec, ...] = ()
env = ev.active_environment()
if env:
dev_specs = tuple(
@@ -2414,11 +2433,17 @@ def setup(self, driver, specs, reuse=None):
self.external_packages()
# TODO: make a config option for this undocumented feature
require_checksum = "SPACK_CONCRETIZER_REQUIRE_CHECKSUM" in os.environ
self.define_package_versions_and_validate_preferences(self.pkgs, require_checksum)
self.define_ad_hoc_versions_from_specs(specs, Provenance.SPEC, require_checksum)
self.define_ad_hoc_versions_from_specs(dev_specs, Provenance.DEV_SPEC, require_checksum)
self.validate_and_define_versions_from_requirements(require_checksum)
checksummed = "SPACK_CONCRETIZER_REQUIRE_CHECKSUM" in os.environ
self.define_package_versions_and_validate_preferences(
self.pkgs, deprecated=deprecated, require_checksum=checksummed
)
self.define_ad_hoc_versions_from_specs(
specs, Provenance.SPEC, require_checksum=checksummed
)
self.define_ad_hoc_versions_from_specs(
dev_specs, Provenance.DEV_SPEC, require_checksum=checksummed
)
self.validate_and_define_versions_from_requirements(require_checksum=checksummed)
self.gen.h1("Package Constraints")
for pkg in sorted(self.pkgs):
@@ -2467,7 +2492,7 @@ def literal_specs(self, specs):
if self.concretize_everything:
self.gen.fact(fn.solve_literal(idx))
def validate_and_define_versions_from_requirements(self, require_checksum: bool):
def validate_and_define_versions_from_requirements(self, *, require_checksum: bool):
"""If package requirements mention concrete versions that are not mentioned
elsewhere, then we need to collect those to mark them as possible
versions. If they are abstract and statically have no match, then we
@@ -2932,7 +2957,16 @@ def _reusable_specs(self, specs):
return reusable_specs
def solve(self, specs, out=None, timers=False, stats=False, tests=False, setup_only=False):
def solve(
self,
specs,
out=None,
timers=False,
stats=False,
tests=False,
setup_only=False,
deprecated=False,
):
"""
Arguments:
specs (list): List of ``Spec`` objects to solve for.
@@ -2943,6 +2977,7 @@ def solve(self, specs, out=None, timers=False, stats=False, tests=False, setup_o
If a tuple of package names, concretize test dependencies for named
packages (defaults to False: do not concretize test dependencies).
setup_only (bool): if True, stop after setup and don't solve (default False).
deprecated (bool): allow deprecated version in the solve
"""
# Check upfront that the variants are admissible
specs = [s.lookup_hash() for s in specs]
@@ -2950,10 +2985,14 @@ def solve(self, specs, out=None, timers=False, stats=False, tests=False, setup_o
reusable_specs.extend(self._reusable_specs(specs))
setup = SpackSolverSetup(tests=tests)
output = OutputConfiguration(timers=timers, stats=stats, out=out, setup_only=setup_only)
result, _, _ = self.driver.solve(setup, specs, reuse=reusable_specs, output=output)
result, _, _ = self.driver.solve(
setup, specs, reuse=reusable_specs, output=output, deprecated=deprecated
)
return result
def solve_in_rounds(self, specs, out=None, timers=False, stats=False, tests=False):
def solve_in_rounds(
self, specs, out=None, timers=False, stats=False, tests=False, deprecated=False
):
"""Solve for a stable model of specs in multiple rounds.
This relaxes the assumption of solve that everything must be consistent and
@@ -2968,6 +3007,7 @@ def solve_in_rounds(self, specs, out=None, timers=False, stats=False, tests=Fals
timers (bool): print timing if set to True
stats (bool): print internal statistics if set to True
tests (bool): add test dependencies to the solve
deprecated (bool): allow deprecated version in the solve
"""
specs = [s.lookup_hash() for s in specs]
reusable_specs = self._check_input_and_extract_concrete_specs(specs)
@@ -2981,7 +3021,7 @@ def solve_in_rounds(self, specs, out=None, timers=False, stats=False, tests=Fals
output = OutputConfiguration(timers=timers, stats=stats, out=out, setup_only=False)
while True:
result, _, _ = self.driver.solve(
setup, input_specs, reuse=reusable_specs, output=output
setup, input_specs, reuse=reusable_specs, output=output, deprecated=deprecated
)
yield result

10
lib/spack/spack/solver/concretize.lp
View File

@@ -1361,16 +1361,6 @@ opt_criterion(75, "requirement weight").
build_priority(PackageNode, Priority)
}.
% Minimize the number of deprecated versions being used
opt_criterion(73, "deprecated versions used").
#minimize{ 0@273: #true }.
#minimize{ 0@73: #true }.
#minimize{
1@73+Priority,PackageNode
: attr("deprecated", PackageNode, _),
build_priority(PackageNode, Priority)
}.
% Minimize the:
% 1. Version weight
% 2. Number of variants with a non default value, if not set

3
lib/spack/spack/spec.py
View File

@@ -2963,8 +2963,9 @@ def _new_concretize(self, tests=False):
if self._concrete:
return
allow_deprecated = spack.config.get("config:deprecated", False)
solver = spack.solver.asp.Solver()
result = solver.solve([self], tests=tests)
result = solver.solve([self], tests=tests, deprecated=allow_deprecated)
result.raise_if_unsat()
# take the best answer

2
lib/spack/spack/test/concretize.py
View File

@@ -1356,7 +1356,7 @@ def test_multivalued_variants_from_cli(self, spec_str, expected_dict):
# Version 1.1.0 is deprecated and should not be selected, unless we
# explicitly asked for that
("deprecated-versions", ["deprecated-versions@1.0.0"]),
("deprecated-versions@1.1.0", ["deprecated-versions@1.1.0"]),
("deprecated-versions@=1.1.0", ["deprecated-versions@1.1.0"]),
],
)
@pytest.mark.only_clingo("Use case not supported by the original concretizer")

4
lib/spack/spack/test/concretize_requirements.py
View File

@@ -545,13 +545,13 @@ def test_reuse_oneof(concretize_scope, create_test_repo, mutable_database, fake_
def test_requirements_are_higher_priority_than_deprecation(concretize_scope, test_repo):
"""Test that users can override a deprecated version with a requirement."""
# @2.3 is a deprecated versions. Ensure that any_of picks both constraints,
# 2.3 is a deprecated versions. Ensure that any_of picks both constraints,
# since they are possible
conf_str = """\
packages:
y:
require:
- any_of: ["@2.3", "%gcc"]
- any_of: ["@=2.3", "%gcc"]
"""
update_packages_config(conf_str)