Closed Bug 833405 Opened 7 years ago Closed 7 years ago

Exempt -Wdeprecated-declarations from FAIL_ON_WARNINGS

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla21

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

Attachments

(1 file)

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.
Attached patch fix v1Splinter Review
Haven't tested this yet, but I think this'll do it.
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Blocks: 832545
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.
Attachment #704932 - Flags: review?(ted)
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 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+
Yes, good call.  I added brief explanation for each exception, and pushed:
  https://hg.mozilla.org/integration/mozilla-inbound/rev/b15fe80b4167
> I added brief explanation for each exception

+1
https://hg.mozilla.org/mozilla-central/rev/b15fe80b4167
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Blocks: 837903
Blocks: 896292
Blocks: 903663
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.