-
Notifications
You must be signed in to change notification settings - Fork 203
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
take into account alternate sysroot for /bin/bash
used by run_cmd
#4646
take into account alternate sysroot for /bin/bash
used by run_cmd
#4646
Conversation
…se sysroot/bin/bash instead of /bin/bash if sysroot is set
…led. Any command run before that with run_cmd should use the regular /bin/bash
Ok, this issue actually does not exist for EB 5.0: the whole
at least for EESSI, the result is that the bash from the prefix is used. That means our issue no longer exists. I'm closing this PR, as it is not relevant. |
@casparvl As discussed, I think it's worth considering this to be merged into I'll take a thorough look into this right now... |
…cated test for run_cmd using with_sysroot=True
/bin/bash
used by run_cmd
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.
lgtm
EasyBuild's
run_cmd
runs commands in a subshell. The shell used is defined in theexec_cmd
. However, in context where the use of asysroot
is configured, one would want to use<sysroot>/bin/bash
instead. This PR implements that change. Note that sincerun_cmd
is also called a few times beforeset_up_configuration
is called, we need to be able to disable the check for thebuild_option('sysroot')
for those instances (similar to how this is already don forbuild_option('hooks')
).Solves this issue.
Edit: hmmm, I made this PR, and then realized, this is probably one of the components that is very different in EB 5.0. And it seems to be indeed...
Someone with more EB 5.0 knowledge needs to give me a pointer of how to fix this for EB 5.0 :)