ASP-based solver: allow to reuse installed externals (#31558)
fixes #31484 Before this change if anything was matching an external condition, it was considered "external" and thus something to be "built". This was happening in particular to external packages that were re-read from the DB, which then couldn't be reused, causing the problems shown in #31484. This PR fixes the issue by excluding specs with a "hash" from being considered "external" * Test that users have a way to select a virtual This ought to be solved by extending the "require" attribute to virtual packages, so that one can: ```yaml mpi: require: 'multi-provider-mpi' ``` * Prevent conflicts to be enforced on specs that can be reused. * Rename the "external_only" fact to "buildable_false", to better reflect its origin
This commit is contained in:
Massimiliano Culpo
GitHub
parent
08261af4ab
commit
0c6e3188ac
@ -49,9 +49,8 @@ packages rather than building its own packages. This may be desirable
|
||||
if machines ship with system packages, such as a customized MPI
|
||||
that should be used instead of Spack building its own MPI.
|
||||
|
||||
External packages are configured through the ``packages.yaml`` file found
|
||||
in a Spack installation's ``etc/spack/`` or a user's ``~/.spack/``
|
||||
directory. Here's an example of an external configuration:
|
||||
External packages are configured through the ``packages.yaml`` file.
|
||||
Here's an example of an external configuration:
|
||||
|
||||
.. code-block:: yaml
|
||||
|
||||
@ -97,11 +96,14 @@ Each package version and compiler listed in an external should
|
||||
have entries in Spack's packages and compiler configuration, even
|
||||
though the package and compiler may not ever be built.
|
||||
|
||||
The packages configuration can tell Spack to use an external location
|
||||
for certain package versions, but it does not restrict Spack to using
|
||||
external packages. In the above example, since newer versions of OpenMPI
|
||||
are available, Spack will choose to start building and linking with the
|
||||
latest version rather than continue using the pre-installed OpenMPI versions.
|
||||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
Prevent packages from being built from sources
|
||||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
Adding an external spec in ``packages.yaml`` allows Spack to use an external location,
|
||||
but it does not prevent Spack from building packages from sources. In the above example,
|
||||
Spack might choose for many valid reasons to start building and linking with the
|
||||
latest version of OpenMPI rather than continue using the pre-installed OpenMPI versions.
|
||||
|
||||
To prevent this, the ``packages.yaml`` configuration also allows packages
|
||||
to be flagged as non-buildable. The previous example could be modified to
|
||||
@ -121,9 +123,15 @@ be:
|
||||
buildable: False
|
||||
|
||||
The addition of the ``buildable`` flag tells Spack that it should never build
|
||||
its own version of OpenMPI, and it will instead always rely on a pre-built
|
||||
OpenMPI. Similar to ``paths``, ``buildable`` is specified as a property under
|
||||
a package name.
|
||||
its own version of OpenMPI from sources, and it will instead always rely on a pre-built
|
||||
OpenMPI.
|
||||
|
||||
.. note::
|
||||
|
||||
If ``concretizer:reuse`` is on (see :ref:`concretizer-options` for more information on that flag)
|
||||
pre-built specs include specs already available from a local store, an upstream store, a registered
|
||||
buildcache or specs marked as externals in ``packages.yaml``. If ``concretizer:reuse`` is off, only
|
||||
external specs in ``packages.yaml`` are included in the list of pre-built specs.
|
||||
|
||||
If an external module is specified as not buildable, then Spack will load the
|
||||
external module into the build environment which can be used for linking.
|
||||
@ -132,6 +140,10 @@ The ``buildable`` does not need to be paired with external packages.
|
||||
It could also be used alone to forbid packages that may be
|
||||
buggy or otherwise undesirable.
|
||||
|
||||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
Non-buildable virtual packages
|
||||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
Virtual packages in Spack can also be specified as not buildable, and
|
||||
external implementations can be provided. In the example above,
|
||||
OpenMPI is configured as not buildable, but Spack will often prefer
|
||||
@ -153,21 +165,37 @@ but more conveniently:
|
||||
- spec: "openmpi@1.6.5%intel@10.1 arch=linux-debian7-x86_64"
|
||||
prefix: /opt/openmpi-1.6.5-intel
|
||||
|
||||
Implementations can also be listed immediately under the virtual they provide:
|
||||
Spack can then use any of the listed external implementations of MPI
|
||||
to satisfy a dependency, and will choose depending on the compiler and
|
||||
architecture.
|
||||
|
||||
In cases where the concretizer is configured to reuse specs, and other ``mpi`` providers
|
||||
(available via stores or buildcaches) are not wanted, Spack can be configured to require
|
||||
specs matching only the available externals:
|
||||
|
||||
.. code-block:: yaml
|
||||
|
||||
packages:
|
||||
mpi:
|
||||
buildable: False
|
||||
openmpi@1.4.3%gcc@4.4.7 arch=linux-debian7-x86_64: /opt/openmpi-1.4.3
|
||||
openmpi@1.4.3%gcc@4.4.7 arch=linux-debian7-x86_64+debug: /opt/openmpi-1.4.3-debug
|
||||
openmpi@1.6.5%intel@10.1 arch=linux-debian7-x86_64: /opt/openmpi-1.6.5-intel
|
||||
mpich@3.3 %clang@9.0.0 arch=linux-debian7-x86_64: /opt/mpich-3.3-intel
|
||||
require:
|
||||
- one_of: [
|
||||
"openmpi@1.4.3%gcc@4.4.7 arch=linux-debian7-x86_64",
|
||||
"openmpi@1.4.3%gcc@4.4.7 arch=linux-debian7-x86_64+debug",
|
||||
"openmpi@1.6.5%intel@10.1 arch=linux-debian7-x86_64"
|
||||
]
|
||||
openmpi:
|
||||
externals:
|
||||
- spec: "openmpi@1.4.3%gcc@4.4.7 arch=linux-debian7-x86_64"
|
||||
prefix: /opt/openmpi-1.4.3
|
||||
- spec: "openmpi@1.4.3%gcc@4.4.7 arch=linux-debian7-x86_64+debug"
|
||||
prefix: /opt/openmpi-1.4.3-debug
|
||||
- spec: "openmpi@1.6.5%intel@10.1 arch=linux-debian7-x86_64"
|
||||
prefix: /opt/openmpi-1.6.5-intel
|
||||
|
||||
Spack can then use any of the listed external implementations of MPI
|
||||
to satisfy a dependency, and will choose depending on the compiler and
|
||||
architecture.
|
||||
This configuration prevents any spec using MPI and originating from stores or buildcaches to be reused,
|
||||
unless it matches the requirements under ``packages:mpi:require``. For more information on requirements see
|
||||
:ref:`package-requirements`.
|
||||
|
||||
.. _cmd-spack-external-find:
|
||||
|
||||
@ -194,11 +222,6 @@ Specific limitations include:
|
||||
* Packages are not discoverable by default: For a package to be
|
||||
discoverable with ``spack external find``, it needs to add special
|
||||
logic. See :ref:`here <make-package-findable>` for more details.
|
||||
* The current implementation only collects and examines executable files,
|
||||
so it is typically only useful for build/run dependencies (in some cases
|
||||
if a library package also provides an executable, it may be possible to
|
||||
extract a meaningful Spec by running the executable - for example the
|
||||
compiler wrappers in MPI implementations).
|
||||
* The logic does not search through module files, it can only detect
|
||||
packages with executables defined in ``PATH``; you can help Spack locate
|
||||
externals which use module files by loading any associated modules for
|
||||
|
||||
@ -1210,10 +1210,11 @@ def external_packages(self):
|
||||
|
||||
self.gen.h2("External package: {0}".format(pkg_name))
|
||||
# Check if the external package is buildable. If it is
|
||||
# not then "external(<pkg>)" is a fact.
|
||||
# not then "external(<pkg>)" is a fact, unless we can
|
||||
# reuse an already installed spec.
|
||||
external_buildable = data.get("buildable", True)
|
||||
if not external_buildable:
|
||||
self.gen.fact(fn.external_only(pkg_name))
|
||||
self.gen.fact(fn.buildable_false(pkg_name))
|
||||
|
||||
# Read a list of all the specs for this package
|
||||
externals = data.get("externals", [])
|
||||
|
||||
@ -276,7 +276,8 @@ error(0, Msg) :- node(Package),
|
||||
conflict(Package, TriggerID, ConstraintID, Msg),
|
||||
condition_holds(TriggerID),
|
||||
condition_holds(ConstraintID),
|
||||
not external(Package). % ignore conflicts for externals
|
||||
not external(Package), % ignore conflicts for externals
|
||||
not hash(Package, _). % ignore conflicts for installed packages
|
||||
|
||||
#defined conflict/4.
|
||||
|
||||
@ -436,7 +437,7 @@ attr("node_compiler_version_satisfies", Package, Compiler, Version)
|
||||
#defined external/1.
|
||||
#defined external_spec/2.
|
||||
#defined external_version_declared/4.
|
||||
#defined external_only/1.
|
||||
#defined buildable_false/1.
|
||||
#defined pkg_provider_preference/4.
|
||||
#defined default_provider_preference/3.
|
||||
#defined node_version_satisfies/2.
|
||||
@ -463,8 +464,10 @@ error(2, "Attempted to use external for '{0}' which does not satisfy any configu
|
||||
version_weight(Package, Weight) :- external_version(Package, Version, Weight).
|
||||
version(Package, Version) :- external_version(Package, Version, Weight).
|
||||
|
||||
% if a package is not buildable (external_only), only externals are allowed
|
||||
external(Package) :- external_only(Package), node(Package).
|
||||
% if a package is not buildable, only externals or hashed specs are allowed
|
||||
external(Package) :- buildable_false(Package),
|
||||
node(Package),
|
||||
not hash(Package, _).
|
||||
|
||||
% a package is a real_node if it is not external
|
||||
real_node(Package) :- node(Package), not external(Package).
|
||||
@ -483,7 +486,8 @@ external(Package) :- external_spec_selected(Package, _).
|
||||
% determine if an external spec has been selected
|
||||
external_spec_selected(Package, LocalIndex) :-
|
||||
external_conditions_hold(Package, LocalIndex),
|
||||
node(Package).
|
||||
node(Package),
|
||||
not hash(Package, _).
|
||||
|
||||
external_conditions_hold(Package, LocalIndex) :-
|
||||
possible_external(ID, Package, LocalIndex), condition_holds(ID).
|
||||
|
||||
@ -1767,3 +1767,90 @@ def test_git_ref_version_errors_if_unknown_version(self, git_ref):
|
||||
match="The reference version 'main' for package 'develop-branch-version'",
|
||||
):
|
||||
s.concretized()
|
||||
|
||||
@pytest.mark.regression("31484")
|
||||
def test_installed_externals_are_reused(self, mutable_database, repo_with_changing_recipe):
|
||||
"""Test that external specs that are in the DB can be reused."""
|
||||
if spack.config.get("config:concretizer") == "original":
|
||||
pytest.xfail("Use case not supported by the original concretizer")
|
||||
|
||||
# Configuration to be added to packages.yaml
|
||||
external_conf = {
|
||||
"changing": {
|
||||
"buildable": False,
|
||||
"externals": [{"spec": "changing@1.0", "prefix": "/usr"}],
|
||||
}
|
||||
}
|
||||
spack.config.set("packages", external_conf)
|
||||
|
||||
# Install the external spec
|
||||
external1 = Spec("changing@1.0").concretized()
|
||||
external1.package.do_install(fake=True, explicit=True)
|
||||
assert external1.external
|
||||
|
||||
# Modify the package.py file
|
||||
repo_with_changing_recipe.change({"delete_variant": True})
|
||||
|
||||
# Try to concretize the external without reuse and confirm the hash changed
|
||||
with spack.config.override("concretizer:reuse", False):
|
||||
external2 = Spec("changing@1.0").concretized()
|
||||
assert external2.dag_hash() != external1.dag_hash()
|
||||
|
||||
# ... while with reuse we have the same hash
|
||||
with spack.config.override("concretizer:reuse", True):
|
||||
external3 = Spec("changing@1.0").concretized()
|
||||
assert external3.dag_hash() == external1.dag_hash()
|
||||
|
||||
@pytest.mark.regression("31484")
|
||||
def test_user_can_select_externals_with_require(self, mutable_database):
|
||||
"""Test that users have means to select an external even in presence of reusable specs."""
|
||||
if spack.config.get("config:concretizer") == "original":
|
||||
pytest.xfail("Use case not supported by the original concretizer")
|
||||
|
||||
# Configuration to be added to packages.yaml
|
||||
external_conf = {
|
||||
"mpi": {"buildable": False},
|
||||
"multi-provider-mpi": {
|
||||
"externals": [{"spec": "multi-provider-mpi@2.0.0", "prefix": "/usr"}]
|
||||
},
|
||||
}
|
||||
spack.config.set("packages", external_conf)
|
||||
|
||||
# mpich and others are installed, so check that
|
||||
# fresh use the external, reuse does not
|
||||
with spack.config.override("concretizer:reuse", False):
|
||||
mpi_spec = Spec("mpi").concretized()
|
||||
assert mpi_spec.name == "multi-provider-mpi"
|
||||
|
||||
with spack.config.override("concretizer:reuse", True):
|
||||
mpi_spec = Spec("mpi").concretized()
|
||||
assert mpi_spec.name != "multi-provider-mpi"
|
||||
|
||||
external_conf["mpi"]["require"] = "multi-provider-mpi"
|
||||
spack.config.set("packages", external_conf)
|
||||
|
||||
with spack.config.override("concretizer:reuse", True):
|
||||
mpi_spec = Spec("mpi").concretized()
|
||||
assert mpi_spec.name == "multi-provider-mpi"
|
||||
|
||||
@pytest.mark.regression("31484")
|
||||
def test_installed_specs_disregard_conflicts(self, mutable_database, monkeypatch):
|
||||
"""Test that installed specs do not trigger conflicts. This covers for the rare case
|
||||
where a conflict is added on a package after a spec matching the conflict was installed.
|
||||
"""
|
||||
if spack.config.get("config:concretizer") == "original":
|
||||
pytest.xfail("Use case not supported by the original concretizer")
|
||||
|
||||
# Add a conflict to "mpich" that match an already installed "mpich~debug"
|
||||
pkg_cls = spack.repo.path.get_pkg_class("mpich")
|
||||
monkeypatch.setitem(pkg_cls.conflicts, "~debug", [(spack.spec.Spec(), None)])
|
||||
|
||||
# If we concretize with --fresh the conflict is taken into account
|
||||
with spack.config.override("concretizer:reuse", False):
|
||||
s = Spec("mpich").concretized()
|
||||
assert s.satisfies("+debug")
|
||||
|
||||
# If we concretize with --reuse it is not, since "mpich~debug" was already installed
|
||||
with spack.config.override("concretizer:reuse", True):
|
||||
s = Spec("mpich").concretized()
|
||||
assert s.satisfies("~debug")
|
||||
|
||||
Loading…
Reference in New Issue
Block a user