diff options
| -rw-r--r-- | lib/spack/llnl/util/link_tree.py | 5 | ||||
| -rw-r--r-- | lib/spack/spack/environment/environment.py | 21 | ||||
| -rw-r--r-- | lib/spack/spack/filesystem_view.py | 30 | ||||
| -rw-r--r-- | lib/spack/spack/test/cmd/env.py | 6 |
4 files changed, 53 insertions, 9 deletions
diff --git a/lib/spack/llnl/util/link_tree.py b/lib/spack/llnl/util/link_tree.py index 52268bc3f8..dece1c5edf 100644 --- a/lib/spack/llnl/util/link_tree.py +++ b/lib/spack/llnl/util/link_tree.py @@ -430,6 +430,11 @@ class MergeConflictError(Exception): pass +class ConflictingSpecsError(MergeConflictError): + def __init__(self, spec_1, spec_2): + super(MergeConflictError, self).__init__(spec_1, spec_2) + + class SingleMergeConflictError(MergeConflictError): def __init__(self, path): super(MergeConflictError, self).__init__("Package merge blocked by file: %s" % path) diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index a46b8f9835..813ffca0b6 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -18,7 +18,9 @@ import ruamel.yaml as yaml import llnl.util.filesystem as fs import llnl.util.tty as tty +import llnl.util.tty.color as clr from llnl.util.lang import dedupe +from llnl.util.link_tree import ConflictingSpecsError from llnl.util.symlink import symlink import spack.compilers @@ -603,7 +605,24 @@ class ViewDescriptor(object): os.unlink(tmp_symlink_name) except (IOError, OSError): pass - raise e + + # Give an informative error message for the typical error case: two specs, same package + # project to same prefix. + if isinstance(e, ConflictingSpecsError): + spec_a = e.args[0].format(color=clr.get_color_when()) + spec_b = e.args[1].format(color=clr.get_color_when()) + raise SpackEnvironmentViewError( + f"The environment view in {self.root} could not be created, " + "because the following two specs project to the same prefix:\n" + f" {spec_a}, and\n" + f" {spec_b}.\n" + " To resolve this issue:\n" + " a. use `concretization:unify:true` to ensure there is only one " + "package per spec in the environment, or\n" + " b. disable views with `view:false`, or\n" + " c. create custom view projections." + ) from e + raise # Remove the old root when it's in the same folder as the new root. This guards # against removal of an arbitrary path when the original symlink in self.root diff --git a/lib/spack/spack/filesystem_view.py b/lib/spack/spack/filesystem_view.py index 058a952216..500adb4e43 100644 --- a/lib/spack/spack/filesystem_view.py +++ b/lib/spack/spack/filesystem_view.py @@ -20,6 +20,7 @@ from llnl.util.filesystem import ( ) from llnl.util.lang import index_by, match_predicate from llnl.util.link_tree import ( + ConflictingSpecsError, DestinationMergeVisitor, LinkTree, MergeConflictSummary, @@ -638,6 +639,22 @@ class SimpleFilesystemView(FilesystemView): def __init__(self, root, layout, **kwargs): super(SimpleFilesystemView, self).__init__(root, layout, **kwargs) + def _sanity_check_view_projection(self, specs): + """A very common issue is that we end up with two specs of the same + package, that project to the same prefix. We want to catch that as + early as possible and give a sensible error to the user. Here we use + the metadata dir (.spack) projection as a quick test to see whether + two specs in the view are going to clash. The metadata dir is used + because it's always added by Spack with identical files, so a + guaranteed clash that's easily verified.""" + seen = dict() + for current_spec in specs: + metadata_dir = self.relative_metadata_dir_for_spec(current_spec) + conflicting_spec = seen.get(metadata_dir) + if conflicting_spec: + raise ConflictingSpecsError(current_spec, conflicting_spec) + seen[metadata_dir] = current_spec + def add_specs(self, *specs, **kwargs): assert all((s.concrete for s in specs)) if len(specs) == 0: @@ -652,6 +669,8 @@ class SimpleFilesystemView(FilesystemView): if kwargs.get("exclude", None): specs = set(filter_exclude(specs, kwargs["exclude"])) + self._sanity_check_view_projection(specs) + # Ignore spack meta data folder. def skip_list(file): return os.path.basename(file) == spack.store.layout.metadata_dir @@ -714,16 +733,17 @@ class SimpleFilesystemView(FilesystemView): for src_root, group in per_source } + def relative_metadata_dir_for_spec(self, spec): + return os.path.join( + self.get_relative_projection_for_spec(spec), spack.store.layout.metadata_dir, spec.name + ) + def link_metadata(self, specs): metadata_visitor = SourceMergeVisitor() for spec in specs: src_prefix = os.path.join(spec.package.view_source(), spack.store.layout.metadata_dir) - proj = os.path.join( - self.get_relative_projection_for_spec(spec), - spack.store.layout.metadata_dir, - spec.name, - ) + proj = self.relative_metadata_dir_for_spec(spec) metadata_visitor.set_projection(proj) visit_directory_tree(src_prefix, metadata_visitor) diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index dc183a84f4..e757384b8f 100644 --- a/lib/spack/spack/test/cmd/env.py +++ b/lib/spack/spack/test/cmd/env.py @@ -1200,7 +1200,7 @@ def test_env_view_fails(tmpdir, mock_packages, mock_stage, mock_fetch, install_m add("libelf") add("libelf cflags=-g") with pytest.raises( - llnl.util.link_tree.MergeConflictSummary, match=spack.store.layout.metadata_dir + ev.SpackEnvironmentViewError, match="two specs project to the same prefix" ): install("--fake") @@ -2830,10 +2830,10 @@ def test_failed_view_cleanup(tmpdir, mock_stage, mock_fetch, install_mockery): all_views = os.path.dirname(resolved_view) views_before = os.listdir(all_views) - # Add a spec that results in MergeConflictError's when creating a view + # Add a spec that results in view clash when creating a view with ev.read("env"): add("libelf cflags=-O3") - with pytest.raises(llnl.util.link_tree.MergeConflictError): + with pytest.raises(ev.SpackEnvironmentViewError): install("--fake") # Make sure there is no broken view in the views directory, and the current |