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

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

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.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Attachment: (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.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Attachment: (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.

Comment 3

[Josh Matthews [:jdm]]

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.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

(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

[(no longer active)]

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!

Comment 6

[Josh Aas]

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.

Comment 7

[Kartikaya Gupta (email:[e-mail])]

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.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Attachment: (1/2) Disable warnings-as-errors in plugin code

Comment 9

[Josh Aas]

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.

Comment 10

[Kartikaya Gupta (email:[e-mail])]

Attachment: (1/2) Disable warnings-as-errors for android in plugin code (v3)

Comment 11

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/integration/mozilla-inbound/rev/350f68cf80b0
https://hg.mozilla.org/integration/mozilla-inbound/rev/07a3cdc2c88f

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/350f68cf80b0
https://hg.mozilla.org/mozilla-central/rev/07a3cdc2c88f