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

RESOLVED FIXED in Firefox 20

Status

()

Firefox for Android
General
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: kats, Assigned: kats)

Tracking

unspecified
Firefox 20
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

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.
Created attachment 684414 [details] [diff] [review]
(1/2) Move enum values into the enum to avoid -Wswitch

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)
Created attachment 684415 [details] [diff] [review]
(2/2) This fixes a bunch of unused variable warnings

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 3

6 years ago
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 5

6 years ago
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 6

6 years ago
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.
Created attachment 686128 [details] [diff] [review]
(1/2) Disable warnings-as-errors in plugin code
Attachment #684414 - Attachment is obsolete: true
Attachment #686128 - Flags: review?(joshmoz)

Comment 9

6 years ago
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.
Created attachment 687087 [details] [diff] [review]
(1/2) Disable warnings-as-errors for android in plugin code (v3)
Attachment #686128 - Attachment is obsolete: true
Attachment #686128 - Flags: review?(joshmoz)
Attachment #687087 - Flags: review?(joshmoz)

Updated

6 years ago
Attachment #687087 - Flags: review?(joshmoz) → review+
Blocks: 816993
https://hg.mozilla.org/mozilla-central/rev/350f68cf80b0
https://hg.mozilla.org/mozilla-central/rev/07a3cdc2c88f
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 20
You need to log in before you can comment on or make changes to this bug.