604850 - Write tools to track build warnings

Status: RESOLVED WORKSFORME

(Firefox Build System :: General, defect)

Description

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: POC

Attached is a proof of concept patch that redirects the output of the compiler to a file and then provides a python script to run over the objdir to collect the warnings.  The final product is a JSON list of the filename, warning count, and list of warnings for every file in the build.

Comment 1

[Robert Sayre]

in the olden days, this tool existed. I believe bsmedberg had resurrected it at one time.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

The warnings tool that used to run on tinderbox is http://mxr.mozilla.org/mozilla/source/webtools/tinderbox/warnings.pl

Comment 3

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

New log: "[{"count": 1, "file": "newfile", "warnings": ["Warning 1"]}, {"count": 3, "file": "samefile", "warnings": ["Warning 2", "Warning 4", "Warning 5"]}]"
Old log: [{"count": 1, "file": "oldfile", "warnings": ["Warning 1"]},
 {"count": 2, "file": "samefile", "warnings": ["Warning 2", "Warning 3"]}]

Example output:
$ python ../tools/warnlog/diff.py log.txt log2.txt
../tools/warnlog/diff.py:38: DeprecationWarning: the sets module is deprecated
  from sets import ImmutableSet
File: 'samefile'
        Warning Added: Warning 4
        Warning Added: Warning 5
        Warning Removed: Warning 3
File: 'newfile'
        Warning Added: Warning 1
File: 'oldfile'
        Warning Removed: Warning 1

Total change: 1

Comment 4

[Benjamin Smedberg]

My tool was at http://hg.mozilla.org/users/bsmedberg_mozilla.com/static-analysis-buildbot/file/19e7a98a8dc4/warning-parser.py and http://hg.mozilla.org/users/bsmedberg_mozilla.com/static-analysis-buildbot/file/19e7a98a8dc4/warningstep.py

It required a -j1 build because otherwise the warnings get interleaved and you can't assign them to locations. It wasn't just counting, it categorized and blamed them.

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I don't think blame on this is useful.  There's no way to accurately determine blame without building previous source revs.  Consider a function where I remove the last use of a local variable: whoever added that variable now gets blamed for the warning unless you build the previous .  The only way to get useful blame is to look at the changes in warnings between changesets, and even then it's not that useful (should the person who includes a header with a bunch of warnings be blamed for those?)

As for categorization, if we can get the JSON out of the build and put it somewhere useful that's easy to do.  Its also worth noting that this approach works with -jN and could easily be run on every build.

Comment 6

[Benjamin Smedberg]

The blame wasn't especially useful, but the by-directory/module reports certainly were.

I fully support doing something within the build system which tees individual build commands through a parsing mechanism, but that was a bit more invasive than I wanted at the time.

Comment 7

[Daniel Holbert [:dholbert]]

Comment on attachment 483707 [details] [diff] [review]
Patch

Great job taking initiative on this! :)

>-	$(AS) -o $@ $(ASFLAGS) -c $<
>+	$(AS) -o $@ $(ASFLAGS) -c $< 

Accidental whitespace-at-end-of-line addition here.  (nix it)

>+# If we're logging warnings, set variables appropriately
>+ifdef MOZ_WARNLOG
>+CC_SUFFIX = $(WARNING_LOGGER)
>+CXX_SUFFIX = $(WARNING_LOGGER)
>+endif # MOZ_WARNLOG
[...]
>+        WARNING_LOGGER='| tee .warnings/$@.log'

So if we build _without_ MOZ_MAKE_FLAGS="-s", will we get build cruft (or other non-warning-output) in the warning logs?

If so, it might be a good idea to enforce MOZ_MAKE_FLAGS="-s" when we're logging warnings.  (This might be completely off-target, though... I'm not actually sure at what level "make -s" does its silencing and whether it would have any effect on this.)

Comment 8

[Serge Gautherie (:sgautherie)]

(In reply to comment #2)
> The warnings tool that used to run on tinderbox is
> http://mxr.mozilla.org/mozilla/source/webtools/tinderbox/warnings.pl

Ftr, I assume this was the script used by dead 'brad' tinderbox (bug 231998).

Comment 9

[Chris Peterson [:cpeterson]]

mach tracks compiler warnings with its warnings-summary and warnings-list commands.