Linux 5.15 Enabling "-Werror" By Default For All Kernel Builds

[arQon]

The reason he does not tolerate 'benign' warnings is that they then build up and it's hard to see a warning that merits attention.

That's the crucial piece. I've enforced -Werror for every project I've led for decades now, for exactly that reason: if there's ANY result from a build other than it being perfectly clean, whatever the error (nee warning) is it needs to be investigated by someone who understands WHY the compiler's complaining and what the best way of handling it is.

Once you get past the trivial stuff like unused vars etc, sometimes the "best way" is to simply pragma the warning off for the specific code block that's triggering it; and sometimes that's needed on a project-wide scale (e.g. VC being unable to compile its own STL without hundreds of warnings because of its debug info problems). But just ignoring them is the worst possible position to take, because when someone causes a NEW warning that indicates a genuine problem, if it's buried in the "usual" 400+ warnings that a build produces they don't even know that they DID introduce a new bug waiting to happen.

It's always painful to retrofit -Werror in a large codebase, and this is probably going to keep bouncing on and off for a couple of years - but this is the right thing to do, and a good goal to reach for (even if only slowly) over time.

[oiaohm]

Well that is true. If it's a config option that can be turned off it's fine. I just react allergically every time I see Werror because it's the no 1 cause of build failures I see on my system, only because I run a newer version of gcc than the devs did and when new gcc versions add new warnings, or improve detection of existing wanings, stuff breaks for no reason.

The problem here this is wrong because the warning are never for no reason.

I maintain that Werror for end-users makes absolutely no sense. New warnings in more recent versions of gcc don't suddenly make the code compile differently, or wrong. New optimizations might, but that's a different issue. During development using Werror is perfectly reasonable, even if warnings aren't always an indication that something is wrong. But for user's it's pointless, they're not going to go in there fix the code, they want something to run.

First problem here is end users the question is serous if they should have to build binaries? Majority of cases is no. So the fact end user is building source code its it own form of failure.

Also when the code in development was build always with -Werror and had no errors and now you are building as a user and you have errors from warnings is a sign of something wrong. The something wrong is building with a untested compiler.

Next you have a mistake here. "gcc don't suddenly make code compile differently" this is not 100 percent true. Some gcc warnings are about undefined behaviour these can in fact at time build differently between gcc or llvm runs let alone different compiler versions completely. So you really do need to check out what warning has in fact come up before turning off -Werror just to make sure its not something horrible fatal. Yes the fact you have had to turn -Werror off this also should mean you do need to build and run the full test suite or find the right compiler.

binarybanana a lot of people like you treat warnings from gcc and llvm way to lightly. New warnings that the upstream developers did not know about is bad news. Yes I personally thing -Werror should be on by default for end user builds with option to switch it off. Mostly so the end user gets a heads up that this code is now throwing warning the upstream developers did not know about. Yes this does mean the end user should be questioning if they need to open a bug upstream and they should be making sure they are going to use what ever this part test suite is to make sure there is no other surprises. Like it or not a -Werror failure is code equal to "canary coal mine" not always a problem but it time wake up you are in a possible dangerous area and take extra due care.

Yes there is a education problem here. People are failing to join up a -Werror build error that that requires them to turn off -Werror as a cause that they need to perform extra diligence they are going in a unexplored area.

This is part of why I say -Werror should be on by default with option to turn it off. Yes we need education that new warnings are not something to be ignored. Remember if you have been caught by -Werror you most likely did not have build and run test suite enabled either. Yes this is the serous catch when you are normally caught by -Werror the correct action is normally not just turn -Werror off but is turn -Werror off and turn test suite on and run the test suite to make sure everything is functional..

[JustRob]

Linux kernel doesn’t use standard IO functions provided by C stdio.h, so this option likely do nothing.

Here are three documented instances of its success during a Kernel build:

http://lkml.iu.edu/hypermail/linux/k...3.1/01949.html
https://patchwork.ozlabs.org/project...canonical.com/
https://oss.oracle.com/pipermail/el-...er/004556.html

Plus it enables -Wall .

[binarybanana]

This depend on you target case.

Remember with the Linux kernel you still have to build config before you can build the kernel. -Werror on by default is not a problem when you turn this off in the build configure you are using.

Stable releases having -Werror on by default for items like the linux kernel that have a lot configure options you should looked and configured at is not really problem.

I will give you if you need to patch the source to turn the -Werror off then the person doing it has made a mistake. -Werror on by default with a build option/configuration to turn it off is totally valid.

There are 4 possible ways here.
1) -Werror on by default with configuration to turn it off.
2) -Werror not on by default with a configuration to turn it on and developers having to remember to turn it on all the time the continuous integration testing. And at some point this gets goofed up resulting in continuous integration testing not doing its job.
3) Hard coded as -Werror always on
4) Hard coded as -Werror always off.

Yes options 3 and 4 are always wrong as they both require patching to change the -Werror mode. Option 4 is really bad because a lot of faults the compiler can find will end up being missed in the development cycles.

Next point Linux kernel stable release are not end of development. LTS branches of the kernel are still getting patched. So stable release of the Linux kernel are still technically a development build. Development builds -Werror on by default is recommend to keep flaws low. The problem here is how to correct balance the fact that the Linux kernel stable releases are development releases. Yes the fun reality that something can be stable and development release at exactly the same time. For the cases where something is a stable and development release at the same time option 1 I personally see as the best choice.

Option 1 is really the best option in most cases and if you cannot do option 1 then option 2 must be done this does have it downside. There are a lot of projects where the stable releases are not fixed in time like the Linux kernel so -Werror should be on by default with configuration to turn it off.

Well that is true. If it's a config option that can be turned off it's fine. I just react allergically every time I see Werror because it's the no 1 cause of build failures I see on my system, only because I run a newer version of gcc than the devs did and when new gcc versions add new warnings, or improve detection of existing wanings, stuff breaks for no reason. I maintain that Werror for end-users makes absolutely no sense. New warnings in more recent versions of gcc don't suddenly make the code compile differently, or wrong. New optimizations might, but that's a different issue. During development using Werror is perfectly reasonable, even if warnings aren't always an indication that something is wrong. But for user's it's pointless, they're not going to go in there fix the code, they want something to run.

[brad0]

Ok. I admit that I was overestimating the intolerance for warnings, even though Linus has never allowed them in his build, some of the other arches are turning up some warnings now...

Whether he allows said code in his build is irrelevant. It's looking at things completely backwards. It's the fact he has his time wasted by things that could be easily prevented from the very beginning. It saves his time and the developers time and other developers time.

[oiaohm]

Werror is OK for dev builds, but for stable releases it's totally nuts. Every time I update gcc stuff breaks because gcc adds new warnings and I have to patch out that shit.

This depend on you target case.

Remember with the Linux kernel you still have to build config before you can build the kernel. -Werror on by default is not a problem when you turn this off in the build configure you are using.

Stable releases having -Werror on by default for items like the linux kernel that have a lot configure options you should looked and configured at is not really problem.

I will give you if you need to patch the source to turn the -Werror off then the person doing it has made a mistake. -Werror on by default with a build option/configuration to turn it off is totally valid.

There are 4 possible ways here.
1) -Werror on by default with configuration to turn it off.
2) -Werror not on by default with a configuration to turn it on and developers having to remember to turn it on all the time the continuous integration testing. And at some point this gets goofed up resulting in continuous integration testing not doing its job.
3) Hard coded as -Werror always on
4) Hard coded as -Werror always off.

Yes options 3 and 4 are always wrong as they both require patching to change the -Werror mode. Option 4 is really bad because a lot of faults the compiler can find will end up being missed in the development cycles.

Next point Linux kernel stable release are not end of development. LTS branches of the kernel are still getting patched. So stable release of the Linux kernel are still technically a development build. Development builds -Werror on by default is recommend to keep flaws low. The problem here is how to correct balance the fact that the Linux kernel stable releases are development releases. Yes the fun reality that something can be stable and development release at exactly the same time. For the cases where something is a stable and development release at the same time option 1 I personally see as the best choice.

Option 1 is really the best option in most cases and if you cannot do option 1 then option 2 must be done this does have it downside. There are a lot of projects where the stable releases are not fixed in time like the Linux kernel so -Werror should be on by default with configuration to turn it off.

[binarybanana]

Werror is OK for dev builds, but for stable releases it's totally nuts. Every time I update gcc stuff breaks because gcc adds new warnings and I have to patch out that shit.

[oiaohm]

-Werror by default is generally considered bad practice on a library cause you never know what a user is going to compile with.

Except this is a part false statement. For security dependant libraries its in fact bad practice not to have -Werror features on by default and instead have option to disable it if required.

Newer compilers necessarily evolve the warnings they emit.

NSA and many other guides cover this. Why on by default if there is new issue you want to error out and not just build silently so that someone in fact has to look at the problem and change something.

Failure in build stage that the developer has to look at that may cause the developer to see the problem is better than having the software deployed on million+ systems with a bug.

at least there is an opt out in kconfig, but seems that will be the first thing disabled in developer builds which might defeat the purpose of the enabled by default setting

This also the problem. You missed that the reverse is the case as well were particular parties can get punished for lack of due care building a kernel without -Werror on and they have had to be making sure they have been turning it manually on.

To be regulation conforming in many countries -Werror features really do need to be on by default. Now if a developer decides to turn that off ouch.

which is to enable Werror in all CI build bots.

Now there is a problem here where people will setup CI at some point and forget to enable -Werror as well. Off switch with stuff like this is a lot safer than the on switch route. If you are stuborn or need to build with -Werror off you still can.

Think about it what is a worse error -Werror being off in CI builds because someone forgot to turn it on resulting in badly coded patches getting mainline or -Werror being on in a individual developer build and it failing because they forgot to turn it off.

The reality here -Werror off by default is a far higher risk of large stuff ups.

[Markopolo]

It seems like a good idea on the face of it but compilers flag up more and more things as warnings with every release, meaning that trying to build older kernels with newer compilers won't work. Good thing it's configurable.

I think this part is key. Newer compilers necessarily evolve the warnings they emit. Putting -Werror by default is generally considered bad practice on a library cause you never know what a user is going to compile with. I understand Linus’ motivation but this is a real downside that his comments don’t seem to acknowledge.

others here mention the alternative that many libraries use, which is to enable Werror in all CI build bots. This way the patches never make it to review but still allow local/user builds even with warnings. Maybe that approach doesn’t work for the kernel due to the uniqueness of its development approach.

at least there is an opt out in kconfig, but seems that will be the first thing disabled in developer builds which might defeat the purpose of the enabled by default setting

[MrCooper]

Now if only the Mesa developers were to do this instead of hiding warnings....

That reference is from 3 years ago.

Nowadays, all Mesa CI build jobs enable -Werror[0]. That means it's not possible to merge changes which cause compiler warnings in the most common build configurations.

This is the best we can do. Enabling -Werror for all builds would leave some users with different compiler versions or unusual build configurations unable to build Mesa.

[0] Some jobs still have to make exceptions for a small number of warnings. The goal is to keep shrinking and eventually eliminate those as the corresponding warnings are fixed.