While I agree with that in general, it seems like the warnings in NSS and
NSPR, at least, are our responsibility, and should be fixed. I notice the
huge number of warnings scrolling by from NSS, in particular, every time I
compile, and they make me worry.
On Fri, May 19, 2017 at 04:27:47PM -0400, Michael Layzell wrote:
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
--
Kris Maglione
Senior Firefox Add-ons Engineer
Mozilla Corporation
Whenever a man has cast a longing eye on offices, a rottenness begins
in his conduct.
--Thomas Jefferson
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
