1311741 - Remove "--non-constant-id" flag from aapt in mach builds

Status: RESOLVED FIXED

(Firefox Build System :: Android Studio and Gradle Integration, defect, P1)

Description

[Andrzej Hunt :ahunt]

Bug 880118 (specifically Bug 880118 comment #24) added the --non-constant-id flag to aapt invocations in mach build's, as part of building the geckoview library. This is needed for R.java's that will be integrated into other projects, but isn't desirable otherwise.

Nowadays geckoview seems to be built using gradle, and gradle is able to do it's own aapt invocations which result in better/correct R.java's being created (including constant id's for normal fennec builds).

Using --non-constant-id is annoying because we can't use resource IDs in switch statements.

I've also been told that modern geckoview no longer bundles resources, which completely avoids the aapt issues altogether, regardless of gradle builds.

I'd like to remove this flag again in order to let us use switch statements for resource IDs (useful for e.g. menu's), and to make development faster (use of switch statements is only a problem in mach builds, most developers use Android Studio with gradle builds, so the problem isn't caught until a |mach build| is done, rather than being made visible when writing switch statements in the first place).

Comment 1

[Andrzej Hunt :ahunt]

Attachment: Bug 1311741 - Remove "--non-constant-id" from aapt invocations

This flag seems to no longer be needed, and prevents the use of switch statements
for resource ID's. It was first introduced for mach based geckoview builds,
however geckoview now seems to be built using gradle which is able to invoke
aapt to produce better R.java's, resulting in constant Resource ID's in Fennec.

Review commit: https://reviewboard.mozilla.org/r/87228/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/87228/

Comment 2

[Nick Alexander :nalexander [he/him]]

(In reply to Andrzej Hunt :ahunt from comment #0)
> Bug 880118 (specifically Bug 880118 comment #24) added the --non-constant-id
> flag to aapt invocations in mach build's, as part of building the geckoview
> library. This is needed for R.java's that will be integrated into other
> projects, but isn't desirable otherwise.

Hmm.  Okay.  I vaguely remember that --non-constant-id impacts *combining* libraries as well, but since we only invoke aapt exactly once in mach builds, that doesn't make sense.

> Nowadays geckoview seems to be built using gradle, and gradle is able to do
> it's own aapt invocations which result in better/correct R.java's being
> created (including constant id's for normal fennec builds).


Correct.
 
> Using --non-constant-id is annoying because we can't use resource IDs in
> switch statements.

Indeed.
 
> I've also been told that modern geckoview no longer bundles resources, which
> completely avoids the aapt issues altogether, regardless of gradle builds.
> 
> I'd like to remove this flag again in order to let us use switch statements
> for resource IDs (useful for e.g. menu's), and to make development faster
> (use of switch statements is only a problem in mach builds, most developers
> use Android Studio with gradle builds, so the problem isn't caught until a
> |mach build| is done, rather than being made visible when writing switch
> statements in the first place).

r+.  Too busy to flag in mozreview.

Comment 3

[Andrzej Hunt :ahunt]

https://hg.mozilla.org/integration/fx-team/rev/9cdd8ed1733a941de56c4d022400c3e4b2ce0d12
Bug 1311741 - Remove "--non-constant-id" from aapt invocations r=nalexander

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9cdd8ed1733a

Comment 5

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8802983 [details]
Bug 1311741 - Remove "--non-constant-id" from aapt invocations

https://reviewboard.mozilla.org/r/87228/#review88194