Chris's Wiki :: blog/programming/ReleaseBuildsNoAbortOnWarnings

Official source release builds should not abort on (compilation) warnings

July 20, 2016

I will put my conclusion up front:

Official source releases should not build software with -Werror or the equivalent.

(By official source releases I mean things like 'we are releasing version 1.x.y of our package today'.)

Perhaps you disagree. Then the rest of this entry is for you.

As I write this, the current release version of Rust is 1.10.0. This version (and probably all previous ones) won't build on the recently released Fedora 24 because of a C compiler issue. This isn't because of bug in the Fedora 24 version of gcc, and it's not due to the Fedora 24 gcc uncovering a previously unrecognized bug in Rust. Instead it's because of, well, this:

src/rt/miniz.c: In function ‘tinfl_decompress’:
src/rt/miniz.c:578:9: error: this ‘for’ clause does not guard... [-Werror=misleading-indentation]
         for ( i = 0; i <= 143; ++i) *p++ = 8; for ( ; i <= 255; ++i) *p++ = 9; for ( ; i <= 279; ++i) *p++ = 7; for ( ; i <= 287; ++i) *p++ = 8;
         ^~~
src/rt/miniz.c:578:47: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the ‘for’
         for ( i = 0; i <= 143; ++i) *p++ = 8; for ( ; i <= 255; ++i) *p++ = 9; for ( ; i <= 279; ++i) *p++ = 7; for ( ; i <= 287; ++i) *p++ = 8;
                                               ^~~
[...]
cc1: all warnings being treated as errors

Rust has opted to compile much or all of the C in its source tree with the gcc options -Wall -Werror, which mean 'emit more or less all warnings that you can, and if you emit any warnings consider this an error and stop'. Fedora 24 is one of the first Linux distributions to ship with gcc 6, and gcc 6 has added some more warnings, which now pick up more nits in the C code, and now Rust doesn't build.

It would be one thing if this was pointing out a previously undiscovered error. But it's not. The code in the Rust 1.10.0 release is merely not ideal, and this example neatly illustrates the problem with making 'not ideal' a fatal error in official source releases. Put simply, the compiler's definition of 'not ideal' changes over time.

When you set your official releases to build with -Wall -Werror or the equivalent, you're putting yourself on quicksand. It's basically guaranteed that future compiler versions will have different opinions on what to warn about, which means that official releases of your software are going to stop building at some point in the future for no good reason. Having your software stop building for no good reason is not helping people.

(I say 'for no good reason' because if it actually built, the release would be no more broken than it was before the new nits were reported. The nits were always there, after all, even on all the systems that didn't report them.)

I think it's fine to use -Werror in development if you want to. But the moment you make a release, my very strong sysadmin opinion is that the job of that release is to build correctly in as many environments as possible, including future environments. An attempt to build a release should fail on a system only if the end result would be incorrect in a new way.

(If a release is incorrect on both system X and system Y but this is only uncovered on system Y, that should not be a fatal error. It's sufficient for release builds to be bug for bug equivalent to each other. This is probably too tricky to do in most cases, although maybe you should provide a 'I don't care, just force it to build' configuration option that suppresses as many compiler errors as possible.)

Another way to put this is that -Wall -Werror is a service for the developers; it surfaces nits and forces developers to fix them. However, releases are not made for developers, they're made for outside people. Forcing developer issues on outside people is both futile (since outside people are not developers) and annoying. Thus actual releases should have things like compiler options reoriented to serve the needs of outside people instead of developers.

Comments on this page:

By Alex Xu (Hello71) at 2016-07-20 08:37:18:

It has been Gentoo policy for many years to always remove -Werror in ebuilds: https://devmanual.gentoo.org/ebuild-writing/common-mistakes/#-werror-compiler-flag-not-removed.

By Anon at 2016-07-20 13:41:34:

I'd argue this is a bug in GCC6 (which itself is still unreleased and experimental) has a bug - it's flagging code that doesn't actually have misleading indentation - rather the author felt like creating a one liner.

It's a shame that Wmisleading-indentation falls under Wall than Wextra (as shown by https://github.com/Barro/compiler-warnings/blob/master/gcc/warnings-gcc-6.txt ). However I think Werror can be used if you explicitly mark which warnings you want to be errors rather than letting the compiler make all warnings errors (but perhaps that makes your compile non-portable)...

By Anon at 2016-07-20 13:55:04:

One more thought - if the default standard the compiler uses changes and you don't specify which standard you're trying to adhere to then you CAN get the case where your code is "more broken" than it was before but a warning might have saved you...

From 31.17.248.110 at 2016-07-20 15:47:48:

You're missing the obvious.

The Rust developers have decided that -Wall -Werror is a part of their official release cycle. So obviously you should immediately file a a release-critical (type blocker) bug against the software.

By cks at 2016-07-20 16:16:07:

The official gcc site says that gcc 6.1 is an official release (and the latest version); gcc 7.0 is what's in development. I'd also argue that this code does have misleading indentation as written. The common idiom for an inline for loop in C is:
for (...) thing;
for (...) { thing; more-thing; }
When you write:
for (...) thing; more-thing;
This certainly can be said to look like 'more-thing;' is intended to be part of the for loop based on indentation, but it actually isn't because the for loop body is only a single statement. This is the same sort of issue as:
if (...) thing; more-thing;
Again, this sort of looks like more-thing; is part of the if body, but it isn't. I think both are worth complaining about under 'misleading indentation'.

As far as filing a release critical blocker bug, the problem is that 1.10.0 has already been released. Rust can release 1.10.1 if they want to, but they absolutely shouldn't go back and change the 1.10.0 release so that it can compile with gcc 6. This especially becomes a problem if you need to build older releases of software for some reason; these older releases are permanently unbuildable and are very unlikely to get new little point releases to fix their build issue.

(This particular problem has already been fixed in the Rust mainline, of course. I don't know if the Rust people are considering a 1.10.1 release or if they ever do such minor releases except for critical issues.)

By John Marshall at 2016-07-21 05:55:03:

Anon's complaints that -Wmisleading-indentation shouldn't be in -Wall are nothing new to the GCC developers. Some reasoning why it is included in -Wall is in https://gcc.gnu.org/ml/gcc/2016-05/msg00046.html and the surrounding thread.

People who like to write one-liners can suppress the warning with
    if (...) { thing; } more-thing;
to emphasise exactly what's controlled by the if, or in other ways. Personally I find this no more objectionable than the if ((a = b)) excess parentheses used to suppress did you mean == warnings that we're all used to.

By Anon at 2016-07-21 16:12:23:

cks, John: I stand corrected. I was wrong to say gcc 6 hadn't been officially released and it does look like it's justifiably hard to get new warnings into -Wall...

By Greg A. Woods at 2016-08-10 20:57:43:

I think in the old days when there were a lot more compilers and system types in use, and a lot more commonly used targets for popular compilers like GCC, that use of -Werror was potentially beneficial in avoiding real problems that might affect an program at run-time if a problem only showed up for some particular compiler or target system. Or at least that's the way I used to worry about it.

These days with fewer and fewer compilers, and an incredibly reduced range of target systems (at least for generally portable code to run on), I would agree this situation is now the other way around.

I do still run into problems with compiler and/or system- or architecture-specific warnings from older toolchains and less common target systems though. Most of the time I'm able to catch all of these myself, but sometimes I rely on strangers to find and report them. In these latter cases I do still waffle on whether -Werror would be the best way to get such reports or not.

By cks at 2016-08-10 21:38:53:

I think that it's often a good idea to use -Werror in the development version, in beta versions, and arguably even in release candidates, because in each case it may smoke out problems and you have the chance to fix any issues that come up before you freeze things. But an actual release is frozen, at which point -Werror is a real problem.

Written on 20 July 2016.

«	How not to set up your DNS (part 23)

My current set of essential extensions for Firefox profiles