bugfix: spack shouldn't fail in an incomplete environment#16473
Merged
bugfix: spack shouldn't fail in an incomplete environment#16473
Conversation
Member
Author
becker33
approved these changes
May 5, 2020
Member
becker33
left a comment
There was a problem hiding this comment.
I probably would have just issued the warnings as they occur, but I don't have a problem with batching them in the return statement either.
Member
Author
|
The issue that is addressing is that both add and rm are called on deactivate, so you get the messages twice. Note that only add prints the warnings. |
39f3c5d to
1894ba3
Compare
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).
1894ba3 to
3446fe0
Compare
Member
|
Late confirmation: fixes the reported issue for me, thanks a lot! |
becker33
pushed a commit
that referenced
this pull request
Jul 11, 2020
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).
vjranagit
pushed a commit
to vjranagit/spack
that referenced
this pull request
Jan 18, 2026
Fixed spack#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 spack#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).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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).