-
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
strip -Werror: all specific or none #30284
Conversation
lib/spack/env/cc
Outdated
@@ -484,6 +485,25 @@ while [ $# -ne 0 ]; do | |||
if [ -z "$arg" ]; then shift; arg="$1"; fi | |||
append other_args_list "-l$arg" | |||
;; | |||
-Werror=*) |
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.
it'd be nice to generalize these with the DTAG stripping we already do to ensure RPATH
-- can the arguments to the wrapper not be a general set of args to strip?
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 have to think though how best to make that happen, maybe we can pass a list of patterns, and if any of them match the argument is removed? That should work as long as we don't need a conditional or other handling inside the wrapper.
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.
Having thought this through more, we can do this, in general, but -Werror
has a bit of a trick attached to it that makes it more complicated. If we want to be able to differentiate between -Werror=<something>
and -Werror
, the longer match has to be explicitly kept before matching the shorter match. I'm considering whether to build out a generic enough mechanism to allow us to pass this in, or handle -Werror=
specially and merge out everything else.
@tgamblin, this should be good for a look when you get a chance. It doesn't actually handle the dtags stuff with the new mechanism, because there are a lot of combinations of those where it's actually pairs of arguments, but it is a generic mechanism we can use to preserve or strip arbitrary flag patterns going forward. |
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.
This is a really clever way to do this within the confines of posix sh!
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.
Great!
Rebased and new tests added, hopefully better coverage of the added/changed lines. |
@trws: style is breaking |
Looks like all is passing again @tgamblin. |
This PR started breaking the leveldb package (see #30799). Is there a way to tell spack "don't touch any of the warning flags!" for a specific package? (could this be done in the package definition, maybe?) |
This reverts commit 330832c.
This reverts commit 330832c. `-Werror` chagnes were unfortunately causing the `rdma-core` build to fail. Reverting on `v0.18`; we can fix this in `develop`
Add a config option to strip `-Werror*` or `-Werror=*` from compile lines everywhere. ```yaml config: keep_werror: false ``` By default, we strip all `-Werror` arguments out of compile lines, to avoid unwanted failures when upgrading compilers. You can re-enable `-Werror` in your builds if you really want to, with either: ```yaml config: keep_werror: all ``` or to keep *just* specific `-Werror=XXX` args: ```yaml config: keep_werror: specific ``` This should make swapping in newer versions of compilers much smoother when maintainers have decided to enable `-Werror` by default.
This reverts commit 330832c.
Add a config option to strip `-Werror*` or `-Werror=*` from compile lines everywhere. ```yaml config: keep_werror: false ``` By default, we strip all `-Werror` arguments out of compile lines, to avoid unwanted failures when upgrading compilers. You can re-enable `-Werror` in your builds if you really want to, with either: ```yaml config: keep_werror: all ``` or to keep *just* specific `-Werror=XXX` args: ```yaml config: keep_werror: specific ``` This should make swapping in newer versions of compilers much smoother when maintainers have decided to enable `-Werror` by default.
This reverts commit 330832c.
Add a config option to strip
-Werror*
or-Werror=*
from compile lines everywhere.By default, we strip all
-Werror
arguments out of compile lines, to avoid unwanted failures when upgrading compilers. You can re-enable-Werror
in your builds if you really want to, with either:or to keep just specific
-Werror=XXX
args:This should make swapping in newer versions of compilers much smoother when maintainers have decided to enable
-Werror
by default.