Closed Bug 1360764 Opened 7 years ago Closed 7 years ago

Print compiler warnings at end of build

Categories

(Firefox Build System :: General, enhancement)

enhancement
Not set
normal

Tracking

(firefox55 fixed)

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: gps, Assigned: gps)

References

Details

Attachments

(3 files)

We currently print an inline compiler warning structured log message next to the compiler output itself. I think this summarized warning message would be more useful if it were printed at the end of the build log alongside all the other compiler warnings, as all the warnings would be in a single, sorted, easy-to-parse-for-humans list.
I don't think I have many problems with the patches themselves, but I'm kind of +/-0 on the whole endeavor.  Applying the patches locally on my Linux box, for instance, does produce a nice warnings summary...but virtually all of the warnings are split between:

1) Third-party code, which we arguably shouldn't be warning for (we turn a lot of warnings off in third-party code, for better or worse); and
2) dom/bindings/ auto-generated code, which I think is mostly just noise; and
3) NSPR/NSS, which are _probably_ worth fixing, but I'm not sure if anybody's particularly motivated to wade into the process there.

And all the warnings that aren't in the above categories are -Wmaybe-uninitialized warnings, which are annoying anyway.

I'd be curious to know what the situation looks like on a Windows machine, whether it's an overwhelming amount of output, since we pay less attention to warnings on Windows, or whether it looks more like the above.

Maybe we should just go with the "change is better than the status quo" for such a minor area, and then deal with the yelling after the fact.
There is a try push where you can see output on different platforms. https://treeherder.mozilla.org/#/jobs?repo=try&revision=99ad92c15772&selectedJob=95368280. Here's a Windows log: https://treeherder.mozilla.org/logviewer.html#?job_id=95368280&repo=try&lineNumber=20525

I'd like to note that your calling out 3 classes of follow-up bugs in your evaluation of this change justifies my rationale for the change: raising visibility of compiler warnings so people fix them. I view every compiler warning as a bug - either "bad" code or a worthless warning. I'm trying to nudge us towards no compiler warnings.
Comment on attachment 8863074 [details]
Bug 1360764 - Decouple warnings database from log parser;

https://reviewboard.mozilla.org/r/134912/#review138628
Attachment #8863074 - Flags: review?(nfroyd) → review+
Comment on attachment 8863075 [details]
Bug 1360764 - Record warnings seen during the current operation;

https://reviewboard.mozilla.org/r/134914/#review138634
Attachment #8863075 - Flags: review?(nfroyd) → review+
Comment on attachment 8863076 [details]
Bug 1360764 - Print compiler warnings at end of the build;

https://reviewboard.mozilla.org/r/134916/#review138636
Attachment #8863076 - Flags: review?(nfroyd) → review+
Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b811b0049408
Decouple warnings database from log parser; r=froydnj
https://hg.mozilla.org/integration/autoland/rev/ae04a15b9fcc
Record warnings seen during the current operation; r=froydnj
https://hg.mozilla.org/integration/autoland/rev/4abe611696ab
Print compiler warnings at end of the build; r=froydnj
https://hg.mozilla.org/mozilla-central/rev/b811b0049408
https://hg.mozilla.org/mozilla-central/rev/ae04a15b9fcc
https://hg.mozilla.org/mozilla-central/rev/4abe611696ab
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Depends on: 1367405
An option to turn this off would be nice...
(In reply to Honza Bambas (:mayhemer) from comment #11)
> An option to turn this off would be nice...

If there's something you don't like about it, please open a new bug describing what the problem is.
Flags: needinfo?(honzab.moz)
Blocks: 1367434
(In reply to Nathan Froyd [:froydnj] from comment #12)
> (In reply to Honza Bambas (:mayhemer) from comment #11)
> > An option to turn this off would be nice...
> 
> If there's something you don't like about it, please open a new bug
> describing what the problem is.

Bug 1367434.  Don't give it much weight, I might be the only one that would like to have a pref for this.  If I stand alone and the option is too complicated, just WONTFIX it.  Thanks.
Flags: needinfo?(honzab.moz)
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: