Closed Bug 1411688 Opened 7 years ago Closed 7 years ago

Make --with-gradle support single-locale repacks

Categories

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

enhancement
Not set
normal

Tracking

(firefox58 fixed)

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: nalexander, Assigned: nalexander)

References

(Blocks 1 open bug)

Details

Attachments

(5 files)

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).
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.
Assignee: nobody → nalexander
Status: NEW → ASSIGNED
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 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 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 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 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 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+
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)
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 → ---
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: 7 years ago7 years ago
Flags: needinfo?(nalexander)
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Product: Firefox for Android → Firefox Build System
Target Milestone: Firefox 58 → mozilla58
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: