Make --with-gradle support single-locale repacks

RESOLVED FIXED in Firefox 58

Status

()

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: nalexander, Assigned: nalexander)

Tracking

(Blocks 1 bug)

unspecified
Firefox 58
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox58 fixed)

Details

Attachments

(5 attachments)

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).
Blocks: gradle-automation-v2
No longer blocks: 1405396
See Also: → 1405396
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
The posted patches are just my WIP.
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.
Depends on: 1413737
Assignee: nobody → nalexander
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
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

2 years ago
mozreview-review
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

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

https://reviewboard.mozilla.org/r/193042/#review201098
Attachment #8922036 - Flags: review?(aki) → review+

Comment 14

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

https://reviewboard.mozilla.org/r/195878/#review201100
Attachment #8924644 - Flags: review?(aki) → review+

Comment 15

2 years ago
mozreview-review
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.
Attachment #8922037 - Flags: review?(aki) → review+

Comment 16

2 years ago
mozreview-review
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.
(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 :)
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.
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 on attachment 8924643 [details]
Bug 1411688 - Part 0: Make --with-gradle handle single-locale repack ABIs.

https://reviewboard.mozilla.org/r/195876/#review201566
Attachment #8924643 - Flags: review?(snorp) → review+
Comment on attachment 8922035 [details]
Bug 1411688 - Part 1: Make --with-gradle handle single-locale repacks.

https://reviewboard.mozilla.org/r/193040/#review201568
Attachment #8922035 - Flags: review?(snorp) → review+

Comment 22

2 years ago
Pushed by nalexander@mozilla.com:
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
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)
Flags: needinfo?(nalexander)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 34

2 years ago
Pushed by nalexander@mozilla.com:
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
Backed out 5 changesets (bug 1411688) for failing Android single-locale repacks. r=backout a=backout

https://hg.mozilla.org/mozilla-central/rev/a73e202ca31d30bc2b96418fa81d1096f8dc731e
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: Firefox 58 → ---

Comment 37

2 years ago
Pushed by ncsoregi@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/958eef714e2b
 Make --with-gradle handle single-locale repack r=snorp a=reland
Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Flags: needinfo?(nalexander)
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
You need to log in before you can comment on or make changes to this bug.