1411688 - Make --with-gradle support single-locale repacks

Status: RESOLVED FIXED

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

Description

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

We're turning off single-locale repacks in Beta and Release: see Bug 1408083.  Some parts of the organization want to keep them for Nightly, but they're almost certainly broken: see Bug 1372911.

Supporting Android single-locale repacks is hard -- the whole process is incredibly fragile -- and it's holding back needed improvements to the build system.  Therefore, this ticket tracks making --with-gradle support single-locale repacks _eventually_, but not necessarily in time for the initial conversion to --with-gradle (in Bug 1405376).

Comment 1

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

Attachment: Bug 1411688 - Part 1: Make --with-gradle handle single-locale repacks.

Single-locale repacks do the following:

Download existing APK; unzip APK; update l10n resources; |mach package| with IS_LANGUAGE_REPACK=1.

This is pretty hard to accommodate, but we can try.  The key issues
here are to recognize when IS_LANGUAGE_REPACK=1 and not ask for l10n
resources (in particular, strings.xml) to be generated.

We do need to include the freshly built classes.dex when repackaging,
because newer Gradle/aapt doesn't preserve the R.java IDs.

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

Comment 2

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

Attachment: Bug 1411688 - Part 2: Include secrets in Android single-locale repacks.

Single-locale repacks need to run aapt (--without-gradle) or Gradle
(--with-gradle).  When running --with-gradle, they need to compile the
Java source code again (in order to produce a fresh R.java with
correct IDs).  That compile will be part of the shipping APK, so it
needs to be configured "the same" as the underlying repacked.  *This
is a significant change in behaviour, but necessary to support newer
Gradle/aapt versions, which do not maintain R.java ID mappings across
invocations.*

Part of the configuration are the secret keys and features that are
gated on them.  This commit makes those secrets available to
single-locale repacks.

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

Comment 3

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

Attachment: Bug 1411688 - Part 4: Make single-locale repacks agree with underlying Nightly builds.

There's nothing that makes sense in the existing setup; we're only not
getting bitten because the set of things that _do_ depend on all of
the flags that differ between the underlying Nightly builds and
single-locale repacks is small, and nobody has complained.  For
example, about:licenses probably does not include the Adjust SDK
license for single-locale repacks.

This patch series recompiles the Java code as part of each
single-locale repack, and that means the feature flags, etc, need to
be the same between the underlying compiled code (from the underlying
Nightly build) and the fresh Java compile.  This patch tries to
harmonize the two.

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

Comment 4

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

The posted patches are just my WIP.

Comment 5

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

Here's what I think needs to happen to address this: the single-locale repack jobs and configuration needs to be build-like.  We need to (re)compile the Fennec Java code and Android resources, which means that

- toolchains have to be the same between build and repack
- secrets and key files need to be the same between build and repack
- build settings need to be the same between build and repack
- basically, it all needs to be the same.

Once that is in place, the existing patches should "just work".  The hard part is making the single-locale repack jobs not be special snowflakes: they need to fetch secrets, set build flags, etc, just like real builds.

Comment 6

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

Attachment: Bug 1411688 - Part 0: Make --with-gradle handle single-locale repack ABIs.

This agrees with
http://searchfox.org/mozilla-central/rev/423b2522c48e1d654e30ffc337164d677f934ec3/mobile/android/base/generate_build_config.py#95.
It should really come from the underlying binaries for repacks, but
right now we only repack ARM builds, so this will do.

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

Comment 7

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

Attachment: Bug 1411688 - Part 3: Set MOZ_UPDATE_CHANNEL in single-locale repacks.

I'm seeing "try" in my repacks, when the underlying build has
"nightly-try".  This should make the two agree.

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

Comment 8

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

Comment on attachment 8922035 [details]
Bug 1411688 - Part 1: Make --with-gradle handle single-locale repacks.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/193040/diff/1-2/

Comment 9

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

Comment on attachment 8922036 [details]
Bug 1411688 - Part 2: Include secrets in Android single-locale repacks.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/193042/diff/1-2/

Comment 10

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

Comment on attachment 8922037 [details]
Bug 1411688 - Part 4: Make single-locale repacks agree with underlying Nightly builds.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/193044/diff/1-2/

Comment 11

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

aki, snorp: this turned out to not be as tricky as I expected.  It requires some small affordances in the Gradle configuration, which mimic the existing affordances in the Makefile.in files; those I've pushed to snorp, since he reviewed some similar work-arounds recently.

The real issue is that the single-locale repack jobs and configuration need to agree with the underlying configuration.  That requires secrets, but it wasn't too hard to get that working; and it requires unifying the configurations.  This is fragile... but the existing system is fragile.

Try builds looked green earlier, but I've pushed French (fr) at

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d87909eb33e04156ff809b73a293a368ca0cc8d2

and we'll see how it does.  I've manually inspected the pre- and post- repack APK files, and they look correct.  I'll make sure that the French build is sensible locally now.

Comment 12

[Aki Sasaki (not active)]

Comment on attachment 8922036 [details]
Bug 1411688 - Part 2: Include secrets in Android single-locale repacks.

https://reviewboard.mozilla.org/r/193042/#review201088

::: commit-message-e1ebb:6
(Diff revision 2)
> +Bug 1411688 - Part 2: Include secrets in Android single-locale repacks. r=aki
> +
> +Single-locale repacks need to run aapt (--without-gradle) or Gradle
> +(--with-gradle).  When running --with-gradle, they need to compile the
> +Java source code again (in order to produce a fresh R.java with
> +correct IDs).  That compile will be part of the shipping APK, so it

Recompiling is not ideal, and on other platforms this might have been a blocker. However, we don't ship android single-locale outside of nightly, so I think this is ok.

Comment 13

[Aki Sasaki (not active)]

Comment on attachment 8922036 [details]
Bug 1411688 - Part 2: Include secrets in Android single-locale repacks.

https://reviewboard.mozilla.org/r/193042/#review201098

Comment 14

[Aki Sasaki (not active)]

Comment on attachment 8924644 [details]
Bug 1411688 - Part 3: Set MOZ_UPDATE_CHANNEL in single-locale repacks.

https://reviewboard.mozilla.org/r/195878/#review201100

Comment 15

[Aki Sasaki (not active)]

Comment on attachment 8922037 [details]
Bug 1411688 - Part 4: Make single-locale repacks agree with underlying Nightly builds.

https://reviewboard.mozilla.org/r/193044/#review201102

This is largely a stamp; if you need a more in-depth review, the l10n or build teams might be able to help.

Comment 16

[Axel Hecht [:Pike]]

Comment on attachment 8924644 [details]
Bug 1411688 - Part 3: Set MOZ_UPDATE_CHANNEL in single-locale repacks.

https://reviewboard.mozilla.org/r/195878/#review201112

::: testing/mozharness/scripts/mobile_l10n.py:195
(Diff revision 1)
> +
> +        if self.query_is_nightly() or self.query_is_nightly_promotion():
> +            if self.query_is_nightly():
> +                # nightly promotion needs to set update_channel but not do all the 'IS_NIGHTLY'
> +                # automation parts like uploading symbols for now
> +                env["IS_NIGHTLY"] = "yes"

repack_env here and below? I think that's what's burning on try right now.

Comment 17

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

(In reply to Axel Hecht [:Pike] from comment #16)
> Comment on attachment 8924644 [details]
> Bug 1411688 - Part 3: Set MOZ_UPDATE_CHANNEL in single-locale repacks.
> 
> https://reviewboard.mozilla.org/r/195878/#review201112
> 
> ::: testing/mozharness/scripts/mobile_l10n.py:195
> (Diff revision 1)
> > +
> > +        if self.query_is_nightly() or self.query_is_nightly_promotion():
> > +            if self.query_is_nightly():
> > +                # nightly promotion needs to set update_channel but not do all the 'IS_NIGHTLY'
> > +                # automation parts like uploading symbols for now
> > +                env["IS_NIGHTLY"] = "yes"
> 
> repack_env here and below? I think that's what's burning on try right now.

Thanks -- this was cargoed from buildbase.py.  Will update and try again :)

Comment 18

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

With Axel's suggestion, this is looking very happy:

⋊> ~/M/gecko wget -O target.fr.apk https://queue.taskcluster.net/v1/task/N2q_zOC5R0WryAuegC6wcw/runs/0/artifacts/public/build/fr/target.apk
--2017-11-02 12:38:07--  https://queue.taskcluster.net/v1/task/N2q_zOC5R0WryAuegC6wcw/runs/0/artifacts/public/build/fr/target.apk
Resolving queue.taskcluster.net... 54.197.255.25, 174.129.218.85, 23.23.74.217
Connecting to queue.taskcluster.net|54.197.255.25|:443... connected.
HTTP request sent, awaiting response... 303 See Other
Location: https://public-artifacts.taskcluster.net/N2q_zOC5R0WryAuegC6wcw/0/public/build/fr/target.apk [following]
--2017-11-02 12:38:07--  https://public-artifacts.taskcluster.net/N2q_zOC5R0WryAuegC6wcw/0/public/build/fr/target.apk
Resolving public-artifacts.taskcluster.net... 52.84.17.138
Connecting to public-artifacts.taskcluster.net|52.84.17.138|:443... connected.
HTTP request sent, awaiting response... 200 OK
Length: 40440196 (39M) [application/vnd.android.package-archive]
Saving to: ‘target.apk’

target.apk                                                100%[====================================================================================================================================>]  38.57M  25.8MB/s    in 1.5s

2017-11-02 12:38:09 (25.8 MB/s) - ‘target.fr.apk’ saved [40440196/40440196]

⋊> ~/M/gecko wget -O target.en-US.apk https://queue.taskcluster.net/v1/task/V1dQ7099TZuBorVC7CKMYQ/runs/0/artifacts/public/build/en-US/target.apk
--2017-11-02 12:38:31--  https://queue.taskcluster.net/v1/task/V1dQ7099TZuBorVC7CKMYQ/runs/0/artifacts/public/build/en-US/target.apk
Resolving queue.taskcluster.net... 54.197.255.25, 174.129.218.85, 23.23.74.217
Connecting to queue.taskcluster.net|54.197.255.25|:443... connected.
HTTP request sent, awaiting response... 303 See Other
Location: https://public-artifacts.taskcluster.net/V1dQ7099TZuBorVC7CKMYQ/0/public/build/en-US/target.apk [following]
--2017-11-02 12:38:32--  https://public-artifacts.taskcluster.net/V1dQ7099TZuBorVC7CKMYQ/0/public/build/en-US/target.apk
Resolving public-artifacts.taskcluster.net... 52.84.17.138
Connecting to public-artifacts.taskcluster.net|52.84.17.138|:443... connected.
HTTP request sent, awaiting response... 200 OK
Length: 40356970 (38M) [application/vnd.android.package-archive]
Saving to: ‘target.en-US.apk’

target.en-US.apk                                          100%[====================================================================================================================================>]  38.49M  29.6MB/s    in 1.3s

2017-11-02 12:38:33 (29.6 MB/s) - ‘target.en-US.apk’ saved [40356970/40356970]

⋊> ~/M/gecko apktool -f d target.en-US.apk                                                                                                                                                                                     12:39:07
I: Using Apktool 2.2.2 on target.en-US.apk
I: Loading resource table...
I: Decoding AndroidManifest.xml with resources...
I: Loading resource table from file: /Users/nalexander/Library/apktool/framework/1.apk
I: Regular manifest package...
I: Decoding file-resources...
I: Decoding values */* XMLs...
I: Baksmaling classes.dex...
I: Copying assets and libs...
I: Copying unknown files...
I: Copying original files...
⋊> ~/M/gecko apktool -f d target.fr.apk                                                                                                                                                                                        12:39:27
I: Using Apktool 2.2.2 on target.fr.apk
I: Loading resource table...
I: Decoding AndroidManifest.xml with resources...
I: Loading resource table from file: /Users/nalexander/Library/apktool/framework/1.apk
I: Regular manifest package...
I: Decoding file-resources...
I: Decoding values */* XMLs...
I: Baksmaling classes.dex...
I: Copying assets and libs...
I: Copying unknown files...
I: Copying original files...
⋊> ~/M/gecko diff -qr target.en-US target.fr                                                                                                                                                                                   12:40:26
Files target.en-US/apktool.yml and target.fr/apktool.yml differ
Files target.en-US/assets/omni.ja and target.fr/assets/omni.ja differ
Files target.en-US/original/META-INF/ANDROIDD.RSA and target.fr/original/META-INF/ANDROIDD.RSA differ
Files target.en-US/original/META-INF/ANDROIDD.SF and target.fr/original/META-INF/ANDROIDD.SF differ
Files target.en-US/original/META-INF/MANIFEST.MF and target.fr/original/META-INF/MANIFEST.MF differ
Files target.en-US/res/values/strings.xml and target.fr/res/values/strings.xml differ

Testing locally now.

Comment 19

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

I pushed the target.fr.apk fetched above to my local device, and I get what looks like a reasonable first screen, with "Recherche ou adresse", "Les plus visites", etc.  So: no scrambled resources at first glance.

Comment 20

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8924643 [details]
Bug 1411688 - Part 0: Make --with-gradle handle single-locale repack ABIs.

https://reviewboard.mozilla.org/r/195876/#review201566

Comment 21

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8922035 [details]
Bug 1411688 - Part 1: Make --with-gradle handle single-locale repacks.

https://reviewboard.mozilla.org/r/193040/#review201568

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a8fd33d8a2af
Part 0: Make --with-gradle handle single-locale repack ABIs. r=snorp
https://hg.mozilla.org/integration/autoland/rev/133417cefdab
Part 1: Make --with-gradle handle single-locale repacks. r=snorp
https://hg.mozilla.org/integration/autoland/rev/3df83a3b7e9c
Part 2: Include secrets in Android single-locale repacks. r=aki
https://hg.mozilla.org/integration/autoland/rev/c313d76f2aa5
Part 3: Set MOZ_UPDATE_CHANNEL in single-locale repacks. r=aki
https://hg.mozilla.org/integration/autoland/rev/8ddf3257a8db
Part 4: Make single-locale repacks agree with underlying Nightly builds. r=aki

Comment 23

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for flake8 linting failure at testing/mozharness/scripts/mobile_l10n.py:

https://hg.mozilla.org/integration/autoland/rev/0de94b56338e29e469b762fe9e98f2b6e7b99c49

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=8ddf3257a8db317403f9be34e92a62569b4add06&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=142053820&repo=autoland
>  TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/toolkit/components/extensions/test/xpcshell/test_ext_i18n_css.js:130:5 | Unexpected var, use let or const instead. (mozilla/var-only-at-top-level)

Comment 24

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

Comment on attachment 8924643 [details]
Bug 1411688 - Part 0: Make --with-gradle handle single-locale repack ABIs.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/195876/diff/1-2/

Comment 25

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

Comment on attachment 8922035 [details]
Bug 1411688 - Part 1: Make --with-gradle handle single-locale repacks.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/193040/diff/2-3/

Comment 26

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

Comment on attachment 8922036 [details]
Bug 1411688 - Part 2: Include secrets in Android single-locale repacks.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/193042/diff/2-3/

Comment 27

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

Comment on attachment 8924644 [details]
Bug 1411688 - Part 3: Set MOZ_UPDATE_CHANNEL in single-locale repacks.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/195878/diff/1-2/

Comment 28

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

Comment on attachment 8922037 [details]
Bug 1411688 - Part 4: Make single-locale repacks agree with underlying Nightly builds.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/193044/diff/2-3/

Comment 29

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

Comment on attachment 8924643 [details]
Bug 1411688 - Part 0: Make --with-gradle handle single-locale repack ABIs.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/195876/diff/2-3/

Comment 30

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

Comment on attachment 8922035 [details]
Bug 1411688 - Part 1: Make --with-gradle handle single-locale repacks.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/193040/diff/3-4/

Comment 31

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

Comment on attachment 8922036 [details]
Bug 1411688 - Part 2: Include secrets in Android single-locale repacks.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/193042/diff/3-4/

Comment 32

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

Comment on attachment 8924644 [details]
Bug 1411688 - Part 3: Set MOZ_UPDATE_CHANNEL in single-locale repacks.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/195878/diff/2-3/

Comment 33

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

Comment on attachment 8922037 [details]
Bug 1411688 - Part 4: Make single-locale repacks agree with underlying Nightly builds.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/193044/diff/3-4/

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2e4e139bb5d7
Part 0: Make --with-gradle handle single-locale repack ABIs. r=snorp
https://hg.mozilla.org/integration/autoland/rev/34f83aab44e6
Part 1: Make --with-gradle handle single-locale repacks. r=snorp
https://hg.mozilla.org/integration/autoland/rev/866854a996b8
Part 2: Include secrets in Android single-locale repacks. r=aki
https://hg.mozilla.org/integration/autoland/rev/8ba514bd8ed6
Part 3: Set MOZ_UPDATE_CHANNEL in single-locale repacks. r=aki
https://hg.mozilla.org/integration/autoland/rev/65d5d13b4ea0
Part 4: Make single-locale repacks agree with underlying Nightly builds. r=aki

Comment 35

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/2e4e139bb5d7
https://hg.mozilla.org/mozilla-central/rev/34f83aab44e6
https://hg.mozilla.org/mozilla-central/rev/866854a996b8
https://hg.mozilla.org/mozilla-central/rev/8ba514bd8ed6
https://hg.mozilla.org/mozilla-central/rev/65d5d13b4ea0

Comment 36

[Natalia Csoregi [:nataliaCs]]

Backed out 5 changesets (bug 1411688) for failing Android single-locale repacks. r=backout a=backout

https://hg.mozilla.org/mozilla-central/rev/a73e202ca31d30bc2b96418fa81d1096f8dc731e

Comment 37

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/958eef714e2b
 Make --with-gradle handle single-locale repack r=snorp a=reland