Closed Bug 1311741 Opened 5 years ago Closed 5 years ago

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

Categories

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

defect

Tracking

(firefox52 fixed)

RESOLVED FIXED
mozilla52
Iteration:
1.7
Tracking Status
firefox52 --- fixed

People

(Reporter: ahunt, Assigned: ahunt)

References

Details

(Whiteboard: [MobileAS])

Attachments

(1 file)

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).
Assignee: nobody → ahunt
Status: NEW → ASSIGNED
Priority: -- → P1
Whiteboard: [MobileAS]
Blocks: 1300144
Iteration: --- → 1.7
(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.
https://hg.mozilla.org/integration/fx-team/rev/9cdd8ed1733a941de56c4d022400c3e4b2ce0d12
Bug 1311741 - Remove "--non-constant-id" from aapt invocations r=nalexander
https://hg.mozilla.org/mozilla-central/rev/9cdd8ed1733a
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
Comment on attachment 8802983 [details]
Bug 1311741 - Remove "--non-constant-id" from aapt invocations

https://reviewboard.mozilla.org/r/87228/#review88194
Attachment #8802983 - Flags: review?(nalexander) → review+
Product: Firefox for Android → Firefox Build System
Target Milestone: Firefox 52 → mozilla52
You need to log in before you can comment on or make changes to this bug.