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.
This commit is contained in:
Harmen Stoppels 2023-11-10 12:32:48 +01:00 committed by Todd Gamblin
parent fe57ec2ab7
commit c266e69cde
3 changed files with 9 additions and 5 deletions

View file

@ -1739,11 +1739,14 @@ def _env_modifications_for_view(
self, view: ViewDescriptor, reverse: bool = False self, view: ViewDescriptor, reverse: bool = False
) -> spack.util.environment.EnvironmentModifications: ) -> spack.util.environment.EnvironmentModifications:
try: 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: except Exception as e:
# Failing to setup spec-specific changes shouldn't be a hard error. # Failing to setup spec-specific changes shouldn't be a hard error.
tty.warn( 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 spack.util.environment.EnvironmentModifications()
return mods.reversed() if reverse else mods return mods.reversed() if reverse else mods

View file

@ -284,7 +284,7 @@ def setup_error(pkg, env):
_, err = capfd.readouterr() _, err = capfd.readouterr()
assert "cmake-client had issues!" in err 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): 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 # test that Spack detects the missing package and fails gracefully
with spack.repo.use_repositories(mock_custom_repository): with spack.repo.use_repositories(mock_custom_repository):
wrong_repo = env("activate", "--sh", "test") 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 assert "Unknown namespace: builtin.mock" in wrong_repo
# test replacing repo fixes it # test replacing repo fixes it
normal_repo = env("activate", "--sh", "test") 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 assert "Unknown namespace: builtin.mock" not in normal_repo

View file

@ -11,6 +11,7 @@
import spack.build_environment import spack.build_environment
import spack.config import spack.config
import spack.error
import spack.spec import spack.spec
import spack.util.environment as environment import spack.util.environment as environment
import spack.util.prefix as prefix import spack.util.prefix as prefix