Control Werror by converting to Wno-error by trws · Pull Request #30882 · spack/spack

trws · 2022-05-26T19:48:34Z

Using -Werror is good practice for development and testing, but causes us a great deal of heartburn supporting multiple compiler versions, especially as newer compiler versions add warnings for released packages. This PR adds support for suppressing -Werror through spack's compiler wrappers. There are currently three modes for the flags:keep_werror setting:

none: (default) cancel all -Werror, -Werror=* and -Werror-* flags by converting them to -Wno-error[=]* flags
specific: preserve explicitly selected warnings as errors, such as -Werror=format-truncation, but reverse the blanket -Werror
all: keeps all -Werror flags

These can be set globally in config.yaml, through the config command-line flags, or overridden by a particular package (some packages use Werror as a proxy for determining support for other compiler features). We chose to use this approach because:

removing -Werror flags entirely broke many build systems, especially autoconf based ones, because of things like checking -Werror=feature and making the assumption that if that did not error other flags related to that feature would also work
attempting to preserve -Werror in some phases but not others caused similar issues
The per-package setting came about because some packages, even with all these protections, still use -Werror unsafely. Currently there are roughly 3 such packages known.

tgamblin · 2022-07-10T18:28:40Z

@trws we should add a package property that overrides this behavior, so that if this does break certain packages we can just add:

class PackageWithInsaneBuildSystem(Package):
    keep_werror = "all"

That would override the config option, and it would allow us to patch troublesome packages on our own time while still handling the vast majority of sane packages. I think this might've actually been the right thing to do with #30284 instead of reverting it.

If this is easier than fixing rdma-core here, I think it's fine to just add this to rdma-core here and to fix rdma-core later.

trws · 2022-11-08T07:39:58Z

@spackbot help

spackbot-app · 2022-11-08T07:39:59Z

You can interact with me in many ways!

@spackbot hello: say hello and get a friendly response back!
@spackbot help or @spackbot commands: see this message
@spackbot run pipeline or @spackbot re-run pipeline: to request a new run of the GitLab CI pipeline
@spackbot rebuild everything: to run a pipeline rebuilding all specs from source.
@spackbot fix style if you have write and would like me to run spack style --fix for you.
@spackbot maintainers or @spackbot request review: to look for and assign reviewers for the pull request.

I'll also help to label your pull request and assign reviewers!
If you need help or see there might be an issue with me, open an issue here

trws · 2022-11-08T07:40:19Z

@spackbot rebuild everything

spackbot-app · 2022-11-08T07:40:22Z

I had a problem triggering the pipeline.

tgamblin · 2022-11-08T07:42:00Z

@spackbot rebuild everything

spackbot-app · 2022-11-08T07:42:03Z

I had a problem triggering the pipeline.

trws · 2022-11-08T07:42:28Z

Maybe the prechecks have to be done first?

@spackbot rebuild everything

spackbot-app · 2022-11-08T07:42:32Z

I've started that pipeline for you!

tgamblin · 2022-11-08T07:43:48Z

I'm wondering if this is based on a develop that is too recent for spackbot to rebuild. I don't think that is a thing yet but not entirely sure as we've been talking about more judicious commit choices. More likely may be that the initial gitlab pipeline hasn't run...

trws · 2022-11-08T07:45:40Z

Could be that, I'm guessing it was that I tried to start a pipeline when the pre-conditions for pipelines hadn't been fulfilled yet. Either way, running now.

tgamblin · 2022-11-08T07:48:10Z

I think your hypothesis is correct

tgamblin · 2022-11-08T17:52:12Z

moving this to 0.20 sadly -- but I think it'll actually be good to have it in develop for a while before we put it in a release.

trws · 2022-11-08T17:55:24Z

Yeah, letting it simmer so we can hopefully catch packages not covered in stacks certainly wont hurt. I'll try to get this updated this morning so we can move it forward.

trws · 2022-11-08T20:30:38Z

@spackbot rebuild everything

trws · 2022-11-22T17:40:22Z

@spackbot rebuild everything

spackbot-app · 2022-11-22T17:40:31Z

I've started that pipeline for you!

Using `-Werror` is good practice for development and testing, but causes us a great deal of heartburn supporting multiple compiler versions, especially as newer compiler versions add warnings for released packages. This PR adds support for suppressing `-Werror` through spack's compiler wrappers. There are currently three modes for the `flags:keep_werror` setting: * `none`: (default) cancel all `-Werror`, `-Werror=*` and `-Werror-*` flags by converting them to `-Wno-error[=]*` flags * `specific`: preserve explicitly selected warnings as errors, such as `-Werror=format-truncation`, but reverse the blanket `-Werror` * `all`: keeps all `-Werror` flags These can be set globally in config.yaml, through the config command-line flags, or overridden by a particular package (some packages use Werror as a proxy for determining support for other compiler features). We chose to use this approach because: 1. removing `-Werror` flags entirely broke *many* build systems, especially autoconf based ones, because of things like checking `-Werror=feature` and making the assumption that if that did not error other flags related to that feature would also work 2. Attempting to preserve `-Werror` in some phases but not others caused similar issues 3. The per-package setting came about because some packages, even with all these protections, still use `-Werror` unsafely. Currently there are roughly 3 such packages known.

In #30882, we made Spack ignore `-Werror` calls so that it could more easily build projects that inject `-Werror` into their builds. We did this by translating them to `-Wno-error` in the compiler wrapper. However, some compilers (like `nvhpc`) do not support `-Wno-error`. We need to exclude them from this feature until they do. - [x] make a property on `PackageBase` for `keep_werror` that knows not to use it for `nvhpc`. Co-authored-by: William Mou <[email protected]>

In #30882, we made Spack ignore `-Werror` calls so that it could more easily build projects that inject `-Werror` into their builds. We did this by translating them to `-Wno-error` in the compiler wrapper. However, some compilers (like `nvhpc`) do not support `-Wno-error`. We need to exclude them from this feature until they do. - [x] make a property on `PackageBase` for `keep_werror` that knows not to use it for `nvhpc`. - [x] update property so that it keeps only the specific `-Werror=...` args for newer nvhpc's, which support `-Wno-error` but not `-Wno-error=...` Co-authored-by: William Mou <[email protected]>

In #30882, we made Spack ignore `-Werror` calls so that it could more easily build projects that inject `-Werror` into their builds. We did this by translating them to `-Wno-error` in the compiler wrapper. However, some compilers (like `nvhpc`) do not support `-Wno-error`. We need to exclude them from this feature until they do. - [x] make a property on `PackageBase` for `keep_werror` that knows not to use it for `nvhpc`. - [x] update property so that it keeps only the specific `-Werror=...` args for newer nvhpc's, which support `-Wno-error` but not `-Wno-error=...` --------- Co-authored-by: William Mou <[email protected]> Co-authored-by: Tom Scogland <[email protected]> Signed-off-by: Todd Gamblin <[email protected]>

In spack#30882, we made Spack ignore `-Werror` calls so that it could more easily build projects that inject `-Werror` into their builds. We did this by translating them to `-Wno-error` in the compiler wrapper. However, some compilers (like `nvhpc`) do not support `-Wno-error`. We need to exclude them from this feature until they do. - [x] make a property on `PackageBase` for `keep_werror` that knows not to use it for `nvhpc`. - [x] update property so that it keeps only the specific `-Werror=...` args for newer nvhpc's, which support `-Wno-error` but not `-Wno-error=...` --------- Co-authored-by: William Mou <[email protected]> Co-authored-by: Tom Scogland <[email protected]> Signed-off-by: Todd Gamblin <[email protected]>

spackbot-app bot added the build-environment label May 26, 2022

trws requested review from becker33 and tgamblin May 26, 2022 19:50

trws mentioned this pull request May 27, 2022

Installation issue: gdb #30836

Closed

4 tasks

joequant mentioned this pull request May 30, 2022

cc: fail wrapper when arguments will always fail #30881

Closed

trws mentioned this pull request Jun 7, 2022

DNM: Test werror conversion #31037

Closed

trws force-pushed the werror-conversion branch from cfeb9cc to 6ee68a0 Compare June 15, 2022 22:02

spackbot-app bot added patch update-package labels Jun 15, 2022

tgamblin mentioned this pull request Jul 10, 2022

openssl: New version 1.1.1q #31487

Merged

sethrj mentioned this pull request Aug 11, 2022

superlu-dist %oneapi: add -Wno-error=implicit-function-declaration #32081

Merged

spackbot-app bot added core PR affects Spack core functionality defaults tests General test capability(ies) labels Nov 8, 2022

tgamblin added this to the v0.20.0 milestone Nov 8, 2022

trws added 8 commits November 21, 2022 23:22

make werror behavior overridable per package

838df24

fix tests

92b1626

avoid unnecessary concretization

61d7229

add support for -Werror-implicit-function-declaration

2df7854

keep werror for gcc, due to autoconf strangeness

7e3216a

expand test coverage, make replacements ordered

11a1d6b

fix quoting corner case impacting gcc

fc159d7

more defensive eval quoting, add GCC-like arg with embedded perens

d259648

trws force-pushed the werror-conversion branch from 75eeb31 to d259648 Compare November 21, 2022 23:22

trws changed the title ~~cc: handle Werror by converting to Wno-error~~ Control Werror by converting to Wno-error Nov 23, 2022

tgamblin approved these changes Nov 23, 2022

View reviewed changes

tgamblin merged commit 0182603 into spack:develop Nov 23, 2022

alalazo mentioned this pull request Nov 30, 2022

Bootstrap most of Spack dependencies using environments #34029

Merged

5 tasks

greenc-FNAL mentioned this pull request Dec 1, 2022

Sync with upstream FNALssi/spack#72

Closed

msimberg mentioned this pull request Jan 30, 2023

-Werror to -Wno-error translation does not work with nvc++ because it doesn't support -Wno-error #35235

Closed

3 tasks

msimberg mentioned this pull request Apr 14, 2023

Keep -Werror flags in CI eth-cscs/DLA-Future#834

Merged

William-Mou mentioned this pull request Apr 17, 2023

Keep -Werror to avoid issue #35235 #36961

Closed

msimberg mentioned this pull request Jul 5, 2023

config: use more specific -Werror=type-safety pmodels/mpich#6581

Merged

4 tasks

tgamblin mentioned this pull request May 12, 2024

nvhpc: Do not use -Wno-error with nvhpc #44142

Merged

1 task

tgamblin mentioned this pull request Sep 18, 2024

py-h5py %[email protected]: let warnings be warnings #34984

Merged

Conversation

trws commented May 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tgamblin commented Jul 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

trws commented Nov 8, 2022

Uh oh!

spackbot-app bot commented Nov 8, 2022

Uh oh!

trws commented Nov 8, 2022

Uh oh!

spackbot-app bot commented Nov 8, 2022

Uh oh!

tgamblin commented Nov 8, 2022

Uh oh!

spackbot-app bot commented Nov 8, 2022

Uh oh!

trws commented Nov 8, 2022

Uh oh!

spackbot-app bot commented Nov 8, 2022

Uh oh!

tgamblin commented Nov 8, 2022

Uh oh!

trws commented Nov 8, 2022

Uh oh!

tgamblin commented Nov 8, 2022

Uh oh!

tgamblin commented Nov 8, 2022

Uh oh!

trws commented Nov 8, 2022

Uh oh!

trws commented Nov 8, 2022

Uh oh!

trws commented Nov 22, 2022

Uh oh!

spackbot-app bot commented Nov 22, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

trws commented May 26, 2022 •

edited

Loading

tgamblin commented Jul 10, 2022 •

edited

Loading