Write tools to track build warnings

RESOLVED WORKSFORME

Status

defect
RESOLVED WORKSFORME
9 years ago
2 years ago

People

(Reporter: khuey, Unassigned)

Tracking

(Blocks 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Posted patch POC (obsolete) — Splinter Review
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.
in the olden days, this tool existed. I believe bsmedberg had resurrected it at one time.
Posted patch PatchSplinter Review
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
Attachment #483698 - Attachment is obsolete: true
Attachment #483707 - Flags: feedback?(dholbert)
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.
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.
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 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.)
Attachment #483707 - Flags: feedback?(dholbert) → feedback+
(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).
mach tracks compiler warnings with its warnings-summary and warnings-list commands.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → WORKSFORME
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.