-
Notifications
You must be signed in to change notification settings - Fork 305
Be careful with PATH-like variables when using buildenv
#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
base: develop
Are you sure you want to change the base?
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) |
And also always use
pushenvfor everything else so users can recover environment variables they may set independently.Fixes #4004