search

cps-org / cps

Common Package Specification — A cross-tool mechanism for locating software dependencies
https://cps-org.github.io/cps/
Other
92 stars 8 forks source link

When is -Werror a usage requirement? #15

Open alexreinking opened 2 years ago

alexreinking commented 2 years ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 6 months ago

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.