From b196e4396a8d2bb21cc50d26114d34cdacb3b727 Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Thu, 7 May 2020 02:30:09 -0700 Subject: bugfix: spack shouldn't fail in an incomplete environment (#16473) Fixed #15884. Spack asks every package linked into an environment to tell us how environment variables should be modified when a spack environment is activated. As part of this, specs in an environment are symlinked into the environment's view (see #13249), and the package calculates environment modifications with *the default view as the prefix*. All of this works nicely for pointing the user's environment at the view *if* every package is successfully linked. Unfortunately, right now we only track what specs "should" be in a view, not which specs actually are. So we end up calculating environment modifications on things that aren't linked into thee view, and the exception isn't caught, so lots of spack commands end up failing. This fixes the issue by ignoring and warning about specs where calculating environment modifications fails. So we can still keep using Spack even if the current environment is incomplete. We should probably also just avoid computing env modifications *entirely* for unlinked packages, but right now that is a slow operation (requires a lot of YAML parsing). We should revisit that when we have some better state management for views, but the fix adopted here will still be necessary, as we want spack commands to be resilient to other types of bugs in `setup_run_environment()` and friends. That code is in packages and we have to assume it could be buggy when we call it outside of builds (as it might fail more than just the build). --- lib/spack/spack/environment.py | 36 +++++++++++++++++++++++++++--------- lib/spack/spack/test/cmd/env.py | 22 ++++++++++++++++++++++ 2 files changed, 49 insertions(+), 9 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py index fff1485e3c..43e91073e3 100644 --- a/lib/spack/spack/environment.py +++ b/lib/spack/spack/environment.py @@ -1091,6 +1091,25 @@ class Environment(object): for view in self.views.values(): view.regenerate(specs, self.roots()) + def _env_modifications_for_default_view(self, reverse=False): + all_mods = spack.util.environment.EnvironmentModifications() + + errors = [] + for _, spec in self.concretized_specs(): + if spec in self.default_view and spec.package.installed: + try: + mods = uenv.environment_modifications_for_spec( + spec, self.default_view) + except Exception as e: + msg = ("couldn't get environment settings for %s" + % spec.format("{name}@{version} /{hash:7}")) + errors.append((msg, str(e))) + continue + + all_mods.extend(mods.reversed() if reverse else mods) + + return all_mods, errors + def add_default_view_to_shell(self, shell): env_mod = spack.util.environment.EnvironmentModifications() @@ -1101,10 +1120,11 @@ class Environment(object): env_mod.extend(uenv.unconditional_environment_modifications( self.default_view)) - for _, spec in self.concretized_specs(): - if spec in self.default_view and spec.package.installed: - env_mod.extend(uenv.environment_modifications_for_spec( - spec, self.default_view)) + mods, errors = self._env_modifications_for_default_view() + env_mod.extend(mods) + if errors: + for err in errors: + tty.warn(*err) # deduplicate paths from specs mapped to the same location for env_var in env_mod.group_by_name(): @@ -1122,11 +1142,9 @@ class Environment(object): env_mod.extend(uenv.unconditional_environment_modifications( self.default_view).reversed()) - for _, spec in self.concretized_specs(): - if spec in self.default_view and spec.package.installed: - env_mod.extend( - uenv.environment_modifications_for_spec( - spec, self.default_view).reversed()) + mods, _ = self._env_modifications_for_default_view(reverse=True) + env_mod.extend(mods) + return env_mod.shell_modifications(shell) def _add_concrete_spec(self, spec, concrete, new=True): diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index 671d9869ed..f8ed37e17c 100644 --- a/lib/spack/spack/test/cmd/env.py +++ b/lib/spack/spack/test/cmd/env.py @@ -163,6 +163,28 @@ def test_env_install_single_spec(install_mockery, mock_fetch): assert e.specs_by_hash[e.concretized_order[0]].name == 'cmake-client' +def test_env_modifications_error_on_activate( + install_mockery, mock_fetch, monkeypatch, capfd): + env('create', 'test') + install = SpackCommand('install') + + e = ev.read('test') + with e: + install('cmake-client') + + def setup_error(pkg, env): + raise RuntimeError("cmake-client had issues!") + + pkg = spack.repo.path.get_pkg_class("cmake-client") + monkeypatch.setattr(pkg, "setup_run_environment", setup_error) + with e: + pass + + _, err = capfd.readouterr() + assert "cmake-client had issues!" in err + assert "Warning: couldn't get environment settings" in err + + def test_env_install_same_spec_twice(install_mockery, mock_fetch, capfd): env('create', 'test') -- cgit v1.2.3-60-g2f50