Closed
Bug 1360764
Opened 7 years ago
Closed 7 years ago
Print compiler warnings at end of build
Categories
(Firefox Build System :: General, enhancement)
Firefox Build System
General
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.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 4•7 years ago
|
||
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.
Assignee | ||
Comment 5•7 years ago
|
||
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 6•7 years ago
|
||
mozreview-review |
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 7•7 years ago
|
||
mozreview-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 8•7 years ago
|
||
mozreview-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
Comment 10•7 years ago
|
||
bugherder |
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
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Comment 11•7 years ago
|
||
An option to turn this off would be nice...
Comment 12•7 years ago
|
||
(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)
Comment 13•7 years ago
|
||
(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)
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•