Closed Bug 1155349 Opened 8 years ago Closed 7 years ago

Port android builds to mozharness+mach.

Categories

(Release Engineering :: Applications: MozharnessCore, defect)

defect
Not set
normal

Tracking

(firefox38.0.5 fixed, firefox39 fixed, firefox40 fixed, firefox41 fixed, firefox-esr31 fixed, firefox-esr38 fixed, b2g-v2.0 fixed, b2g-v2.1 fixed, b2g-v2.1S fixed, b2g-v2.2 fixed, b2g-master fixed)

RESOLVED FIXED
Tracking Status
firefox38.0.5 --- fixed
firefox39 --- fixed
firefox40 --- fixed
firefox41 --- fixed
firefox-esr31 --- fixed
firefox-esr38 --- fixed
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed
b2g-v2.1S --- fixed
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: mshal, Assigned: mshal)

References

Details

Attachments

(9 files)

We'll want to have a mozharness script for android builds. Based on the work that dustin has done so far, this looks like it'll either be a simple android_build.py, or possibly just reuse fx_desktop_build.py.
Attached patch mc-androidSplinter Review
The easy part - we just need to turn off l10n-check in the automation steps. The defaults are fine for the rest.
Attached patch bb-android.patchSplinter Review
buildbot-configs changes to turn on mozharness+mach for android in 40+
Attached patch androidSplinter Review
mozharness part 1 - configs for building android. Since the automation steps are driven by the mozconfigs, we can re-use fx_desktop_build. The only annoying thing here is how the build variants are specified - if there is a better solution there, I'm all for it.
Attached patch android-l10nSplinter Review
mozharness part 2 - support for multil10n.py.

Android nightlies do both an en-US build and a multilocale build, where the en-US build is uploaded to an 'en-US/' subdirectory on FTP (the changes to query_mach_build_env() are to support this subdirectory). This patch adds support for a multi-l10n action in the base build script, but it's effectively unique to android builds. In order to re-use the existing multi-locale configs, I added a wrapper config (android-mozharness-build.json) which modifies the work_dir and such to reflect mozharness' directory layout instead of buildbot's. We can't change the multi-locale configs directly since they are shared by the separate l10n jobs.
Attachment #8599428 - Flags: review?(winter2718)
Attachment #8599429 - Flags: review?(winter2718)
Attachment #8599430 - Flags: review?(winter2718)
Attachment #8599433 - Flags: review?(winter2718)
Attachment #8599428 - Flags: review?(winter2718) → review+
Comment on attachment 8599429 [details] [diff] [review]
bb-android.patch

Review of attachment 8599429 [details] [diff] [review]:
-----------------------------------------------------------------

Looks straight-forward to me.
Attachment #8599429 - Flags: review?(winter2718) → review+
Comment on attachment 8599430 [details] [diff] [review]
android

Review of attachment 8599430 [details] [diff] [review]:
-----------------------------------------------------------------

Also looks fair, unfortunately I'm not away of a better way to specify the various build variants are specified (looks like what you did is par for the course).
Attachment #8599430 - Flags: review?(winter2718) → review+
Comment on attachment 8599433 [details] [diff] [review]
android-l10n

Review of attachment 8599433 [details] [diff] [review]:
-----------------------------------------------------------------

Aside from being sad at the complexity of basebuilder.py, I don't see any problems mh wise; but I'm not well versed in android or multi-locale builds. Interested to see if Dustin has any change requests after he applies this to his work.
Attachment #8599433 - Flags: review?(winter2718) → review+
Keywords: leave-open
I ran these in my docker container (in progress in bug 1125973), and got

20:47:09     INFO - abs_src_mozconfig: /home/worker/workspace/build/src/mobile/android/config/mozconfigs/android/nightly
20:47:09    FATAL - "abs_src_mozconfig" path could not be determined. Please make sure it is a valid path off of "abs_src_dir"

can you help figure out what that might mean?  Desktop builds using the same container worked (mostly; tests failed, but I think that was a missing Xvfb).
(In reply to Dustin J. Mitchell [:dustin] from comment #12)
> I ran these in my docker container (in progress in bug 1125973), and got
> 
> 20:47:09     INFO - abs_src_mozconfig:
> /home/worker/workspace/build/src/mobile/android/config/mozconfigs/android/
> nightly
> 20:47:09    FATAL - "abs_src_mozconfig" path could not be determined. Please
> make sure it is a valid path off of "abs_src_dir"
> 
> can you help figure out what that might mean?  Desktop builds using the same
> container worked (mostly; tests failed, but I think that was a missing Xvfb).

It looks like the base "android" build was removed in bug 1126360. I setup the mozharness builds to match those in buildbot, and we still have an "android" in there, though it appears to be no longer used. Can you try to pass in a --custom-build-variant-cfg parameter and see if that works? Here are the possible values for android:

+                    '--custom-build-variant-cfg', 'api-9',
+                    '--custom-build-variant-cfg', 'api-11',
+                    '--custom-build-variant-cfg', 'x86',
+                    '--custom-build-variant-cfg', 'debug',
+                    '--custom-build-variant-cfg', 'api-9-debug',
+                    '--custom-build-variant-cfg', 'api-11-debug',

(In the buildbot+mozharness case, these are passed in from buildbot).

Perhaps the default android variant should be api-9 since plain "android" no longer exists?
great, thanks!
Comment on attachment 8599429 [details] [diff] [review]
bb-android.patch

The buildbot-configs change was backed out because neither mozharness nor mach are able to set packageUrl correctly.
Attachment #8599429 - Flags: checked-in+
This stores the value of PACKAGE in mach_build_properties.json, and uses it to set packageUrl. The packageFilename parameter is needed since the same logic exists in mozharness for taskcluster uploads.

In particular, this lets us be explicit with which file is the package, rather than assuming it's any .zip/.bz2/.apk that isn't in an exclusion list, which is rather error prone:

http://mxr.mozilla.org/build/source/buildbotcustom/process/factory.py#227
Attachment #8605980 - Flags: review?(mh+mozilla)
Attached patch packageNameSplinter Review
With packageFilename from mach_build_properties.json, we can fixup the packageUrl override for S3 uploads in mozharness. This should solve the issue that caused the android switchover to get backed out.
Attachment #8605982 - Flags: review?(winter2718)
Comment on attachment 8605982 [details] [diff] [review]
packageName

Review of attachment 8605982 [details] [diff] [review]:
-----------------------------------------------------------------

Ship it!
Attachment #8605982 - Flags: review?(winter2718) → review+
Attachment #8605980 - Flags: review?(mh+mozilla) → review+
Comment on attachment 8599429 [details] [diff] [review]
bb-android.patch

https://hg.mozilla.org/build/buildbot-configs/rev/b75f5cfeebc6

I bumped the version 40 check to version 41 because releases moved since then.
Attachment #8599429 - Flags: checked-in+
Did this intentionally change the destination of android builds from http://ftp.mozilla.org/pub/mozilla.org/mobile/tinderbox-builds/ to http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/ ?
Flags: needinfo?(mshal)
(In reply to Bob Clary [:bc:] from comment #26)
> Did this intentionally change the destination of android builds from
> http://ftp.mozilla.org/pub/mozilla.org/mobile/tinderbox-builds/ to
> http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/ ?

Nope, not intentional. Looks like I missed setting a variable in the mozharness configs - I'll test out a fix & hopefully get a patch out today.
Flags: needinfo?(mshal)
Attached patch android-mobileSplinter Review
Sets stage_product to 'mobile' for android builds. This puts them in the right subdirectory, instead of the default 'firefox'.
Attachment #8609492 - Flags: review?(jlund)
Attachment #8609492 - Flags: review?(jlund) → review+
Note this still changed the urls to the builds. Now the build is located in the en-US subdirectory:

http://ftp.mozilla.org/pub/mozilla.org/mobile/tinderbox-builds/mozilla-inbound-android-api-9/1432570281/en-US/

while before the en-US subdirectory wasn't used:

http://ftp.mozilla.org/pub/mozilla.org/mobile/tinderbox-builds/mozilla-inbound-android-api-9/1432218258/

Again, if this is intentional I can live with it, but it does impose a tax on any consumers of mobile builds. We are already having to plan for the move to s3 at some point and these paper cuts simply add to the burden.
Flags: needinfo?(mshal)
FYI, https://hg.mozilla.org/build/buildbot-configs/rev/b75f5cfeebc6 disabled mozharness for desktop builds on aurora and beta. jlund tells me that we didn't actually want that on beta, but aurora is a regression.
(In reply to Michael Shal [:mshal] from comment #18)
> Created attachment 8605980 [details] [diff] [review]
> 0001-Bug-1155349-set-packageFilename-in-mach-properties.patch

Since this is now required to build on try, can we land it on every single active release branch, so we don't have to have the method for testing a backport to aurora be "push aurora to try, wait, discover that all your builds burned, ask on #developers, be told you have to stick bd6fb3d49f40 in your patch queue, push to try again"?
as nick mentioned, https://bug1155349.bugzilla.mozilla.org/attachment.cgi?id=8599429 disables m-a mh desktop builds. This is because desktop_mozharness_builds_enabled is branch wide config item. We have enabled m-a as a mozharness 'able' build branch already so anything that has 'mozharness_desktop_build' (a platform item), will use mozharness.

I think the solution here is to remove the platform level item for android builds that are less than m-c

http://people.mozilla.org/~jlund/reverse_the_disabling_of_mh_desktop_builds.patch

fwiw, the issue, unrelated to this bug, that caused m-b desktop builds to use mh will be addressed here: https://bugzil.la/1155349

builderlist diff of this patch: http://people.mozilla.org/~jlund/reverse_the_disabling_of_mh_desktop_builds.patch
Attachment #8610340 - Flags: review?(mshal)
> fwiw, the issue, unrelated to this bug, that caused m-b desktop builds to
> use mh will be addressed here: https://bugzil.la/1155349

derp, I mean https://bugzil.la/1144475
Comment on attachment 8610340 [details] [diff] [review]
re-enables_mh_desktop_builds_on_m-a.patch

>diff --git a/mozilla/config.py b/mozilla/config.py
>index d40ad0b..7c44ac9 100644
>--- a/mozilla/config.py
>+++ b/mozilla/config.py
>@@ -3008,7 +3008,9 @@ for name, branch in items_at_least(BRANCHES, 'gecko_version', 39):
> for name, branch in items_before(BRANCHES, 'gecko_version', 41):
>     for platform in branch['platforms'].keys():
>         if 'android' in platform:
>-            branch['desktop_mozharness_builds_enabled'] = False
>+            # we don't want to disable the branch level item: "desktop_mozharness_builds_enabled"
>+            # we do wnat to remove the platform level item: "mozharness_desktop_build"

wnat -> want
Attachment #8610340 - Flags: review?(mshal) → review+
Comment on attachment 8610340 [details] [diff] [review]
re-enables_mh_desktop_builds_on_m-a.patch

thanks and updated:

remote:   https://hg.mozilla.org/build/buildbot-configs/rev/c1256cb8daad
Attachment #8610340 - Flags: checked-in+
Since the multi-l10n step only happens on nightly builds, we should only create the en-US subdirectory on nightlies. This makes it so we only pay attention to the multi_locale config option on nightly builds.
Flags: needinfo?(mshal)
Attachment #8610818 - Flags: review?(jlund)
Attachment #8610818 - Flags: review?(jlund) → review+
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
You need to log in before you can comment on or make changes to this bug.