Skip to content
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

When is -Werror a usage requirement? #15

Open
alexreinking opened this issue Sep 27, 2021 · 3 comments
Open

When is -Werror a usage requirement? #15

alexreinking opened this issue Sep 27, 2021 · 3 comments

Comments

@alexreinking
Copy link

alexreinking commented Sep 27, 2021

I think the answer to this is "never", and I think it should be excluded from the package specification. I'm referring to this part of the spec.

https://github.com/mwoehlke/cps/blob/30140eb6a4a96db35b6da15f61a2f4761cf91a44/features.rst#featureoptnofeaturewarnerror

To be sure, abstracting these options across compilers is itself a noble goal, but it is unclear to me why this abstraction needs to take place in a package spec. Such a feature seems more at home in a build system.

Unlike the language standards and threads options, warnings have no impact whatsoever on whether or not a consuming build succeeds and produces usable binaries. Adding -Werror to the mix becomes toxic. If I use -Werror and manually disable certain warnings, another package should not be able to enable them for me (nor be incompatible for this reason alone). On the other hand, if I do not wish to enable -Werror when testing with a nightly build of clang, then a package should not be able to force -Werror on me.

There is no shortage of examples of havoc wreaked by -Werror. Getting it injected transitively is a special kind of hell for package maintainers and first-party developers alike. Please remove this (or at least require that implementations provide a hook for disabling it).

@bretbrownjr
Copy link
Collaborator

I also vote "never". Treating warnings as errors is actually a property of the workflow, not a property of the project itself. So I'm mostly in favor of removing that part of the spec.

That being said, it's true that people do like smuggling flags in places like this. It's possible that modeling this behavior is wise just so I have the "nevermind... warnings are actually warnings" flag I can use later. See CMake's --compile-no-warning-as-error option for an analogous example.

@dcbaker
Copy link
Collaborator

dcbaker commented Sep 23, 2023

I actually agree never is the right answer, but by specifying that it should be here it’s easier for a build system to ignore and/or override it. Meson has a similar option to turn werror on and off globally

@mwoehlke
Copy link
Member

I'm pretty sure I had a specific example in mind when I wrote that bit... but I strongly suspect it wasn't a -Werror=foo needed, but a -Wno-error=foo. (For symmetry, the spec supports both...) Similar for -Wno-foo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants