Mark toolkit/components/downloads/ as FAIL_ON_WARNINGS

RESOLVED FIXED in mozilla25

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

Trunk
mozilla25
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
The directory toolkit/components/downloads/ is build-warning-free on all supported platforms.

Filing this bug to mark it as FAIL_ON_WARNINGS, to keep it that way.

Here's a fully-green Try run, with the annotation added, to prove that this is safe:
 https://tbpl.mozilla.org/?tree=Try&rev=53b34881aba0
(Assignee)

Comment 1

6 years ago
Posted patch patch v1Splinter Review
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attachment #773420 - Flags: review?(mak77)
(Assignee)

Comment 2

6 years ago
(Aside: on my local machine, this directory does hit a few instances of "-Wdeprecated-declarations" warnings (for using deprecated GTK APIs), but those won't cause problems here because we've already excepted -Wdeprecated-declarations from FAIL_ON_WARNINGS, over in bug 833405.)
Comment on attachment 773420 [details] [diff] [review]
patch v1

Review of attachment 773420 [details] [diff] [review]:
-----------------------------------------------------------------

I honestly trust your checks and the tryserver here. Since I don't have anything against fail on warning, it's fine.
Attachment #773420 - Flags: review?(mak77) → review+
(Assignee)

Comment 4

6 years ago
Thanks!

Side note: I verified locally that this was being enforced -- I added an unused variable to one of the .cpp files in this directory, and it caused a build error (as you'd expect).
https://hg.mozilla.org/mozilla-central/rev/4df2e13d4157
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Hey Daniel,

I introduced a protocol buffer library as a dependency in https://bugzilla.mozilla.org/show_bug.cgi?id=837199 that is not compatible with this change. A lot of this code is automatically generated by the protocol buffer compiler and the bug to fix this has been open for more than a year: http://code.google.com/p/protobuf/issues/detail?id=361&colspec=ID%20Type%20Status%20Priority%20FixedIn%20Owner%20Summary

Is it possible to revert this?

Monica
Flags: needinfo?(dholbert)
(Assignee)

Comment 8

6 years ago
Backed out:
  https://hg.mozilla.org/integration/mozilla-inbound/rev/7645c1ac2ec3
so as not to block Monica from landing the 3rd-party header mentioned in comment 7 ( csd.pb.h ), which apparently has a few build warnings.

Per irc discussion, Monica's going to file a bug on those warnings, blocking this one, so that once we (hopefully) address them (or once upstream addresses them), we can re-land this annotation.
Status: RESOLVED → REOPENED
Flags: needinfo?(dholbert) → needinfo?(mmc)
Resolution: FIXED → ---
(Assignee)

Comment 9

6 years ago
Actually, from skimming the build logs from the Try run for bug 837199, I'm not convinced that protobuf is actually triggering any build warnings in toolkit/components/downloads/ ...

The code.google.com issue flagged in comment 7 seems to be for warnings in protobuf .cc files, and some of those are shown in the Try run's build logs, but those files don't live in toolkit/components/downloads/, so they shouldn't cause any trouble here.

Maybe I'm missing something? If not, then we don't need to file a new bug (blocking this one) after all, and this never actually needed to be backed out & can be re-landed.
Hi Daniel,

Thanks for helping me :) I am recompiling with my local fixes and your original patch:

 https://tbpl.mozilla.org/?tree=Try&rev=356a26b7fa58

Monica
Flags: needinfo?(mmc)
(Assignee)

Comment 12

6 years ago
Per IRC discussion with Monica, she agrees that Comment 9 is probably right -- the only build warnings introduced in this dir in Bug 837199 were in Mozilla code, and Monica's fixed those locally.

So, I re-landed this:
 https://hg.mozilla.org/integration/mozilla-inbound/rev/c434fcf6825b
https://hg.mozilla.org/mozilla-central/rev/c434fcf6825b
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.