Closed Bug 603266 Opened 10 years ago Closed 10 years ago

Android release package/upload fixes

Categories

(Release Engineering :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: aki, Assigned: aki)

References

Details

Attachments

(4 files, 1 obsolete file)

Once bug 567873 lands, Android release packaging/uploads will need to change as well.  This is on the default branch, while the patches in bug 567873 are on the buildbot-0.8.0 branch, so a decent amount of code will need to be ported.

Dependent on the post_upload.py fixes in bug 601323.
This should also cover Android multilocale release builds.
This is the patch for our tools repo for my 4.0b2 staging runs (and will be for our production run(s).

It allows us to bump mobile-browser/confvars.sh versions, and fixes mobile candidates uploading... which are technically bug 600624 and bug 601323, but since my configs and buildbotcustom patches are all in one piece (sorry), I'm uploading all 4 patches here.

Ideally this'll get reviewed and landed before EOD today, so we're ready for go-to-build (and if that's not late today or tomorrow, Lukas can get a staging run in, maybe).

I've tested, in no particular order, over the past 4 days:

a) that I haven't borked Maemo5-gtk nightly builds/uploads
b) that Maemo5-gtk release multilocale builds upload correctly and spawn l10n repacks correctly
c) that Android release builds build multilocale and upload properly
d) that linux-i686 desktop release builds upload properly
e) macosx-i686 desktop release builds, when done manually (moz2-darwin9-slave08 seems to have constant dns issues when run through buildbot) upload properly
f) win32-i686 is just borked. i think the windows slave on sm02 needs reimaging
Attachment #485810 - Flags: review?(coop)
This is a diff of the PRODUCTION tag against default tip in mozharness.
I primarily added 4.0_release_android.json and staging_4.0_release_android.json config files (and ran tests w/ the latter) and updated multil10n.py to grok the l10n-changesets_mobile-2.0.json format.
Attachment #485811 - Flags: review?(coop)
Ok.

* added a dummy, but smaller l10n-changesets_mobile-2.0.json for staging (it was just {} before, since pre-4.0b2 was non-localized).
* copied l10n-changesets_mobile-1.1.json to l10n-changesets_mobile-2.0.json in production, then added android-multilocale to the platforms wherever there was maemo-multilocale.  The changesets will need to be updated before we go to build.
* un-disabled multilocale in release-fennec-2.0.py
* updated release-fennec-2.0.py pre-changesets for production.
* changed bumpFiles to confvars.sh
* changed default update channel from betatest to beta-cck-test (see bug 602329 comment 2 )
* added multilocale bits to the android factory
Attachment #485812 - Flags: review?(coop)
Attached patch 4.0b2 buildbotcustom (obsolete) — Splinter Review
Buildbotcustom side.

This includes ports from 0.8.0 for relevant code from:

* bug 567873 - android packaging should use standard packaging code
* bug 603822 - android nightlies broken
* bug 563382 - android multilocale

I'm not entirely sure why I had to touch MaemoBuildFactory.prepUpload(), but afterwards the multilocale release Maemo build uploaded correctly, as well as the nightly.

I probably could have not touched a lot of AndroidBuildFactory since we only run Android release builds in 07x, but I'm trying not to diverge too much. Failing somewhat at that.

Ideally we get Maemo multilocale and single locale repacks into 08x pronto, then get release builds on 08x as well.  However, getting that done and baked before 4.0b3, isn't likely, especially since I'm out around that time.

... As for the previousApk stuff, I'm assuming that the apk will be named fennec-version.locale.arch.apk, which isn't actually true for 4.0b1 (fennec.apk).  However, in the interest of prepping for 4.0b3 in my absence, I went ahead and created the relevant softlinks on stage.m.o so this should Just Work.
Attachment #485815 - Flags: review?(coop)
Attachment #485810 - Flags: review?(coop) → review+
Comment on attachment 485811 [details] [diff] [review]
mozharness diffs for 4.0b2

>diff --git a/lib/base/script.py b/lib/base/script.py
>--- a/lib/base/script.py
>+++ b/lib/base/script.py
>@@ -103,16 +103,17 @@ class BaseScript(object):
>         self.info("mkdir: %s" % path)
>         if not os.path.exists(path):
>             if not self.config['noop']:
>                 os.makedirs(path)
>         else:
>             self.info("Already exists.")
> 
>     def rmtree(self, path, error_level='error', exit_code=-1):
>+# TODO windows rmtree

Are you looking for this?

http://hg.mozilla.org/build/buildbot/file/71c8d03e9010/slave/buildslave/commands/utils.py#l25

>diff --git a/scripts/multil10n.py b/scripts/multil10n.py
>--- a/scripts/multil10n.py
>+++ b/scripts/multil10n.py
>@@ -6,16 +6,18 @@ of MaemoBuildFactory.  However, this was
> requiring runtime step truncation/creation with large amounts of build
> properties that disallowed the use of "Force Build" for any multi-locale
> nightly.
> 
> To improve things, we're moving the logic slave-side where a dedicated
> slave can use its cycles determining which locales to repack.
> 
> Currently oriented towards Android multilocale, bug 563382.
>+
>+TODO: finish work to make this a standalone runnable script.
[deletia]
>+                # TODO this belongs in a shared function for releases.
>@@ -273,16 +284,20 @@ class MultiLocaleRepack(MercurialScript)
>+        # TODO a lot of the lines of code here are determining paths.
>+        # Each of these action methods should be able to call a single
>+        # function that returns a dictionary of these that we can use
>+        # so we don't have to keep redefining them.

Followup bug(s)?
Attachment #485811 - Flags: review?(coop) → review+
Comment on attachment 485812 [details] [diff] [review]
4.0b2 buildbot-configs

>diff --git a/mozilla2-staging/release-fennec-2.0.py b/mozilla2-staging/release-fennec-2.0.py
>--- a/mozilla2-staging/release-fennec-2.0.py
>+++ b/mozilla2-staging/release-fennec-2.0.py
>@@ -16,33 +16,33 @@ mozRelbranchOverride   = ''
> l10nRelbranchOverride   = ''
> mobileRelbranchOverride   = ''
> l10nRepoClonePath   = 'l10n-central'
> l10nRepoPath        = 'users/stage-ffxbld'
> l10nRevisionFile    = 'l10n-changesets_mobile-2.0.json'
> productName         = 'fennec'
> appName             = 'mobile'
> mergeLocales        = True
>-disableMultiLocale  = True
>+disableMultiLocale  = False

I would prefer "enableMultiLocale = True" instead of the double-negative, but I won't block on that.


>-        ausPreviousUploadDir = "%s/%s/%s/%%(previous_buildid)s/en-US/betatest" % \
>+        ausPreviousUploadDir = "%s/%s/%s/%%(previous_buildid)s/en-US/beta-cck-test" % \

Is that really our channel name? Sigh.
Attachment #485812 - Flags: review?(coop) → review+
Comment on attachment 485815 [details] [diff] [review]
4.0b2 buildbotcustom

>@@ -7393,19 +7439,28 @@ class AndroidBuildFactory(MobileBuildFac
>         self.addBaseRepoSteps()
>         self.getMozconfig()
>         self.addPreBuildSteps()
>         self.addBuildSteps()
>         self.addPackageSteps()
>         if self.createSnippet:
>             self.addUpdateSteps()
>         self.addSymbolSteps()
>-        self.addUploadSteps(platform=self.uploadPlatform)
>+        if self.multiLocale:
>+            self.addMakeUploadSteps(subdir="en-US")
>+        else:
>+            self.addMakeUploadSteps()
>         if self.triggerBuilds:
>             self.addTriggeredBuildsSteps()
>+        if self.multiLocale:
>+            self.addMultiLocaleSteps()
>+            self.addPackageSteps(locale='multi')
>+            if self.createSnippet:
>+                self.addUpdateSteps()
>+            self.addMakeUploadSteps(locale='multi')

You're running self.addUpdateSteps() twice here if self.multiLocale is set.

>@@ -7525,18 +7583,18 @@ class AndroidBuildFactory(MobileBuildFac
>             milestone=self.baseUploadDir,
>             baseurl='%s/nightly' % self.downloadBaseURL,
>             hashType=self.hashType)
>         )
> 
>     def addUpdateSteps(self):
>         # Normally we'd make a mar first, but we'll create a snippet of
>         # the apk for now.
>-        self.addFilePropertiesSteps(filename='fennec.apk',
>-                                    directory='%s/%s/%s/embedding/android' % \
>+        self.addFilePropertiesSteps(filename='fennec*.apk',
>+                                    directory='%s/%s/%s/dist' % \
>                                       (self.baseWorkDir, self.branchName,
>                                        self.objdir),
>                                     fileType='completeMar',

Should the fileType be set to something different then?
Attachment #485815 - Flags: review?(coop) → review-
(In reply to comment #8)
> You're running self.addUpdateSteps() twice here if self.multiLocale is set.

if you switch the buildbotcustom patch to only run addUpdateSteps once, you can consider that an r+.
(In reply to comment #8)
> Comment on attachment 485815 [details] [diff] [review]
> 4.0b2 buildbotcustom
> 
> >@@ -7393,19 +7439,28 @@ class AndroidBuildFactory(MobileBuildFac
> >         self.addBaseRepoSteps()
> >         self.getMozconfig()
> >         self.addPreBuildSteps()
> >         self.addBuildSteps()
> >         self.addPackageSteps()
> >         if self.createSnippet:
> >             self.addUpdateSteps()
> >         self.addSymbolSteps()
> >-        self.addUploadSteps(platform=self.uploadPlatform)
> >+        if self.multiLocale:
> >+            self.addMakeUploadSteps(subdir="en-US")
> >+        else:
> >+            self.addMakeUploadSteps()
> >         if self.triggerBuilds:
> >             self.addTriggeredBuildsSteps()
> >+        if self.multiLocale:
> >+            self.addMultiLocaleSteps()
> >+            self.addPackageSteps(locale='multi')
> >+            if self.createSnippet:
> >+                self.addUpdateSteps()
> >+            self.addMakeUploadSteps(locale='multi')
> 
> You're running self.addUpdateSteps() twice here if self.multiLocale is set.

I think the idea is to create an en-US snippet and a multi snippet, which I am doing in nightlies.
I'm not entirely sure if that's the right approach or if it's buggy or not.  We will probably only be shipping one of these, and the release snippet stuff still requires manual intervention to update file size + hash post-signing, (plus moving channels) so I'm fine with just doing this for the multi.

> 
> >@@ -7525,18 +7583,18 @@ class AndroidBuildFactory(MobileBuildFac
> >             milestone=self.baseUploadDir,
> >             baseurl='%s/nightly' % self.downloadBaseURL,
> >             hashType=self.hashType)
> >         )
> > 
> >     def addUpdateSteps(self):
> >         # Normally we'd make a mar first, but we'll create a snippet of
> >         # the apk for now.
> >-        self.addFilePropertiesSteps(filename='fennec.apk',
> >-                                    directory='%s/%s/%s/embedding/android' % \
> >+        self.addFilePropertiesSteps(filename='fennec*.apk',
> >+                                    directory='%s/%s/%s/dist' % \
> >                                       (self.baseWorkDir, self.branchName,
> >                                        self.objdir),
> >                                     fileType='completeMar',
> 
> Should the fileType be set to something different then?

I believe I'm setting completeMar specifically because CreateCompleteUpdateSnippet has that hardcoded, and I'm effectively creating a complete snippet, just with the apk instead of a mar.

But agreed, it's a misnomer.

I'm kind of leaning towards waiting to see if/when Android moves to using complete and partial mars.
(In reply to comment #7)
> >+disableMultiLocale  = False
> 
> I would prefer "enableMultiLocale = True" instead of the double-negative, but I
> won't block on that.

Me too.
Go to build for build1 is probably tomorrow; I can try to fix afterwards.

> >+        ausPreviousUploadDir = "%s/%s/%s/%%(previous_buildid)s/en-US/beta-cck-test" % \
> 
> Is that really our channel name? Sigh.

Yes, and it's actually the best-sounding solution to the problem atm.

(In reply to comment #6)
> >+# TODO windows rmtree
> 
> Are you looking for this?
> 
> http://hg.mozilla.org/build/buildbot/file/71c8d03e9010/slave/buildslave/commands/utils.py#l25

Yes.  Bear pasted that to me earlier today after adrianp ran into that in #seneca.  I'll get that in in a bit :)

> >+TODO: finish work to make this a standalone runnable script.
> [deletia]
> >+                # TODO this belongs in a shared function for releases.
> >@@ -273,16 +284,20 @@ class MultiLocaleRepack(MercurialScript)
> >+        # TODO a lot of the lines of code here are determining paths.
> >+        # Each of these action methods should be able to call a single
> >+        # function that returns a dictionary of these that we can use
> >+        # so we don't have to keep redefining them.
> 
> Followup bug(s)?

Sure.
I'll probably work on getting mozharness landed in build/mozharness first (trying to throttle the non-urgent patches I'm sending you, honest!) and then file followup bugs for important TODOs.

Thanks Coop!
> > >+        ausPreviousUploadDir = "%s/%s/%s/%%(previous_buildid)s/en-US/beta-cck-test" % \
> > Is that really our channel name? Sigh.
> Yes, and it's actually the best-sounding solution to the problem atm.

Don't forget the app.update.url.override, where we copy from app.update.url and s/%CHANNEL%/betatest/.
(In reply to comment #12)
> > > >+        ausPreviousUploadDir = "%s/%s/%s/%%(previous_buildid)s/en-US/beta-cck-test" % \
> > > Is that really our channel name? Sigh.
> > Yes, and it's actually the best-sounding solution to the problem atm.
> 
> Don't forget the app.update.url.override, where we copy from app.update.url and
> s/%CHANNEL%/betatest/.

Right, getting the long app.update.url copied and edited properly on device seems tricky, but I can ask QA if that's doable.
Comment on attachment 485811 [details] [diff] [review]
mozharness diffs for 4.0b2

PRODUCTION tag moved: http://hg.mozilla.org/users/asasaki_mozilla.com/mozharness/rev/f1f10f3cf9be

(getting review to land in build/mozharness on my post-4.0b2 shortlist)
Attachment #485811 - Flags: checked-in+
> if you switch the buildbotcustom patch to only run addUpdateSteps once, you can
> consider that an r+.

7444         if not self.multiLocale and self.createSnippet:
7445             self.addUpdateSteps()

done.
Marking r+.
Attachment #485815 - Attachment is obsolete: true
Attachment #485972 - Flags: review+
Filed bug 608851 for mozharness windows rmtree.
Filed bug 608854 for finishing the multil10n.py script.
Bug 608430 is tracking the issues we ran into for Android uploads from 4.0b2, and I'm going to rename disableMultiLocale -> enableMultiLocale there.
Bug 574473 tracks getting mozharness into build/mozharness.

I think I've tracked all the relevant followup work noted here; resolving.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.