From a2a6e65e279cb14c74eb008a90e4934a20c749fc Mon Sep 17 00:00:00 2001 From: Greg Becker Date: Sun, 14 May 2023 04:36:03 -0700 Subject: concretizer: don't change concrete environments without `--force` (#37438) If a user does not explicitly `--force` the concretization of an entire environment, Spack will try to reuse the concrete specs that are already in the lockfile. --------- Co-authored-by: becker33 Co-authored-by: Todd Gamblin --- lib/spack/docs/environments.rst | 23 +++++----- lib/spack/spack/environment/environment.py | 72 ++++++++++++++++++++++-------- lib/spack/spack/test/cmd/env.py | 8 +++- 3 files changed, 71 insertions(+), 32 deletions(-) (limited to 'lib') diff --git a/lib/spack/docs/environments.rst b/lib/spack/docs/environments.rst index 415d590882..74831073ed 100644 --- a/lib/spack/docs/environments.rst +++ b/lib/spack/docs/environments.rst @@ -347,7 +347,7 @@ the Environment and then install the concretized specs. (see :ref:`build-jobs`). To speed up environment builds further, independent packages can be installed in parallel by launching more Spack instances. For example, the following will build at most four packages in parallel using - three background jobs: + three background jobs: .. code-block:: console @@ -395,7 +395,7 @@ version (and other constraints) passed as the spec argument to the For packages with ``git`` attributes, git branches, tags, and commits can also be used as valid concrete versions (see :ref:`version-specifier`). -This means that for a package ``foo``, ``spack develop foo@git.main`` will clone +This means that for a package ``foo``, ``spack develop foo@git.main`` will clone the ``main`` branch of the package, and ``spack install`` will install from that git clone if ``foo`` is in the environment. Further development on ``foo`` can be tested by reinstalling the environment, @@ -589,10 +589,11 @@ user support groups providing a large software stack for their HPC center. .. admonition:: Re-concretization of user specs - When using *unified* concretization (when possible), the entire set of specs will be - re-concretized after any addition of new user specs, to ensure that - the environment remains consistent / minimal. When instead unified concretization is - disabled, only the new specs will be concretized after any addition. + The ``spack concretize`` command without additional arguments will *not* change any + previously concretized specs. This may prevent it from finding a solution when using + ``unify: true``, and it may prevent it from finding a minimal solution when using + ``unify: when_possible``. You can force Spack to ignore the existing concrete environment + with ``spack concretize -f``. ^^^^^^^^^^^^^ Spec Matrices @@ -1121,19 +1122,19 @@ index once every package is pushed. Note how this target uses the generated SPACK ?= spack BUILDCACHE_DIR = $(CURDIR)/tarballs - + .PHONY: all - + all: push - + include env.mk - + example/push/%: example/install/% @mkdir -p $(dir $@) $(info About to push $(SPEC) to a buildcache) $(SPACK) -e . buildcache create --allow-root --only=package --directory $(BUILDCACHE_DIR) /$(HASH) @touch $@ - + push: $(addprefix example/push/,$(example/SPACK_PACKAGE_IDS)) $(info Updating the buildcache index) $(SPACK) -e . buildcache update-index --directory $(BUILDCACHE_DIR) diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index e49d7854c4..bee1d629a6 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -16,7 +16,7 @@ import time import urllib.parse import urllib.request import warnings -from typing import Any, Dict, List, Optional, Union +from typing import Any, Dict, List, Optional, Set, Tuple, Union import llnl.util.filesystem as fs import llnl.util.tty as tty @@ -1365,64 +1365,98 @@ class Environment: msg = "concretization strategy not implemented [{0}]" raise SpackEnvironmentError(msg.format(self.unify)) - def _concretize_together_where_possible(self, tests=False): + def _get_specs_to_concretize( + self, + ) -> Tuple[Set[spack.spec.Spec], Set[spack.spec.Spec], List[spack.spec.Spec]]: + """Compute specs to concretize for unify:true and unify:when_possible. + + This includes new user specs and any already concretized specs. + + Returns: + Tuple of new user specs, user specs to keep, and the specs to concretize. + + """ + # Exit early if the set of concretized specs is the set of user specs + new_user_specs = set(self.user_specs) - set(self.concretized_user_specs) + kept_user_specs = set(self.user_specs) & set(self.concretized_user_specs) + if not new_user_specs: + return new_user_specs, kept_user_specs, [] + + concrete_specs_to_keep = [ + concrete + for abstract, concrete in self.concretized_specs() + if abstract in kept_user_specs + ] + + specs_to_concretize = list(new_user_specs) + concrete_specs_to_keep + return new_user_specs, kept_user_specs, specs_to_concretize + + def _concretize_together_where_possible( + self, tests: bool = False + ) -> List[Tuple[spack.spec.Spec, spack.spec.Spec]]: # Avoid cyclic dependency import spack.solver.asp # Exit early if the set of concretized specs is the set of user specs - user_specs_did_not_change = not bool( - set(self.user_specs) - set(self.concretized_user_specs) - ) - if user_specs_did_not_change: + new_user_specs, _, specs_to_concretize = self._get_specs_to_concretize() + if not new_user_specs: return [] - # Proceed with concretization self.concretized_user_specs = [] self.concretized_order = [] self.specs_by_hash = {} result_by_user_spec = {} solver = spack.solver.asp.Solver() - for result in solver.solve_in_rounds(self.user_specs, tests=tests): + for result in solver.solve_in_rounds(specs_to_concretize, tests=tests): result_by_user_spec.update(result.specs_by_input) result = [] for abstract, concrete in sorted(result_by_user_spec.items()): + if abstract in new_user_specs: + result.append((abstract, concrete)) + else: + assert (abstract, concrete) in result self._add_concrete_spec(abstract, concrete) - result.append((abstract, concrete)) return result - def _concretize_together(self, tests=False): + def _concretize_together( + self, tests: bool = False + ) -> List[Tuple[spack.spec.Spec, spack.spec.Spec]]: """Concretization strategy that concretizes all the specs in the same DAG. """ # Exit early if the set of concretized specs is the set of user specs - user_specs_did_not_change = not bool( - set(self.user_specs) - set(self.concretized_user_specs) - ) - if user_specs_did_not_change: + new_user_specs, kept_user_specs, specs_to_concretize = self._get_specs_to_concretize() + if not new_user_specs: return [] - # Proceed with concretization self.concretized_user_specs = [] self.concretized_order = [] self.specs_by_hash = {} try: concrete_specs = spack.concretize.concretize_specs_together( - *self.user_specs, tests=tests + *specs_to_concretize, tests=tests ) except spack.error.UnsatisfiableSpecError as e: # "Enhance" the error message for multiple root specs, suggest a less strict # form of concretization. if len(self.user_specs) > 1: + e.message += ". " + if kept_user_specs: + e.message += ( + "Couldn't concretize without changing the existing environment. " + "If you are ok with changing it, try `spack concretize --force`. " + ) e.message += ( - ". Consider setting `concretizer:unify` to `when_possible` " - "or `false` to relax the concretizer strictness." + "You could consider setting `concretizer:unify` to `when_possible` " + "or `false` to allow multiple versions of some packages." ) raise - concretized_specs = [x for x in zip(self.user_specs, concrete_specs)] + # zip truncates the longer list, which is exactly what we want here + concretized_specs = [x for x in zip(new_user_specs | kept_user_specs, concrete_specs)] for abstract, concrete in concretized_specs: self._add_concrete_spec(abstract, concrete) return concretized_specs diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index 0bde94a45b..a52b2f3839 100644 --- a/lib/spack/spack/test/cmd/env.py +++ b/lib/spack/spack/test/cmd/env.py @@ -2404,7 +2404,11 @@ def test_concretize_user_specs_together(): # Concretize a second time using 'mpich2' as the MPI provider e.remove("mpich") e.add("mpich2") - e.concretize() + + # Concretizing without invalidating the concrete spec for mpileaks fails + with pytest.raises(spack.error.UnsatisfiableSpecError): + e.concretize() + e.concretize(force=True) assert all("mpich2" in spec for _, spec in e.concretized_specs()) assert all("mpich" not in spec for _, spec in e.concretized_specs()) @@ -2435,7 +2439,7 @@ def test_duplicate_packages_raise_when_concretizing_together(): e.add("mpich") with pytest.raises( - spack.error.UnsatisfiableSpecError, match=r"relax the concretizer strictness" + spack.error.UnsatisfiableSpecError, match=r"You could consider setting `concretizer:unify`" ): e.concretize() -- cgit v1.2.3-60-g2f50