560911 - add android build steps to the build environment

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Mike Taylor [:bear]]

Create the factories and staging config items to start producing binaries

Comment 1

[Mike Taylor [:bear]]

Attachment: [WIP] add android to build steps

Comment 2

[Mike Taylor [:bear]]

Attachment: [WIP] add android to build steps

Comment 3

[Aki Sasaki (not active)]

Comment on attachment 440820 [details] [diff] [review]
[WIP] add android to build steps

>diff --git a/mobile-rw/config-production.py 
b/mobile-rw/config-production.py

We don't have any tests for android yet, and they're going to be different from non-SUT tests, so don't touch mobile-rw*/ or mobile*/ at all.

The builders should only be in mozilla2*/

>diff --git a/mozilla2-staging/config.py b/mozilla2-staging/config.py
>--- a/mozilla2-staging/config.py
>+++ b/mozilla2-staging/config.py
>@@ -191,17 +191,17 @@ PLATFORM_VARS = {
>                 'CHOWN_ROOT': '~/bin/chown_root',
>                 'CHOWN_REVERT': '~/bin/chown_revert',
>             },
>             'enable_opt_unittests': True,
>             'enable_checktests': True,
>             'talos_masters': GLOBAL_VARS['talos_masters'],
>         },
>         'macosx64': {
>-            'base_name': 'OS X 10.6 %(branch)s',
>+            'base_name': 'OS X 10.6.2 %(branch)s',

Is this an intentional change?

>+MOBILE_BRANCHES['mobile-trunk']['platforms']['android-r7']['base_name'] = 'Android Fennec Desktop mozilla-central'

"Desktop" here means non-mobile platform, e.g. desktop OSX.

Probably should be "Android Fennec mozilla-central"
       buildsBeforeReboot=pf['builds_before_reboot'],
>+                sb_target=pf.get('sb_target', ''),  #BEAR

Not sure why you have a scratchbox target if you aren't building in scratchbox ?

>+                mobileObjdir=pf.get('mobile_objdir', 'mobile'),
>+                xulrunnerObjdir=pf.get('xulrunner_objdir', 'xulrunner'),

Let's not use these.  Remove... let's not build xulrunner first.

>+                packageGlobList=pf.get('glob_list', ['mobile/dist/*.tar.bz2',
>+                                                     'xulrunner/dist/*.tar.bz2']),
>+                debs=pf.get('debs', False),

Your upload list should probably be something like dist/*.apk right?

Where is that built?

Why are we referencing debs here?

>+            mobile_nightly_factory = MaemoBuildFactory(

You probably want to use the same Android factory for your nightlies, but have clobber/nightly set to True.

Comment 4

[Aki Sasaki (not active)]

Comment on attachment 440821 [details] [diff] [review]
[WIP] add android to build steps

>+        elif platform.startswith("android"):
>+            packageFilename = '*.firefox.apk'

What is the package called at the end?

>+# Largely built from PackagedUnittests
>+class AndroidUnittestFactory(BuildFactory):

This is most likely invalid.
This bug is about the builds, not unit tests...
You probably don't have to touch mobiletestfactory.py at all on this patch.

Where is the AndroidBuildFactory you imported in the previous patch?  I doubt these patches pass checkconfig, correct?

Comment 5

[Mike Taylor [:bear]]

Attachment: [WIP] add Android to buildbot-configs

Comment 6

[Mike Taylor [:bear]]

Attachment: [WIP] add android to build steps

Comment 7

[Mike Taylor [:bear]]

Attachment: [WIP] add Android to buildbot-configs

Comment 8

[Aki Sasaki (not active)]

Comment on attachment 441833 [details] [diff] [review]
[WIP] add android to build steps

This looks good; pretty much a straight copy of the MobileDesktopBuildFactory afaict.

Something you might want to do is set some defaults in this line:

def __init__(self, uploadPlatform=None, **kwargs):

like

def __init__(self, uploadPlatform='linux', packageGlobList=['dist/*.apk'], **kwargs):

... overridable, but sensible defaults.

Comment 9

[Aki Sasaki (not active)]

Comment on attachment 441834 [details] [diff] [review]
[WIP] add Android to buildbot-configs

>+MOBILE_BRANCHES['mobile-trunk']['platforms']['android-r7']['mozconfig'] = 'android/mozilla-central/nightly'

This is good, but you didn't actually create one. Or if you did, you didn't |hg add mozilla2-staging/android| so it shows up in the patch.  Also, I'm wondering if it should be mobile-browser rather than mozilla-central, since mozilla-central might imply Firefox desktop rather than Fennec mobile.

>+MOBILE_BRANCHES['mobile-trunk']['l10n_platforms']['android-r7'] = 'linux'

This line will actually start triggering repacks for Android, which we definitely aren't ready for. Either comment this out or remove it til we are ready :)

>diff --git a/mozilla2-staging/mobile_master.py b/mozilla2-staging/mobile_master.py
>--- a/mozilla2-staging/mobile_master.py
>+++ b/mozilla2-staging/mobile_master.py
>+    'android-r7': MOBILE_SLAVES['android-r7'],

This is the only line you added to mobile_master.py, which means you didn't

a) import AndroidBuildFactory
b) create an Android mobile_dep_factory or mobile_nightly_factory (search for MaemoBuildFactory to see how that's done for Maemo, but lean towards the config settings for MobileDesktop).

This is why you're getting the test-masters exception; it's finding android in mobile_config and putting it in the scheduler, but not creating a builder for it in the for loop starting at line 168.

This isn't too far away though -- once you add those you'll be able to checkconfig, and then we'll get you a staging env to test a build or ten on :D

Comment 10

[Mike Taylor [:bear]]

(In reply to comment #8)
> (From update of attachment 441833 [details] [diff] [review])
> This looks good; pretty much a straight copy of the MobileDesktopBuildFactory
> afaict.

yep - it is an exact copy to be honest...

> Something you might want to do is set some defaults in this line:
> 
> def __init__(self, uploadPlatform=None, **kwargs):
> 
> like
> 
> def __init__(self, uploadPlatform='linux', packageGlobList=['dist/*.apk'],
> **kwargs):
> 
> ... overridable, but sensible defaults.

I'll add some now

Comment 11

[John Ford [:jhford] CET/CEST Berlin Time]

(In reply to comment #10)
> (In reply to comment #8)
> > (From update of attachment 441833 [details] [diff] [review] [details])
> > This looks good; pretty much a straight copy of the MobileDesktopBuildFactory
> > afaict.
> 
> yep - it is an exact copy to be honest...

if it is an exact copy could we rename MobileDesktopBuildFactory to MobileGeneralBuildFactory or something similar and avoid the duplication?

Comment 12

[Aki Sasaki (not active)]

(In reply to comment #11)
> if it is an exact copy could we rename MobileDesktopBuildFactory to
> MobileGeneralBuildFactory or something similar and avoid the duplication?

Pretty sure we're going to be adding some Android signing steps at some point.

Comment 13

[John Ford [:jhford] CET/CEST Berlin Time]

when we do that we could subclass MobileGeneralBuildFactory to do the special signing steps.

Comment 14

[Mike Taylor [:bear]]

Once I figure out the differences I'll poke you John to work on the refactoring.

Comment 15

[Mike Taylor [:bear]]

Attachment: [WIP] add Android to buildbot-configs

Comment 16

[Mike Taylor [:bear]]

Attachment: [WIP] add android to build steps

Comment 17

[Mike Taylor [:bear]]

Attachment: [WIP] add Android to buildbot-configs

Comment 18

[Mike Taylor [:bear]]

Attachment: [WIP] add Android to buildbot-configs

Comment 19

[Aki Sasaki (not active)]

Comment on attachment 441940 [details] [diff] [review]
[WIP] add Android to buildbot-configs

This looks ready for testing.

Comment 20

[Mike Taylor [:bear]]

build is showing all green but with two work arounds:

1 - make package is not in place so hacked steps to run manually
2 - make upload is hacked to handle location of apk build from 1 above

Comment 21

[(not currently active) Ted Mielczarek]

We should be fixing stuff like "make package" and "make upload" in the build system if they're broken. I see you've already filed a bug on "make package", please file one on "make upload" too, if you need that to work.

Comment 22

[Mike Taylor [:bear]]

(In reply to comment #21)
> We should be fixing stuff like "make package" and "make upload" in the build
> system if they're broken. I see you've already filed a bug on "make package",
> please file one on "make upload" too, if you need that to work.

I'm not sure if a "make upload" bug will be required if they "make package" work follows the normal patterns

Comment 23

[Mike Taylor [:bear]]

Attachment: add android to build steps

required changes to enable Android builds, albeit with some work-arounds to handle "make package" and "make upload" issues

Comment 24

[Mike Taylor [:bear]]

Attachment: add Android to buildbot-configs

changes required to enable Android builds

Comment 25

[Mike Taylor [:bear]]

Attachment: add Android to buildbot-configs

remove --disable-tests

Comment 26

[Aki Sasaki (not active)]

Comment on attachment 442806 [details] [diff] [review]
add Android to buildbot-configs

>+ac_add_options --disable-tests

We are going to need tests if we're going to be running unit tests soon.

You can enable tests by removing this line (it's on by default) and running the make package-tests step (assuming this works, which might not be a good assumption).

If make package-tests doesn't work, we need to file a bug that blocks the unit tests/talos bug 561908.

> ### mozilla-central
> MOBILE_BRANCHES['mobile-trunk']['main_config'] = config.BRANCHES['mozilla-central']
>-MOBILE_BRANCHES['mobile-trunk']['repo_path'] = 'mozilla-central'
>+MOBILE_BRANCHES['mobile-trunk']['repo_path'] = 'users/vladimir_mozilla.com/mozilla-droid'#'mozilla-central'
>+MOBILE_BRANCHES['mobile-trunk']['revision'] = 'android'  #required only for android staging build #hackalert

Ok, this was good for your single build, but now you're saying *all* mobile trunk builds are going to use vlad's repo instead of m-c.  Grounds for r- right here.

So you need to either:

a) set repo_path at the platform level for Android, and check for it in mobile_master.py when you create the AndroidBuildFactory builders, or
b) Create a MOBILE_BRANCHES['android-trunk'] that only has android as a platform.

Both of these are workarounds until the user repo can land in m-c.


>+        elif 'android' in platform:
>+            mobile_dep_factory = AndroidBuildFactory(
>+                hgHost=mainConfig['hghost'],
>+                repoPath=branch['repo_path'],

This line is what you would change ^^, potentially to pf.get('repo_path', branch['repo_path'])

>+                packageGlobList=pf.get('glob_list', ['dist/*.apk',]),

You're specifying 'dist/*.apk' here as your packageGlobList, then hardcoding '../embedding/android/fennec.apk' in your actual upload step.

Same nits for the nightly builder.

Also, you're missing the production changes (mozilla2/).

Comment 27

[Aki Sasaki (not active)]

Comment on attachment 442808 [details] [diff] [review]
add Android to buildbot-configs

Same as the previous patch with the following changes:

* you have tests enabled (good)
* i don't think you've tested that it works with tests enabled

Comment 28

[Aki Sasaki (not active)]

Comment on attachment 442805 [details] [diff] [review]
add android to build steps

>+class AndroidBuildFactory(MobileBuildFactory):
>+    def __init__(self, uploadPlatform='linux',
>+                 packageGlobList=['embedding/android/*.apk'], **kwargs):

You did update the packageGlobList :)
But only in one location, and then you didn't use self.packageGlob in the upload step.

>+    def addPackageSteps(self):
>+        if self.env is None:
>+            envJava = {}
>+        else:
>+            envJava = self.env.copy()
>+
>+        envJava['PATH'] = '/tools/jdk6/bin:%s' % envJava.get('PATH', os.environ['PATH'])

Ugh.
Could you add a comment that references the 'make package' bug # and that this ugliness will go away once that's fixed?

>+        #self.addStep(ShellCommand,
>+        #    name='make_pkg_tests',
>+        #    command=['make', 'package-tests'],
>+        #    workdir='%s/%s/%s' % (self.baseWorkDir,
>+        #        self.branchName, self.objdir),
>+        #    env=self.env,
>+        #    haltOnFailure=True,
>+        #)

Again, we should see if make package-tests works.

If yes: do it, and upload the tests (this will require a packageGlob update)
If no: file another bug since we need 'make package-tests' for unit testing.

>+
>+    def addUploadSteps(self, platform):
>+        self.addStep(SetProperty,
>+            name="get_buildid",
>+            command=['python', 'config/printconfigsetting.py',
>+                     'dist/bin/application.ini',
>+                     'App', 'BuildID'],
>+            property='buildid',
>+            workdir='%s/%s' % (self.baseWorkDir, self.branchName),
>+            description=['getting', 'buildid'],
>+            descriptionDone=['got', 'buildid']   
>+        )
>+        self.addStep(MozillaStageUpload,
>+            name="upload_to_stage",
>+            description=['upload','to','stage'],
>+            objdir=self.branchName,
>+            username=self.stageUsername,
>+            milestone=self.baseUploadDir,
>+            remoteHost=self.stageServer,
>+            remoteBasePath=self.stageBasePath,
>+            platform=platform,
>+            group=self.stageGroup,
>+            packageGlob='../embedding/android/fennec.apk',
>+            sshKey=self.stageSshKey,
>+            uploadCompleteMar=False,
>+            releaseToLatest=self.nightly,
>+            releaseToDated=self.nightly,
>+            releaseToTinderboxBuilds=True,
>+            tinderboxBuildsDir=self.baseUploadDir,
>+            remoteCandidatesPath=self.stageBasePath,
>+            dependToDated=True,
>+            workdir='%s/%s/%s' % (self.baseWorkDir, self.branchName,
>+                                        self.objdir)
>+        )

Hm, did you actually have to override this, or did you just have to change your packageGlob to be accurate? (Did you actually change anything else?)


Also -- a missing point on the configs -- turn off the triggers.  As in set the builder trigger configs to False.

Comment 29

[Aki Sasaki (not active)]

We've got the bugs, but I think we should go ahead with new patches with no package-tests... we shouldn't block nightlies on those.

Comment 30

[Mike Taylor [:bear]]

Attachment: add android to build steps

Comment 31

[Mike Taylor [:bear]]

Attachment: add Android to buildbot-configs

Comment 32

[Aki Sasaki (not active)]

Comment on attachment 443119 [details] [diff] [review]
add android to build steps

>+class AndroidBuildFactory(MobileBuildFactory):
>+    def __init__(self, uploadPlatform='linux',
>+                       packageGlobList=['dist/*.apk',], **kwargs):

I thought this was ../embedding/android/*.apk, and dist/*.apk failed.  No?


>+        self.addStep(ShellCommand,
>+           name='make_pkg_tests',
>+           command=['make', 'package-tests'],
>+           workdir='%s/%s/%s' % (self.baseWorkDir,
>+               self.branchName, self.objdir),
>+           env=self.env,
>+           haltOnFailure=True,
>+        )

I thought this failed.  Please remove; we want a working Android build.

Comment 33

[Aki Sasaki (not active)]

Comment on attachment 443120 [details] [diff] [review]
add Android to buildbot-configs

This appears to be the factory patch, not the configs patch.

Comment 34

[Mike Taylor [:bear]]

Attachment: add Android to buildbot-configs

mozilla2-staging and mozilla2 changes

Comment 35

[Mike Taylor [:bear]]

Attachment: add android to build steps

changes to factory for Android with make package and make package-tests work-arounds and also a hack to allow upload step behave until the package bugs are fixed

Comment 36

[Aki Sasaki (not active)]

Comment on attachment 443129 [details] [diff] [review]
add android to build steps

>+    def addUploadSteps(self, platform):
>+        self.addStep(SetProperty,
>+            name="get_buildid",
>+            command=['python', 'config/printconfigsetting.py',
>+                     'dist/bin/application.ini',
>+                     'App', 'BuildID'],
>+            property='buildid',
>+            workdir='%s/%s' % (self.baseWorkDir, self.branchName),
>+            description=['getting', 'buildid'],
>+            descriptionDone=['got', 'buildid']   
>+        )
>+        self.addStep(MozillaStageUpload,

We're very close.

Three things:

1) pet peeve nit: trailing whitespace after the descriptionDone.

2) Looks like the only reason you need to override addUploadSteps is you're not setting an objdir, correct?  I'd very much like to see this be an objdir build... worth a shot in staging while we're at it.

3) If that works, we can not override addUploadSteps at all, unless I'm missing something.

Comment 37

[Aki Sasaki (not active)]

Comment on attachment 443128 [details] [diff] [review]
add Android to buildbot-configs

>diff --git a/mozilla2-staging/android/mobile-browser/nightly/mozconfig b/mozilla2-staging/android/mobile-browser/nightly/mozconfig
>new file mode 100644
>--- /dev/null
>+++ b/mozilla2-staging/android/mobile-browser/nightly/mozconfig
>@@ -0,0 +1,41 @@
>+mk_add_options JAVAC=/tools/jdk6/bin/javac

objdir...  but you know that :)

>+            nightlyWorkDir  = pf['base_workdir']  + '-nightly'

I think this ends up with a android-trunk-nightly/build-nightly, which I'd
rather not have.  A lot of our clobbery-goodness assumes a directory of
"build" and anything else requires additional hackery.

Noticed this b/c I logged into linux-slave17 to doublecheck apk locations.

>+            nightlyWorkDir  = pf['base_workdir']  + '-nightly'

same here.

>+                triggerBuilds = True,
>+                triggeredSchedulers=triggeredSchedulers,

We don't have any triggeredBuilds or triggeredSchedulers yet, so these should probably be False.

That's something you want to watch out for -- unless there's something you explicitly want turned off in staging but on in production, your production/staging configs should be the same.

But again, very very close. Objdir is the main thing, and that just came up today.

Comment 38

[Mike Taylor [:bear]]

Attachment: add Android to buildbot-configs

Comment 39

[Mike Taylor [:bear]]

Attachment: add android to build steps

Comment 40

[Mike Taylor [:bear]]

Attachment: add Android to buildbot-configs

Comment 41

[Mike Taylor [:bear]]

Attachment: add android to build steps

Comment 42

[Mike Taylor [:bear]]

Attachment: add Android to buildbot-configs

helps if you s/android/android2/ in all of the spots

Comment 43

[Aki Sasaki (not active)]

Comment on attachment 443446 [details] [diff] [review]
add android to build steps

http://hg.mozilla.org/build/buildbotcustom/rev/819ba16f21c8

Comment 44

[Aki Sasaki (not active)]

Comment on attachment 443465 [details] [diff] [review]
add Android to buildbot-configs

http://hg.mozilla.org/build/buildbot-configs/rev/f3bf0ac7bcee

(staging only)

Comment 45

[Aki Sasaki (not active)]

Bear: if you're hitting broken compilations, you can temporarily change your android2 mozRevision to something known&good (mobileRevision might work too, for m-b)

Comment 46

[Aki Sasaki (not active)]

I believe that:

 - attachment 443465 [details] [diff] [review] is obsoleted by bug 562842
 - this bug is fixed