-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve error message for requirements #33988
Improve error message for requirements #33988
Conversation
@adamjstewart Not sure you consider this sufficient, but it's a low hanging fruit to improve the error message. |
Much better than it was before! |
@adamjstewart Then if you approve I'll merge it as soon as tests pass. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like error messages created in the Spack core referring to spack commands, since spack commands are just one of the many ways this error can be triggered. This also causes weird error messages in buildcache commands for example, where some errors refer to spack buildcache
args when using spack install
or the other way around; they take different flags. So preferably this error message is created a few layers up. But at least something is better, so hitting approve.
Hmm, I don't get your comment. This is a concretization error, that could happen only because of something in |
@spackbot run pipeline |
I've started that pipeline for you! |
No, The point is that guessing the path that leads to an error where the error happens is always wrong, it should just bubble up as an exception until it reaches a layer where it's clear "you have this offending config file on the filesystem". |
@haampie I get where you're coming from. I just tried to install a package but had a typo in the name, and Spack told me I needed to run |
refers #33985