557260 - use MercurialBuildFactory for mobile builds

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

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

Once we can build Fennec like we build firefox, we should look at trying to use the MercurialBuildFactory to do mobile builds.

Two major roadblocks:
1. scratchbox needs to be transparent to MercurialBuildFactory
2. mobile-browser checkouts need to be done

Comment 1

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

going to be looking at this while working on bug 523946

Comment 2

[Aki Sasaki (not active)]

I'm semi-duplicating a lot of NightlyBuildFactory in Mobile/Android build factories to hack in Android updates.

If we move to MercurialBuildFactory, we should also look into using NightlyBuildFactory.

Comment 3

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

Sounds like a good idea!

If we were to add mobile style builds as an option to the base class __init__ method, we could have the source steps understand the difference between.  Beyond the extra source steps we are going to either have to figure out the scratchbox issue (somewhere I have a ShellCommand that made this easier) or use the current factories for maemo builds.

If we do this, we'd be moving android from mobile_platforms to platforms most likely, and if we do that we would need to differentiate between android-fennec and android-firefox (should we start producing that).  The easiest way would probably be to make the platform be 'android-phone'/'android-tablet' or 'android-firefox'/'android-fennec'.  This would be the same for linux, win32 and macosx desktop builds.

Comment 4

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

Attachment: simple graphserver patch to allow these builds to go green in staging

Comment 5

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

Attachment: add a scratchbox command that makes scratchbox (mostly) invisible

This is a patch that adds a ShellCommand derived class to make scratchbox mostly invisible.  For non-scratchbox using platforms, it is essentially a renamed ShellCommand.  For scratchbox using platforms, we take the workdir and command arguments and create a command string that otherwise works.  The workdir can be a string or WithProperties and the command can be a list, a string or a single WithProperties.  If the command is a list, it can contain WithProperties or strings.

The workdir kwarg is mandatory because it is not easy/possible to figure this out for the platforms that use scratchbox.

The code needed in generateBranchObjects to use these options is

+            mobile=pf.get('enable_mobile', False),
+            mobileRepoPath=pf.get('mobile_repo_path'),
+            use_scratchbox=pf.get('use_scratchbox'),
+            scratchbox_target=pf.get('scratchbox_target'),

and each ScratchboxCommand instance uses the basedir property to get the correct workdir, similar to

+         workdir=WithProperties('%(basedir)s/build'),

While build is hardcoded, it is an assumption made is so many places that I feel like it is not particularly egregious.

Comment 6

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

Attachment: [wip] this is a work in progress buildbot-configs patch

wip patch which is able to do dep/nightly builds for linux desktop fennec builds as well as run to the compile step for maemo.  There is tons left to do here before it is even feedback? ready, but thought i'd post wip patch for the curious.

Comment 7

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

Attachment: [wip] this is a work in progress buildbotcustom patch

like the buildbot-configs patch, this is not ready for review/feedback? yet, so mainly up here for the curious.

should have mentioned in the configs patch that the change to OBJDIR is not a needed/desired thing, it was a quick debugging thing that i haven't yet reverted

Comment 8

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

(In reply to comment #5)

> The workdir kwarg is mandatory because it is not easy/possible to figure this
> out for the platforms that use scratchbox.

wait, shouldn't I be able to access the properties in the .start method?

Comment 9

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

Tested this with the staging clobberer and it does clobber properly


Checking clobber URL: http://build.mozilla.org/stage-clobberer/index.php?master=http%3A%2F%2Fstaging-master.build.mozilla.org%3A8011%2F&slave=linux-ix-slave02&builddir=cen-linuxmaemo&branch=mozilla-central&buildername=Maemo+5+GTK+mozilla-central+build
cen-linuxmaemo:Our last clobber date:  2011-02-09 10:57:13
cen-linuxmaemo:Server clobber date:    2011-02-15 09:29:47
cen-linuxmaemo:Server is forcing a clobber, initiated by [e-mail]
cen-linuxmaemo:Clobbering...
Skipping tools
Skipping last-clobber
Removing codesize-auto-old.log
Removing codesize-auto.log
Removing build/
Removing codesize-auto-diff.log

Comment 10

[Chris AtLee [:catlee]]

Comment on attachment 511110 [details] [diff] [review]
add a scratchbox command that makes scratchbox (mostly) invisible

So I think this looks ok...one question is how does passing environment variables down into scratchbox work?

Comment 11

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

(In reply to comment #10)
> Comment on attachment 511110 [details] [diff] [review]
> add a scratchbox command that makes scratchbox (mostly) invisible
> 
> So I think this looks ok...one question is how does passing environment
> variables down into scratchbox work?

that is the -k argument to moz_scratchbox

Comment 12

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

Attachment: buildbot-configs patch on 3606:69e900bf01ea

This is a patch on 3606:69e900bf01ea .  Shelving these patches while aki implements mozharness based builds.

Comment 13

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

Attachment: buildbotcustom patch on 1299:cd7e22d28d84

Buildbot custom changes required to integrate mobile builds into mercurial build factory

Comment 14

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

marking this bug as WONTFIX as aki thinks mozharness will be ready to replace this in a month.

Comment 16

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

We decided today that we are going to move forward with this work and use it for our mobile builds.

adding 600317 as dep because it adds stage_platform support

Comment 17

[Aki Sasaki (not active)]

* multilocale
** This requires mozharness, 2 passes of uploads. At least one of the uploads needs to go in a subdirectory.
** make package/upload steps require different logic depending if it's multilocale.
** This may require small mozharness tweaks, probably mostly dealing with paths.

* 4.0.x support
** Mobile branch polling
** Clone mobile branch, show mobile changeset link if appropriate

* Android
** We don't use MARs for Android updates; we use the apk. Maybe we can override the default NightlyBuildFactory logic inside of AndroidBuildFactory ?
** The snippets go in a different update path than Firefox update snippets, but that should hopefully be easily configurable.
** Android currently requires envJava to be set during the build (can be config driven).  However, it needs JARSIGNER to be in the env during packaging, *but this cannot be set during the build*.
*** This is a makefile bug that should be fixed, but until it is, we can't build with the same env all the way through.

* Uploads
** Latest dir isn't latest-BRANCH, it's latest-BRANCH-PLATFORM.
** Candidates dir isn't nightly/, it's candidates/.
** I believe the above is an improvement over desktop logic, and would argue that if one changes, it should be desktop.

* Rollout
** As mentioned before, we probably have to clobber everything when we first roll out.  We can test by taking an existing dep build directory and switching factories and trying to build without a clobber, but it's probably safest to clobber.

* Try
** Uploads and email are different -- we should make sure this still works.
** Trychooser changes?

* Misc
** Symbols go in a different directory tree, but that should be easily configurable.
** If you happen to solve bug 592078 while you're here (rename android-r7 to android) I won't argue.  Not a blocker.
** If you happen to solve bug 592082 while you're here (refactor mobile mozconfigs; don't forget mobile-tryserver/ =P ) I won't argue.  Not a blocker.


Builds: linux, win32, osx, android, maemo5 {gtk,qt}. Android-debug is coming.
Linux, win32, osx trigger repacks.
Android (and android-debug) trigger tests... IIRC maemo still polls.
Android and maemo5* will have multilocale nightlies on certain branches.

I *think* that's it.

Comment 18

[Aki Sasaki (not active)]

** If you happen to solve bug 648399 while you're here (send mobile builds/tests to the Firefox tinderbox page) I won't argue.  Not a blocker.

Comment 20

[Aki Sasaki (not active)]

* For nightly single locale repacks (l10n), we need to not break what's in generateMobileBranchObjects currently (run tools scripts for linux/macosx/win32 mobile desktop nightlies).  Anything else single-locale related is outside the scope of this bug.

I didn't include that in comment 17 because I was thinking "what might break if we change MobileBuildFactory?" and not "what might break if we get rid of generateMobileBranchObjects?".  Hopefully this doesn't complicate things too much more.

Comment 21

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

Not addressing everything quite yet, but I had a couple questions and comments, so I am replying to the points that I have questions or comments for.

(In reply to comment #17)>

> * Uploads
> ** Latest dir isn't latest-BRANCH, it's latest-BRANCH-PLATFORM.

addressed, example http://staging-stage.build.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux-android/

Using /firefox/ in there is because i haven't switched that over yet, it is planned to remain as /mobile/ in final patches.

> ** Candidates dir isn't nightly/, it's candidates/.
> ** I believe the above is an improvement over desktop logic, and would argue
> that if one changes, it should be desktop.

Both of these are outside the scope of this bug and should be decided/implemented in the bug that moves release automation to the work from this bug

> * Rollout
> ** As mentioned before, we probably have to clobber everything when we first
> roll out.  We can test by taking an existing dep build directory and switching
> factories and trying to build without a clobber, but it's probably safest to
> clobber.

I *think* everything should be getting a different build directory, and if that is the case, the old builds will time off after purge old builds threshold is exceeded or the drive fills up and they get clobbered.  During the rollout of this patch, old maemo build dirs will become unused and will remain un-clobberable.  Once this patch is deployed we can rm -rf the appropriate build dirs.  Maybe through puppet?

> * Try
> ** Uploads and email are different -- we should make sure this still works.

Because this is using the standard desktop configuration logic, we should be ok here.  It *should* use whatever we have implemented for desktop.  Try is not my top priority right now, but I do plan to test all of this for the final patch.

> ** Trychooser changes?

Spoke briefly with Lukas yesterday and we think that these platforms should work, with some ugliness.  I will look at the code and add some mappings so that the 'linux-android' platform can be used as 'android'.  The name 'linux-android' is required because of how our automation requires the platform string to start with 'linux' if you want to get linux style automation.

>
> * Misc
> ** If you happen to solve bug 592078 while you're here (rename android-r7 to
> android) I won't argue.  Not a blocker.

Already done, very very easy

> ** If you happen to solve bug 592082 while you're here (refactor mobile
> mozconfigs; don't forget mobile-tryserver/ =P ) I won't argue.  Not a blocker.

Already done, very easy and this cleans things up by allowing us to use %(branch)s like desktop

> Builds: linux, win32, osx, android, maemo5 {gtk,qt}. Android-debug is coming.


tested so far: linux, android, maemo5-gtk.  Others should be easy to test

> Linux, win32, osx trigger repacks.

so single locale for linux, win32 and osx?  It looks like the win32 builds/repacks are broken right now.  I will see if I can fix them during this, but will not block on them

> Android (and android-debug) trigger tests... IIRC maemo still polls.

Maemo still polls and isn't worth the effort to switch at this point.  A sample unit test and talos sendchange are pasted below

    master: staging-master.build.mozilla.org:9009
    branch: mozilla-central-linux-talos
    revision: c7f62d818af25b3e62e2f0c35b27214a4209a898
    comments: 
    user: sendchange
    files: ['http://staging-stage.build.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-android/1304102243/fennec-6.0a1.en-US.eabi-arm.apk']     properties: [('buildid', '20110429113723'), ('builduid', u'84036b3ded8142ad9f9fdb13db9f99e9')]

    master: staging-master.build.mozilla.org:9009
    branch: mozilla-central-linux-android-opt-unittest
    revision: c7f62d818af25b3e62e2f0c35b27214a4209a898
    comments: 
    user: sendchange-unittest
    files: ['http://staging-stage.build.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-android/1304102243/fennec-6.0a1.en-US.eabi-arm.apk', 'http://staging-stage.build.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-android/1304102243/fennec-6.0a1.en-US.eabi-arm.tests.zip']     properties: [('buildid', '20110429113723'), ('builduid', u'84036b3ded8142ad9f9fdb13db9f99e9')]


> I *think* that's it.

(In reply to comment #18)
> ** If you happen to solve bug 648399 while you're here (send mobile
> builds/tests to the Firefox tinderbox page) I won't argue.  Not a blocker.

Already done by me not adding special cases for mobile to go to a different tinderbox tree

(In reply to comment #20)
> * For nightly single locale repacks (l10n), we need to not break what's in
> generateMobileBranchObjects currently (run tools scripts for linux/macosx/win32
> mobile desktop nightlies).  Anything else single-locale related is outside the
> scope of this bug.

I am pretty much copying and pasting the the single locale config logic and modifying it as needed to make sure it works with the new config style

> I didn't include that in comment 17 because I was thinking "what might break if
> we change MobileBuildFactory?" and not "what might break if we get rid of
> generateMobileBranchObjects?".  Hopefully this doesn't complicate things too
> much more.

Nope, shouldn't.

Comment 22

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

(In reply to comment #21) 
> > * Uploads
> > ** Latest dir isn't latest-BRANCH, it's latest-BRANCH-PLATFORM.
> 
> addressed, example
> http://staging-stage.build.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux-android/


Sorry, should read as.  Previous link is left over from before the stage_platform patch merged into my working copies.

http://staging-stage.build.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-android/

Comment 23

[Aki Sasaki (not active)]

(In reply to comment #21)
> Not addressing everything quite yet, but I had a couple questions and comments,
> so I am replying to the points that I have questions or comments for.

Ok.

> 
> > * Rollout
> > ** As mentioned before, we probably have to clobber everything when we first
> > roll out.  We can test by taking an existing dep build directory and switching
> > factories and trying to build without a clobber, but it's probably safest to
> > clobber.
> 
> I *think* everything should be getting a different build directory, and if that
> is the case, the old builds will time off after purge old builds threshold is
> exceeded or the drive fills up and they get clobbered.  During the rollout of
> this patch, old maemo build dirs will become unused and will remain
> un-clobberable.  Once this patch is deployed we can rm -rf the appropriate
> build dirs.  Maybe through puppet?

We'll probably have problems with disk space on the VMs at the very least if we don't do something to clobber.

> > * Try
> > ** Uploads and email are different -- we should make sure this still works.
> 
> Because this is using the standard desktop configuration logic, we should be ok
> here.  It *should* use whatever we have implemented for desktop.  Try is not my
> top priority right now, but I do plan to test all of this for the final patch.

Try needs to not break when we roll it out, so good :)

> > ** Trychooser changes?
> 
> Spoke briefly with Lukas yesterday and we think that these platforms should
> work, with some ugliness.  I will look at the code and add some mappings so
> that the 'linux-android' platform can be used as 'android'.  The name
> 'linux-android' is required because of how our automation requires the platform
> string to start with 'linux' if you want to get linux style automation.

I'm not thrilled with the name linux-android, but I haven't really dug into the specifics.

Comment 24

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

Mobile MercurialBuildFactory Blockers

Desktop builds:
-need to verify that latest-branch-platform directories are populated
--alternative (maybe better?) approach would be to use properties to not rely on latest-* directores having correct content
-linux builds working, needs some polish
-win32/osx to be tested, they aren't difficult to test

Scratchbox builds:
-since my last rebase to latest buildbotcustom/configs scratchbox support broke, need to figure out why.
-before this broke, the only thing blocking this was multi-l10n

Android builds:
-building + signing properly, needs same polish as desktop
-blocked on multi-l10n like maemo
-need to look into android snippet creation


General Polish Issues:
-builds are uploading into /pub/mozilla.org/firefox/ instead of /pub/mozilla.org/mobile/

Comment 25

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

(In reply to comment #24)
> Mobile MercurialBuildFactory Blockers
> 
> Desktop builds:
> -need to verify that latest-branch-platform directories are populated

This is controlled by the -b parameter of post_upload.py.  Because desktop behaviour is to use 'branch' and mobile is to use 'branch-platform', I see the options being:

-standardize on one and inform everyone of the change
-adding an option to buildbotcustom.process.factory.postUploadCmdPrefix that includes arbitrary text as a suffix to the branch
-create a -p option on post_upload.py that understands how to map platforms.

I don't think that the first option is at all in scope of this bug and I feel like option three is on the scope-creepish side, especially considering that mobile currently is implementing option two.  I will be moving forward with option two.

Comment 26

[Aki Sasaki (not active)]

Attachment: rm dist/fennec* in mozharness

Comment 27

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

Attachment: buildbot-configs v1

- fix mozconfigs
- mark a bunch of variables that are only needed for legacy code
- add mobile platforms to 'platforms' dict
- add stage_product to each platform
- block mobile_platforms being set on all branches other than mozilla-2.1 and try where there isn't yet support
- need to disable android/maemo nightlies for mozilla-central still

Comment 28

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

Attachment: buildbotcustom v1

- adds scratchbox shellcommand and setproperty classes that understand how to properly mangle command and workdir to run scratchbox commands transparently
- adds branch variable 'stage_product' which is what the top level directory on stage is.  currently firefox or mobile.
- change 'stage_base_path' to mean the part of the stage path that does *not* include the product (i.e. the old meaning but without firefox)
  - this means that xulrunner's stage_base_path doesn't need to be a second variable
- add android signing logic to the packaging steps, eventually this will be fixed in m-c
- adds the ability to let a platform optionally use 'branch-platform' formatting on stage because that's what mobile does.
- Moves make package before make package-tests.  This is flawed but is a needed workaround while we fix the makefile
- log uploading changes:
  - use the 'stage_platform' variable to decide which platform to upload to.
  - use 'stage_product' to decide where logs go
- Allow a platform to optionally use mobile l10n script style repack logic instead of plain desktop (copied from generateMobileBranchObjects)

Comment 29

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

Attachment: tools v1

- rework the mobile repack script slightly

Comment 30

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

A patch that adds the pseudo-machines to graphserver still needs to be written

Comment 31

[[e-mail] (:bhearsum)]

Comment on attachment 531264 [details] [diff] [review]
tools v1

Your comment about looking for a platforms key, and falling back to checking mobile_platforms is much better than the current code. Please do it!

Comment 32

[[e-mail] (:bhearsum)]

Comment on attachment 531262 [details] [diff] [review]
buildbot-configs v1

The "remove mobile-mercuriabuildfactory" comments aren't self explanatory to me, can you change them to be?

mobile_platforms don't need to be set for any of the branches that match the default (eg, mozilla-central through to mozilla-2.0). Also, please drop the commented out lines around the default of this variable.

You've got references to jford_mozilla.com still, please get rid of those :)

Please use "hg copy" on the mozconfigs, it makes review & history much clearer.

MOZ_SYMBOLS_EXTRA_BUILDID still needs to get set somewhere, per-branch, or android-r7 symbols from different branches could get uploaded to the same place.

Do you need to set mozharness_config and/or enable_mobile_dep somewhere, too?

Please make this comment more verbose:
>     PROJECT_BRANCHES[branch]['mobile_platforms'] = {} # NO!

Can you explain why deleting platforms from BRANCHES['try'] helps TryBuildFactory "understand the new stage_product and stage_platform variables as well as understanding how to do some commands scratchboxishly", as well as why we need a workaround for try at all?

Comment 33

[[e-mail] (:bhearsum)]

Comment on attachment 531263 [details] [diff] [review]
buildbotcustom v1


>+        codesighs = pf.get('enable_codesighs', config.get('enable_codesighs',True))

This breaks assumptions about how config and pf interact. Namely that enable_codesighs in 'pf' takes precedence over the same variable in 'config'. What are you trying to accomplish here? We can find a better way to do it.

>             codesighs = False
>             if not pf.get('enable_unittests'):
>                 uploadPackages = pf.get('packageTests', False)
>@@ -1009,6 +1015,14 @@ def generateBranchObjects(config, name):
>         else:
>             factory_class = NightlyBuildFactory
>             uploadSymbols = False
>+        # This is needed to make sure that mobile things come from the mobile
>+        # stage directory instead of the firefox one.  The hardcoded 'firefox'
>+        # string is used to preserve existing behaviour
>+        if pf.has_key('stage_product'):
>+            stageBasePath = '%s/%s' % (config['stage_base_path'],
>+                                       pf['stage_product'])
>+        else:
>+            stageBasePath = '%s/firefox' % config['stage_base_path']

You're setting "stage_product" for all platforms in config.py, is this conditional necessary?


>             triggeredSchedulers=None
>-            if config['enable_l10n'] and platform in config['l10n_platforms'] and \
>-               nightly_builder in l10nNightlyBuilders:
>-                triggeredSchedulers=[l10nNightlyBuilders[nightly_builder]['l10n_builder']]
>+            mobile_l10n_scheduler_name = '%s-%s-l10n' % (name, platform)
>+            if config['enable_l10n'] and pf.get('is_mobile_l10n'):

Please key this section off of l10n_chunks existing/not being None, too.


>+        pkg_patterns = []
>+        for product in ('firefox', 'fennec'):
>+            pkg_patterns.append('%s/dist/%s-*' % (self.mozillaObjdir,
>+                                                  product))

Hardcoding--. Any reason you can't use product_name for this? You can override it as "fennec" for all of the mobile platforms.


feedback+, but r- for the specific reasons above. I didn't look at the MercurialBuildFactory changes in great detail because I don't feel fully qualified to review mobile build logic. Good work pushing this through, John.

Comment 34

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

(In reply to comment #33)
> Comment on attachment 531263 [details] [diff] [review] [review]
> buildbotcustom v1
> 
> 
> >+        codesighs = pf.get('enable_codesighs', config.get('enable_codesighs',True))
> 
> This breaks assumptions about how config and pf interact. Namely that
> enable_codesighs in 'pf' takes precedence over the same variable in
> 'config'. What are you trying to accomplish here? We can find a better way
> to do it.

There are some platforms that don't support codesighs at all, but the branch still needs/wants codesighs.  The assumption here is that no platform is going to set pf['enable_codesighs'] to true, which I would agree is flawed.  I could change it so that codesighs will not be run if the branch disables it
regardless of what the platform wants and have the pf setting control whether codesighs is run if the branch allows it

> You're setting "stage_product" for all platforms in config.py, is this
> conditional necessary?

hmm, no.  I had that in there because it was a late decision to set stage_product on all platforms.  I will remove it.

> >+            if config['enable_l10n'] and pf.get('is_mobile_l10n'):
> 
> Please key this section off of l10n_chunks existing/not being None, too.

Ok, will do
 
> Hardcoding--. Any reason you can't use product_name for this? You can
> override it as "fennec" for all of the mobile platforms.

Yes, because product_name is not set in a valid way anymore.  It is a branch level option and I am not sure what all the consequences are of changing it to a platform level option.  As this was hardcoded before, I don't see this as a massive problem

> feedback+, but r- for the specific reasons above. I didn't look at the
> MercurialBuildFactory changes in great detail because I don't feel fully
> qualified to review mobile build logic. Good work pushing this through, John.

Great,  thanks for taking a look :)

Comment 35

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

(In reply to comment #32)
> Comment on attachment 531262 [details] [diff] [review] [review]
> buildbot-configs v1
> 
> The "remove mobile-mercuriabuildfactory" comments aren't self explanatory to
> me, can you change them to be?

Sure, but without resorting to a bunch of mutli-line comments, they will be short
 
> mobile_platforms don't need to be set for any of the branches that match the
> default (eg, mozilla-central through to mozilla-2.0). Also, please drop the
> commented out lines around the default of this variable.

Actually, they do for split repo branches.  Supporting split repos is not a high priority for the first pass, those branches with split repos will not be supported by this patch.  Because the mobile_platforms dictionary is a copy from mozilla-central, which is now blank,  we need to set it to have the appropriate platforms.

> You've got references to jford_mozilla.com still, please get rid of those :)

yep, staging leftovers.  sorry about that.
 
> Please use "hg copy" on the mozconfigs, it makes review & history much
> clearer.

I did, but the mozconfigs weren't word for word identical
 
> MOZ_SYMBOLS_EXTRA_BUILDID still needs to get set somewhere, per-branch, or
> android-r7 symbols from different branches could get uploaded to the same
> place.

K, will do

> Do you need to set mozharness_config and/or enable_mobile_dep somewhere, too?

When I have support for mozharness multi-l10n, yes

> Please make this comment more verbose:
> >     PROJECT_BRANCHES[branch]['mobile_platforms'] = {} # NO!

Heh, I think i can do that :)


> Can you explain why deleting platforms from BRANCHES['try'] helps
> TryBuildFactory "understand the new stage_product and stage_platform
> variables as well as understanding how to do some commands scratchboxishly",
> as well as why we need a workaround for try at all?

The reason is long and it might end up being easier for me to fix try than to explain why try isn't being fixed

Comment 36

[[e-mail] (:bhearsum)]

(In reply to comment #34)
> (In reply to comment #33)
> > Comment on attachment 531263 [details] [diff] [review] [review] [review]
> > buildbotcustom v1
> > 
> > 
> > >+        codesighs = pf.get('enable_codesighs', config.get('enable_codesighs',True))
> > 
> > This breaks assumptions about how config and pf interact. Namely that
> > enable_codesighs in 'pf' takes precedence over the same variable in
> > 'config'. What are you trying to accomplish here? We can find a better way
> > to do it.
> 
> There are some platforms that don't support codesighs at all, but the branch
> still needs/wants codesighs.  The assumption here is that no platform is
> going to set pf['enable_codesighs'] to true, which I would agree is flawed. 
> I could change it so that codesighs will not be run if the branch disables it
> regardless of what the platform wants and have the pf setting control
> whether codesighs is run if the branch allows it

How about this: Drop the branch-level enable_codesighs, and use:
codesighs = pf.get('enable_codesighs', False) in misc.py.

This should reduce the amount of config.py lines needed, and clean it up in general. If you do that, don't forget to drop the hardcoding for debug/windows a bit further down in misc.py.

> > Hardcoding--. Any reason you can't use product_name for this? You can
> > override it as "fennec" for all of the mobile platforms.
> 
> Yes, because product_name is not set in a valid way anymore.  It is a branch
> level option and I am not sure what all the consequences are of changing it
> to a platform level option.  As this was hardcoded before, I don't see this
> as a massive problem

Another victim of my broken mental model. You're right, so this part is fine as is.

(In reply to comment #35)
> (In reply to comment #32)
> > Comment on attachment 531262 [details] [diff] [review] [review] [review]
> > buildbot-configs v1
> > 
> > The "remove mobile-mercuriabuildfactory" comments aren't self explanatory to
> > me, can you change them to be?
> 
> Sure, but without resorting to a bunch of mutli-line comments, they will be
> short

Even something like "TODO: remove when MobileBuildFactory dies" would be sufficient.


> > mobile_platforms don't need to be set for any of the branches that match the
> > default (eg, mozilla-central through to mozilla-2.0). Also, please drop the
> > commented out lines around the default of this variable.
> 
> Actually, they do for split repo branches.  Supporting split repos is not a
> high priority for the first pass, those branches with split repos will not
> be supported by this patch.  Because the mobile_platforms dictionary is a
> copy from mozilla-central, which is now blank,  we need to set it to have
> the appropriate platforms.

Per IRC, the overrides do the same thing as the default, and thus aren't needed.

> > Please use "hg copy" on the mozconfigs, it makes review & history much
> > clearer.
> 
> I did, but the mozconfigs weren't word for word identical

These patches don't reflect that...if you did, you should see things like:
copy from mozilla2/linux/mozilla-central/codecoverage/mozconfig
copy to mozilla2/linux/mozilla-beta/codecoverage/mozconfig

followed by a diff of what actually did change in the new version.

The "New file..." means "hg add" was used on them, instead.


> > Can you explain why deleting platforms from BRANCHES['try'] helps
> > TryBuildFactory "understand the new stage_product and stage_platform
> > variables as well as understanding how to do some commands scratchboxishly",
> > as well as why we need a workaround for try at all?
> 
> The reason is long and it might end up being easier for me to fix try than
> to explain why try isn't being fixed

Works for me =).

Comment 37

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

(In reply to comment #36)
> > > Please use "hg copy" on the mozconfigs, it makes review & history much
> > > clearer.
> > 
> > I did, but the mozconfigs weren't word for word identical
> 
> These patches don't reflect that...if you did, you should see things like:
> copy from mozilla2/linux/mozilla-central/codecoverage/mozconfig
> copy to mozilla2/linux/mozilla-beta/codecoverage/mozconfig
> 
> followed by a diff of what actually did change in the new version.
> 
> The "New file..." means "hg add" was used on them, instead.

That only happens if the source file is already tracked.  I would have to commit the mozilla-central mozconfigs the do the hg cp.

[jhford@singe ~]$ mkdir test
[jhford@singe ~]$ cd test
[jhford@singe test]$ echo 'john' > a
[jhford@singe test]$ hg init
[jhford@singe test]$ hg add a
[jhford@singe test]$ hg diff
diff --git a/a b/a
new file mode 100644
--- /dev/null
+++ b/a
@@ -0,0 +1,1 @@
+john
[jhford@singe test]$ hg cp a b
a has not been committed yet, so no copy data will be stored for b.
[jhford@singe test]$ hg diff
diff --git a/a b/a
new file mode 100644
--- /dev/null
+++ b/a
@@ -0,0 +1,1 @@
+john
diff --git a/b b/b
new file mode 100644
--- /dev/null
+++ b/b
@@ -0,0 +1,1 @@
+john
[jhford@singe test]$

Comment 38

[Aki Sasaki (not active)]

Comment on attachment 531262 [details] [diff] [review]
buildbot-configs v1

>diff --git a/mozilla/config.py b/mozilla/config.py
>--- a/mozilla/config.py
>+++ b/mozilla/config.py
>@@ -19,18 +19,17 @@ GLOBAL_VARS = {
>-    'stage_username_mobile': 'ffxbld',
>-    'stage_base_path': '/home/ftp/pub/firefox',
>-    'stage_base_path_xulrunner': '/home/ftp/pub/xulrunner',
>-    'stage_base_path_mobile': '/home/ftp/pub/mobile',
>+    'stage_username_mobile': 'ffxbld', # remove mobile-mercurialbuildfactory
>+    'stage_base_path': '/home/ftp/pub',
>+    'stage_base_path_mobile': '/home/ftp/pub/mobile', # remove mobile-mercurialbuildfactory

I think you're removing stage_base_path_xulrunner -- is that intended?

>+        'linux-mobile': {},
>+        'linux-maemo5gtk': {},
>+        'linux-maemo5qt': {},
>+        'linux-android': {},
>+        'linux-android-debug': {},

Not a fan of these names, but I'm not going to block on that.
As mentioned in IRC, these change the sendchange platforms, so we need to update mozilla-tests/config.py so the Tegras don't stop testing builds.
Not sure if it's easier to do that, or make our code aware of android as a platform.

>+            'base_name': 'Android %(branch)s',

This is different than before (Android R7) but that's fine. We need to enable scraping.

>+            'stage_platform': "maemo5gtk",

This will change the upload platform from maemo5-gtk to maemo5gtk, is that intended?

>+            'stage_platform': "maemo5qt",

Same here.

>+            'stage_platform': "linux-mobile",

This has been 'linux'. If we're uploading to mobile/, there's no conflict.

>+            'base_name': 'Windows Mobile Desktop %(branch)s',
This was WINNT 5.2 Mobile Desktop.  The new name is fine, scraping.

>+            'stage_platform': "win32-mobile",

'win32'.

>+            'base_name': 'OS X 10.5 Mobile Desktop %(branch)s',

This was OS X 10.5.2 Mobile Desktop.  Getting rid of the final .2 is probably good, but requires scraping.

>+            'stage_platform': "macosx-mobile",

'macosx'.

>-BRANCHES['mozilla-central']['mobile_platforms']['android-r7']['env']['MOZ_SYMBOLS_EXTRA_BUILDID'] = 'mozilla-central'

As Ben mentioned, we need this.

>-BRANCHES['mozilla-central']['mobile_platforms']['android-debug']['enable_mobile_dep'] = True

Where/how are you keeping old-style nightlies around until you fix symbols+updates+multilocale?  We should turn off new-style nightlies until that's fixed as well.  Android, Maemo5 GTK, Maemo5 QT, for all branches with a mozharness config.  (mozilla-central, mozilla-beta, mozilla-aurora,

>+        if platform.endswith('debug') and 'linux-android' not in platform:
>+            # As this condition was, it broke on linux-android-debug
>             BRANCHES[branch]['platforms'][platform]['mozconfig'] = platform.split('-')[0] + '/' + branchConfig.get('mozconfig_dir', 'generic') + '/debug'

This would work with android-debug, but we'll see which solution we go with.

>diff --git a/mozilla2/linux-android-debug/mozilla-aurora/nightly/mozconfig b/mozilla2/linux-android-debug/mozilla-aurora/nightly/mozconfig

I just added |ac_add_options --with-branding=mobile/branding/aurora| to the bottom of every mozilla-aurora mobile mozconfig, including l10n.

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

Could you add those?

>+++ b/mozilla2/linux-android-debug/mozilla-beta/nightly/mozconfig

I'll be working on adding the beta branding to these, but don't have a confirmation as to how that'll look yet.  If bug 655107 lands before this, we may need to massage the mozilla-beta mozconfigs similarly.

How closely do I need to look at the mozconfigs, otherwise? It's looking like you created them new rather than copied?  If so, is there a reason for that? Copying would seem to be less risk.

Nightlies with updates+multilocale are definitely a hard blocker here... aiui you're going to re-enable them in the mobile_platforms, no?

Comment 39

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

(In reply to comment #38)
> Comment on attachment 531262 [details] [diff] [review] [review]
> buildbot-configs v1

> I think you're removing stage_base_path_xulrunner -- is that intended?

yes, it is taken care of in misc.py and release.py
 
> >+        'linux-mobile': {},
> >+        'linux-maemo5gtk': {},
> >+        'linux-maemo5qt': {},
> >+        'linux-android': {},
> >+        'linux-android-debug': {},
> 
> Not a fan of these names, but I'm not going to block on that.
> As mentioned in IRC, these change the sendchange platforms, so we need to
> update mozilla-tests/config.py so the Tegras don't stop testing builds.
> Not sure if it's easier to do that, or make our code aware of android as a
> platform.

Another option is to change the sendchange branches to use stage_platform instead of the complete platform

Pretty name issues:
> >+            'base_name': 'Android %(branch)s',
leaving as is (aiui, this is desired)
> >+            'base_name': 'OS X 10.5 Mobile Desktop %(branch)s',
renaming back to 10.5.2

Upload platform issues: 
> >+            'stage_platform': "maemo5gtk",
> >+            'stage_platform': "maemo5qt",
renaming to maemo5-gtk and maemo5-qt
> >+            'stage_platform': "linux-mobile",
> >+            'stage_platform': "win32-mobile",
> >+            'stage_platform': "macosx-mobile",
While I prefer leaving -mobile in the upload platform, I don't feel strongly and it does remove some of the churn from this patch


> >-BRANCHES['mozilla-central']['mobile_platforms']['android-r7']['env']['MOZ_SYMBOLS_EXTRA_BUILDID'] = 'mozilla-central'
> 
> As Ben mentioned, we need this.

Put back in for new version of patch

> >-BRANCHES['mozilla-central']['mobile_platforms']['android-debug']['enable_mobile_dep'] = True
> 
> Where/how are you keeping old-style nightlies around until you fix
> symbols+updates+multilocale?  We should turn off new-style nightlies until
> that's fixed as well.  Android, Maemo5 GTK, Maemo5 QT, for all branches with
> a mozharness config.  (mozilla-central, mozilla-beta, mozilla-aurora,

I forgot to enable old-style nightly (only) builds, will add that back to patch for central, beta and aurora

> >+        if platform.endswith('debug') and 'linux-android' not in platform:
> >+            # As this condition was, it broke on linux-android-debug
> >             BRANCHES[branch]['platforms'][platform]['mozconfig'] = platform.split('-')[0] + '/' + branchConfig.get('mozconfig_dir', 'generic') + '/debug'
> 
> This would work with android-debug, but we'll see which solution we go with.

This is a hack and I don't feel strongly about it other than making sure the correct mozconfig is used


> >diff --git a/mozilla2/linux-android-debug/mozilla-aurora/nightly/mozconfig b/mozilla2/linux-android-debug/mozilla-aurora/nightly/mozconfig
> 
> I just added |ac_add_options --with-branding=mobile/branding/aurora| to the
> bottom of every mozilla-aurora mobile mozconfig, including l10n.
> 
> http://hg.mozilla.org/build/buildbot-configs/rev/edca67d5e0f0
> 
> Could you add those?

I can (and have)

> >+++ b/mozilla2/linux-android-debug/mozilla-beta/nightly/mozconfig
> 
> I'll be working on adding the beta branding to these, but don't have a
> confirmation as to how that'll look yet.  If bug 655107 lands before this,
> we may need to massage the mozilla-beta mozconfigs similarly.

Good to know

> How closely do I need to look at the mozconfigs, otherwise? It's looking
> like you created them new rather than copied?  If so, is there a reason for
> that? Copying would seem to be less risk.

I wanted to clean them up and there were things that would break new style.  I think we should definately look at them closer than a skim

> Nightlies with updates+multilocale are definitely a hard blocker here...
> aiui you're going to re-enable them in the mobile_platforms, no?

I am leaning more towards putting my efforts in fixing everything instead of figuring out how to selectively add/remove platforms.  I am going to take a look at the multi-l10n/snippets blockers today.

Comment 40

[Aki Sasaki (not active)]

Comment on attachment 531182 [details] [diff] [review]
rm dist/fennec* in mozharness

Tested; works.

However, this doesn't help John's step-ordering problem at all.

Using John's current patch:

1) package en-US
2) package-tests en-US
3) multil10n
4) package multi (rm dist/fennec*)
5) package-tests multi
6) upload en-US (gone)
7) upload multi

Using this, and removing the rm -rf dist/fennec* from make package:

1) package en-US
2) package-tests en-US
3) multil10n (rm dist/fennec*)
4) package multi
5) package-tests multi
6) upload en-US (gone)
7) upload multi

Without rm'ing either place:

1) package en-US
2) package-tests en-US
3) multil10n (overwrite/corrupt en-US and multi dirs+files)
4) package multi
5) package-tests multi
6) upload en-US (partially en-US, partially corrupt multi)
7) upload multi (corrupt)

The real fix here is to upload en-US before running the multil10n step.

Comment 41

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

Comment on attachment 531182 [details] [diff] [review]
rm dist/fennec* in mozharness

Looks good to me.  Thanks!

Comment 42

[Aki Sasaki (not active)]

Comment on attachment 531182 [details] [diff] [review]
rm dist/fennec* in mozharness

http://hg.mozilla.org/build/mozharness/rev/ad19b5950001

Comment 43

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

Attachment: do the package clobbering as part of the packaging steps in MobileBuildFactory

this patch applies to both buildbot-0.7 and default branches of buildbotcustom and does clobbering as part of the packaging steps.

I have a user repo with the proposed patch at /users/jford_mozilla.com/mobile-5.0 and moz2-master2 setup for doing a nightly of maemo5gtk.

As discussed, Aki will finish testing this patch.

Comment 44

[Aki Sasaki (not active)]

Comment on attachment 532356 [details] [diff] [review]
do the package clobbering as part of the packaging steps in MobileBuildFactory

This patch overall is good, but has some nits:

>diff --git a/process/factory.py b/process/factory.py
>--- a/process/factory.py
>+++ b/process/factory.py
>@@ -5769,6 +5769,15 @@ class MobileDesktopBuildFactory(MobileBu
> 
>     def addPackageSteps(self):
>         self.addStep(ShellCommand,
>+            name='rm_mobile_pkg',
>+            command=['rm', '-rvf', 'dist/fennec*'],
>+            workdir='%s/%s/%s' % (self.baseWorkDir,
>+            self.branchName, self.objdir),
>+            description=['make', 'mobile', 'package'],

NIT: description is wrong.

>+            name='rm_pkg',
>+            command=[self.scratchboxPath, '-p', '-d',
>+                     '%s' % (self.objdirRelPath),
>+                     'rm -rfv dist/fennec*'],
>+            description=['make', 'package'],

NIT: here too.

>@@ -7519,6 +7536,14 @@ class AndroidBuildFactory(MobileBuildFac
>             makePackageTestsCommand += ['AB_CD=%s' % locale]
> 
>         self.addStep(ShellCommand,
>+            name='rm_android_pkg',
>+            command=makePackageCommand,
>+            workdir='%s/%s/%s' % (self.baseWorkDir, self.branchName, self.objdir),
>+            description=['rm', '-rfv', 'dist/fennec*'],

Here you have the right description, but you're running |make package| a second time.  This is more serious than the other nits; I'm not sure what the side effects are at this point.

(This step may not be strictly needed, even, since Android multilocale uses mozharness on 0.7 as well.)

I was, however, able to do a full staging run of Fennec 5.0b1 build 3 without bustage.  I then tested the OSX mobile desktop build locally, and the Maemo 5 GTK multilocale deb on N900 in both en-US and es-ES.

So I'm marking this r- because of the Android 2nd make package, but r=me with the above comments addressed.  This will need to land in the buildbot-0.7 branch, then merge into the production-0.7 branch and get updated on production-master02:/tools/buildbotcustom/buildbotcustom/.

Comment 45

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

Attachment: do the package clobbering as part of the packaging steps in MobileBuildFactory v2

Nits addressed and corrected.  We should also land this on default/production to ensure that dep builds have the same fix.

Comment 46

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

Attachment: python script to create snippets for android apks

This script will be enabled on a per-platform basis to allow platforms to individually opt-in to generating android snippets.  This script could be generalized for all nightly updates with an unscoped amount of effort.

The values passed into the script will be obtained from a combination of config and production_config or staging_config variables.  The values will come from:
--download-base-url == branch configs's 'mobile_download_base_url' key
--download-subdir == name + stage_platform in misc.py
--abi == platform's 'Android_arm-eabi-gcc3' key
--aus-base-path == branch config's 'aus2_mobile_base_upload_dir' key
--aus-host == branch config's 'aus2_host' key
--aus-user == branch config's 'aus2_user' key
--aus-ssh-key == branch config's 'aus2_ssh_key' key
apk filename will be the basename of the packageUrl propery

The following is output of a test.  In order to be able to force the version of the previous apk file, I downloaded the file outside the script and commented out removal and fetching portions of getPreviousBuildID.

jhford-wifi:mozilla jhford$ curl -O http://ftp.mozilla.org/pub/mozilla.org/mobile/nightly/2011/05/2011-05-15-04-mozilla-central-android-r7/gecko-unsigned-unaligned.apk
jhford-wifi:mozilla jhford$ curl -O http://ftp.mozilla.org/pub/mozilla.org/mobile/nightly/2011/05/2011-05-16-04-mozilla-central-android-r7/fennec-6.0a1.multi.eabi-arm.apk
jhford-wifi:mozilla jhford$ mv gecko-unsigned-unaligned.apk previous.apk
jhford-wifi:mozilla jhford$ python android_snippet.py --download-base-url http://staging-stage.build.mozilla.org/pub/mozilla.org/mobile --download-subdir mozilla-central-android --abi Android_arm-eabi-gcc3 --aus-base-path /opt/aus2/incoming/2/Fennec/mozilla-central --aus-host localhost --aus-user jhford --aus-ssh-key ~/.ssh/jhford fennec-6.0a1.multi.eabi-arm.apk 
Build ID for fennec-6.0a1.multi.eabi-arm.apk is 20110516042149
Version for fennec-6.0a1.multi.eabi-arm.apk is 6.0a1
Build ID for previous.apk is 20110515043952
Version for previous.apk is 6.0a1
Complete snippet (indented 4 spaces, line start/end marked with ^ and $
    ^version=1$
    ^type=complete$
    ^url=http://staging-stage.build.mozilla.org/pub/mozilla.org/mobile/nightly/2011/05/2011-05-16-04-mozilla-central-android/fennec-6.0a1.multi.eabi-arm.apk$
    ^hashFunction=sha512$
    ^hashValue=bc5f16d0a54fd02efddd2324a21bce21bf196bc0315f2c1d29a91a87ade4c008d8f91b010e7c92fa13df3488b9773e4ab6920bf6ee2ddc66044713448bef2e43$
    ^size=15040006$
    ^build=20110516042149$
    ^appv=6.0a1$
    ^extv=6.0a1$
Running ['ssh', '-l', 'jhford', '-i', '/Users/jhford/.ssh/jhford', 'localhost', 'mkdir', '-p', '/opt/aus2/incoming/2/Fennec/mozilla-central/Android_arm-eabi-gcc3/20110515043952/en-US']
Running ['scp', '-i', '/Users/jhford/.ssh/jhford', 'complete.txt', 'partial.txt', 'jhford@localhost:/opt/aus2/incoming/2/Fennec/mozilla-central/Android_arm-eabi-gcc3/20110515043952/en-US']
complete.txt                                                                                                                                                                                                                                                                             100%  392     0.4KB/s   00:00    
partial.txt                                                                                                                                                                                                                                                                              100%    0     0.0KB/s   00:00    
Running ['ssh', '-l', 'jhford', '-i', '/Users/jhford/.ssh/jhford', 'localhost', 'mkdir', '-p', '/opt/aus2/incoming/2/Fennec/mozilla-central/Android_arm-eabi-gcc3/20110516042149/en-US']
Running ['ssh', '-l', 'jhford', '-i', '/Users/jhford/.ssh/jhford', 'localhost', 'touch /opt/aus2/incoming/2/Fennec/mozilla-central/Android_arm-eabi-gcc3/20110516042149/en-US/complete.txt /opt/aus2/incoming/2/Fennec/mozilla-central/Android_arm-eabi-gcc3/20110516042149/en-US/partial.txt /opt/aus2/incoming/2/Fennec/mozilla-central/Android_arm-eabi-gcc3/20110516042149/en-US']
jhford-wifi:mozilla jhford$ find /opt
/opt
/opt/aus2
/opt/aus2/incoming
/opt/aus2/incoming/2
/opt/aus2/incoming/2/Fennec
/opt/aus2/incoming/2/Fennec/mozilla-central
/opt/aus2/incoming/2/Fennec/mozilla-central/Android_arm-eabi-gcc3
/opt/aus2/incoming/2/Fennec/mozilla-central/Android_arm-eabi-gcc3/20110515043952
/opt/aus2/incoming/2/Fennec/mozilla-central/Android_arm-eabi-gcc3/20110515043952/en-US
/opt/aus2/incoming/2/Fennec/mozilla-central/Android_arm-eabi-gcc3/20110515043952/en-US/complete.txt
/opt/aus2/incoming/2/Fennec/mozilla-central/Android_arm-eabi-gcc3/20110515043952/en-US/partial.txt
/opt/aus2/incoming/2/Fennec/mozilla-central/Android_arm-eabi-gcc3/20110516042149
/opt/aus2/incoming/2/Fennec/mozilla-central/Android_arm-eabi-gcc3/20110516042149/en-US
/opt/aus2/incoming/2/Fennec/mozilla-central/Android_arm-eabi-gcc3/20110516042149/en-US/complete.txt
/opt/aus2/incoming/2/Fennec/mozilla-central/Android_arm-eabi-gcc3/20110516042149/en-US/partial.txt
jhford-wifi:mozilla jhford$ find /opt -type f -exec file {} \;
/opt/aus2/incoming/2/Fennec/mozilla-central/Android_arm-eabi-gcc3/20110515043952/en-US/complete.txt: ASCII text
/opt/aus2/incoming/2/Fennec/mozilla-central/Android_arm-eabi-gcc3/20110515043952/en-US/partial.txt: empty
/opt/aus2/incoming/2/Fennec/mozilla-central/Android_arm-eabi-gcc3/20110516042149/en-US/complete.txt: empty
/opt/aus2/incoming/2/Fennec/mozilla-central/Android_arm-eabi-gcc3/20110516042149/en-US/partial.txt: empty
jhford-wifi:mozilla jhford$ cat -vet /opt/aus2/incoming/2/Fennec/mozilla-central/Android_arm-eabi-gcc3/20110515043952/en-US/complete.txt
version=1$
type=complete$
url=http://staging-stage.build.mozilla.org/pub/mozilla.org/mobile/nightly/2011/05/2011-05-16-04-mozilla-central-android/fennec-6.0a1.multi.eabi-arm.apk$
hashFunction=sha512$
hashValue=bc5f16d0a54fd02efddd2324a21bce21bf196bc0315f2c1d29a91a87ade4c008d8f91b010e7c92fa13df3488b9773e4ab6920bf6ee2ddc66044713448bef2e43$
size=15040006$
build=20110516042149$
appv=6.0a1$
extv=6.0a1$

Comment 47

[Aki Sasaki (not active)]

Comment on attachment 532874 [details]
python script to create snippets for android apks

This currently exits 1 if it fails to download the previous apk.  I think the desired behavior is to create our size 0 snippets in our current buildid dir and fail but don't {halt,flunk}OnFailure.  We also want to fail if we can't upload.

This may take a little massaging to create/upload release snippets, but those aren't currently used, and are outside the scope of this bug.

Without going too deep, and other than the first issue above, this looks good.

Comment 48

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

(In reply to comment #46)
> Created attachment 532874 [details] [review]
> python script to create snippets for android apks

This was not done in buildbot logic as a subclass of MercurialBuildFactory because

a) scripting using buildbot steps/properties is painful
b) subclassing makes generateBranchObjects messier
c) doing this logic with a bunch of buildbot steps has master side step start/stop overhead, this helps remove some of it
d) established convention is to deal with platform specific logic directly in MercurialBuildFactory depending on the platform being targeted
e) it is harder to test/debug buildbot logic than it is with a script
f) we are generally moving towards splitting logic out into scripts from buildbot steps
g) it is easier to write tests for the script if we desire to do so in future

Comment 49

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

Attachment: buildbot-configs v3

address review feedback. This patch does not have mozconfigs, and as such isn't going to be the final patch.  At this point, however, I think the only things left to modify will be the mozconfigs which I can work on while this review happens.  I have left in some staging cruft as I know this isn't going to be the absolute final cut of the patch.

Comment 50

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

Attachment: buildbotcustom v3

This patch adds multi-locale for android and maemo5-gtk as well as android nightly snippet generation in addition to addressing review feedback.

Comment 51

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

Attachment: tools v3

Actually, there isn't a situation where the mobile_platform key is used.

Comment 52

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

Attachment: mozharness v3

The same changes would be required for all branches/platforms we want to support.  Will wait on r+ on these changes before making more of the changes.

v3 to match the version of the other patches

Comment 53

[Aki Sasaki (not active)]

Comment on attachment 533187 [details] [diff] [review]
mozharness v3

>--- a/configs/multi_locale/trunk_maemo5_gtk.json
>+++ b/configs/multi_locale/mozilla-central_linux-maemo5-gtk.json
>@@ -3,18 +3,18 @@
>     "sbox_home": "/builds/scratchbox/users/cltbld/home/cltbld/",
>     "sbox_root": "/builds/scratchbox/users/cltbld",
>     "log_name": "multilocale",
>-    "objdir": "objdir",
>-    "locales_file": "mozilla-central/mobile/locales/maemo-locales",
>+    "objdir": "obj-firefox",
>+    "locales_file": "build/mobile/locales/maemo-locales",
>     "locales_dir": "mobile/locales",
>     "ignore_locales": ["en-US", "multi"],
>     "repos": [{
>         "repo": "http://hg.mozilla.org/mozilla-central",
>         "tag": "default",
>-        "dir_name": "mozilla-central"
>+        "dir_name": "build"
>     },{
>         "repo": "http://hg.mozilla.org/build/buildbot-configs",
>         "tag": "default",
>-        "dir_name": "buildbot-configs"
>+        "dir_name": "build/configs"
>     },{
>         "repo": "http://hg.mozilla.org/build/tools",
>         "tag": "default",
>@@ -27,6 +27,6 @@
>     "hg_l10n_tag": "default",
>     "l10n_dir": "l10n-central",
>     "merge_locales": true,
>-    "mozilla_dir": "mozilla-central",
>-    "mozconfig": "buildbot-configs/mozilla2/mobile/maemo5-gtk/mobile-browser/nightly/mozconfig"
>+    "mozilla_dir": "build",
>+    "mozconfig": "buildbot/configs/mozilla2/mobile/maemo5-gtk/mobile-browser/nightly/mozconfig"
> }

Hm, should this mozconfig be build/configs/... ?

Also, I'm going to guess we need mozilla-beta_*.json and mozilla-aurora_*.json updated as well.

Comment 54

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

(In reply to comment #53)
> >-    "mozilla_dir": "mozilla-central",
> >-    "mozconfig": "buildbot-configs/mozilla2/mobile/maemo5-gtk/mobile-browser/nightly/mozconfig"
> >+    "mozilla_dir": "build",
> >+    "mozconfig": "buildbot/configs/mozilla2/mobile/maemo5-gtk/mobile-browser/nightly/mozconfig"
> > }
> 
> Hm, should this mozconfig be build/configs/... ?

I think that is a typo

> Also, I'm going to guess we need mozilla-beta_*.json and
> mozilla-aurora_*.json updated as well.

Yep!

Comment 55

[[e-mail] (:bhearsum)]

Comment on attachment 533186 [details] [diff] [review]
tools v3

I'd like to see this script more in line with the other build/tools scripts in the following ways:
- Use existing library functions where appropriate (eg, commands.py:run_remote_cmd instead of re-implementing it as ssh)
- Use Python logging facilities
- Put the following functions in lib/python somewhere, and write tests for them: hashFile, _scp


What's the l10n change here? It doesn't seem to fit in with this bug at all.

Comment 56

[[e-mail] (:bhearsum)]

Comment on attachment 533185 [details] [diff] [review]
buildbotcustom v3

I don't see a need for createAndroidSnippet to exist. Please use the existing flag, and 'if platform is android' where necessary instead. 

Don't specify "/tools/python-2.6.5/bin/python" here, that's asking for bustage down the road. You should be using /tools/python/bin/python, the same as the rest of the build process uses (which also happens to be first in PATH).

I have to admit, I don't really know how to properly review this patch given the scope and # of possible code paths, so I'm really trusting a lot in your testing.

Comment 57

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

(In reply to comment #55)
> Comment on attachment 533186 [details] [diff] [review] [review]
> tools v3
> 
> I'd like to see this script more in line with the other build/tools scripts
> in the following ways:
> - Use existing library functions where appropriate (eg,
> commands.py:run_remote_cmd instead of re-implementing it as ssh)
> - Use Python logging facilities
> - Put the following functions in lib/python somewhere, and write tests for
> them: hashFile, _scp

Can I do that as a follow up after the bulk of this lands?  These are non-trivial changes, which is ok, but they are going to delay landing for at least a day

> What's the l10n change here? It doesn't seem to fit in with this bug at all.

This is the change required to get single locale repacks working now that the configs are on a different branch.

Comment 58

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

(In reply to comment #56)
> Comment on attachment 533185 [details] [diff] [review] [review]
> buildbotcustom v3
> 
> I don't see a need for createAndroidSnippet to exist. Please use the
> existing flag, and 'if platform is android' where necessary instead. 

This flag isn't actually deciding whether snippet style should be 'android' or 'regular', it is saying 'generate android snippets independently of normal snippets'.  I could structure it so that the normal codepath for self.createSnippets is taken, but that is adding a bunch of extra branch points in the codepath and requires retesting.
 
> Don't specify "/tools/python-2.6.5/bin/python" here, that's asking for
> bustage down the road. You should be using /tools/python/bin/python, the
> same as the rest of the build process uses (which also happens to be first
> in PATH).

Yes, but that is python 2.5 and this script needs python 2.6.  As we run buildbot on these slaves with python 2.6, I don't see this as a problem.  

Could I file a follow up bug, noting in createAndroidSnippet that until the bug is resolved that we might have a source of bustage there.  The followup bug would be to create standard symlinks for python 2.6 that omit the minor version number (i.e. /tools/python-2.6 is a symlink to /tools/python-2.6.5).

> I have to admit, I don't really know how to properly review this patch given
> the scope and # of possible code paths, so I'm really trusting a lot in your
> testing.

Yes, it is a near complete reimplementation of our mobile infrastructure and is very difficult to test/review as many of our standard tools aren't really useful (esp. dump masters).  There are bound to be issues arising from this patch, I think the important thing to do is make sure we land this early in the day and watch the trees like a hawk for the rest of the day and a couple days after.  Ideally, we will be able to run this in pre-production for at least a day to let it bake, but I don't know how that is going to work with other code landings happening in the meantime.

Comment 59

[[e-mail] (:bhearsum)]

(In reply to comment #57)
> (In reply to comment #55)
> > Comment on attachment 533186 [details] [diff] [review] [review] [review]
> > tools v3
> > 
> > I'd like to see this script more in line with the other build/tools scripts
> > in the following ways:
> > - Use existing library functions where appropriate (eg,
> > commands.py:run_remote_cmd instead of re-implementing it as ssh)
> > - Use Python logging facilities
> > - Put the following functions in lib/python somewhere, and write tests for
> > them: hashFile, _scp
> 
> Can I do that as a follow up after the bulk of this lands?  These are
> non-trivial changes, which is ok, but they are going to delay landing for at
> least a day

Please do it in the first landing, we (as a group) have a poor track record at coming back to these things.

> > What's the l10n change here? It doesn't seem to fit in with this bug at all.
> 
> This is the change required to get single locale repacks working now that
> the configs are on a different branch.

Ah, ok.

(In reply to comment #58)
> (In reply to comment #56)
> > Comment on attachment 533185 [details] [diff] [review] [review] [review]
> > buildbotcustom v3
> > 
> > I don't see a need for createAndroidSnippet to exist. Please use the
> > existing flag, and 'if platform is android' where necessary instead. 
> 
> This flag isn't actually deciding whether snippet style should be 'android'
> or 'regular', it is saying 'generate android snippets independently of
> normal snippets'.  I could structure it so that the normal codepath for
> self.createSnippets is taken, but that is adding a bunch of extra branch
> points in the codepath and requires retesting.

Is there a situation where we would be generating both? If not, I don't see why they need to be separate.

> > Don't specify "/tools/python-2.6.5/bin/python" here, that's asking for
> > bustage down the road. You should be using /tools/python/bin/python, the
> > same as the rest of the build process uses (which also happens to be first
> > in PATH).
> 
> Yes, but that is python 2.5 and this script needs python 2.6.  As we run
> buildbot on these slaves with python 2.6, I don't see this as a problem.  

> Could I file a follow up bug, noting in createAndroidSnippet that until the
> bug is resolved that we might have a source of bustage there.  The followup
> bug would be to create standard symlinks for python 2.6 that omit the minor
> version number (i.e. /tools/python-2.6 is a symlink to /tools/python-2.6.5).

We use it to launch Buildbot, yes, but everything in the build *process* uses 2.5. Keeping that consistentency is important IMHO.

Which part of the script requires Python 2.6? I'm happy to help find a replacement.

> > I have to admit, I don't really know how to properly review this patch given
> > the scope and # of possible code paths, so I'm really trusting a lot in your
> > testing.
> 
> Yes, it is a near complete reimplementation of our mobile infrastructure and
> is very difficult to test/review as many of our standard tools aren't really
> useful (esp. dump masters).  There are bound to be issues arising from this
> patch, I think the important thing to do is make sure we land this early in
> the day and watch the trees like a hawk for the rest of the day and a couple
> days after.  Ideally, we will be able to run this in pre-production for at
> least a day to let it bake, but I don't know how that is going to work with
> other code landings happening in the meantime.

I'm glad you'll be keeping a close eye on things.

Comment 60

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

(In reply to comment #59)
> (In reply to comment #57)
> > (In reply to comment #55)
> > > Comment on attachment 533186 [details] [diff] [review] [review] [review] [review]
> > > tools v3
> > > 
> > > I'd like to see this script more in line with the other build/tools scripts
> > > in the following ways:
> > > - Use existing library functions where appropriate (eg,
> > > commands.py:run_remote_cmd instead of re-implementing it as ssh)
> > > - Use Python logging facilities
> > > - Put the following functions in lib/python somewhere, and write tests for
> > > them: hashFile, _scp
> > 
> > Can I do that as a follow up after the bulk of this lands?  These are
> > non-trivial changes, which is ok, but they are going to delay landing for at
> > least a day
> 
> Please do it in the first landing, we (as a group) have a poor track record
> at coming back to these things.

My only concern is time.  I can change this behaviour.
 
> (In reply to comment #58)
> > (In reply to comment #56)
> > > Comment on attachment 533185 [details] [diff] [review] [review] [review] [review]
> > > buildbotcustom v3
> > > 
> > > I don't see a need for createAndroidSnippet to exist. Please use the
> > > existing flag, and 'if platform is android' where necessary instead. 
> > 
> > This flag isn't actually deciding whether snippet style should be 'android'
> > or 'regular', it is saying 'generate android snippets independently of
> > normal snippets'.  I could structure it so that the normal codepath for
> > self.createSnippets is taken, but that is adding a bunch of extra branch
> > points in the codepath and requires retesting.
> 
> Is there a situation where we would be generating both? If not, I don't see
> why they need to be separate.

Ok, I will move the android logic into addCreateSnippetsSteps and use |if 'android' in self.platform_variations:| to decide whether to use android or normal logic.

> > > Don't specify "/tools/python-2.6.5/bin/python" here, that's asking for
> > > bustage down the road. You should be using /tools/python/bin/python, the
> > > same as the rest of the build process uses (which also happens to be first
> > > in PATH).
> > 
> > Yes, but that is python 2.5 and this script needs python 2.6.  As we run
> > buildbot on these slaves with python 2.6, I don't see this as a problem.  
> 
> > Could I file a follow up bug, noting in createAndroidSnippet that until the
> > bug is resolved that we might have a source of bustage there.  The followup
> > bug would be to create standard symlinks for python 2.6 that omit the minor
> > version number (i.e. /tools/python-2.6 is a symlink to /tools/python-2.6.5).
> 
> We use it to launch Buildbot, yes, but everything in the build *process*
> uses 2.5. Keeping that consistentency is important IMHO.
> 
> Which part of the script requires Python 2.6? I'm happy to help find a
> replacement.

Specifically zipfile.ZipFile.open.  It is new in python 2.6, and the zipfile library is a mess without that function.  I can revert to calling the unzip application and opening the file on the filesystem.

Comment 61

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

Comment on attachment 507905 [details] [diff] [review]
simple graphserver patch to allow these builds to go green in staging

not going to enable codesighs in this patch.  This patch will be required if we want to enable codesighs for mobile in the future.  codesighs seemed to work on mobile platforms just fine.

Comment 62

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

Comment on attachment 532671 [details] [diff] [review]
do the package clobbering as part of the packaging steps in MobileBuildFactory v2

http://hg.mozilla.org/build/buildbotcustom/rev/dbe12d03c8a8

Comment 63

[Aki Sasaki (not active)]

Comment on attachment 533185 [details] [diff] [review]
buildbotcustom v3

>@@ -979,7 +982,8 @@ def generateBranchObjects(config, name):
>         unittestBranch = "%s-%s-opt-unittest" % (name, platform)
>         tinderboxBuildsDir = None
>         if platform.find('-debug') > -1:
>-            leakTest = True
>+            # Some platforms can't run on the build host
>+            leakTest = pf.get('enable_leaktests', True)
>             codesighs = False
>             if not pf.get('enable_unittests'):
>                 uploadPackages = pf.get('packageTests', False)
>@@ -989,12 +993,13 @@ def generateBranchObjects(config, name):
>             # Platform already has the -debug suffix
>             unittestBranch = "%s-%s-unittest" % (name, platform)
>             tinderboxBuildsDir = "%s-%s" % (name, platform)
>-        elif pf.get('enable_opt_unittests'):
>-            packageTests = True
>+        else:
>+            codesighs = pf.get('enable_codesighs', True)
>+            leakTest = False
> 
>         # Allow for test packages on platforms that can't be tested
>         # on the same master.
>-        packageTests = pf.get('packageTests', packageTests)
>+        packageTests = pf.get('packageTests', pf.get('enable_opt_unittests'))

As mentioned in IRC, this regresses debug packageTests behavior, and your pastebinned fix looks good.

>+                    factory = ScriptFactory(
>+                        scriptRepo='%s%s' % (config['hgurl'],
>+                                              'users/jford_mozilla.com/tools'),
>+                                              #config['build_tools_repo_path']),
>+                        interpreter='bash',

As mentioned in your comment, this patch needs some cleanup.
Rebasing would be good as well.

>-    def getPackageFilename(self, platform):
>-        if platform.startswith("linux64"):
>-            packageFilename = '*.linux-x86_64.tar.bz2'
>+    def getPackageFilename(self, platform, platform_variation):
>+        if 'android' in platform_variation:
>+            packageFilename = '*arm.apk' #the arm.apk is to avoid
>+                                         #unsigned/unaligned apks
>         elif platform.startswith("linux"):
>-            packageFilename = '*.linux-i686.tar.bz2'
>+            packageFilename = '*.linux-*.tar.bz2'

As mentioned in IRC, this is probably harmless, but I can see possibly having 64- and 32- bit files in the same objdir at some point.

Probably not too risky, but explicitly leaving linux64 in there seems a tiny percentage safer.

... was this for Maemo?  I'm going to guess yes.

>-                 stagePlatform=None, testPrettyNames=False, l10nCheckTest=False, 
>+                 stagePlatform=None, testPrettyNames=False, l10nCheckTest=False,

Removing trailing whitespace ++

>+        assert scratchbox_target is None, 'unimplemented'

Heh.

>-        self.latestDir = '/pub/mozilla.org/%s' % self.productName + \
>+        self.latestDir = '/pub/mozilla.org/%s' % self.stageProduct + \
>                          '/nightly/latest-%s' % self.branchName

Do you need

        if self.post_upload_include_platform:
            self.latestDir += "-%s" self.stagePlatform

?            

>+            self.addStep(ShellCommand,
>+                name='hg_update',
>+                command=['hg', 'update', '-r', tag],
>+                description=['updating', 'mozconfigs'],

NIT: Description's wrong here.

>+        if not uploadMulti:
>+            for master, warn, retries in self.talosMasters:

This seems like the wrong thing to key off of for sendchanges.
We could add a doSendchange=True, or split sendchanges to their own function.
Not a blocker.

Cleanup and packageTests need to be addressed here, but we're close.

Comment 64

[Aki Sasaki (not active)]

Comment on attachment 533184 [details] [diff] [review]
buildbot-configs v3

>@@ -791,18 +1107,26 @@ BRANCHES = {
>     'mozilla-central': {
>     },
>     'shadow-central': {
>-        'mobile_platforms': {},
>     },
>     'mozilla-beta': {
>     },
>     'mozilla-aurora': {
>     },
>     'mozilla-2.0': {
>-        'mobile_platforms': {},
>     },

To make up for removing |'mobile_platforms': {},|, do we need to remove the mobile platforms from 'platforms' ?

>-BRANCHES['try']['repo_path'] = 'try'
>+BRANCHES['try']['repo_path'] = 'users/jford_mozilla.com/try'

As mentioned in your comment, the patch needs some cleanup.


I'd definitely like to doublecheck the mozconfigs when you're ready.
We've done quite a bit of branding changes -- maemo+android nightly are nightly-branded, maemo+android aurora are aurora-branded, and maemo+android beta are beta branded. And mobile desktop is never branded.

Comment 65

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

(In reply to comment #63)
> Comment on attachment 533185 [details] [diff] [review] [review]
> buildbotcustom v3

> As mentioned in IRC, this regresses debug packageTests behavior, and your
> pastebinned fix looks good.

Fixed

> >-    def getPackageFilename(self, platform):
> >-        if platform.startswith("linux64"):
> >-            packageFilename = '*.linux-x86_64.tar.bz2'
> >+    def getPackageFilename(self, platform, platform_variation):
> >+        if 'android' in platform_variation:
> >+            packageFilename = '*arm.apk' #the arm.apk is to avoid
> >+                                         #unsigned/unaligned apks
> >         elif platform.startswith("linux"):
> >-            packageFilename = '*.linux-i686.tar.bz2'
> >+            packageFilename = '*.linux-*.tar.bz2'
> 
> As mentioned in IRC, this is probably harmless, but I can see possibly
> having 64- and 32- bit files in the same objdir at some point.
> 
> Probably not too risky, but explicitly leaving linux64 in there seems a tiny
> percentage safer.
> 
> ... was this for Maemo?  I'm going to guess yes.

Fixed!  There is a linux64 condition and I have added a maemo condition

> >-        self.latestDir = '/pub/mozilla.org/%s' % self.productName + \
> >+        self.latestDir = '/pub/mozilla.org/%s' % self.stageProduct + \
> >                          '/nightly/latest-%s' % self.branchName
> 
> Do you need
> 
>         if self.post_upload_include_platform:
>             self.latestDir += "-%s" self.stagePlatform
> 
> ?            

Strictly speaking, self.latestDir isn't used by this patch, but probably best to make this fit with the other uses of stagePlatform.  Fixed.

> >+            self.addStep(ShellCommand,
> >+                name='hg_update',
> >+                command=['hg', 'update', '-r', tag],
> >+                description=['updating', 'mozconfigs'],
> 
> NIT: Description's wrong here.

Fixed

> >+        if not uploadMulti:
> >+            for master, warn, retries in self.talosMasters:
> 
> This seems like the wrong thing to key off of for sendchanges.
> We could add a doSendchange=True, or split sendchanges to their own function.
> Not a blocker.

We could, but i don't think it is really worth it until we change how en-US/multi builds are done.    

> Cleanup and packageTests need to be addressed here, but we're close.

Cool!

Comment 66

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

(In reply to comment #64)
> Comment on attachment 533184 [details] [diff] [review] [review]
> buildbot-configs v3
> 
> >@@ -791,18 +1107,26 @@ BRANCHES = {
> >     'mozilla-central': {
> >     },
> >     'shadow-central': {
> >-        'mobile_platforms': {},
> >     },
> >     'mozilla-beta': {
> >     },
> >     'mozilla-aurora': {
> >     },
> >     'mozilla-2.0': {
> >-        'mobile_platforms': {},
> >     },
> 
> To make up for removing |'mobile_platforms': {},|, do we need to remove the
> mobile platforms from 'platforms' ?

Ahh, thanks for catching that.  Fixed.


> I'd definitely like to doublecheck the mozconfigs when you're ready.
> We've done quite a bit of branding changes -- maemo+android nightly are
> nightly-branded, maemo+android aurora are aurora-branded, and maemo+android
> beta are beta branded. And mobile desktop is never branded.

Tomorrow shall be known as the day of mozconfig and mozharness changes.

Comment 67

[[e-mail] (:bhearsum)]

(In reply to comment #60)
> (In reply to comment #59)
> > (In reply to comment #57)
> > > (In reply to comment #55)
> > > > Comment on attachment 533186 [details] [diff] [review] [review] [review] [review] [review]
> > > > tools v3
> > > > 
> > > > I'd like to see this script more in line with the other build/tools scripts
> > > > in the following ways:
> > > > - Use existing library functions where appropriate (eg,
> > > > commands.py:run_remote_cmd instead of re-implementing it as ssh)
> > > > - Use Python logging facilities
> > > > - Put the following functions in lib/python somewhere, and write tests for
> > > > them: hashFile, _scp
> > > 
> > > Can I do that as a follow up after the bulk of this lands?  These are
> > > non-trivial changes, which is ok, but they are going to delay landing for at
> > > least a day
> > 
> > Please do it in the first landing, we (as a group) have a poor track record
> > at coming back to these things.
> 
> My only concern is time.  I can change this behaviour.

Sorry, I'm holding firm on this. Please address the original comments above.

> > > > Don't specify "/tools/python-2.6.5/bin/python" here, that's asking for
> > > > bustage down the road. You should be using /tools/python/bin/python, the
> > > > same as the rest of the build process uses (which also happens to be first
> > > > in PATH).
> > > 
> > > Yes, but that is python 2.5 and this script needs python 2.6.  As we run
> > > buildbot on these slaves with python 2.6, I don't see this as a problem.  
> > 
> > > Could I file a follow up bug, noting in createAndroidSnippet that until the
> > > bug is resolved that we might have a source of bustage there.  The followup
> > > bug would be to create standard symlinks for python 2.6 that omit the minor
> > > version number (i.e. /tools/python-2.6 is a symlink to /tools/python-2.6.5).
> > 
> > We use it to launch Buildbot, yes, but everything in the build *process*
> > uses 2.5. Keeping that consistentency is important IMHO.
> > 
> > Which part of the script requires Python 2.6? I'm happy to help find a
> > replacement.
> 
> Specifically zipfile.ZipFile.open.  It is new in python 2.6, and the zipfile
> library is a mess without that function.  I can revert to calling the unzip
> application and opening the file on the filesystem.

.read() is present in 2.5, and if you wrap it in a StringIO it's a drop-in replacement for .open():
>>> z = ZipFile("gecko-unsigned-unaligned.apk")
>>> StringIO(z.read("application.ini")).read()
'[App]\nVendor=Mozilla\nName=Fennec\nVersion=6.0a1\nBuildID=20110519042057\nSourceRepository=http://hg.mozilla.org/mozilla-central\nSourceStamp=4fd08a1e0644\nID={a23983c0-fd0e-11dc-95ff-0800200c9a66}\n\n[Gecko]\nMinVersion=1.9.2b5pre\nMaxVersion=6.0a1\n\n[XRE]\nEnableExtensionManager=1\n\n[Crash Reporter]\nEnabled=1\nServerURL=https://crash-reports.mozilla.com/submit\n'

Comment 68

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

(In reply to comment #67)
> (In reply to comment #60)
> > My only concern is time.  I can change this behaviour.
> 
> Sorry, I'm holding firm on this. Please address the original comments above.

I wasn't being precise in my diction.  I was trying to say that yes, I was going to do as you requested and change the behaviour.

Comment 69

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

Attachment: mozilla-central mozconfigs

This patch has just mozconfig changes for mozilla-central.  Safe to land before we land anything else and if done soon enough, will make adding other mozconfigs easier.

Comment 70

[Aki Sasaki (not active)]

Comment on attachment 533824 [details] [diff] [review]
mozilla-central mozconfigs

I think you missed macosx-mobile's objdir.

When copying to create generic/project branch mozconfigs, please remove branding.
You'll need to create mozilla-{aurora,beta} mozconfigs, and add l10n-mozconfigs later.

This shouldn't harm anything, however, so r=me with the macosx objdir.

Comment 71

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

Attachment: buildbot-configs v4

address review comments, mozilla/config.py should be pretty much final.  mozconfigs for mozilla-central already have r+

the mozconfigs have nothing added

jhford-wifi:buildbot-configs jhford$ hg diff mozilla2 | grep "^+[^+]"
jhford-wifi:buildbot-configs jhford$ 

and nothing removed other than MOZ_OBJDIR settings

jhford-wifi:buildbot-configs jhford$ hg diff mozilla2 | grep "^-[^-]" | grep -vi objdir
jhford-wifi:buildbot-configs jhford$

Comment 72

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

Attachment: buildbotcustom v4

While the script still needs to change for android snippets, the interface too it wont.

Comment 73

[Aki Sasaki (not active)]

Comment on attachment 533987 [details] [diff] [review]
buildbot-configs v4

>+        'linux-android': {

In bug 655046, I'm coming to the conclusion that the android-r7/android platform difference between mozilla/config.py and mozilla-tests/config.py is breaking trychooser.

There are a number of ways to get around this -- hack trychooser to special case 'android-r7' (now 'linux-android'), or change mozilla-tests/config.py to use 'android-r7' or 'linux-android' as its dictionary keys.

To me, the right fix seems to be naming it 'android'.  I'm fine doing that after this set of patches land, but it'll be right afterwards.

>+            'mozconfig': 'linux-android/%(branch)s/nightly',

This is cool, but does mean that if we don't override this, we need to have a linux-android/BRANCH directory for each branch we build.

I'm not entirely sure how the non- mozilla-____ branches work (are they all PROJECT_BRANCHES ?) but it might be easiest to set this to generic/nightly and special case m-c and mozilla-aurora.

> PROJECTS = {
>@@ -791,18 +1107,41 @@ BRANCHES = {
>     'mozilla-central': {
>     },
>     'shadow-central': {
>-        'mobile_platforms': {},
>     },

This will enable mobile builds for s-c, which probably isn't too terrible.
It'll also enable mobile desktop builds for shadow-central, which I think aren't wanted.

>     'try': {
>-        'mobile_platforms': {
>-            'maemo5-gtk': {},
>-            'maemo5-qt': {},
>-            'android-r7': {},
>-        },
>     },

This will enable android-debug for try, which probably isn't too terrible.
It'll also enable mobile desktop builds for try, which I think aren't wanted.

>@@ -910,6 +1242,7 @@ for branch in BRANCHES.keys():
> ######## mozilla-central
> # This is a path, relative to HGURL, where the repository is located
> # HGURL + repo_path should be a valid repository
>+#BRANCHES['mozilla-central']['repo_path'] = 'mozilla-central'
> BRANCHES['mozilla-central']['repo_path'] = 'mozilla-central'

Intentional? :)

>-BRANCHES['try']['mobile_platforms']['android-r7']['mozconfig'] = 'mobile-try/android'
>-BRANCHES['try']['mobile_platforms']['maemo5-gtk']['mozconfig'] = 'mobile-try/maemo5-gtk/'
>-BRANCHES['try']['mobile_platforms']['maemo5-qt']['mozconfig'] = 'mobile-try/maemo5-qt'

As mentioned above.
I think it's going to be looking for linux-android/try/..., which doesn't exist.

>     # point to the mozconfigs, default is generic
>     for platform in BRANCHES[branch]['platforms']:
>-        if platform.endswith('debug'):
>+        if platform.endswith('debug') and 'linux-android' not in platform:
>             BRANCHES[branch]['platforms'][platform]['mozconfig'] = platform.split('-')[0] + '/' + branchConfig.get('mozconfig_dir', 'generic') + '/debug'
>         elif platform.endswith('qt'):
>             BRANCHES[branch]['platforms'][platform]['mozconfig'] = 'linux/' + branchConfig.get('mozconfig_dir', 'generic') + '/qt'

Will this set maemo5-qt's mozconfig to linux/... ?

We don't set android/maemo/mobile desktop mozconfigs here at all (except for maemo5-qt).  We either need a generic mozconfig and set it here or above (as mentioned) or we need to create mozconfig directories for every single branch.

>diff --git a/mozilla2/mobile/android/mozilla-beta/debug/mozconfig b/mozilla2/linux-android-debug/mozilla-beta/nightly/mozconfig
>copy from mozilla2/mobile/android/mozilla-beta/debug/mozconfig
>copy to mozilla2/linux-android-debug/mozilla-beta/nightly/mozconfig

We will probably not be doing mozilla-beta nightlies, which means these aren't needed, but they don't hurt. I'd rather have them and remove them later.


I imported both v4 patches, and ran test-masters.sh.
This seems problematic:

(bb08)deathduck:~/src/jhford/buildbot-configs [13:49:06]
573$ ./test-masters.sh 
staging-scheduler_master... OK
staging-builder_master1... Traceback (most recent call last):
  File "/Users/asasaki/wrk/virtualenv/bb08/lib/python2.6/site-packages/buildbot-0.8.2_hg_a63f22816750_production_0.8-py2.6.egg/buildbot/scripts/runner.py", line 1039, in doCheckConfig
    ConfigLoader(configFileName=configFileName)
  File "/Users/asasaki/wrk/virtualenv/bb08/lib/python2.6/site-packages/buildbot-0.8.2_hg_a63f22816750_production_0.8-py2.6.egg/buildbot/scripts/checkconfig.py", line 31, in __init__
    self.loadConfig(configFile, check_synchronously_only=True)
  File "/Users/asasaki/wrk/virtualenv/bb08/lib/python2.6/site-packages/buildbot-0.8.2_hg_a63f22816750_production_0.8-py2.6.egg/buildbot/master.py", line 783, in loadConfig
    % b['name'])
ValueError: duplicate builder name Maemo 5 GTK electrolysis build
Broken pieces are in master_dir

We definitely need to fix this, as well as our non- m-c/m-beta/m-aurora mozconfigs.

Comment 74

[Aki Sasaki (not active)]

Comment on attachment 533988 [details] [diff] [review]
buildbotcustom v4

Overall I think this is good, though you may need to revisit to fix the test-masters error above.

>         if self.packageTests:
>-            self.addStep(ShellCommand,
>+            self.addStep(ScratchboxCommand(
>              name='make_pkg_tests',

*snip*

>         # Get package details
>-        packageFilename = self.getPackageFilename(self.platform)
>+        packageFilename = self.getPackageFilename(self.platform,
>+                                                  self.platform_variation)
>         if packageFilename and 'rpm' not in self.platform_variation:
>-            self.addFilePropertiesSteps(filename=packageFilename, 
>+            self.addFilePropertiesSteps(filename=packageFilename,
>                                         directory='build/%s/dist' % self.mozillaObjdir,
>                                         fileType='package',
>                                         haltOnFailure=True)
>+        # Maemo special cases
>+        if 'maemo' in self.complete_platform:
>+            self.addStep(ScratchboxCommand(
>+                name='make_deb',

I don't want to rekindle the argument here, especially since we've got fixes for the rm -rf patch to land.

However, if we keep this patch as is:

* the rm -rf patch cannot land before this lands and gets a reconfig
* at which point all branches need the rm -rf patch, or mobile tests will be missing
* we have a very small window to get the patch landed into m-c, mozilla-beta, and mozilla-aurora
* project branches will be broken, as well as potentially try builds that aren't rebased properly
* our preproduction testing will be broken since we can't land the rm -rf patch before this patch lands in production

If we move the |if 'maemo'... make_deb| above |make package-tests|:

* the rm -rf patch can land at any point after this lands in production
* we could land the rm -rf patch in m-c and let it percolate through m-aurora, m-beta, and all project branches in its own good time, without any warning newsgroup posts and approvals and other time-sensitive coordination
* we can test end-to-end in preproduction and staging without any sort of user-repo gymastics before this patch lands in production

To me, it's a no-brainer to move it.
I'm fine doing that as an immediate followup patch once this lands on default if you're unhappy about that.

Comment 75

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

(In reply to comment #74)
> I don't want to rekindle the argument here, especially since we've got fixes
> for the rm -rf patch to land.
> 
> However, if we keep this patch as is:
> 
> * the rm -rf patch cannot land before this lands and gets a reconfig
> * at which point all branches need the rm -rf patch, or mobile tests will be
> missing
> * we have a very small window to get the patch landed into m-c,
> mozilla-beta, and mozilla-aurora
> * project branches will be broken, as well as potentially try builds that
> aren't rebased properly
> * our preproduction testing will be broken since we can't land the rm -rf
> patch before this patch lands in production
> 
> If we move the |if 'maemo'... make_deb| above |make package-tests|:
> 
> * the rm -rf patch can land at any point after this lands in production
> * we could land the rm -rf patch in m-c and let it percolate through
> m-aurora, m-beta, and all project branches in its own good time, without any
> warning newsgroup posts and approvals and other time-sensitive coordination
> * we can test end-to-end in preproduction and staging without any sort of
> user-repo gymastics before this patch lands in production

I don't know why this is coming up yet again.  If the concern is that we don't want to have to land the m-c, m-b, m-a and project patches during the downtime for this patch, we can land https://bug557260.bugzilla.mozilla.org/attachment.cgi?id=532671 on default and production.  Once that patch is merged to the production branch, it is safe to land the product build system patch at any time before this patch lands.  
 
> To me, it's a no-brainer to move it.

Is it?  Did you know that symbols won't be uploaded to stage if we don't land the product patch?  Changing where symbols are done is outside of the addUploadSteps function.  This change would introduce more risk than landing a one line change to product repositories that we have tested extensively.

> I'm fine doing that as an immediate followup patch once this lands on
> default if you're unhappy about that.

Given that we have a full mitigation strategy outlined above, I don't know why this is required.

Comment 76

[Aki Sasaki (not active)]

(In reply to comment #75)
> If the concern is that we
> don't want to have to land the m-c, m-b, m-a and project patches during the
> downtime for this patch, we can land
> https://bug557260.bugzilla.mozilla.org/attachment.cgi?id=532671 on default
> and production.  Once that patch is merged to the production branch, it is
> safe to land the product build system patch at any time before this patch
> lands.

That works as well. Want to aim for tomorrow's reconfig for that?

Comment 77

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

(In reply to comment #73)
> Comment on attachment 533987 [details] [diff] [review] [review]
> buildbot-configs v4
> 
> >+        'linux-android': {
> 
> In bug 655046, I'm coming to the conclusion that the android-r7/android
> platform difference between mozilla/config.py and mozilla-tests/config.py is
> breaking trychooser.

lets fix trychooser to understand 'android'

> There are a number of ways to get around this -- hack trychooser to special
> case 'android-r7' (now 'linux-android'), or change mozilla-tests/config.py
> to use 'android-r7' or 'linux-android' as its dictionary keys.

I think using linux-android in mozilla-tests is the right solution, with trychooser understanding that 'android' is an alias for 'linux-android'.

> To me, the right fix seems to be naming it 'android'.  I'm fine doing that
> after this set of patches land, but it'll be right afterwards.

The reason that I am using 'linux-%s' for platforms that are run with linux automation is that we don't need to change the entire mercurial build factory every time we add a new platform that also builds on linux.  If we rename linux-android to android on the build side, it logically follows that we need to rename linux-maemo5-gtk and linux-maemo5-qt to maemo5-gtk and maemo5-qt.  If we do that, what is currently described as 

   if platform.startswith('linux'):

would need to become 

  if platform.startswith('linux') or platform.startswith('android') or platform.startswith('maemo5'):

There are also a lot of steps that we currently use the self.platform MercurialBuildFactory variable, which is set to 'linux' when using linux-android.  If we change to using 'android',  we will have to modify all classes used in MercurialBuildFactory that depend on platform being 'linux' when running linux style automation. 

> >+            'mozconfig': 'linux-android/%(branch)s/nightly',
> 
> This is cool, but does mean that if we don't override this, we need to have
> a linux-android/BRANCH directory for each branch we build.

Yes, that is done currently for non-mobile builds.  This patch is about unifying the logic, so lets use the existing MercurialBuildFactory logic and file follow-up bugs if the desire is to change that.

> I'm not entirely sure how the non- mozilla-____ branches work (are they all
> PROJECT_BRANCHES ?) but it might be easiest to set this to generic/nightly
> and special case m-c and mozilla-aurora.

That isn't how non-mobile builds work, lets either fix everything as a follow-up or stick with the non-mobile configuration logic.

> > PROJECTS = {
> >@@ -791,18 +1107,41 @@ BRANCHES = {
> >     'mozilla-central': {
> >     },
> >     'shadow-central': {
> >-        'mobile_platforms': {},
> >     },
> 
> This will enable mobile builds for s-c, which probably isn't too terrible.
> It'll also enable mobile desktop builds for shadow-central, which I think
> aren't wanted.

I don't know what the desired coverage is, I think we need to talk to product/branch owners to figure this out.

> >     'try': {
> >-        'mobile_platforms': {
> >-            'maemo5-gtk': {},
> >-            'maemo5-qt': {},
> >-            'android-r7': {},
> >-        },
> >     },
> 
> This will enable android-debug for try, which probably isn't too terrible.

I wonder if the trychooser will allow us to deal with debug android the same way we deal with debug linux/win/mac

> It'll also enable mobile desktop builds for try, which I think aren't wanted.

well, it will make them available on try.  If the user doesn't ask for mobile desktop and try chooser is mandatory (which I believe it is soon), they won't get mobile desktop.

> 
> >@@ -910,6 +1242,7 @@ for branch in BRANCHES.keys():
> > ######## mozilla-central
> > # This is a path, relative to HGURL, where the repository is located
> > # HGURL + repo_path should be a valid repository
> >+#BRANCHES['mozilla-central']['repo_path'] = 'mozilla-central'
> > BRANCHES['mozilla-central']['repo_path'] = 'mozilla-central'
> 
> Intentional? :)

Nope!  the second line was where my user repo went, strange that i corrected it instead of deleting the staging line.

> 
> >-BRANCHES['try']['mobile_platforms']['android-r7']['mozconfig'] = 'mobile-try/android'
> >-BRANCHES['try']['mobile_platforms']['maemo5-gtk']['mozconfig'] = 'mobile-try/maemo5-gtk/'
> >-BRANCHES['try']['mobile_platforms']['maemo5-qt']['mozconfig'] = 'mobile-try/maemo5-qt'
> 
> As mentioned above.
> I think it's going to be looking for linux-android/try/..., which doesn't
> exist.

It doesn't exist in this patch, but will in the final changeset

> 
> >     # point to the mozconfigs, default is generic
> >     for platform in BRANCHES[branch]['platforms']:
> >-        if platform.endswith('debug'):
> >+        if platform.endswith('debug') and 'linux-android' not in platform:
> >             BRANCHES[branch]['platforms'][platform]['mozconfig'] = platform.split('-')[0] + '/' + branchConfig.get('mozconfig_dir', 'generic') + '/debug'
> >         elif platform.endswith('qt'):
> >             BRANCHES[branch]['platforms'][platform]['mozconfig'] = 'linux/' + branchConfig.get('mozconfig_dir', 'generic') + '/qt'
> 
> Will this set maemo5-qt's mozconfig to linux/... ?

hmm, yes.

> We don't set android/maemo/mobile desktop mozconfigs here at all (except for
> maemo5-qt).  We either need a generic mozconfig and set it here or above (as
> mentioned) or we need to create mozconfig directories for every single
> branch.

See http://hg.mozilla.org/build/buildbot-configs/file/6fa2a8b0d677/mozilla/config.py#l1506 which sets the mozconfig correctly.

> >diff --git a/mozilla2/mobile/android/mozilla-beta/debug/mozconfig b/mozilla2/linux-android-debug/mozilla-beta/nightly/mozconfig
> >copy from mozilla2/mobile/android/mozilla-beta/debug/mozconfig
> >copy to mozilla2/linux-android-debug/mozilla-beta/nightly/mozconfig
> 
> We will probably not be doing mozilla-beta nightlies, which means these
> aren't needed, but they don't hurt. I'd rather have them and remove them
> later.

A result of me not knowing what desired coverage is :)

> I imported both v4 patches, and ran test-masters.sh.
> This seems problematic:
> 
> (bb08)deathduck:~/src/jhford/buildbot-configs [13:49:06]
> 573$ ./test-masters.sh 
> staging-scheduler_master... OK
> staging-builder_master1... Traceback (most recent call last):
>   File
> "/Users/asasaki/wrk/virtualenv/bb08/lib/python2.6/site-packages/buildbot-0.8.
> 2_hg_a63f22816750_production_0.8-py2.6.egg/buildbot/scripts/runner.py", line
> 1039, in doCheckConfig
>     ConfigLoader(configFileName=configFileName)
>   File
> "/Users/asasaki/wrk/virtualenv/bb08/lib/python2.6/site-packages/buildbot-0.8.
> 2_hg_a63f22816750_production_0.8-py2.6.egg/buildbot/scripts/checkconfig.py",
> line 31, in __init__
>     self.loadConfig(configFile, check_synchronously_only=True)
>   File
> "/Users/asasaki/wrk/virtualenv/bb08/lib/python2.6/site-packages/buildbot-0.8.
> 2_hg_a63f22816750_production_0.8-py2.6.egg/buildbot/master.py", line 783, in
> loadConfig
>     % b['name'])
> ValueError: duplicate builder name Maemo 5 GTK electrolysis build
> Broken pieces are in master_dir

Avoiding staging things caused me to leave out my project_branches.py changes

diff --git a/mozilla/project_branches.py b/mozilla/project_branches.py
--- a/mozilla/project_branches.py
+++ b/mozilla/project_branches.py
@@ -4,7 +4,6 @@ PROJECT_BRANCHES = {
     'accessibility': {
         'enable_nightly': True,
         'enable_mobile': False,
-        'mobile_platforms': {},
         # only want a11y so turn off the default set
         'talos_suites': {
             'dirty': 0,
@@ -24,7 +23,6 @@ PROJECT_BRANCHES = {
         'enable_nightly': True,
         # need both of these to turn off mobile completely because of key in generic config.py
         'enable_mobile': False,
-        'mobile_platforms': {},
         'platforms': {
             'macosx-debug': {
                 'dont_build': True,
@@ -42,24 +40,6 @@ PROJECT_BRANCHES = {
     },
     'electrolysis': {
         'mozconfig_dir': 'electrolysis',
-        'mobile_platforms': {
-            'maemo5-gtk': {
-                'mozconfig': 'mobile/maemo5-gtk/mobile-e10s/nightly',
-            },
-            'maemo5-qt': {
-                'mozconfig': 'mobile/maemo5-qt/mobile-e10s/nightly',
-            },
-            'linux': {
-                'mozconfig': 'mobile/linux-i686/mobile-e10s/nightly',
-            },
-            'win32': {
-                'mozconfig': 'mobile/win32-i686/mobile-e10s/nightly',
-            },
-            'macosx': {
-                'mozconfig': 'mobile/macosx-i686/mobile-e10s/nightly',
-            },
-            'android-r7': {},
-        },
         'enable_talos': False,
     },
     'graphics':{
@@ -95,7 +75,6 @@ PROJECT_BRANCHES = {
     'private-browsing': {
         'enable_talos': False,
         'enable_mobile': False,
-        'mobile_platforms': {},
         'enable_nightly': True,
     },
     'services-central': {

> We definitely need to fix this, as well as our non- m-c/m-beta/m-aurora
> mozconfigs.

Yep, desired coverage is something we need to clarify with product owners

Comment 78

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

(In reply to comment #76)
> (In reply to comment #75)
> > If the concern is that we
> > don't want to have to land the m-c, m-b, m-a and project patches during the
> > downtime for this patch, we can land
> > https://bug557260.bugzilla.mozilla.org/attachment.cgi?id=532671 on default
> > and production.  Once that patch is merged to the production branch, it is
> > safe to land the product build system patch at any time before this patch
> > lands.
> 
> That works as well. Want to aim for tomorrow's reconfig for that?

Cool, I'll land on default today.

Comment 79

[Aki Sasaki (not active)]

(In reply to comment #77)
> I think using linux-android in mozilla-tests is the right solution, with
> trychooser understanding that 'android' is an alias for 'linux-android'.
> 
> > To me, the right fix seems to be naming it 'android'.  I'm fine doing that
> > after this set of patches land, but it'll be right afterwards.
> 
> The reason that I am using 'linux-%s' for platforms that are run with linux
> automation is that we don't need to change the entire mercurial build
> factory every time we add a new platform that also builds on linux.  If we
> rename linux-android to android on the build side, it logically follows that
> we need to rename linux-maemo5-gtk and linux-maemo5-qt to maemo5-gtk and
> maemo5-qt.  If we do that, what is currently described as 
> 
>    if platform.startswith('linux'):
> 
> would need to become 
> 
>   if platform.startswith('linux') or platform.startswith('android') or
> platform.startswith('maemo5'):

As mentioned in IRC, I don't like how we're overloading platform here, but we're already overloading platform.  Patches for your review to make trychooser work with linux-android in bug 655046.

> > >+            'mozconfig': 'linux-android/%(branch)s/nightly',
> > 
> > This is cool, but does mean that if we don't override this, we need to have
> > a linux-android/BRANCH directory for each branch we build.
> 
> Yes, that is done currently for non-mobile builds.  This patch is about
> unifying the logic, so lets use the existing MercurialBuildFactory logic and
> file follow-up bugs if the desire is to change that.
> 
> > I'm not entirely sure how the non- mozilla-____ branches work (are they all
> > PROJECT_BRANCHES ?) but it might be easiest to set this to generic/nightly
> > and special case m-c and mozilla-aurora.
> 
> That isn't how non-mobile builds work, lets either fix everything as a
> follow-up or stick with the non-mobile configuration logic.

Ok, just making sure we have a mozconfig for every branch we build
(missing some in this patch.

> I don't know what the desired coverage is, I think we need to talk to
> product/branch owners to figure this out.

Stuart gave us some pointers that we tried to match, and Lukas followed branch request forms that specified whether branch owners wanted mobile or not.

Rather than enable a bunch more builds than exist currently, ask everyone a second time whether they want those additional builds, and remove thme again later, I think it makes sense to match what we have now.

> > >     'try': {
> > >-        'mobile_platforms': {
> > >-            'maemo5-gtk': {},
> > >-            'maemo5-qt': {},
> > >-            'android-r7': {},
> > >-        },
> > >     },
> > 
> > This will enable android-debug for try, which probably isn't too terrible.
> 
> I wonder if the trychooser will allow us to deal with debug android the same
> way we deal with debug linux/win/mac
> 
> > It'll also enable mobile desktop builds for try, which I think aren't wanted.
> 
> well, it will make them available on try.  If the user doesn't ask for
> mobile desktop and try chooser is mandatory (which I believe it is soon),
> they won't get mobile desktop.

try: -a will build mobile desktop, which is probably not desired behavior.
I think it's safest to disable them.

> > As mentioned above.
> > I think it's going to be looking for linux-android/try/..., which doesn't
> > exist.
> 
> It doesn't exist in this patch, but will in the final changeset

Ok. You probably know this, but any project branch or try's mozconfig should not have branding.

> See
> http://hg.mozilla.org/build/buildbot-configs/file/6fa2a8b0d677/mozilla/
> config.py#l1506 which sets the mozconfig correctly.

Yeah, that looks like it'll work, and it sounds like you've got generic mozconfigs on the way. Cool.

>      'private-browsing': {
>          'enable_talos': False,
>          'enable_mobile': False,
> -        'mobile_platforms': {},
>          'enable_nightly': True,
>      },

We should probably remove the mobile platforms from platforms here, since that's how the config is now.

Comment 80

[[e-mail] (:bhearsum)]

Comment on attachment 533987 [details] [diff] [review]
buildbot-configs v4

Will wait for the next version before reviewing.

Comment 81

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

(In reply to comment #79)
> As mentioned in IRC, I don't like how we're overloading platform here, but
> we're already overloading platform.  Patches for your review to make
> trychooser work with linux-android in bug 655046.

Patches reviewed

> Stuart gave us some pointers that we tried to match, and Lukas followed
> branch request forms that specified whether branch owners wanted mobile or
> not.
> 
> Rather than enable a bunch more builds than exist currently, ask everyone a
> second time whether they want those additional builds, and remove thme again
> later, I think it makes sense to match what we have now.

Makes sense.  I was under the impression that we didn't have mobile for some branches because it was awkward to add mobile support.  I'll cut a patch that has matching coverage.

> try: -a will build mobile desktop, which is probably not desired behavior.
> I think it's safest to disable them.

My understanding is that 'try: -a' was supposed to give as close to a mozilla-central push as possible.  If we don't enable mobile desktop builds in a way that 'try: -a' picks up, my understanding is that you can't have a mobile desktop build on try at all.  As for resource wasting, developers are already firmly in the driver's seat when it comes to judicious use of try resources and they may need or want mobile desktop builds.

It is a trivially easy patch to write to disable the mobile desktop builds on try and I don't have a preference one way or the other.  I am happy to disable them on try.

> Ok. You probably know this, but any project branch or try's mozconfig should
> not have branding.

Yep, I know this :)

> >      'private-browsing': {
> >          'enable_talos': False,
> >          'enable_mobile': False,
> > -        'mobile_platforms': {},
> >          'enable_nightly': True,
> >      },
> 
> We should probably remove the mobile platforms from platforms here, since
> that's how the config is now.

Will do

(In reply to comment #80)
> Comment on attachment 533987 [details] [diff] [review] [review]
> buildbot-configs v4
> 
> Will wait for the next version before reviewing.

K, makes sense.

Comment 82

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

Attachment: tools changes for single locale repacks

These are the changes needed to do single locale repacks using new mobile automation.  I had to teach the script what the stage_platform is so that mozconfigs can use (for example) linux-mobile and the post upload command can use linux.

Comment 83

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

Attachment: mozconfigs

This patch copies all the mozconfigs to the new locations.  We should hg rm mozilla2/mobile and mozilla2/mobile-try (for non-ff4 mozconfigs) once we are sure that we have no more running old style automation builds

Comment 84

[Aki Sasaki (not active)]

Comment on attachment 535180 [details] [diff] [review]
mozconfigs

I imported these into a fresh checkout.

(bb08)deathduck:~/src/jhford/buildbot-configs/mozilla2 [15:39:02]
547$ grep -r brand linux-android
linux-android/electrolysis/nightly/mozconfig:ac_add_options --with-branding=mobile/branding/nightly
linux-android/generic/nightly/mozconfig:ac_add_options --with-branding=mobile/branding/nightly
linux-android/mozilla-aurora/nightly/mozconfig:ac_add_options --with-branding=mobile/branding/aurora
linux-android/mozilla-beta/nightly/mozconfig:ac_add_options --with-branding=mobile/branding/beta
linux-android/mozilla-central/nightly/mozconfig:ac_add_options --with-branding=mobile/branding/nightly
linux-android/tracemonkey/nightly/mozconfig:ac_add_options --with-branding=mobile/branding/nightly
linux-android/ux/nightly/mozconfig:ac_add_options --with-branding=mobile/branding/nightly

electrolysis, generic, tracemonkey, and ux should not have branding.
This is also a problem for linux-maemo5-gtk and linux-maemo5-qt.


(bb08)deathduck:~/src/jhford/buildbot-configs/mozilla2 [15:40:23]
551$ diff linux-android/electrolysis/nightly/mozconfig linux-android/generic/nightly/mozconfig 
(bb08)deathduck:~/src/jhford/buildbot-configs/mozilla2 [15:40:56]
552$ !!:s,generic,tracemonkey
diff linux-android/electrolysis/nightly/mozconfig linux-android/tracemonkey/nightly/mozconfig 
(bb08)deathduck:~/src/jhford/buildbot-configs/mozilla2 [15:41:09]
553$ !!:s,tracemonkey,ux
diff linux-android/electrolysis/nightly/mozconfig linux-android/ux/nightly/mozconfig

These are all the same.
Same with linux-android-debug, linux-mobile, and macosx-mobile.
linux-maemo5-gtk, linux-maemo5-qt, and win32-mobile have differing electrolysis mozconfigs.

I think it might be easiest to all use the generic mozconfigs for non- {mozilla-central,mozilla-aurora,mozilla-beta} branches, with exceptions.  Softlinks would also work.  This is not a blocker, just an observation.  If we create branch-specific mozconfigs like this, one of the things we need to doublecheck in preproduction/staging is that all branch builds have the mozconfigs they're expecting.

r- for branding; that should hopefully be a quick fix.

Comment 85

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

Attachment: mozconfigs v3

i was under the impression that nightly branding was the same as no branding.  corrected patch.

Comment 86

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

Comment on attachment 535255 [details] [diff] [review]
mozconfigs v3

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

Comment 87

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

Attachment: mozharness changes

these changes have been working for the multi-l10n repacks on staging-master for mozilla-central, same changes to aurora and beta.

Comment 88

[Armen [:armenzg]]

(In reply to comment #86)
> Comment on attachment 535255 [details] [diff] [review] [review]
> mozconfigs v3
> 
> http://hg.mozilla.org/build/buildbot-configs/rev/47d5646788ea

This got merged.

Comment 89

[Aki Sasaki (not active)]

Comment on attachment 535261 [details] [diff] [review]
mozharness changes

These look good, and since we're only adding files, this can land at any time.

It looks like we may be missing a mozilla-central_linux-maemo5-gtk.json .  I'm not too worried about this since our running multilocale Maemo5 QT is of questionable use, but if we're scheduling these, it should have a config.

Comment 90

[Aki Sasaki (not active)]

er, missing a mozilla-central_linux-maemo5-qt.json.

Comment 91

[[e-mail] (:bhearsum)]

Comment on attachment 535163 [details] [diff] [review]
tools changes for single locale repacks

Per real life chat, this patch will de-supported single locale repacks for 4.0.x. Somebody with authority needs to sign off on shutting those off when this lands.

If they do, you need to also remove the command line option --mobile-branch, as it does nothing.

Comment 92

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

(In reply to comment #89)
> Comment on attachment 535261 [details] [diff] [review] [review]
> mozharness changes
> 
> These look good, and since we're only adding files, this can land at any
> time.
> 
> It looks like we may be missing a mozilla-central_linux-maemo5-gtk.json . 
> I'm not too worried about this since our running multilocale Maemo5 QT is of
> questionable use, but if we're scheduling these, it should have a config.

(In reply to comment #90)
> er, missing a mozilla-central_linux-maemo5-qt.json.

As written, my patch does not do multi-l10n for maemo5-qt.  I would like to keep to one landing per repo as much as possible, so if we need those repacks, I'll hold off on landing

Comment 93

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

Attachment: tools changes for single locale repacks v2

Completely remove second repository support.  The script factory used for this script now has

                    factory = ScriptFactory(
                        scriptRepo='%s%s' % (config['hgurl'],
                                              config['build_tools_repo_path']),
                        interpreter='bash',
                        scriptName='scripts/l10n/nightly_mobile_repacks.sh',
                        extra_args=[platform, stage_platform,
                                    getRealpath('localconfig.py'),
                                    str(pf['l10n_chunks']), str(n)]
                    )

Comment 94

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

Attachment: buildbot-configs v5

This also needs a slightly updated buildbotcustom patch (v5).  The patch chunk (including unrelated, r+'d code) is below to save review time reading the whole buildbotcustom patch again.

diff --git a/misc.py b/misc.py
--- a/misc.py
+++ b/misc.py
@@ -966,12 +966,25 @@ def generateBranchObjects(config, name):
             )
     branchObjects['schedulers'].append(weekly_scheduler)
 
+    # This section is to make it easier to disable certain products.
+    # Ideally we could specify a shorter platforms key on the branch,
+    # but that doesn't work
+    enabled_platforms = []
     for platform in sorted(config['platforms'].keys()):
+        pf = config['platforms'][platform]
+        if pf['stage_product'] in config['enabled_products']:
+            enabled_platforms.append(platform)
+
+    for platform in enabled_platforms:
         # shorthand
         pf = config['platforms'][platform]
 
-        leakTest = False
-        codesighs = config.get('enable_codesighs',True)
+        # The stage platform needs to be used by the factory __init__ methods
+        # as well as the log handler status target.  Instead of repurposing the
+        # platform property on each builder, we will create a new property
+        # on the needed builders
+        stage_platform = pf.get('stage_platform', platform)
+
         uploadPackages = True
         uploadSymbols = False
         packageTests = False

Comment 95

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

Attachment: tools for android snippets

(In reply to comment #55)
> Comment on attachment 533186 [details] [diff] [review] [review]
> tools v3
> 
> I'd like to see this script more in line with the other build/tools scripts
> in the following ways:
> - Use existing library functions where appropriate (eg,
> commands.py:run_remote_cmd instead of re-implementing it as ssh)

fixed

> - Use Python logging facilities

fixed

> - Put the following functions in lib/python somewhere, and write tests for
> them: hashFile, _scp

filed bug 660245 and assigned to myself to work on.  This is a great thing to do, but it shouldn't block this patch (which blocks the whole bug landing, which blocks mobile releases on 0.8, which if missed, means another release we have to do on buildbot 0.7).


> > Specifically zipfile.ZipFile.open.  It is new in python 2.6, and the zipfile
> > library is a mess without that function.  I can revert to calling the unzip
> > application and opening the file on the filesystem.
> 
> .read() is present in 2.5, and if you wrap it in a StringIO it's a drop-in
> replacement for .open():

I have taken your suggestion and implemented it, but have left the .open call in comments.

Comment 96

[Aki Sasaki (not active)]

Comment on attachment 535425 [details] [diff] [review]
tools changes for single locale repacks v2

If we land on default this weekend, we may need to turn off or hide 4.0 nightly repacks.

Comment 97

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

Comment on attachment 535648 [details] [diff] [review]
tools for android snippets

changeset:   1456:47207577e830
tag:         tip
user:        John Ford <[e-mail]>
date:        Fri May 27 16:34:53 2011 -0400
summary:     bug 557260 - add script to generate nightly android snippets r=bhearsum

Comment 98

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

Comment on attachment 535261 [details] [diff] [review]
mozharness changes

changeset:   475:4622243f0388
tag:         tip
user:        John Ford <[e-mail]>
date:        Fri May 27 16:35:40 2011 -0400
summary:     bug 557260 - add configs for doing multi-l10n on new style automation r=aki

Comment 99

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

Comment on attachment 533988 [details] [diff] [review]
buildbotcustom v4

changeset:   1561:c928ed85bcf8
user:        John Ford <[e-mail]>
date:        Fri May 27 16:26:05 2011 -0400
summary:     bug 557260 - changes to automation to allow mobile product to be built with desktop automation r=aki+bhearsum p=jhford

Comment 100

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

Comment on attachment 535440 [details] [diff] [review]
buildbot-configs v5

changeset:   4167:99539903d320
user:        John Ford <[e-mail]>
date:        Fri May 27 16:23:06 2011 -0400
summary:     bug 557260 - enable mobile platforms on new automation r=aki+bhearsum p=jhford

A bit of confusion here.  I thought that Ben had reviewed the patch when I commited and pushed.  Spoke with ben on irc and he is fine with leaving it with Aki's review.  Apologies.

Comment 101

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

Had to land a couple patches,

http://hg.mozilla.org/build/buildbotcustom/rev/cd07adce87fd because i forgot to remove a staging remnant.
http://hg.mozilla.org/build/buildbotcustom/rev/892fa7ef7183 because something strange happened while tested code related to that section

Comment 102

[Aki Sasaki (not active)]

I dump_masters.sh'ed, then grepped the output for mozconfig.
You're missing these mozconfigs:

mozilla2/linux-android-debug/shadow-central/nightly/mozconfig
mozilla2/linux-android/shadow-central/nightly/mozconfig
mozilla2/linux-maemo5-gtk/shadow-central/nightly/mozconfig
mozilla2/linux-maemo5-qt/shadow-central/nightly/mozconfig
mozilla2/linux-mobile/shadow-central/nightly/mozconfig
mozilla2/macosx-mobile/mozilla-aurora/shark/mozconfig
mozilla2/macosx-mobile/mozilla-central/shark/mozconfig
mozilla2/macosx-mobile/shadow-central/nightly/mozconfig
mozilla2/macosx-mobile/tracemonkey/shark/mozconfig
mozilla2/win32-mobile/shadow-central/nightly/mozconfig

I'd guess we can create s-c mozconfigs and turn off shark.

Comment 103

[Aki Sasaki (not active)]

* Preproduction keeps timing out on its preprod releases (8020). Not your bug, but it means we don't have any release coverage yet.
* Preproduction build master looks very red.  Not sure how useful it'll be.  I've grabbed some slaves to do more targeted testing.
* We're getting preproduction exceptions on stage_platform.
* We should probably turn off ccache for win32-mobile and macosx-mobile. They're throwing errors and they don't look like they're enabled on macosx64 and win32.

Comment 104

[Aki Sasaki (not active)]

macosx-mobile uploadsymbols is broken because it's setting  SYMBOL_SERVER_SSH_KEY=/home/cltbld/.ssh/ffxbld_dsa

(should be /Users/cltbld/...)

Comment 105

[Aki Sasaki (not active)]

(In reply to comment #103)
> * We're getting preproduction exceptions on stage_platform.

This appears to be firefox desktop l10n jobs that need stage_platform set.

Comment 106

[Aki Sasaki (not active)]

The compile step on all windows (desktop+mobile) builds may be broken because the path is in the form of /e/builds/... rather than e:\builds\...

Comment 107

[Aki Sasaki (not active)]

Attachment: fix win32 compile step

It's been dying with:

make -f client.mk build MOZ_BUILD_DATE=20110528073018
 in dir /e/builds/moz2_slave/cen-w32-mb-ntly/build (timeout 10800 secs)

...

make: client.mk: No such file or directory
make: *** No rule to make target `client.mk'.  Stop.

This should hopefully fix it (testing atm)

Comment 108

[Aki Sasaki (not active)]

Attachment: fix macosx-mobile uploadsymbols

Comment 109

[Aki Sasaki (not active)]

Comment on attachment 535897 [details] [diff] [review]
fix win32 compile step

Looks like this needs to happen for all ScratchboxCommands (or ScratchboxCommand needs to set the workdir differently on windows).

I'll be submitting new configs/custom patches after some more testing.

Comment 110

[Aki Sasaki (not active)]

Attachment: configs fixes

Comment 111

[Aki Sasaki (not active)]

Attachment: buildbotcustom fixes

* turned off macosx-mobile shark builds
* added stage_platform and product props to mobile/desktop l10n
* fixed win32 ScratchboxCommand/ScratchboxProperty workdirs -- there's probably a more elegant way to do this, but this keeps them from burning.
* changed talosBranch to use self.complete_platform.  (Android talos sendchanges were sendchanging to BRANCH-linux-talos, which would have killed android tests and burned some linux runs.)  I don't think we want to run talos on anything else with dashes in the platform name, so I *think* this is good.

I'm going to keep running builds against these patches tonight/tomorrow.

Comment 112

[Aki Sasaki (not active)]

Hm, my configs comment didn't make it:

* disabled 4.x fennec desktop l10n -- the nightly builders were trying to trigger missing builders and going red.
* disabled macosx-mobile shark
* disabled macosx-mobile and win32-mobile ccache, which was going red. rm_old_symbols is still failing, but i'm leaving that for now to see if it stays that way.
* fixed SYMBOL_SERVER_SSH_KEY for macosx_mobile
* added shadow-central mozconfigs (softlinks for posix; hg cp's for windows)

This is what I'm currently testing on my staging server, plus setting the build_tools_repo_path to my user repo.

Comment 113

[Aki Sasaki (not active)]

* win32-mobile uploadsymbols is broken; I've got a patch
* win32-mobile dies on |make installer|; I've got a patch
* macosx-mobile |make package| is broken, but it's broken in production as well.
* android nightlies aren't creating snippets.  I've got a patch, but it turns on MAR creation as well, so it needs work.

I was able to clone users/prepr-ffxbld/mozilla-2.0 (see bug 660514) and kicked off a preproduction release.  The win32 Scratchbox* step breakage broke those, but I pushed my patched buildbotcustom up and that seems to have fixed that.  I'd still feel safest if someone did a staging release after these patches go live.

I'm going to let these tests run a bit longer, and try to fix the android snippet problem.

Comment 114

[Aki Sasaki (not active)]

I fixed Android to create android snippets without touching MARs, but it's still dying because packageFileName is set to en-US, and it's [correctly] working on the multi apk.

We probably need to set packageFileName again after running the multilocale script.

Comment 115

[Aki Sasaki (not active)]

Attachment: working configs

* DONE shadow-central mozconfigs
* DONE macosx make uploadsymbols fix

* DONE ccache off for macosx-mobile, win32-mobile
* DONE stop macosx-mobile shark builds
* DONE ScratchboxCommand, ScratchboxProperty fixes for win32
* DONE stage_platform for l10n
* DONE remove fennec 4 l10n trigger from nightlies
* DONE android talos sendchange branch wrong

* DONE win32-mobile doesn't do make installer

* macosx-mobile make_pkg broken, but broken in production as well.
* DONE android m-c nightly not creating snippets
** also had to fall back to create_mobile_snippet to not create tracemonkey/jaegermonkey etc. snippets

* IN PROGRESS preproduction release
** we should probably do a staging release before 5.0b4

* PENDING get reviews

Comment 116

[Aki Sasaki (not active)]

Attachment: working buildbotcustom fixes

Comment 117

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

Comment on attachment 536033 [details] [diff] [review]
working configs

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

Comment 118

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

Comment on attachment 536034 [details] [diff] [review]
working buildbotcustom fixes

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

::: process/factory.py
@@ +1386,5 @@
>                  haltOnFailure=True,
>              ))
>          # Windows special cases
>          if self.platform.startswith("win") and \
> +           'mobile' not in self.complete_platform and \

should we use stage_product instead of a substring in the platform name?  As is, i doubt this is going to cause a problem.

@@ +2294,5 @@
>          MercurialBuildFactory.addCreateUpdateSteps(self)
>          if self.createPartial:
>              self.addCreatePartialUpdateSteps()
>  
> +#aki

staging/dev leftovers?

@@ +2479,5 @@
>                  sb=self.use_scratchbox,
>                  timeout=40*60 # 40 minutes
>              ))
>  
> +        talosBranch = "%s-%s-talos" % (self.branchName, self.complete_platform)

This is going to change how sendchanges are sent for non-mobile platforms, but because we only do talos on platforms that have no '-' chars in them, this should be safe.  Good thing to do overall though.

Comment 119

[[e-mail] (:bhearsum)]

Tons of exceptions on preproduction master like this:
--------------------------------------------------------------------------------
Exception in /builds/buildbot/builder-master/twistd.log:
2011-05-29 20:46:57-0700 [-] Unhandled Error
	Traceback (most recent call last):
	  File "/tools/python/lib/python2.6/threading.py", line 504, in __bootstrap
	    self.__bootstrap_inner()
	  File "/tools/python/lib/python2.6/threading.py", line 532, in __bootstrap_inner
	    self.run()
	  File "/tools/python/lib/python2.6/threading.py", line 484, in run
	    self.__target(*self.__args, **self.__kwargs)
	--- <exception caught here> ---
	  File "/builds/buildbot/builder-master/sandbox/lib/python2.6/site-packages/twisted/python/threadpool.py", line 207, in _worker
	    result = context.call(ctx, function, *args, **kwargs)
	  File "/builds/buildbot/builder-master/sandbox/lib/python2.6/site-packages/twisted/python/context.py", line 59, in callWithContext
	    return self.currentContext().callWithContext(ctx, func, *args, **kw)
	  File "/builds/buildbot/builder-master/sandbox/lib/python2.6/site-packages/twisted/python/context.py", line 37, in callWithContext
	    return func(*args,**kw)
	  File "/builds/buildbot/buildbotcustom/buildbotcustom/status/log_handlers.py", line 83, in handleLogs
	    cmd = properties.render(cmd)
	  File "/builds/buildbot/builder-master/sandbox/lib/python2.6/site-packages/buildbot-0.8.2_hg_3dc678eecd11_production_0.8-py2.6.egg/buildbot/process/properties.py", line 108, in render
	    return [ self.render(e) for e in value ]
	  File "/builds/buildbot/builder-master/sandbox/lib/python2.6/site-packages/buildbot-0.8.2_hg_3dc678eecd11_production_0.8-py2.6.egg/buildbot/process/properties.py", line 106, in render
	    return value.render(self.pmap)
	  File "/builds/buildbot/builder-master/sandbox/lib/python2.6/site-packages/buildbot-0.8.2_hg_3dc678eecd11_production_0.8-py2.6.egg/buildbot/process/properties.py", line 194, in render
	    s = self.fmtstring % pmap
	  File "/builds/buildbot/builder-master/sandbox/lib/python2.6/site-packages/buildbot-0.8.2_hg_3dc678eecd11_production_0.8-py2.6.egg/buildbot/process/properties.py", line 169, in __getitem__
	    rv = properties[key]
	  File "/builds/buildbot/builder-master/sandbox/lib/python2.6/site-packages/buildbot-0.8.2_hg_3dc678eecd11_production_0.8-py2.6.egg/buildbot/process/properties.py", line 50, in __getitem__
	    rv = self.properties[name][0]
	exceptions.KeyError: 'stage_platform'

Comment 120

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

(In reply to comment #119)
> Tons of exceptions on preproduction master like this:
> -----------------------------------------------------------------------------
> ---
> Exception in /builds/buildbot/builder-master/twistd.log:
> 2011-05-29 20:46:57-0700 [-] Unhandled Error
> 	Traceback (most recent call last):
> 	  File "/tools/python/lib/python2.6/threading.py", line 504, in __bootstrap
> 	    self.__bootstrap_inner()
> 	  File "/tools/python/lib/python2.6/threading.py", line 532, in
> __bootstrap_inner
> 	    self.run()
> 	  File "/tools/python/lib/python2.6/threading.py", line 484, in run
> 	    self.__target(*self.__args, **self.__kwargs)
> 	--- <exception caught here> ---
> 	  File
> "/builds/buildbot/builder-master/sandbox/lib/python2.6/site-packages/twisted/
> python/threadpool.py", line 207, in _worker
> 	    result = context.call(ctx, function, *args, **kwargs)
> 	  File
> "/builds/buildbot/builder-master/sandbox/lib/python2.6/site-packages/twisted/
> python/context.py", line 59, in callWithContext
> 	    return self.currentContext().callWithContext(ctx, func, *args, **kw)
> 	  File
> "/builds/buildbot/builder-master/sandbox/lib/python2.6/site-packages/twisted/
> python/context.py", line 37, in callWithContext
> 	    return func(*args,**kw)
> 	  File
> "/builds/buildbot/buildbotcustom/buildbotcustom/status/log_handlers.py",
> line 83, in handleLogs
> 	    cmd = properties.render(cmd)
> 	  File
> "/builds/buildbot/builder-master/sandbox/lib/python2.6/site-packages/
> buildbot-0.8.2_hg_3dc678eecd11_production_0.8-py2.6.egg/buildbot/process/
> properties.py", line 108, in render
> 	    return [ self.render(e) for e in value ]
> 	  File
> "/builds/buildbot/builder-master/sandbox/lib/python2.6/site-packages/
> buildbot-0.8.2_hg_3dc678eecd11_production_0.8-py2.6.egg/buildbot/process/
> properties.py", line 106, in render
> 	    return value.render(self.pmap)
> 	  File
> "/builds/buildbot/builder-master/sandbox/lib/python2.6/site-packages/
> buildbot-0.8.2_hg_3dc678eecd11_production_0.8-py2.6.egg/buildbot/process/
> properties.py", line 194, in render
> 	    s = self.fmtstring % pmap
> 	  File
> "/builds/buildbot/builder-master/sandbox/lib/python2.6/site-packages/
> buildbot-0.8.2_hg_3dc678eecd11_production_0.8-py2.6.egg/buildbot/process/
> properties.py", line 169, in __getitem__
> 	    rv = properties[key]
> 	  File
> "/builds/buildbot/builder-master/sandbox/lib/python2.6/site-packages/
> buildbot-0.8.2_hg_3dc678eecd11_production_0.8-py2.6.egg/buildbot/process/
> properties.py", line 50, in __getitem__
> 	    rv = self.properties[name][0]
> 	exceptions.KeyError: 'stage_platform'

stage_platform will probably need to be set for all builders then, instead of the targetted ones.

Comment 121

[Phil Ringnalda (:philor)]

(In reply to comment #115)
> * macosx-mobile make_pkg broken, but broken in production as well.

Applying attachment 535462 [details] [diff] [review] from bug 659718 will fix that, if you want to see that it's the only bustage.

Comment 122

[Armen [:armenzg]]

I run ./test-masters.sh ~/repos/tools/buildfarm/maintenance/production-masters.json and made me add this line.

diff --git a/mozilla/config.py b/mozilla/config.py
--- a/mozilla/config.py
+++ b/mozilla/config.py
@@ -26,4 +26,5 @@ GLOBAL_VARS = {
     'stage_username_mobile': 'ffxbld', # TODO: remove when MobileBuildFactory dies
     'stage_base_path': '/home/ftp/pub',
+    'stage_base_path_xulrunner': '/home/ftp/pub/xulrunner',
     'stage_base_path_mobile': '/home/ftp/pub/mobile', # TODO: remove when MobileBuildFactory dies
     'stage_group': None,

Comment 123

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

(In reply to comment #122)
> I run ./test-masters.sh
> ~/repos/tools/buildfarm/maintenance/production-masters.json and made me add
> this line.
> 
> diff --git a/mozilla/config.py b/mozilla/config.py
> --- a/mozilla/config.py
> +++ b/mozilla/config.py
> @@ -26,4 +26,5 @@ GLOBAL_VARS = {
>      'stage_username_mobile': 'ffxbld', # TODO: remove when
> MobileBuildFactory dies
>      'stage_base_path': '/home/ftp/pub',
> +    'stage_base_path_xulrunner': '/home/ftp/pub/xulrunner',
>      'stage_base_path_mobile': '/home/ftp/pub/mobile', # TODO: remove when
> MobileBuildFactory dies
>      'stage_group': None,

We need to fix test_masters.sh to make sure that we can remove that line because it isn't what is used in the configuration anymore.  We now use stage_base_path + '/xulrunner' in the actual configuration logic.

Comment 124

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

(In reply to comment #123)
> We need to fix test_masters.sh to make sure that we can remove that line
> because it isn't what is used in the configuration anymore.  We now use
> stage_base_path + '/xulrunner' in the actual configuration logic.

turns out this was due to a custom repo and configs repo not on matching branches, which we expect not to work.  Sorted out over irc.

Comment 125

[[e-mail] (:bhearsum)]

Attachment: ensure /builds/slaves/DELETETHIS is missing

[10:31] <jhford-running-downtime> bhearsum|buildduty: could you do me a favour?
[10:32] <jhford-running-downtime> and write/deploy a puppet patch to delete a directory on the slave?
[10:32] <jhford-running-downtime> i am moving the build directories on the slaves to /builds/slave/DELETETHIS
[10:33] <jhford-running-downtime> instead of waiting for the rm -rf /builds/scratchbox.... to finish
[10:33] <jhford-running-downtime> i am moving the directories to a different location
[10:33] <jhford-running-downtime> where we can delete on startup
[10:34] <bhearsum|buildduty> sure
[10:34] <bhearsum|buildduty> 32-bit linux only, right?
[10:34] <jhford-running-downtime> yes

Comment 126

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 536293 [details] [diff] [review]
ensure /builds/slaves/DELETETHIS is missing

lgtm, just promise me you'll revert it soon :)

Comment 127

[[e-mail] (:bhearsum)]

(In reply to comment #126)
> Comment on attachment 536293 [details] [diff] [review] [review]
> ensure /builds/slaves/DELETETHIS is missing
> 
> lgtm, just promise me you'll revert it soon :)

This part's on you, John :)

Comment 128

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

(In reply to comment #127)
> (In reply to comment #126)
> > Comment on attachment 536293 [details] [diff] [review] [review] [review]
> > ensure /builds/slaves/DELETETHIS is missing
> > 
> > lgtm, just promise me you'll revert it soon :)
> 
> This part's on you, John :)

Okay!  I'll let this sit for a couple days to make sure that all the slaves get the directory deleted

The following slaves will need manual cleanup because I could not connect to them.

moz2-linux-slave17
linux-ix-slave09
linux-ix-slave10
linux-ix-slave11
linux-ix-slave23
linux-ix-slave29
linux-ix-slave01
linux-ix-slave03
linux-ix-slave06
linux-ix-slave13
linux-ix-slave16
linux-ix-slave17
linux-ix-slave33
linux-ix-slave34
linux-ix-slave35
linux-ix-slave42

Comment 129

[[e-mail] (:bhearsum)]

Comment on attachment 536293 [details] [diff] [review]
ensure /builds/slaves/DELETETHIS is missing

>diff --git a/os/centos.pp b/os/centos.pp
>index 4cbd3eb..e9c7a74 100644
>--- a/os/centos.pp
>+++ b/os/centos.pp
>@@ -65,6 +65,14 @@ class centos5 {
>         }
>         default: {
>             file {
>+                # This is a one-off deletion added while deploying bug 557260
>+                # We had to delete a bunch of old scratchbox directories
>+                # because they would break builds, and it was easier to move
>+                # them to this directory and have Puppet clean them up than to
>+                # wait for deletes to finish over ssh sessions.
>+                "/builds/slaves/DELETETHIS":

Aki points out this is a typo, and should read "slave" not "slaves". I landed a bustage fix: http://hg.mozilla.org/build/puppet-manifests/rev/2d2a9aa290cb

Comment 130

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

Attachment: bind mount /builds/moz2_slave as well

bind mount /builds/moz2_slave to make this work again for vms that use the old slave dir.

Comment 131

[[e-mail] (:bhearsum)]

Comment on attachment 536362 [details] [diff] [review]
bind mount /builds/moz2_slave as well

I don't think this will work unless you ensure moz2_slave exists. You need to add a rule like this one: http://hg.mozilla.org/build/puppet-manifests/file/tip/os/centos.pp#l142

and then change the require to: [Package::Install_rpm["scratchbox"], File["/builds/moz2_slave"]]

Comment 132

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

Attachment: bind mount /builds/moz2_slave as well v2

ensure directory exists

Comment 133

[[e-mail] (:bhearsum)]

Comment on attachment 536366 [details] [diff] [review]
bind mount /builds/moz2_slave as well v2

Turns out we can fix this in slavealloc by changing the basedir for the few linux slaves using moz2_slave.

Comment 134

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

Attachment: fix the staging_platform exceptions

follow up patch.  this adds a builder property which will allow the log upload status handler to figure out where on ftp to put the log

Comment 135

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

Attachment: fix the staging_platform exceptions v2

only the dep builds were broken so only fix them.

Comment 136

[Aki Sasaki (not active)]

Comment on attachment 536387 [details] [diff] [review]
fix the staging_platform exceptions v2

>diff --git a/misc.py b/misc.py
>--- a/misc.py
>+++ b/misc.py
>@@ -1424,8 +1424,8 @@ def generateBranchObjects(config, name):
>                     'nextSlave': _nextSlowSlave,
>                     'properties': {'branch': name,
>                                    'platform': platform,
>+                                   'product': pf['stage_product'],
>                                    'stage_platform': stage_platform,
>-                                   'product': pf['stage_product'],
>                                    'slavebuilddir': reallyShort('%s-%s-shark' % (name, platform))},

No change needed here, but this is harmless.

>                 }
>                 branchObjects['builders'].append(mozilla2_shark_builder)
>@@ -1462,7 +1462,11 @@ def generateBranchObjects(config, name):
>                 'factory': mozilla2_l10n_dep_factory,
>                 'category': name,
>                 'nextSlave': _nextL10nSlave(),
>-                'properties': {'branch': name, 'platform': platform,'slavebuilddir': reallyShort('%s-%s-l10n-dep' % (name, platform))},
>+                'properties': {'branch': name,
>+                               'platform': platform,
>+                               'stage_platform': stage_platform,
>+                               'product': pf['stage_product',

Missing a ] .  r=me with that fixed.

>+                               'slavebuilddir': reallyShort('%s-%s-l10n-dep' % (name, platform))},
>             }
>             branchObjects['builders'].append(mozilla2_l10n_dep_builder)
>

Comment 137

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

Comment on attachment 536387 [details] [diff] [review]
fix the staging_platform exceptions v2

http://hg.mozilla.org/build/buildbotcustom/rev/413aa9c1c864

Comment 138

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

Attachment: fix mobile snippets

this patch fixes logic issues with the snippet flags.

Comment 139

[Aki Sasaki (not active)]

Comment on attachment 536402 [details] [diff] [review]
fix mobile snippets

Slightly worried that we should be setting ausargs to {} for non-snippet platforms/branches, but I think I was only hitting problems with aus2_mobile_base_upload_dir not being defined on all branches; aus2_base_upload_dir is everywhere create_snippet is set.

If you're also worried, we can add an |if create_snippet:| after the final else: here, and set ausargs to {} if that isn't set.  But this is probably good.

Comment 140

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: m557260-puppet-manifests-p1-r1.patch

I deleted all of the DELETETHIS directories that weren't already gone via cssh, so this is no longer needed.

Comment 141

[Aki Sasaki (not active)]

Comment on attachment 536402 [details] [diff] [review]
fix mobile snippets

>diff --git a/misc.py b/misc.py
>--- a/misc.py
>+++ b/misc.py
>@@ -1220,21 +1220,16 @@ def generateBranchObjects(config, name):
>                 multiargs['multiLocaleConfig'] = multi_config_name
> 
>             create_snippet = config['create_snippet']
>-            if 'android' in platform:
>-                if config['create_mobile_snippet']:

There's a reason why I tested for create_mobile_snippet -- I didn't want to turn on useless Android updates for jaegermonkey, tracemonkey, and ux.
In fact, the mozconfig probably doesn't have updates turned on, so the snippet generation may fail out in tonight's nightly.

We either need to add updates to the mozconfigs (and then they'll still be useless unless we turn on the channels in AUS, if that's even wanted), or we should go back to checking for create_mobile_snippet or some other config flag that tells us we don't want to generate Android updates on these project branches.

Comment 142

[Aki Sasaki (not active)]

Attachment: turn off android updates for project branches

Comment 143

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

This work has landed and looks to be running smoothly so far.  If there are any issues related to this patch please file a follow up bug.

Rob, Sheila, what should I do with the status-firefox5/6 flags now that this work has been landed?

Comment 144

[Justin Wood (:Callek)]

Yes I know this comment is "to little to late" but adding here for posterity atm

(In reply to Ben Hearsum [:bhearsum] from comment #33)
> >+        pkg_patterns = []
> >+        for product in ('firefox', 'fennec'):
> >+            pkg_patterns.append('%s/dist/%s-*' % (self.mozillaObjdir,
> >+                                                  product))
> 
> Hardcoding--. Any reason you can't use product_name for this? You can
> override it as "fennec" for all of the mobile platforms.

(In reply to John Ford [:jhford] from comment #34)
> Yes, because product_name is not set in a valid way anymore.  It is a branch
> level option and I am not sure what all the consequences are of changing it
> to a platform level option.  As this was hardcoded before, I don't see this
> as a massive problem

The code below this was:
command="rm -rf %s/dist/%s-* %s/dist/install/sea/*.exe " %
(self.mozillaObjdir, self.productName, self.mozillaObjdir),

Which surely is not hardcoded to just Firefox, so this is much more hardcoded than discussed in your retort to it here.

/me is thinking of how it breaks SeaMonkey and other dependant apps... (this is MercurialBuildFactory, used by all of us)

I plan to add 'seamonkey' to this spot and merge to default in a future bug though, just as an fyi.