From 4e876b4014cccf477dce155159198a5dc9c0730b Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Thu, 2 May 2024 05:05:26 +0200 Subject: [PATCH] Allow more control over which specs are reused (#42782) This PR gives users finer control over which specs are reused during concretization. The value of the `concretizer:reuse` config option now can take an object with the following properties: - `roots`: true if reusing roots, false if reusing just dependencies - `exclude`: list of constraints used to select reusable specs - `include`: list of constraints used to select reusable specs - `from`: allows to select the sources of reused specs ### Examples #### Reuse only specs compiled with GCC ```yaml concretizer: reuse: roots: true include: - "%gcc" ``` #### `openmpi` must be used from externals, and it must be the only external used ```yaml concretizer: reuse: roots: true from: - type: local exclude: - "openmpi" - type: buildcache exclude: - "openmpi" - type: external include: - "openmpi" ``` --- lib/spack/docs/build_settings.rst | 75 ++++++++- lib/spack/spack/schema/concretizer.py | 29 +++- lib/spack/spack/solver/asp.py | 232 ++++++++++++++++++++------ lib/spack/spack/test/concretize.py | 75 +++++++++ 4 files changed, 357 insertions(+), 54 deletions(-) diff --git a/lib/spack/docs/build_settings.rst b/lib/spack/docs/build_settings.rst index f58c9f4c97..d545c70d18 100644 --- a/lib/spack/docs/build_settings.rst +++ b/lib/spack/docs/build_settings.rst @@ -21,23 +21,86 @@ is the following: Reuse already installed packages -------------------------------- -The ``reuse`` attribute controls whether Spack will prefer to use installed packages (``true``), or -whether it will do a "fresh" installation and prefer the latest settings from -``package.py`` files and ``packages.yaml`` (``false``). -You can use: +The ``reuse`` attribute controls how aggressively Spack reuses binary packages during concretization. The +attribute can either be a single value, or an object for more complex configurations. + +In the former case ("single value") it allows Spack to: + +1. Reuse installed packages and buildcaches for all the specs to be concretized, when ``true`` +2. Reuse installed packages and buildcaches only for the dependencies of the root specs, when ``dependencies`` +3. Disregard reusing installed packages and buildcaches, when ``false`` + +In case a finer control over which specs are reused is needed, then the value of this attribute can be +an object, with the following keys: + +1. ``roots``: if ``true`` root specs are reused, if ``false`` only dependencies of root specs are reused +2. ``from``: list of sources from which reused specs are taken + +Each source in ``from`` is itself an object: + +.. list-table:: Attributes for a source or reusable specs + :header-rows: 1 + + * - Attribute name + - Description + * - type (mandatory, string) + - Can be ``local``, ``buildcache``, or ``external`` + * - include (optional, list of specs) + - If present, reusable specs must match at least one of the constraint in the list + * - exclude (optional, list of specs) + - If present, reusable specs must not match any of the constraint in the list. + +For instance, the following configuration: + +.. code-block:: yaml + + concretizer: + reuse: + roots: true + from: + - type: local + include: + - "%gcc" + - "%clang" + +tells the concretizer to reuse all specs compiled with either ``gcc`` or ``clang``, that are installed +in the local store. Any spec from remote buildcaches is disregarded. + +To reduce the boilerplate in configuration files, default values for the ``include`` and +``exclude`` options can be pushed up one level: + +.. code-block:: yaml + + concretizer: + reuse: + roots: true + include: + - "%gcc" + from: + - type: local + - type: buildcache + - type: local + include: + - "foo %oneapi" + +In the example above we reuse all specs compiled with ``gcc`` from the local store +and remote buildcaches, and we also reuse ``foo %oneapi``. Note that the last source of +specs override the default ``include`` attribute. + +For one-off concretizations, the are command line arguments for each of the simple "single value" +configurations. This means a user can: .. code-block:: console % spack install --reuse -to enable reuse for a single installation, and you can use: +to enable reuse for a single installation, or: .. code-block:: console spack install --fresh to do a fresh install if ``reuse`` is enabled by default. -``reuse: dependencies`` is the default. .. seealso:: diff --git a/lib/spack/spack/schema/concretizer.py b/lib/spack/spack/schema/concretizer.py index 329f777f19..e1c4d64ce1 100644 --- a/lib/spack/spack/schema/concretizer.py +++ b/lib/spack/spack/schema/concretizer.py @@ -9,13 +9,40 @@ """ from typing import Any, Dict +LIST_OF_SPECS = {"type": "array", "items": {"type": "string"}} + properties: Dict[str, Any] = { "concretizer": { "type": "object", "additionalProperties": False, "properties": { "reuse": { - "oneOf": [{"type": "boolean"}, {"type": "string", "enum": ["dependencies"]}] + "oneOf": [ + {"type": "boolean"}, + {"type": "string", "enum": ["dependencies"]}, + { + "type": "object", + "properties": { + "roots": {"type": "boolean"}, + "include": LIST_OF_SPECS, + "exclude": LIST_OF_SPECS, + "from": { + "type": "array", + "items": { + "type": "object", + "properties": { + "type": { + "type": "string", + "enum": ["local", "buildcache", "external"], + }, + "include": LIST_OF_SPECS, + "exclude": LIST_OF_SPECS, + }, + }, + }, + }, + }, + ] }, "enable_node_namespace": {"type": "boolean"}, "targets": { diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index b3be999a1b..e56fe92b57 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -6,6 +6,7 @@ import collections.abc import copy import enum +import functools import itertools import os import pathlib @@ -1221,6 +1222,9 @@ def pkg_rules(self, pkg, tests): def trigger_rules(self): """Flushes all the trigger rules collected so far, and clears the cache.""" + if not self._trigger_cache: + return + self.gen.h2("Trigger conditions") for name in self._trigger_cache: cache = self._trigger_cache[name] @@ -1234,6 +1238,9 @@ def trigger_rules(self): def effect_rules(self): """Flushes all the effect rules collected so far, and clears the cache.""" + if not self._effect_cache: + return + self.gen.h2("Imposed requirements") for name in self._effect_cache: cache = self._effect_cache[name] @@ -1614,6 +1621,27 @@ def external_packages(self): packages_yaml = _external_config_with_implicit_externals(spack.config.CONFIG) self.gen.h1("External packages") + spec_filters = [] + concretizer_yaml = spack.config.get("concretizer") + reuse_yaml = concretizer_yaml.get("reuse") + if isinstance(reuse_yaml, typing.Mapping): + default_include = reuse_yaml.get("include", []) + default_exclude = reuse_yaml.get("exclude", []) + for source in reuse_yaml.get("from", []): + if source["type"] != "external": + continue + + include = source.get("include", default_include) + exclude = source.get("exclude", default_exclude) + spec_filters.append( + SpecFilter( + factory=lambda: [], + is_usable=lambda x: True, + include=include, + exclude=exclude, + ) + ) + for pkg_name, data in packages_yaml.items(): if pkg_name == "all": continue @@ -1622,7 +1650,6 @@ def external_packages(self): if pkg_name not in spack.repo.PATH: continue - self.gen.h2("External package: {0}".format(pkg_name)) # Check if the external package is buildable. If it is # not then "external()" is a fact, unless we can # reuse an already installed spec. @@ -1632,7 +1659,17 @@ def external_packages(self): # Read a list of all the specs for this package externals = data.get("externals", []) - external_specs = [spack.spec.parse_with_version_concrete(x["spec"]) for x in externals] + candidate_specs = [ + spack.spec.parse_with_version_concrete(x["spec"]) for x in externals + ] + + external_specs = [] + if spec_filters: + for current_filter in spec_filters: + current_filter.factory = lambda: candidate_specs + external_specs.extend(current_filter.selected_specs()) + else: + external_specs.extend(candidate_specs) # Order the external versions to prefer more recent versions # even if specs in packages.yaml are not ordered that way @@ -3564,25 +3601,159 @@ def _has_runtime_dependencies(spec: spack.spec.Spec) -> bool: return True +class SpecFilter: + """Given a method to produce a list of specs, this class can filter them according to + different criteria. + """ + + def __init__( + self, + factory: Callable[[], List[spack.spec.Spec]], + is_usable: Callable[[spack.spec.Spec], bool], + include: List[str], + exclude: List[str], + ) -> None: + """ + Args: + factory: factory to produce a list of specs + is_usable: predicate that takes a spec in input and returns False if the spec + should not be considered for this filter, True otherwise. + include: if present, a "good" spec must match at least one entry in the list + exclude: if present, a "good" spec must not match any entry in the list + """ + self.factory = factory + self.is_usable = is_usable + self.include = include + self.exclude = exclude + + def is_selected(self, s: spack.spec.Spec) -> bool: + if not self.is_usable(s): + return False + + if self.include and not any(s.satisfies(c) for c in self.include): + return False + + if self.exclude and any(s.satisfies(c) for c in self.exclude): + return False + + return True + + def selected_specs(self) -> List[spack.spec.Spec]: + return [s for s in self.factory() if self.is_selected(s)] + + @staticmethod + def from_store(configuration, include, exclude) -> "SpecFilter": + """Constructs a filter that takes the specs from the current store.""" + packages = _external_config_with_implicit_externals(configuration) + is_reusable = functools.partial(_is_reusable, packages=packages, local=True) + factory = functools.partial(_specs_from_store, configuration=configuration) + return SpecFilter(factory=factory, is_usable=is_reusable, include=include, exclude=exclude) + + @staticmethod + def from_buildcache(configuration, include, exclude) -> "SpecFilter": + """Constructs a filter that takes the specs from the configured buildcaches.""" + packages = _external_config_with_implicit_externals(configuration) + is_reusable = functools.partial(_is_reusable, packages=packages, local=False) + return SpecFilter( + factory=_specs_from_mirror, is_usable=is_reusable, include=include, exclude=exclude + ) + + +def _specs_from_store(configuration): + store = spack.store.create(configuration) + with store.db.read_transaction(): + return store.db.query(installed=True) + + +def _specs_from_mirror(): + try: + return spack.binary_distribution.update_cache_and_get_specs() + except (spack.binary_distribution.FetchCacheError, IndexError): + # this is raised when no mirrors had indices. + # TODO: update mirror configuration so it can indicate that the + # TODO: source cache (or any mirror really) doesn't have binaries. + return [] + + +class ReuseStrategy(enum.Enum): + ROOTS = enum.auto() + DEPENDENCIES = enum.auto() + NONE = enum.auto() + + +class ReusableSpecsSelector: + """Selects specs that can be reused during concretization.""" + + def __init__(self, configuration: spack.config.Configuration) -> None: + self.configuration = configuration + self.store = spack.store.create(configuration) + self.reuse_strategy = ReuseStrategy.ROOTS + + reuse_yaml = self.configuration.get("concretizer:reuse", False) + self.reuse_sources = [] + if not isinstance(reuse_yaml, typing.Mapping): + if reuse_yaml is False: + self.reuse_strategy = ReuseStrategy.NONE + if reuse_yaml == "dependencies": + self.reuse_strategy = ReuseStrategy.DEPENDENCIES + self.reuse_sources.extend( + [ + SpecFilter.from_store( + configuration=self.configuration, include=[], exclude=[] + ), + SpecFilter.from_buildcache( + configuration=self.configuration, include=[], exclude=[] + ), + ] + ) + else: + roots = reuse_yaml.get("roots", True) + if roots is True: + self.reuse_strategy = ReuseStrategy.ROOTS + else: + self.reuse_strategy = ReuseStrategy.DEPENDENCIES + default_include = reuse_yaml.get("include", []) + default_exclude = reuse_yaml.get("exclude", []) + default_sources = [{"type": "local"}, {"type": "buildcache"}] + for source in reuse_yaml.get("from", default_sources): + include = source.get("include", default_include) + exclude = source.get("exclude", default_exclude) + if source["type"] == "local": + self.reuse_sources.append( + SpecFilter.from_store(self.configuration, include=include, exclude=exclude) + ) + elif source["type"] == "buildcache": + self.reuse_sources.append( + SpecFilter.from_buildcache( + self.configuration, include=include, exclude=exclude + ) + ) + + def reusable_specs(self, specs: List[spack.spec.Spec]) -> List[spack.spec.Spec]: + if self.reuse_strategy == ReuseStrategy.NONE: + return [] + + result = [] + for reuse_source in self.reuse_sources: + result.extend(reuse_source.selected_specs()) + + # If we only want to reuse dependencies, remove the root specs + if self.reuse_strategy == ReuseStrategy.DEPENDENCIES: + result = [spec for spec in result if not any(root in spec for root in specs)] + + return result + + class Solver: """This is the main external interface class for solving. It manages solver configuration and preferences in one place. It sets up the solve and passes the setup method to the driver, as well. - - Properties of interest: - - ``reuse (bool)`` - Whether to try to reuse existing installs/binaries - """ def __init__(self): self.driver = PyclingoDriver() - - # These properties are settable via spack configuration, and overridable - # by setting them directly as properties. - self.reuse = spack.config.get("concretizer:reuse", True) + self.selector = ReusableSpecsSelector(configuration=spack.config.CONFIG) @staticmethod def _check_input_and_extract_concrete_specs(specs): @@ -3596,39 +3767,6 @@ def _check_input_and_extract_concrete_specs(specs): spack.spec.Spec.ensure_valid_variants(s) return reusable - def _reusable_specs(self, specs): - reusable_specs = [] - if self.reuse: - packages = _external_config_with_implicit_externals(spack.config.CONFIG) - # Specs from the local Database - with spack.store.STORE.db.read_transaction(): - reusable_specs.extend( - s - for s in spack.store.STORE.db.query(installed=True) - if _is_reusable(s, packages, local=True) - ) - - # Specs from buildcaches - try: - reusable_specs.extend( - s - for s in spack.binary_distribution.update_cache_and_get_specs() - if _is_reusable(s, packages, local=False) - ) - except (spack.binary_distribution.FetchCacheError, IndexError): - # this is raised when no mirrors had indices. - # TODO: update mirror configuration so it can indicate that the - # TODO: source cache (or any mirror really) doesn't have binaries. - pass - - # If we only want to reuse dependencies, remove the root specs - if self.reuse == "dependencies": - reusable_specs = [ - spec for spec in reusable_specs if not any(root in spec for root in specs) - ] - - return reusable_specs - def solve( self, specs, @@ -3654,7 +3792,7 @@ def solve( # Check upfront that the variants are admissible specs = [s.lookup_hash() for s in specs] reusable_specs = self._check_input_and_extract_concrete_specs(specs) - reusable_specs.extend(self._reusable_specs(specs)) + reusable_specs.extend(self.selector.reusable_specs(specs)) setup = SpackSolverSetup(tests=tests) output = OutputConfiguration(timers=timers, stats=stats, out=out, setup_only=setup_only) result, _, _ = self.driver.solve( @@ -3683,7 +3821,7 @@ def solve_in_rounds( """ specs = [s.lookup_hash() for s in specs] reusable_specs = self._check_input_and_extract_concrete_specs(specs) - reusable_specs.extend(self._reusable_specs(specs)) + reusable_specs.extend(self.selector.reusable_specs(specs)) setup = SpackSolverSetup(tests=tests) # Tell clingo that we don't have to solve all the inputs at once diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py index f69ab54a58..cf20934a00 100644 --- a/lib/spack/spack/test/concretize.py +++ b/lib/spack/spack/test/concretize.py @@ -2823,3 +2823,78 @@ def test_concretization_version_order(): Version("develop"), # likely development version Version("2.0"), # deprecated ] + + +@pytest.mark.only_clingo("Original concretizer cannot reuse specs") +@pytest.mark.parametrize( + "roots,reuse_yaml,expected,not_expected,expected_length", + [ + ( + ["mpileaks"], + {"roots": True, "include": ["^mpich"]}, + ["^mpich"], + ["^mpich2", "^zmpi"], + 2, + ), + ( + ["mpileaks"], + {"roots": True, "include": ["externaltest"]}, + ["externaltest"], + ["^mpich", "^mpich2", "^zmpi"], + 1, + ), + ], +) +@pytest.mark.usefixtures("database", "mock_store") +@pytest.mark.not_on_windows("Expected length is different on Windows") +def test_filtering_reused_specs( + roots, reuse_yaml, expected, not_expected, expected_length, mutable_config, monkeypatch +): + """Tests that we can select which specs are to be reused, using constraints as filters""" + # Assume all specs have a runtime dependency + monkeypatch.setattr(spack.solver.asp, "_has_runtime_dependencies", lambda x: True) + mutable_config.set("concretizer:reuse", reuse_yaml) + selector = spack.solver.asp.ReusableSpecsSelector(mutable_config) + specs = selector.reusable_specs(roots) + + assert len(specs) == expected_length + + for constraint in expected: + assert all(x.satisfies(constraint) for x in specs) + + for constraint in not_expected: + assert all(not x.satisfies(constraint) for x in specs) + + +@pytest.mark.usefixtures("database", "mock_store") +@pytest.mark.parametrize( + "reuse_yaml,expected_length", + [({"from": [{"type": "local"}]}, 17), ({"from": [{"type": "buildcache"}]}, 0)], +) +@pytest.mark.not_on_windows("Expected length is different on Windows") +def test_selecting_reused_sources(reuse_yaml, expected_length, mutable_config, monkeypatch): + """Tests that we can turn on/off sources of reusable specs""" + # Assume all specs have a runtime dependency + monkeypatch.setattr(spack.solver.asp, "_has_runtime_dependencies", lambda x: True) + mutable_config.set("concretizer:reuse", reuse_yaml) + selector = spack.solver.asp.ReusableSpecsSelector(mutable_config) + specs = selector.reusable_specs(["mpileaks"]) + assert len(specs) == expected_length + + +@pytest.mark.parametrize( + "specs,include,exclude,expected", + [ + # "foo" discarded by include rules (everything compiled with GCC) + (["cmake@3.27.9 %gcc", "foo %clang"], ["%gcc"], [], ["cmake@3.27.9 %gcc"]), + # "cmake" discarded by exclude rules (everything compiled with GCC but cmake) + (["cmake@3.27.9 %gcc", "foo %gcc"], ["%gcc"], ["cmake"], ["foo %gcc"]), + ], +) +def test_spec_filters(specs, include, exclude, expected): + specs = [Spec(x) for x in specs] + expected = [Spec(x) for x in expected] + f = spack.solver.asp.SpecFilter( + factory=lambda: specs, is_usable=lambda x: True, include=include, exclude=exclude + ) + assert f.selected_specs() == expected