#2055: replace --enable-new-dtags with --disable-new-dtags instead of remove… #2131
Conversation
… from command-line
easybuild/scripts/rpath_args.py
Outdated
| @@ -93,6 +93,8 @@ | |||
|
|
|||
| # filter out --enable-new-dtags if it's used; | |||
| # this would result in copying rpath to runpath, meaning that $LD_LIBRARY_PATH is taken into account again | |||
| elif arg == '--enable-new-dtags': | |||
| cmd_args.append('--disable-new-dtags'); | |||
| elif arg != '--enable-new-dtags': | |||
There was a problem hiding this comment.
with introducing the arg == '--enable-new-dtags' above, we can just make this else: rather than elif ...:?
easybuild/scripts/rpath_args.py
Outdated
| @@ -93,6 +93,8 @@ | |||
|
|
|||
| # filter out --enable-new-dtags if it's used; | |||
There was a problem hiding this comment.
@jdonners please also fix this comment, to something like # replace --enable-new-dtags with --disable-new-dtags if it's used
the last bit makes me wonder: should we just always inject --disable-new-dtags (maybe combined with filtering out --enable-new-dtags, to be sure)?
There was a problem hiding this comment.
actually, we already inject --disable-new-dtags, see line 114... so how can this patch fix anything?
There was a problem hiding this comment.
it fixes the cases where the command-line consists of ... -Wl, --enable-new-dtags -fomit-frame-pointer. In the current version, rpath_args.py would modify this into -Wl, -fomit-frame-pointer and the option -fomit-frame-pointer is passed to the linker, which fails because it is an unknown option. The fix just passes the same option --disable-new-dtags multiple times to the linker, which shouldn't be a problem.
There was a problem hiding this comment.
Ah, ok, good point...
Maybe clarify that with a comment here, since I suspect otherwise people (e.g. me) will go wonder again why we don't just drop --enable-new-dtags or why we possible add --disable-new-dtags multiple times... :)
|
@jdonners Are you up for making the requested changes so we can merge this in? See http://easybuild.readthedocs.io/en/latest/Contributing.html#updating-existing-pull-requests |
|
The checks will fail, since it expects only once --disable-new-dtags, while it now includes the same flag twice. |
|
@jdonners Can you also change the test accordingly? see |
|
Thanks for the fix @jdonners! |
… from command-line