Closed
Bug 833405
Opened 11 years ago
Closed 11 years ago
Exempt -Wdeprecated-declarations from FAIL_ON_WARNINGS
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla21
People
(Reporter: dholbert, Assigned: dholbert)
References
Details
Attachments
(1 file)
2.21 KB,
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
I think we should exempt -Wdeprecated-declarations from the purview of FAIL_ON_WARNINGS. This build warning is in the category of "nice to know about, but there's not necessarily anything we can do about it".[1] I don't think we should allow it to break our builds (if a function is deprecated out from under us all of a sudden), nor should it prevent directories from being marked otherwise-warning-free just because they happen to use a method that's deprecated on a particular platform. [1] See e.g. bug 832548 comment 1, about a deprecated Mac OS X function that has no available replacement.
Assignee | ||
Comment 1•11 years ago
|
||
Haven't tested this yet, but I think this'll do it.
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Comment 2•11 years ago
|
||
This makes a lot of sense, at least on the Mac. Apple often deprecates stuff just to "de-document" it (to get people to stop using it), without (apparently) ever intending to actually get rid of it.
Assignee | ||
Updated•11 years ago
|
Attachment #704932 -
Flags: review?(ted)
Assignee | ||
Comment 3•11 years ago
|
||
Try push, w/ this patch included: https://tbpl.mozilla.org/?tree=Try&rev=161fee613cbd Ignore the red mac build and the red android build -- those failed for unrelated reasons. The green mac build is the relevant one. :) That green build has compile commands that include: > -Werror -Wno-error=uninitialized -Wno-error=deprecated-declarations (as expected), and it successfully built with gfx/thebes marked as FAIL_ON_WARNINGS (in another patch), even with a deprecated-declaration warning in that directory (as expected).
Comment 4•11 years ago
|
||
Comment on attachment 704932 [details] [diff] [review] fix v1 Review of attachment 704932 [details] [diff] [review]: ----------------------------------------------------------------- ::: configure.in @@ +1399,5 @@ > fi > fi > + WARNINGS_AS_ERRORS='-Werror' > + WARNINGS_AS_ERRORS="$WARNINGS_AS_ERRORS -Wno-error=uninitialized" > + WARNINGS_AS_ERRORS="$WARNINGS_AS_ERRORS -Wno-error=deprecated-declarations" Do you think we should add a brief 1-line description indicating why we have each of these exceptions?
Attachment #704932 -
Flags: review?(ted) → review+
Assignee | ||
Comment 5•11 years ago
|
||
Yes, good call. I added brief explanation for each exception, and pushed: https://hg.mozilla.org/integration/mozilla-inbound/rev/b15fe80b4167
Comment 6•11 years ago
|
||
> I added brief explanation for each exception
+1
Comment 7•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b15fe80b4167
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
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
•