-
Notifications
You must be signed in to change notification settings - Fork 305
Be more careful with PATH-like environment variables when using buildenv + use pushenv for other environment variables
#4007
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
Conversation
|
@boegelbot please test @ jsc-zen3 |
|
@ocaisa: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de PR test command '
Test results coming soon (I hope)... - notification for comment with ID 3611079310 processed Message to humans: this is just bookkeeping information for me, |
|
Test report by @boegelbot Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (total: 18 secs) (1 easyconfigs in total) |
| path_like_vars = {key for key, _ in COMPILER_MAP_CLASS[SearchPaths]} | ||
| path_like_vars.add('LIBRARY_PATH') | ||
| path_like_vars.add('LD_LIBRARY_PATH') | ||
| for key, val in sorted(self.toolchain.vars.items()): |
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.
For the easybuild configuration of EESSI/2025.06 I noted that header libraries are always set in these items (C_INCLUDE_PATH etc.). Since the easyblock used setenv(), the existing paths were being wiped away. This fixes that specific problem in a general way.
In EESSI/2023.06 this does not seem to be a problem as CPATH was not being set. Regardless, all angles are covered now and it is also more flexible by the use of pushenv() in general.
|
|
@boegelbot please test @ jsc-zen3 |
|
@ocaisa: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de PR test command '
Test results coming soon (I hope)... - notification for comment with ID 3611371502 processed Message to humans: this is just bookkeeping information for me, |
|
Test report by @boegelbot Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (total: 15 secs) (1 easyconfigs in total) |
…ame 'build_envvars' to 'build_env_vars'
|
@boegelbot please test @ jsc-zen3 |
PATH-like variables when using buildenvPATH-like environment variables when using buildenv + use pushenv for other environment variables
|
@boegel: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de PR test command '
Test results coming soon (I hope)... - notification for comment with ID 3630933025 processed Message to humans: this is just bookkeeping information for me, |
|
Test report by @boegelbot Overview of tested easyconfigs (in order)
Build succeeded for 2 out of 2 (total: 51 secs) (2 easyconfigs in total) |
boegel
left a comment
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
And also always use
pushenvfor everything else so users can recover environment variables they may set independently.Fixes #4004