Closed Bug 814418 Opened 7 years ago Closed 7 years ago

Make Fennec build with NDK r8c (gcc 4.6 toolchain) when --enable-warnings-as-errors is set

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 20

People

(Reporter: kats, Assigned: kats)

References

Details

Attachments

(2 files, 2 obsolete files)

Right now when building with NDK r8c using the standard "nightly" mozconfig at mobile/android/config/mozconfigs/android/nightly the build fails because gcc 4.6 is stricter with warnings, and a few of them are in directories where no warnings are allowed. This causes the build to fail and blocks us from upgrading to r8c by default.
There were a bunch of errors that looked like this:

/home/kats/zspace/mozilla-git/dom/plugins/base/nsNPAPIPlugin.cpp:2239:5: error: case value '1001' not in enumerated type 'NPNVariable' [-Werror=switch]
/home/kats/zspace/mozilla-git/dom/plugins/base/nsNPAPIPlugin.cpp:2225:5: error: case value '1002' not in enumerated type 'NPNVariable' [-Werror=switch]
/home/kats/zspace/mozilla-git/dom/plugins/base/nsNPAPIPlugin.cpp:2197:5: error: case value '1003' not in enumerated type 'NPNVariable' [-Werror=switch]

This patch fixes them by moving the #define'd values into the enum.
Attachment #684414 - Flags: review?(josh)
This touches a variety of code so I'm not sure who all should be reviewing it. Tagging ehsan to start, but please add any additional reviewers you think are necessary.
Attachment #684415 - Flags: review?(ehsan)
Comment on attachment 684414 [details] [diff] [review]
(1/2) Move enum values into the enum to avoid -Wswitch

I suspect you want the other Josh here.
Attachment #684414 - Flags: review?(josh) → review?(joshmoz)
(In reply to Josh Matthews [:jdm] from comment #3)
> I suspect you want the other Josh here.

Oh, whoops. Sorry 'bout that.


Also I pushed a try run with these patches last night, they look pretty green: https://tbpl.mozilla.org/?tree=Try&rev=1404a5331a27
Comment on attachment 684415 [details] [diff] [review]
(2/2) This fixes a bunch of unused variable warnings

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

r=me, your friendly review sucker!
Attachment #684415 - Flags: review?(ehsan) → review+
Comment on attachment 684414 [details] [diff] [review]
(1/2) Move enum values into the enum to avoid -Wswitch

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

We keep npapi.h in sync with the official repository:

http://code.google.com/p/npapi-sdk/

That doesn't include any android support, and I don't know of any plans to do so. Would be better to find a way to do this without modifying npapi.h.
Attachment #684414 - Flags: review?(joshmoz) → review-
Josh: Are there other files nearby that are also kept in sync that I should avoid touching, or is it just npapi.h? Also would you be opposed to disabling that warning via pragma/compiler options and/or turning off warnings-as-errors in that folder? Just trying to get a sense of which possible solutions would be most acceptable to you.
Attachment #684414 - Attachment is obsolete: true
Attachment #686128 - Flags: review?(joshmoz)
Comment on attachment 686128 [details] [diff] [review]
(1/2) Disable warnings-as-errors in plugin code

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

Can you turn off warnings as errors only for a particular platform? I'd be fine with that for now.

The only files that are kept in sync are npapi.h, npfunctions.h, nptypes.h, and npruntime.h. They're all in the same directory.
Attachment #687087 - Flags: review?(joshmoz) → review+
https://hg.mozilla.org/mozilla-central/rev/350f68cf80b0
https://hg.mozilla.org/mozilla-central/rev/07a3cdc2c88f
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 20
You need to log in before you can comment on or make changes to this bug.