Closed Bug 604850 Opened 14 years ago Closed 8 years ago

Write tools to track build warnings


(Firefox Build System :: General, defect)

Windows 7
Not set


(Not tracked)



(Reporter: khuey, Unassigned)


(Blocks 1 open bug)



(1 file, 1 obsolete file)

Attached 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.
Attached 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/ log.txt log2.txt
../tools/warnlog/ 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 and

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]

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
>+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+
Blocks: buildwarning
(In reply to comment #2)
> The warnings tool that used to run on tinderbox is

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.
Closed: 8 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.