From c266e69cde1d330737afc8b77e53933bc26f3c62 Mon Sep 17 00:00:00 2001 From: Harmen Stoppels Date: Fri, 10 Nov 2023 12:32:48 +0100 Subject: [PATCH] env: compute env mods only for installed roots (#40997) And improve the error message (load vs unload). Of course you could have some uninstalled dependency too, but as long as it doesn't implement `setup_run_environment` etc, I don't think it hurts to attempt to load the root anyways, given that failure to do so is a warning, not a fatal error. --- lib/spack/spack/environment/environment.py | 7 +++++-- lib/spack/spack/test/cmd/env.py | 6 +++--- lib/spack/spack/user_environment.py | 1 + 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index 8ddd7f8d3b..cf6dffcb0d 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -1739,11 +1739,14 @@ def _env_modifications_for_view( self, view: ViewDescriptor, reverse: bool = False ) -> spack.util.environment.EnvironmentModifications: try: - mods = uenv.environment_modifications_for_specs(*self.concrete_roots(), view=view) + with spack.store.STORE.db.read_transaction(): + installed_roots = [s for s in self.concrete_roots() if s.installed] + mods = uenv.environment_modifications_for_specs(*installed_roots, view=view) except Exception as e: # Failing to setup spec-specific changes shouldn't be a hard error. tty.warn( - "couldn't load runtime environment due to {}: {}".format(e.__class__.__name__, e) + f"could not {'unload' if reverse else 'load'} runtime environment due " + f"to {e.__class__.__name__}: {e}" ) return spack.util.environment.EnvironmentModifications() return mods.reversed() if reverse else mods diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index 983a778e96..c3a7551e94 100644 --- a/lib/spack/spack/test/cmd/env.py +++ b/lib/spack/spack/test/cmd/env.py @@ -284,7 +284,7 @@ def setup_error(pkg, env): _, err = capfd.readouterr() assert "cmake-client had issues!" in err - assert "Warning: couldn't load runtime environment" in err + assert "Warning: could not load runtime environment" in err def test_activate_adds_transitive_run_deps_to_path(install_mockery, mock_fetch, monkeypatch): @@ -502,12 +502,12 @@ def test_env_activate_broken_view( # test that Spack detects the missing package and fails gracefully with spack.repo.use_repositories(mock_custom_repository): wrong_repo = env("activate", "--sh", "test") - assert "Warning: couldn't load runtime environment" in wrong_repo + assert "Warning: could not load runtime environment" in wrong_repo assert "Unknown namespace: builtin.mock" in wrong_repo # test replacing repo fixes it normal_repo = env("activate", "--sh", "test") - assert "Warning: couldn't load runtime environment" not in normal_repo + assert "Warning: could not load runtime environment" not in normal_repo assert "Unknown namespace: builtin.mock" not in normal_repo diff --git a/lib/spack/spack/user_environment.py b/lib/spack/spack/user_environment.py index 5d1561a8ea..6e1c798e51 100644 --- a/lib/spack/spack/user_environment.py +++ b/lib/spack/spack/user_environment.py @@ -11,6 +11,7 @@ import spack.build_environment import spack.config +import spack.error import spack.spec import spack.util.environment as environment import spack.util.prefix as prefix