With regard to ALLOW_COMPILER_WARNINGS, I'm strongly of the opinion that we should be providing an option to hide all warnings from modules marked as ALLOW_COMPILER_WARNINGS now, as they are only in "external" libraries which most of us are not working on, and they really clog up my compiler output and make me have to scroll up many many pages in order to see the build errors which are actually my fault. (see this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1301688). Unfortunately, I don't know enough about the build system to write a patch to do this.
On Fri, May 19, 2017 at 2:59 PM, David Major <dma...@mozilla.com> wrote: > I'm not sure if this is exactly the same as the ALLOW_COMPILER_WARNINGS > that we use for warnings-on-errors, but FWIW as of a couple of months > ago I cleaned out the last warning-allowance in our "own" code. > ALLOW_COMPILER_WARNINGS usage is now only in external libraries (I'm > counting NSS and NSPR as "external" for this purpose since I can't land > code to m-c directly). > > On Fri, May 19, 2017, at 02:55 PM, Kris Maglione wrote: > > I've actually been meaning to post about this, with some questions. > > > > I definitely like that we now print warnings at the end of builds, since > > it > > makes them harder to ignore. But the current amount of warnings spew at > > the > > end of every build is problematic, especially when a build fails and I > > need > > to scroll up several pages to find out why. > > > > Can we make some effort to get clean warnings output at the end of > > standard > > builds? A huge chunk of the warnings come from NSS and NSPR, and should > > be > > easily fixable. Most of the rest seem to come from vendored libraries, > > and > > should probably just be whitelisted if we can't get fixes upstreamed. > > > > On Fri, May 19, 2017 at 11:44:08AM -0700, Gregory Szorc wrote: > > >`mach build` attempts to parse compiler warnings to a persisted > "database." > > >You can view a list of compiler warnings post build by running `mach > > >warnings-list`. The intent behind this feature was to make it easier to > > >find and fix compiler warnings. After all, something out of sight is > out of > > >mind. > > > > > >There have been a few recent changes to increase the visibility of > compiler > > >warnings with the expectation being that raising visibility will > increase > > >the chances of someone addressing them. After all, a compiler warning is > > >either a) valid and should be fixed or b) invalid and should be ignored. > > >Either way, a compiler warning shouldn't exist. > > > > > >First, `mach build` now prints a list of all parsed compiler warnings at > > >the end of the build. More details are in the commit message: > > >https://hg.mozilla.org/mozilla-central/rev/4abe611696ab > > > > > >Second, Perfherder now records the compiler warning count as a metric. > This > > >means we now have alerts when compiler warnings are added or removed, > like > > >[1]. And, we can easily see graphs of compiler warnings over time, like > > >[2]. The compiler warnings are also surfaced in the "performance" tab of > > >build jobs on Treeherder, like [3]. > > > > > >The Perfherder alerts and tracking are informational only: nobody will > be > > >backing you out merely because you added a compiler warning. While the > > >possibility of doing that now exists, I view that decision as up to the > C++ > > >module. I'm not going to advocate for it. So if you feel passionately, > take > > >it up with them :) > > > > > >Astute link clickers will note that the count metrics in CI are often > noisy > > >- commonly fluctuating by a value of 1-2. I suspect there are race > > >conditions with interleaved process output and/or bugs in the compiler > > >warnings parser/aggregator. Since the values are currently advisory > only, > > >I'm very much taking a "perfect is the enemy of good" attitude and have > no > > >plans to improve the robustness. > > > > > >[1] https://treeherder.mozilla.org/perf.html#/alerts?id=6700 > > >[2] https://mzl.la/2qFN0me and https://mzl.la/2rAkWR5 > > >[3] > > >https://treeherder.mozilla.org/#/jobs?repo=autoland&; > selectedJob=100509284 > > >_______________________________________________ > > >dev-platform mailing list > > >dev-platform@lists.mozilla.org > > >https://lists.mozilla.org/listinfo/dev-platform > > > > -- > > Kris Maglione > > Senior Firefox Add-ons Engineer > > Mozilla Corporation > > > > The X server has to be the biggest program I've ever seen that doesn't > > do anything for you. > > --Ken Thompson > > > > _______________________________________________ > > dev-platform mailing list > > dev-platform@lists.mozilla.org > > https://lists.mozilla.org/listinfo/dev-platform > _______________________________________________ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
