Need mobile release builds

RESOLVED FIXED

Status

Release Engineering
General
RESOLVED FIXED
9 years ago
5 years ago

People

(Reporter: aki, Assigned: aki)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fennec l10n])

Attachments

(4 attachments, 7 obsolete attachments)

(Assignee)

Description

9 years ago
We've danced around this problem a little too often.

The release builds should hopefully be able to:

- build from tags. we'll need to specify two (mobile-browser + m-1.9.2)
- have different mozconfigs. We should move official branding out of the nightly mozconfigs and into the release ones.

at least.  These'll need to trigger l10n + multi-l10n jobs as well.

Updated

9 years ago
Blocks: 513761

Updated

9 years ago
No longer blocks: 513761
Whiteboard: [fennec l10n]
Component: Release Engineering → Release Engineering: Future
(Assignee)

Comment 1

9 years ago
Is it going to be a problem that the multiL10n job finishes before the l10n repacks?

We've got l10n revisions in the release configs too.

Another is I need to make sure the release deb signing stuff works, at least manually.  Automated would be better.  But updating the repo every RC isn't necessarily ideal, either... is it?

Comment 2

9 years ago
single-locale en-US should upload to a different location that multi, so those shouldn't interfere anymore.

With respect to the input data, we should talk about that in more detail. Like, maemo-locales is not necessarily the right file to control what goes into the release multi-locale builds.

My personal thinking is to wrap shipped-locales, l10n-changesets, and maemo-locales into one file to be landed in buildbot-configs, and to make the multi-locale build be something like a platform.

Regarding the initial comment, the release factories should be careful when building from tags, as tags can be shifted between builds by basically anybody. Which is close to impossible to control on l10n, at which point I'd rather see us using the specified changesets.
(Assignee)

Updated

9 years ago
Assignee: nobody → aki
Component: Release Engineering: Future → Release Engineering

Comment 3

9 years ago
Bug 524519 might not completely block this bug but in the end it makes sense to use post_upload.py to do things the same way as in Firefox's releases.
Depends on: 524519

Updated

9 years ago
No longer depends on: 524519
(Assignee)

Comment 4

9 years ago
So it looks like Firefox releases have the following:

ReleaseTaggingFactory: I think we may be able to use the same factory.

SingleSourceFactory: Ditto. We'll need to use two of these.

(talos sendchanges): Skip, use the poller, or manually sendchange for now.

ReleaseBuildFactory: We need this... at the very least enable tags/revisions.

ReleaseRepackFactory: We need this too.

L10nVerifyFactory: Not sure if this is easy or not.  Possibly skip?

ReleaseUpdatesFactory: This is covered by the deb repo signing.  This will probably need to be updated to work with the release layout.

UpdateVerifyFactory, ReleaseFinalVerification: Looks AUS-specific. Skip.
(In reply to comment #4)
> UpdateVerifyFactory, ReleaseFinalVerification: Looks AUS-specific. Skip.

The analogues of these would be a check on a staging copy of deb repo (on a local server), and a check that a release copy is working. I'm not sure if release will point to releases.m.o or bouncer or <someplace> but if there's a difference then it's worth verifying both. 

For Firefox builds, the UpdateVerifyFactory diffs old_release + update vs new release, for both types of updates. The FinalVerification just pulls the AUS snippet on releasetest, and then checks that the bouncer urls in that return files.
(Assignee)

Updated

9 years ago
Depends on: 531873
(Assignee)

Comment 6

9 years ago
I think I might be nearly code complete for a completely untested and probably broken

* tagging
* source
* build
* repacks

http://hg.mozilla.org/users/asasaki_mozilla.com/mobile-release-configs/
http://hg.mozilla.org/users/asasaki_mozilla.com/mobile-release-custom/

I'm missing uploads currently; I'll probably reach for a quick scp fix.  I'm thinking pub/mozilla.org/mobile/candidates/VERSION-candidates/buildN/PLATFORM

I'll need to run a test release (or ten, more likely) after writing uploads. This will collide with any Firefox release testing unless I figure out a way around the single stage-ffxbld account (use my hg account?)

I'll also need to write and test the release repository creation and signing. For this to work for real, I'll need to know where the repositories are going to live... bouncer won't work for ftp.m.o, correct? Are we exporting mobile/repos/ to mofo or download or ...?  The .install file will have a hardcoded url in it.
(Assignee)

Comment 7

9 years ago
Wrote the uploads (still untested) -- checked into the above repos.

I don't have the release repository creation fully written yet -- I'm thinking about making this a 2 parter.  1st would be to update a set of release-test-repos which allows people to test the release debs in a non-official location.  The second would be after everything's signed off and would be tracked by bouncer.

I can test this over the weekend and/or all-hands.
(Assignee)

Comment 8

9 years ago
Repo creation and tagging worked. MultiSourceFactory worked, minus upload (looks like it's hanging. I may need to change the environment settings for mobile, and it may or may not be trying to upload to production stage.m.o).

They don't seem to be triggering each other the way Dependent schedulers should; I'm hoping that's just a naming issue.
(Assignee)

Comment 9

9 years ago
Created attachment 416619 [details] [diff] [review]
post_upload diff

Allow for a different subdirectory than "nightly" for candidate builds (I'm going with mobile/candidates/... instead of firefox/nightly/...)

I'm gonna shotgun these reviews around; comments are welcome from whomever.
Attachment #416619 - Flags: review?(nrthomas)
Comment on attachment 416619 [details] [diff] [review]
post_upload diff

>diff --git a/stage/post_upload.py b/stage/post_upload.py
>+#CANDIDATES_URL_PATH = "http://staging-stage.build.mozilla.org/pub/mozilla.org/%(product)s/$(parentdir)/%(version)s-candidates/build%(buildnumber)s"
> # For production
>+CANDIDATES_URL_PATH = "http://stage.mozilla.org/pub/mozilla.org/%(product)s/$(parentdir)/%(version)s-candidates/build%(buildnumber)s"

r+ if you squash the perl/shell-ism and use %(parentdir)s.
Attachment #416619 - Flags: review?(nrthomas) → review+
(Assignee)

Comment 11

9 years ago
Comment on attachment 416619 [details] [diff] [review]
post_upload diff

Oops.  I was certain I did that... the patch must have altered itself =P

http://hg.mozilla.org/build/tools/rev/12eed3e812ef
Attachment #416619 - Flags: checked-in+
(Assignee)

Comment 12

9 years ago
Created attachment 416660 [details] [diff] [review]
also update the candidatesPath with parentdir

The other patch only updates the url output from the step. This patch updates the location the binaries get copied to.

Tested in staging:
http://staging-stage.build.mozilla.org/pub/mozilla.org/mobile/candidates/1.0-candidates/build1/source/
Attachment #416660 - Flags: review?(nrthomas)
(Assignee)

Updated

9 years ago
Summary: Need 1.9.2 maemo release builds → Need mobile release builds
(Assignee)

Comment 13

9 years ago
Created attachment 416732 [details]
scratch l10n-changesets in json for mobile

This is a go at creating a combo l10n-changesets / maemo-locales / all|shipped-locales file in json.

The locales and changesets are taken directly from mozilla2-staging/l10n-changesets_mozilla-1.9.2 and may be different from the actual values we want.  The maemo-multilocale locales are taken from maemo-locales.

Axel, Coop: could you verify that this is an acceptable file format?
I'll attach a test python script that parses this.
(Assignee)

Comment 14

9 years ago
Created attachment 416733 [details]
test.py to parse l10n-changesets_mobile-1.0.json
(Assignee)

Comment 15

9 years ago
Current status:

- Axel has suggestions to improve the json format. I'm going to finish up what I'm doing and if I have time left over I'll try to implement those.

- repo setup, tagging, source, and build worked, except compare_locales failed after I disabled merge_locales, with my old changeset info.  I'm trying another test run with the json set to "default" for every l10n revision to see if I can get it to pass.

- I haven't yet tested the single-locale stuff, but I should hopefully be able to wrap that up tomorrow. I'm hoping to get patches up for review before bhearsum flies back east.

- After single locales are done I'll work on release repositories -- probably include the fixes for bug 533341 and test.

  - These should go in two stages -- one repo for release repository testing, and another repo for the live release.  I imagine skipping the former would be slightly easier for me, but harder for testing.

Updated

9 years ago
Depends on: 533997
(Assignee)

Comment 16

9 years ago
Created attachment 416940 [details] [diff] [review]
nthomas' solution to post_upload.py

Testing on staging-stage currently.
This isn't able to upload to both nightly/ and candidates/ in the same step, but I'm not sure if that's ever wanted/needed.  If it isn't, and if the test passes, I'll r and land.
(In reply to comment #16)
> This isn't able to upload to both nightly/ and candidates/ in the same step,
> but I'm not sure if that's ever wanted/needed.  If it isn't, and if the test
> passes, I'll r and land.

Can't imagine we'll ever need that for desktop builds.
(Assignee)

Comment 18

9 years ago
Comment on attachment 416940 [details] [diff] [review]
nthomas' solution to post_upload.py

worked on staging-stage.
Attachment #416940 - Flags: review+
(Assignee)

Updated

9 years ago
Attachment #416660 - Attachment is obsolete: true
Attachment #416660 - Flags: review?(nrthomas)
(Assignee)

Comment 19

9 years ago
Comment on attachment 416940 [details] [diff] [review]
nthomas' solution to post_upload.py

http://hg.mozilla.org/build/tools/rev/a9ebfa761780

updated surf + staging-stage
Attachment #416940 - Flags: checked-in+
(Assignee)

Comment 20

9 years ago
Ok, I've got repacks triggered and going green and uploading.  (A few are going orange in compare_locales; the final locale/changeset list should fix that.)

I'll copy the staging stuff into the production configs and update, then submit patches for review and work on bug 533997.
(Assignee)

Comment 21

9 years ago
Created attachment 416981 [details] [diff] [review]
configs for mobile release builds
Attachment #416732 - Attachment is obsolete: true
Attachment #416733 - Attachment is obsolete: true
Attachment #416981 - Flags: review?(bhearsum)
(Assignee)

Comment 22

9 years ago
Created attachment 416982 [details] [diff] [review]
factories for mobile release builds
Attachment #416982 - Flags: review?(bhearsum)
(Assignee)

Updated

9 years ago
Attachment #416982 - Flags: review?(l10n)
(Assignee)

Comment 23

9 years ago
Comment on attachment 416981 [details] [diff] [review]
configs for mobile release builds

with the caveat that I'm going to revisit the json at some point.
Attachment #416981 - Flags: review?(l10n)
(Assignee)

Comment 24

9 years ago
anyone else want in on this? me, i'll r? any [relevant] person here.
(Assignee)

Updated

9 years ago
Attachment #416981 - Flags: review?(nrthomas)
(Assignee)

Comment 25

9 years ago
Comment on attachment 416981 [details] [diff] [review]
configs for mobile release builds

The more the merrier. If I get one r+ and no r- I'll probably just land it.
(Assignee)

Updated

9 years ago
Attachment #416982 - Flags: review?(nrthomas)

Updated

9 years ago
Attachment #416982 - Flags: review?(l10n) → review-

Comment 26

9 years ago
Comment on attachment 416982 [details] [diff] [review]
factories for mobile release builds

I'll give an r- on this one, found a few things that we should get fixed:

+ platformLocales = {}

can we use defaultdict?

-                         'hg -R '+self.origSrcDir+' pull -r default ; ' +
+                         'hg -R '+self.origSrcDir+' pull -r '+
+                         self.l10nTag+' ; ' +

Pulling -r is weird here, in particular as you're pulling something that's different from en_revision in the else clause.

+            self.addStep(ShellCommand,
+             name='merge_locale',

etc should just be one addStep AFAICT, with flunkOnFailure = not self.mergeLocales.

Not sure if the removal of the mergedir should be optional, but I guess better safe than sorry there.

In MobileBuildFactory, there's an hg update, I'd make that hg update -C to make sure you can safely cross branches in all cases. Cross-check on hg version for that, though.

Nit, I prefer docstrings to be indented.

Blocker, if l10n-merge is off, you should not pass LOCALE_MERGEDIR to make installers-ab-CD nor to make chrome-ab-CD.
Comment on attachment 416981 [details] [diff] [review]
configs for mobile release builds

>diff --git a/mozilla2/linux/mobile-browser/nightly/mozconfig b/mozilla2/linux/mobile-browser/nightly/mozconfig

>-ac_add_app_options mobile --enable-official-branding

>diff --git a/mozilla2/linux/mobile-browser/release/mozconfig b/mozilla2/linux/mobile-browser/release/mozconfig

>+# mobile options
>+
>+ac_add_app_options mobile --enable-application=mobile
>+ac_add_app_options mobile --with-libxul-sdk=../xulrunner/dist

Removing branding from the nightly config is good, but shouldn't you also be adding --enable-official-branding to the release config?
(Assignee)

Comment 28

9 years ago
(In reply to comment #27)
> Removing branding from the nightly config is good, but shouldn't you also be
> adding --enable-official-branding to the release config?

Nice catch.
New patches incoming...
(Assignee)

Comment 29

9 years ago
Still building; I'll check to see if it worked in the morning.
Mainly worried about the pull -r bit.
(Assignee)

Comment 30

9 years ago
I'm getting

RuntimeError: File "brand.dtd" not found in /home/cltbld/build/maemo-1.9.2-release/users/stage-ffxbld/ru/mobile/branding/official

for each of the locales.

Is that just the --enable-official-branding breaking on the multi-locale repack?
Comment on attachment 416982 [details] [diff] [review]
factories for mobile release builds

\>+        if self.mergeLocales:
>+            self.addStep(ShellCommand,
>+             name='merge_locale',
>+             command=['python',
>+                      '../../../compare-locales/scripts/compare-locales'] +
>+                      mergeLocaleOptions +
>+                      ["l10n.ini",
>+                      "../../../%s" % self.l10nRepoPath,
>+                      WithProperties('%(locale)s')],
>+             description='comparing locale',
>+             env={'PYTHONPATH': ['../../../compare-locales/lib']},
>+             flunkOnFailure=False,
>+             warnOnFailure=True,
>+             workdir="%s/%s/%s/locales" % (self.baseWorkDir,
>+                                           self.origSrcDir,
>+                                           self.appName),
>+            )
>+        else:
>+            self.addStep(ShellCommand,
>+             name='merge_locale',
>+             command=['python',
>+                      '../../../compare-locales/scripts/compare-locales'] +
>+                      mergeLocaleOptions +
>+                      ["l10n.ini",
>+                      "../../../%s" % self.l10nRepoPath,
>+                      WithProperties('%(locale)s')],
>+             description='comparing locale',
>+             env={'PYTHONPATH': ['../../../compare-locales/lib']},
>+             haltOnFailure=True,
>+             workdir="%s/%s/%s/locales" % (self.baseWorkDir,
>+                                           self.origSrcDir,
>+                                           self.appName),
>+            )

What's going on here? Looks like the only difference in the cases is whether to fail or not. If that's the case, you need a better variable name than 'mergeLocales'.



>+class MultiSourceFactory(ReleaseFactory):
>+    def __init__(self, productName, version, baseTag, stagingServer,
>+                 stageUsername, stageSshKey, buildNumber, autoconfDirs=['.'],
>+                 buildSpace=1, repoConfig=None, uploadProductName=None,
>+                 parentDir="nightly", **kwargs):

Please choose a better name for 'parentDir' - something that indicates it's remote.

>+        ReleaseFactory.__init__(self, buildSpace=buildSpace, **kwargs)
>+        releaseTag = '%s_RELEASE' % (baseTag)
>+        bundleFiles = []
>+        sourceTarball = 'source/%s-%s.source.tar.bz2' % (productName,
>+                                                         version)
>+        if not uploadProductName:
>+            uploadProductName = productName
>+        if not repoConfig:
>+            repoConfig = [{
>+                'repoPath': self.repoPath,
>+                'location': self.branchName,
>+                'bundleName': '%s-%s.bundle' % (productName, version)
>+            }]

This seems messy...we should just force consumers to pass in the repo config.


> class MobileBuildFactory(MozillaBuildFactory):
>     def __init__(self, configRepoPath, mobileRepoPath, platform,
>                  configSubDir, mozconfig, objdir="objdir",
>                  stageUsername=None, stageSshKey=None, stageServer=None,
>                  stageBasePath=None, stageGroup=None,
>                  baseUploadDir=None, baseWorkDir='build', nightly=False,
>-                 clobber=False, env=None, **kwargs):
>+                 clobber=False, env=None, mobileRevision='default',
>+                 mozRevision='default', **kwargs):

You shouldn't be using 'default' anywhere. I know you override with a tag for releases, but this will change behaviour of nightly builds. Anytime a non-default branch changes lands we'll end up building the tip of default instead of it. Even worse, tbpl will always lie about these builds. IMHO it would be better to do like MercurialBuildFactory and default revisions to 'None', and only do 'hg up -r' if a revision is passed. This goes for the l10n parts, too.


> class MaemoBuildFactory(MobileBuildFactory):
>     def __init__(self, baseBuildDir, scratchboxPath="/scratchbox/moz_scratchbox",
>                  multiLocale = False,
>+                 uploadMultiLocaleInDir=False,

This isn't used anywhere - scrap it?


>+    def addUploadSteps(self, platform):
>+        self.addStep(ShellCommand,
>+         command='ssh -l '+self.stageUsername+' '+
>+                 '-i '+self.stageSshKey+' '+self.stageServer+' '+
>+                 'mkdir -p '+self.stageBasePath+' && '+
>+                 'scp -i '+self.stageSshKey+' '+self.packageGlob+' '+
>+                 self.stageUsername+'@'+self.stageServer+':'+
>+                 self.stageBasePath+' && '+
>+                 'ssh -l '+self.stageUsername+' -i '+self.stageSshKey+' '+
>+                 self.stageServer+' chmod -R 755 '+self.stageBasePath,
>+         timeout=30*60,
>+         haltOnFailure=True,
>+         workdir='%s/%s/%s' % (self.baseWorkDir, self.branchName,
>+                               self.objdir)
>+        )

I know that 'make upload' isn't ready for mobile yet, but can you use MozillaStageUpload in the interim? You might have to manually create the dir first but it seems cleaner than this.


>+    def addUploadSteps(self, platform):
>+        self.addStep(ShellCommand,
>+         command=WithProperties('ssh -l '+self.stageUsername+' '+
>+                 '-i '+self.stageSshKey+' '+self.stageServer+' '+
>+                 'mkdir -p '+self.stageBasePath+' && '+
>+
>+                 'scp -i '+self.stageSshKey+' '+self.packageGlob+' '+
>+                 self.stageUsername+'@'+self.stageServer+':'+
>+                 self.stageBasePath+' && '+
>+                 'ssh -l '+self.stageUsername+' -i '+self.stageSshKey+' '+
>+                 self.stageServer+' chmod -R 755 '+self.stageBasePath),
>+         timeout=30*60,
>+         haltOnFailure=True,
>+         workdir='%s/%s/dist' % (self.baseWorkDir, self.origSrcDir)
>+        )

Same thing here.


r- for this version. A couple asides:
* I'd like to see the results of a staging run of this stuff

And on a more general project note...I don't think we should rush this just because Fennec is having a release soon. I would *much* rather see a slower pace and better/cleaner code even if it means doing parts of 1.0 by hand. We had to do this for the first few releases of 3.1/3.5, for what it's worth.
Attachment #416982 - Flags: review?(bhearsum) → review-
(Assignee)

Comment 32

9 years ago
(In reply to comment #31)
> What's going on here? Looks like the only difference in the cases is whether to
> fail or not. If that's the case, you need a better variable name than
> 'mergeLocales'.

Yeah, Axel pointed this out above; addressed.

> This seems messy...we should just force consumers to pass in the repo config.

I wanted it to just handle a switch from SingleSourceFactory easily, but I can do that too.

> You shouldn't be using 'default' anywhere. I know you override with a tag for
> releases, but this will change behaviour of nightly builds.

The nightly builds are currently explicitly pulling 'default', so this won't be changing the behavior of nightly builds.

However, understood that this can cause issues, so I'll look at MercurialBuildFactory.

> This isn't used anywhere - scrap it?

Sure.

> I know that 'make upload' isn't ready for mobile yet, but can you use
> MozillaStageUpload in the interim? You might have to manually create the dir
> first but it seems cleaner than this.

afaict, MozillaStageUpload handles tinderbox-builds and nightly, and doesn't handle candidates at all.  This is just the ssh/scp command from MozillaStageUpload, changed to work for candidate builds.  Do you want me to change MozillaStageUpload to handle candidate builds?

> r- for this version. A couple asides:
> * I'd like to see the results of a staging run of this stuff

Sure.  Axel's requested changes have run on sm03; I'll do another run with your changes.

> And on a more general project note...I don't think we should rush this just
> because Fennec is having a release soon. I would *much* rather see a slower
> pace and better/cleaner code even if it means doing parts of 1.0 by hand. We
> had to do this for the first few releases of 3.1/3.5, for what it's worth.

I have a feeling that doing this manually may be error prone. If I'm going to make a mistake, I'd rather make the mistake in automation where it can be fixed for posterity, rather than a typo or mental lapse in manual steps where I can make the exact same mistake again later.

But yes, I think manually building or resorting to nightlies will be the fallback if this doesn't work.
(In reply to comment #32)
> > And on a more general project note...I don't think we should rush this just
> > because Fennec is having a release soon. I would *much* rather see a slower
> > pace and better/cleaner code even if it means doing parts of 1.0 by hand. We
> > had to do this for the first few releases of 3.1/3.5, for what it's worth.
> 
> I have a feeling that doing this manually may be error prone. If I'm going to
> make a mistake, I'd rather make the mistake in automation where it can be fixed
> for posterity, rather than a typo or mental lapse in manual steps where I can
> make the exact same mistake again later.

It is *definitely* more error prone, but that's not what we should be optimizing for yet. In the medium and long term we are much better off doing 1 or 2 releases partly manually if it means we have a cleaner, better, and/or more robust automation system that takes a bit longer.
Comment on attachment 416981 [details] [diff] [review]
configs for mobile release builds

>diff --git a/mozilla2-staging/l10n-changesets_mobile-1.0.json b/mozilla2-staging/l10n-changesets_mobile-1.0.json
>new file mode 100644
>--- /dev/null
>+++ b/mozilla2-staging/l10n-changesets_mobile-1.0.json
>@@ -0,0 +1,174 @@
>+{
>+  "af": {
>+    "revision": "default",
>+    "platforms": ["maemo"]
>+  },

I assume we'll be using specific changesets when we do a real release?



>diff --git a/mozilla2-staging/mobile_master.py b/mozilla2-staging/mobile_master.py
>--- a/mozilla2-staging/mobile_master.py
>+++ b/mozilla2-staging/mobile_master.py
>@@ -41,16 +41,24 @@ m = {}
> 
> m['builders'] = []
> m['schedulers'] = []
> m['change_source'] = []
> m['status'] = []
> 
> mobileBuilders = []
> 
>+import release_mobile_master
>+reload(release_mobile_master)

What's the rationale behind importing this here, as opposed to in master-main.cfg, like the other automation? Not sure I have an opinion on this, I'm curious what you think though.

>new file mode 100644
>--- /dev/null
>+++ b/mozilla2-staging/release-fennec-mozilla-1.9.2.py
>@@ -0,0 +1,53 @@
>+hgUsername          = 'stage-ffxbld'
>+hgSshKey            = '~cltbld/.ssh/ffxbld_dsa'
>+mobileBranchNick    = 'mobile-1.9.2'

I think 'mobileBranchConfigKey' or something to that effect would be more intuitive here.

>+version             = '1.0'
>+appVersion          = version
>+milestone           = '1.9.2b1'

You're going to want to use the following for the first 1.0 RC:
version    = 1.0rc1
appVersion = 1.0
milestone  = 1.9.2

The version/appVersion difference will end up with the builds having 'rc1' in their filenames - so we can ship it as such. When we ship 1.0 for real, we take whatever the latest rc is and do some renames (eg, https://wiki.mozilla.org/Releases/Firefox_3.5/BuildNotes#Stage_and_Rename_files).


>+enUSPlatforms       = {
>+    'maemo': {
>+        'config_platform': 'linux',
>+        'mobile_config_platform': 'linux-gnueabi-arm',
>+    },
>+}
>+l10nPlatforms       = {
>+    'maemo': {
>+        'l10n_platform': 'linux',
>+    },
>+}

I'm not a big fan of having the same variables using a different format. It's going to make merging this and release_{master,config}.py merging harder when the time comes. How about something like:
enUSPlatforms = ('linux')
l10nPlatforms = enUSPlatforms
mobilePlatforms = {'linux': 'linux-gnueabi-arm'}

You could also move the platform -> mobile platform translation into a helper function in buildbotcustom.


>-source_factory = SingleSourceFactory(
>+source_factory = MultiSourceFactory(

Given the 'untested as a drop in replacement' comment in the buildbotcustom side, let's not change this yet ;-).


>+mobile_source_factory = MultiSourceFactory(
>+    hgHost=branchConfig['hghost'],
>+    buildToolsRepoPath=branchConfig['build_tools_repo_path'],
>+    repoPath=mozSourceRepoPath,
>+    repoConfig=[{
>+        'repoPath': mozSourceRepoPath,
>+        'location': mozSourceRepoName,
>+        'bundleName': '%s-%s_%s.bundle' % (productName, version,
>+                                           mozSourceRepoName),
>+    },{
>+        'repoPath': mobileSourceRepoPath,
>+        'location': '%s/mobile' % mozSourceRepoName,
>+        'bundleName': '%s-%s_%s.bundle' % (productName, version,
>+                                           mobileSourceRepoName),
>+    }],

This is sufficiently complex to warrant constructing it outside of this. Please move it.


>+repo_setup_scheduler = Scheduler(
>+    name='mobile_repo_setup',
>+    branch=mobileSourceRepoPath,
>+    treeStableTimer=0,
>+    builderNames=['mobile_repo_setup'],
>+    fileIsImportant=lambda c: not isHgPollerTriggered(c, branchConfig['hgurl'])
>+)
>+schedulers.append(repo_setup_scheduler)

repo_setup is only for staging - we don't need any of the references to it in mozilla2/. You'll have to adjust the tagging scheduler so it fires, too.


This is pretty good, but there's a few things that need to be fixed, and I'd *really* like to see comments about variable inconsistency be addressed.
Attachment #416981 - Flags: review?(bhearsum) → review-
(Assignee)

Comment 35

9 years ago
(In reply to comment #34)
> I assume we'll be using specific changesets when we do a real release?

Yes =)
Also, since I have to keep l10n-merge on for official branding, we need to pare down the list as the repacks won't fail out on compare_locales.

> What's the rationale behind importing this here, as opposed to in
> master-main.cfg, like the other automation? Not sure I have an opinion on this,
> I'm curious what you think though.

To keep from having the mobile release on all masters.  But I'm not really tied to it either way.

> I think 'mobileBranchConfigKey' or something to that effect would be more
> intuitive here.

Ok. I refer to (mobile-trunk|mobile-1.9.2) as the branch nick elsewhere, but can rename.

> You're going to want to use the following for the first 1.0 RC:
> version    = 1.0rc1
> appVersion = 1.0
> milestone  = 1.9.2
> 
> The version/appVersion difference will end up with the builds having 'rc1' in
> their filenames - so we can ship it as such. When we ship 1.0 for real, we take
> whatever the latest rc is and do some renames (eg,
> https://wiki.mozilla.org/Releases/Firefox_3.5/BuildNotes#Stage_and_Rename_files).

Cool, this is very useful info.

> I'm not a big fan of having the same variables using a different format.

Ok, I'll revisit this & the other comments.

> repo_setup is only for staging - we don't need any of the references to it in
> mozilla2/. You'll have to adjust the tagging scheduler so it fires, too.

Nice catch. I think I removed the factory but forgot the scheduler.

> This is pretty good, but there's a few things that need to be fixed, and I'd
> *really* like to see comments about variable inconsistency be addressed.

I've addressed Axel's comments and am working on yours; I should have something working and tested tonight.  I'll try to leave a full run (from repo setup to repacks) on sm03 for later perusal.
(In reply to comment #35)
> (In reply to comment #34)
> > I assume we'll be using specific changesets when we do a real release?
> 
> Yes =)
> Also, since I have to keep l10n-merge on for official branding, we need to pare
> down the list as the repacks won't fail out on compare_locales.
> 

Sounds good.

> > What's the rationale behind importing this here, as opposed to in
> > master-main.cfg, like the other automation? Not sure I have an opinion on this,
> > I'm curious what you think though.
> 
> To keep from having the mobile release on all masters.  But I'm not really tied
> to it either way.

I think having it on all masters is a feature, actually. I suspect we'll hit cases in the mobile world where we have to spin a maintenance release and a beta at the same time. If you don't have any objection to it, we should enable that right the get go.

> > I think 'mobileBranchConfigKey' or something to that effect would be more
> > intuitive here.
> 
> Ok. I refer to (mobile-trunk|mobile-1.9.2) as the branch nick elsewhere, but
> can rename.

OK, I didn't know that. Keep it the same for consistency, if you want.

> I've addressed Axel's comments and am working on yours; I should have something
> working and tested tonight.  I'll try to leave a full run (from repo setup to
> repacks) on sm03 for later perusal.

Great!
(In reply to comment #30)
> 
> Is that just the --enable-official-branding breaking on the multi-locale
> repack?

Axel,Seth: This impacts branding. On multi-locale as well as single-locale debs, branded builds are showing up as "Fennec", not "Firefox". However, en-US single-locale builds *are* branded "Firefox". Aki thinks this is related to l10n merge and branding, but we're not certain. Do you have any suggestions or can you help investigate?

Stuart: I'd nominate this for fennec 1.0 blocker, but I dont see flag for that in this bug. If this feels like a blocker to you, I can spin off a separate bug and nom it?
(Assignee)

Comment 38

9 years ago
Created attachment 417654 [details] [diff] [review]
configs, version 3? 4?
Attachment #416981 - Attachment is obsolete: true
Attachment #417654 - Flags: review?(bhearsum)
Attachment #416981 - Flags: review?(nrthomas)
Attachment #416981 - Flags: review?(l10n)
(Assignee)

Comment 39

9 years ago
Created attachment 417655 [details] [diff] [review]
latest buildbotcustom patch

I think this addresses all comments except

Axel -- I still need to revisit the json file format at some point, but that can be a later incremental change after I get release deb repos going (and the rest of this patch landed), and

Ben -- I haven't yet fixed pulling default in the build factory. However, this isn't a regression, just a bug that's been there from the beginning.

I *think* this latest run on staging 3 will go without some annoying borkage.  If so, binaries will go here:

http://staging-stage.build.mozilla.org/pub/mozilla.org/mobile/candidates/
Attachment #417655 - Flags: review?(l10n)
(Assignee)

Updated

9 years ago
Attachment #417655 - Flags: review?(bhearsum)
(Assignee)

Updated

9 years ago
Attachment #416982 - Attachment is obsolete: true
Attachment #416982 - Flags: review?(nrthomas)
(Assignee)

Comment 40

9 years ago
Yay!  Everything went well from repo setup #46 on.
http://staging-stage.build.mozilla.org/pub/mozilla.org/mobile/candidates/1.0rc1-candidates/build1/

sources are in source/
builds are in maemo/ , and I finally got the repacks to stop trying to upload nightly/ and tinderbox-builds/ dirs in there.

logs/configs are on staging-master 8012.  Repacks are still going, green/orange (we'll need to make sure the unfinished locales aren't in the final changesets file)
Comment on attachment 417654 [details] [diff] [review]
configs, version 3? 4?

>+baseTag             = 'FENNEC_1_0'

This should be FENNEC_1_0rc1, just like version - sorry I didn't mention it before.

>+oldVersion          = '1.0b6pre'

Is this just for testing? I wouldn't expect anything with 'pre' in the real world.


It doesn't look like your Tag step is working quite right. It should be bumping the versions in the mobile-browser repo to remove the 'pre'. This will require an update to version-bump.pl and passing confvars.sh as a bump file. Easy enough to do manually for now, so this can be a follow-up if you'd like.

We should probably do something about the naming of the of the XULRunner deb, too. It's not very great to have a buildid in there for a release version. This part isn't a blocking issue though, I think.
Comment on attachment 417655 [details] [diff] [review]
latest buildbotcustom patch

>         self.addStep(ShellCommand,
>          name='merge_locale',
>          command=['python',
>-                  '../../../compare-locales/scripts/compare-locales',
>-                  '-m', 'merged',
>-                  "l10n.ini",
>+                  '../../../compare-locales/scripts/compare-locales'] +
>+                  mergeLocaleOptions +
>+                  ["l10n.ini",

The name "merge_locale" is still throwing me off here. Can you change it to 'run_compare_locales' or something - to more accurately reflect reality in both the merge and non-merge case? In the non-merge case, the name currently lies.


>+class MultiSourceFactory(ReleaseFactory):
>+    def __init__(self, productName, version, baseTag, stagingServer,
>+                 stageUsername, stageSshKey, buildNumber, autoconfDirs=['.'],
>+                 buildSpace=1, repoConfig=None, uploadProductName=None,
>+                 stageNightlyDir="nightly", **kwargs):
>+        ReleaseFactory.__init__(self, buildSpace=buildSpace, **kwargs)
>+        releaseTag = '%s_RELEASE' % (baseTag)
>+        bundleFiles = []
>+        sourceTarball = 'source/%s-%s.source.tar.bz2' % (productName,
>+                                                         version)
>+        if not uploadProductName:
>+            uploadProductName = productName
>+
>+        """repoConfig = [{
>+               'repoPath': repoPath,
>+               'location': branchName,
>+               'bundleName': '%s-%s.bundle' % (productName, version)
>+           }]"""
>+        assert repoConfig

What's going on here? I don't think you can use triple quotes as a way to exec()...

>>> """repoConfig = [{
...        'repoPath': repoPath,
...        'location': branchName,
...        'bundleName': '%s-%s.bundle' % (productName, version)
...    }]"""
"repoConfig = [{\n       'repoPath': repoPath,\n       'location': branchName,\n       'bundleName': '%s-%s.bundle' % (productName, version)\n   }]"
>>> assert repoConfig
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
NameError: name 'repoConfig' is not defined



> class MobileBuildFactory(MozillaBuildFactory):
>     def __init__(self, configRepoPath, mobileRepoPath, platform,
>                  configSubDir, mozconfig, objdir="objdir",
>                  stageUsername=None, stageSshKey=None, stageServer=None,
>                  stageBasePath=None, stageGroup=None,
>                  baseUploadDir=None, baseWorkDir='build', nightly=False,
>-                 clobber=False, env=None, **kwargs):
>+                 clobber=False, release=False, env=None,
>+                 mobileRevision='default',
>+                 mozRevision='default', **kwargs):

Please drop the 'release' options and move it to subclass, like we do for the other en-US/l10n release work.

> class MobileNightlyRepackFactory(BaseRepackFactory):
>     extraConfigureArgs = []
> 
>-    def __init__(self, enUSBinaryURL, hgHost=None, 
>+    def __init__(self, enUSBinaryURL, hgHost=None,
>                  nightly=True,
>+                 release=False,

Same thing here - you've even got a class to do it in already :).


>diff --git a/steps/transfer.py b/steps/transfer.py

This stuff is mostly fine. The XPIs don't end up in the same directory as the ones for Desktop Firefox. They should be in maemo/xpi/ab-CD.xpi. I don't think this is a blocking issue, though.


Random note: If Mobile Firefox ends up like the Fx/Tb relationship we'll end up shipping Mobile Firefox off of the Desktop Firefox relbranch. This is already a supported scenario so it doesn't affect your work here, but just a note that we'll have to start setting relbranchOverride if we do that, even for build1.

Good progress here, but the repoConfig thing doesn't look like it will work and the release work in the factories needs to be moved.
Attachment #417655 - Flags: review?(bhearsum) → review-

Comment 43

9 years ago
Is the official-branding thing resolved?

Comment 44

9 years ago
Observations on stagingmaster:8012:

- repacks don't have official-branding on
- can we do a test build with l10n-merge off?
(Assignee)

Comment 45

9 years ago
official branding is not resolved.
(Assignee)

Comment 46

9 years ago
(In reply to comment #42)
> The name "merge_locale" is still throwing me off here. Can you change it to
> 'run_compare_locales' or something - to more accurately reflect reality in both
> the merge and non-merge case? In the non-merge case, the name currently lies.

The step is "compare_locales" but the option is to turn on/off l10n-merge.
In l10n-merge, you use the -m merge options, and you don't fail if it errors out.  Without l10n-merge, you don't use those options, and you do fail if it errors out.  Given that I believe mergeLocales is a fine name.

Axel?

> >+        """repoConfig = [{
> >+               'repoPath': repoPath,
> >+               'location': branchName,
> >+               'bundleName': '%s-%s.bundle' % (productName, version)
> >+           }]"""
> >+        assert repoConfig
> 
> What's going on here? I don't think you can use triple quotes as a way to
> exec()...

This is a comment.

> 
> >>> """repoConfig = [{
> ...        'repoPath': repoPath,
> ...        'location': branchName,
> ...        'bundleName': '%s-%s.bundle' % (productName, version)
> ...    }]"""
> "repoConfig = [{\n       'repoPath': repoPath,\n       'location':
> branchName,\n       'bundleName': '%s-%s.bundle' % (productName, version)\n  
> }]"
> >>> assert repoConfig
> Traceback (most recent call last):
>   File "<stdin>", line 1, in <module>
> NameError: name 'repoConfig' is not defined

repoConfig is set in the release_mobile_config.py, which is softlinked to the release-fennec* file.

> Please drop the 'release' options and move it to subclass, like we do for the
> other en-US/l10n release work.

Ok.  This means I need to override the addUpload function, which I was trying to avoid, but I can do that.

I think this is the only point that needs to be resolved; is that agreed?
(Assignee)

Comment 47

9 years ago
(In reply to comment #46)

> This is a comment.
> repoConfig is set in the release_mobile_config.py, which is softlinked to the
> release-fennec* file.

Also, this is directly due to "This seems messy...we should just force consumers to pass in the repo config." back in comment 31.
(In reply to comment #46)
> (In reply to comment #42)
> > The name "merge_locale" is still throwing me off here. Can you change it to
> > 'run_compare_locales' or something - to more accurately reflect reality in both
> > the merge and non-merge case? In the non-merge case, the name currently lies.
> 
> The step is "compare_locales" but the option is to turn on/off l10n-merge.
> In l10n-merge, you use the -m merge options, and you don't fail if it errors
> out.  Without l10n-merge, you don't use those options, and you do fail if it
> errors out.  Given that I believe mergeLocales is a fine name.
> 

Right, the step is "compare locale", but the name it is "merge_locale" - which is confusing. Basically, I'm asking for this:
>         self.addStep(ShellCommand,
>          name='merge_locale',
to be changed to:
>         self.addStep(ShellCommand,
>          name='run_compare_locales',

Does that make sense, or do I still have my wires crossed?

> Axel?
> 
> > >+        """repoConfig = [{
> > >+               'repoPath': repoPath,
> > >+               'location': branchName,
> > >+               'bundleName': '%s-%s.bundle' % (productName, version)
> > >+           }]"""
> > >+        assert repoConfig
> > 
> > What's going on here? I don't think you can use triple quotes as a way to
> > exec()...
> 
> This is a comment.
> 
Can you move it to directly below the constructor, and maybe add a brief explanation? Where it is right now, and the lack of explanation is pretty 
confusing.

> > Please drop the 'release' options and move it to subclass, like we do for the
> > other en-US/l10n release work.
> 
> Ok.  This means I need to override the addUpload function, which I was trying
> to avoid, but I can do that.

Yeah, this will be consistent with what we do elsewhere.

> I think this is the only point that needs to be resolved; is that agreed?

I'd like to see the other stuff in this comment addressed too, but I think this are pretty trivial.
(Assignee)

Comment 49

9 years ago
(In reply to comment #48)
> Right, the step is "compare locale", but the name it is "merge_locale" - which
> is confusing.

Oh, ok.  Sorry, sleep dep and/or my general stupidity.

> I'd like to see the other stuff in this comment addressed too, but I think this
> are pretty trivial.

Yeah.  I've got 'em all.

Running a non-l10n-merge build for Axel (which will fail); then running a build and repack only with the new patch, then submitting the new patch for review.

Comment 50

9 years ago
AFAICT, the official branding stuff is bug 439453, marking the dependency. I reopened that bug.
Depends on: 439453
No longer depends on: 439453
(Assignee)

Comment 51

9 years ago
Created attachment 417785 [details] [diff] [review]
latest mobile release configs

Just removes the nightly/release options.
Attachment #417654 - Attachment is obsolete: true
Attachment #417785 - Flags: review?(bhearsum)
Attachment #417654 - Flags: review?(bhearsum)
(Assignee)

Comment 52

9 years ago
Created attachment 417786 [details] [diff] [review]
latest mobile release factories

Deals with bhearsum's comments, and adds some '-C's to hg updates for Axel, and removes a LOCALE_MERGEDIR if not self.mergeLocales for Axel as well.

The non-mergeLocales multilocale build failed until Gavin landed http://hg.mozilla.org/mobile-browser/rev/23ef409d62cf ; that should now be fixed.

However, after switching off mergeLocales and updating the mobile-browser/mozilla-1.9.2 revisions to match, I ran into bug 534961 for the single locale repacks.  That's r+'ed and just needs to land.
Attachment #417655 - Attachment is obsolete: true
Attachment #417786 - Flags: review?(bhearsum)
Attachment #417655 - Flags: review?(l10n)
(Assignee)

Comment 53

9 years ago
Comment on attachment 417786 [details] [diff] [review]
latest mobile release factories

In case you want to take another look at the latest patch.
Attachment #417786 - Flags: review?(l10n)

Comment 54

9 years ago
Comment on attachment 417786 [details] [diff] [review]
latest mobile release factories

Glanced at the patch and at one interdiff, as I'm in a hurry. r=me with a nit, there's some "FIXME" code which probably has historic roots at https://bugzilla.mozilla.org/attachment.cgi?id=417786&action=diff#a/misc.py_sec2. Remove that?
Attachment #417786 - Flags: review?(l10n) → review+
Comment on attachment 417785 [details] [diff] [review]
latest mobile release configs

As per IRC, please remove oldVersion and any other unused config vars. r=bhearsum with that change.
Attachment #417785 - Flags: review?(bhearsum) → review+
Comment on attachment 417786 [details] [diff] [review]
latest mobile release factories

This looks good to me.
Attachment #417786 - Flags: review?(bhearsum) → review+
(Assignee)

Comment 57

9 years ago
Comment on attachment 417785 [details] [diff] [review]
latest mobile release configs

http://hg.mozilla.org/build/buildbot-configs/rev/508c02b0ca32
Attachment #417785 - Flags: checked-in+
(Assignee)

Comment 58

9 years ago
Comment on attachment 417786 [details] [diff] [review]
latest mobile release factories

http://hg.mozilla.org/build/buildbotcustom/rev/51c501b26f09
Attachment #417786 - Flags: checked-in+
(Assignee)

Updated

9 years ago
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.