Announcement

Collapse
No announcement yet.

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

Collapse
X
 
  • Filter
  • Time
  • Show
Clear All
new posts

  • #31
    Originally posted by set135
    This does not change the bar for code getting in. This is only for robots who otherwise might not flag warnings. Linus has never tolerated warnings and if you ask him to pull code that outputs warnings he will likely not accept it. He also hates useless warnings and patches that do nothing useful except shut them up. So, either the warning is helpful and the code is fixed, or it's useless and shut off.
    This is not quite right. Linus of the lInux kernel when a warning is in fact useless to disable it usage a bug report has to be submitted to the compiler developer and be confirmed that this is a bug even with this its not sure that the warning test will be turned off globally.



    There are few examples in the Linux kernel
    #pragma GCC diagnostic ignored "-Wsuggest-attribute=format"
    this has only been required 10 times across the complete kernel. This has a fairly good success rate of being 1000 to 1 yes over 10000 faults were found when that was first turned on only 10 were in fact errors.

    Best practise is really to leave as many compiler error detections on as you can and tag out the areas that those features don't work.

    Linus will accept patches that disable invalid warnings from being displayed he will be demanding you have confirmation from the compiler developer before submit the patch or he will have your head.

    Comment


    • #32
      Well considering most of Linus' legendary rants have been about code quality this is at the very least tonally consistent with the guy.

      I also wouldn't be the least bit surprised if this is primarily aimed at graphics driver devs as they are pretty infamous for their nonchalant attitude towards code quality and even serious warnings thrown up by compilers.

      Comment


      • #33
        It's surprising it hasn't been a case until now. OpenSSL has been doing this for ages.

        Comment


        • #34
          well... well.. linus is looking out for errors on his branch for a quite sometime and probably notoiced or better irked with simple or medium issues which can be fixed before sending patches.

          lets see how this will effect devs and the developnent process of kernel itself.

          Comment


          • #35
            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.

            Comment


            • #36
              Originally posted by Chewi View Post
              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

              Comment


              • #37
                Originally posted by Markopolo View Post
                -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.

                Originally posted by Markopolo View Post
                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.

                Originally posted by Markopolo View Post
                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.

                Originally posted by Markopolo View Post
                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.

                Comment


                • #38
                  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.

                  Comment


                  • #39
                    Originally posted by binarybanana View Post
                    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.

                    Comment


                    • #40
                      Originally posted by set135
                      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.

                      Comment

                      Working...
                      X