The idea is drastically reduce the time when we're _not_ offering a major update. So after a 3.0.x release we'd create a major update path to the latest 3.5 release. If a 3.5 was to closely follow a 3.0 then it may make sense to not enable it immediately.

> So after a 3.0.x release we'd create a major update path to the latest > 3.5 release. We'd be generating the major update snippets during the release process, but I don't recall the discussions about when they'd be enabled.

I'm going to take this off of Nick's plate.

As I see it, there's a few things to do here: * Make the HG automation capable of producing a set of major update snippets. * Decide where/when we want it to run (as part of the "from" release"? the "to" release? both?), and enable it there * Make update verify scripts work for MU testing out of the box. It would also be good to make patcher a bit smarter in --download, and only download the files that are actually required -- but that's not necessarily a blocker.

(In reply to comment #3) > * Decide where/when we want it to run (as part of the "from" release"? the "to" > release? both?), and enable it there After some IRC discussion, we decided not to integrate with the actual releases on the first pass. We'll automate it, but not have it triggered as part of the existing release process. When we get into a good groove with when it's run, the from/to releases, etc. we can talk about integrating it.

This is a quick and dirty-ish solution that lets us skip downloading complete MARs. Manually creating a symlink is still required between --download and --create-patches, and for that reason I'm not entirely happy with it. I'm going to see if I can build a workaround for that into patcher in a relatively clean way.

Needs a real run-through, but passes checkconfig.

This version is similar to the last, but I've renamed the command line option to be a bit more verbose, and fixed the "ValidateToolsDir" test towards the beginning to be skipped if --no-partial-mars is specified - which lets us save a ton of time by not cloning a source repository.

I gave this a good run through in both major update mode and a 3.5.6 - 3.5.7 run. It seems to work correctly for both scenarios. I'm not sure how useful the unit tests are, I found myself needing to update the tests + real code at the same time every time I made a change. They might be better off if there was a different subroutine for major updates rather than sharing it with the minor ones. It's still probably useful in the future for making sure we don't break existing behaviour.

This needs another run through and some additional verification still, after the patcher patch in checked in. (I couldn't run it with --no-partials and buildTools skipped today). I believe this is most of the way there, though. Nick, do you mind giving me some feedback on the approach? Probably not useful to do a full review until I've finished all my testing.

Comment on attachment 424297 [details] [diff] [review] a completed --no-partial-mars patch for patcher This isn't working out so well with further testing. I don't think it's going to work out as part of this bug. Removing review.

Comment on attachment 424306 [details] [diff] [review] patcher config bootstrapper >diff --git a/release/patcher-config-bootstrap.pl b/release/patcher-config-bootstrap.pl Maybe patcher-config-creator.pl to avoid overloading "bootstrap" ? >+sub GetProductDetails { This, the conversion of the locale data structure to exceptions, the substitutions on prettyVersion, and the test comparisons look like good candidates for putting in a common library, to share with patcher-config-bump.pl. Not asking for switching the latter over now, but could we make a start in that direction by doing it for this script ? Perhaps if we do that in hg, and after Bootstrap dies we can bring anything else over from MozBuild:Util and Bootstrap:Util that we need (and break out the wooden stakes for their cold cold hearts ...) >+sub BootstrapPatcherConfig { >+ my @channels = ('beta'); >+ if ($useBetaChannel) { >+ push(@channels, 'release'); >+ } Woah! That's a bit different to what that var was originally intended for (pushing separately to the beta channel ahead of a release). I guess we always do that now so it's fair enough. >+sub ExecuteTest { I agree that duplicating just about all the logic here isn't the way to go, so r-. Is there anything stopping us from checking in a reference config for each test ? We'd just have to get into the habit of running the unit tests before submitting any patches for review.

(In reply to comment #13) > (From update of attachment 424306 [details] [diff] [review]) > >diff --git a/release/patcher-config-bootstrap.pl b/release/patcher-config-bootstrap.pl > > Maybe patcher-config-creator.pl to avoid overloading "bootstrap" ? > > >+sub GetProductDetails { > > This, the conversion of the locale data structure to exceptions, the > substitutions on prettyVersion, and the test comparisons look like good > candidates for putting in a common library, to share with > patcher-config-bump.pl. Not asking for switching the latter over now, but could > we make a start in that direction by doing it for this script ? Perhaps if we > do that in hg, and after Bootstrap dies we can bring anything else over from > MozBuild:Util and Bootstrap:Util that we need (and break out the wooden stakes > for their cold cold hearts ...) Sure, I can do that. > >+sub BootstrapPatcherConfig { > >+ my @channels = ('beta'); > >+ if ($useBetaChannel) { > >+ push(@channels, 'release'); > >+ } > > Woah! That's a bit different to what that var was originally intended for > (pushing separately to the beta channel ahead of a release). I guess we always > do that now so it's fair enough. A fair point. I was also thinking of having a --channel argument to this script, which could be passed multiple times. Would you prefer that? With it, we could handle the channels inside of the factories rather than in here. > >+sub ExecuteTest { > > I agree that duplicating just about all the logic here isn't the way to go, so > r-. Is there anything stopping us from checking in a reference config for each > test ? We'd just have to get into the habit of running the unit tests before > submitting any patches for review. Sort of....the difficulty here is that some version of Config::General aren't predictable in the order in which they print. So you may end up with spurious failures because one prints alphabetically, and one prints in the order in which keys() returns things - or something. I should probably just take the hit and figure out which version prints alphabetically, and add some comments / code that warn about other versions.

Comment on attachment 424307 [details] [diff] [review] release updates factor, mu factory this is somewhat obsolete at this point. I'll be posting a new one soon

Nick, in this patch I've created a library for all the sharable Perl methods. I'm open to other layouts if my names don't make sense. I've also fixed the unit tests to not suck - I've got an accompanying patch to patcher-configs that adds the test configs. I've also fixed some minor bugs with the actual creator - details urls for major update vs minor update, for one. When I was creating the unit test configs I noticed that we didn't set prettyVersion for the 3.5.7 - 3.6rc2 MU. I need to figure out what the current state of affairs is there and ensure that this the creator script does the right thing. Apologies for the massive patch, but my mq series got all mixed up :-(.

This patch drops the --no-partial-mar and associated logic and fixes a couple of misc. issues found in testing

I think the 1.9.1 config is complete at this point, but I still need to update the others. I chose to use leave out a Scheduler for the Major Update factory entirely and used Triggerable to fire the update verify jobs. I figured this is much simpler than having to logon to a master and figure out all the right sendchange options to pass.

This patch is ready for review. It includes a major update generation builder, as well as update verify builders. I've doing a 3.5.7 -> 3.6rc2 MU which went fabulously. I verified it against the 3.5.7->3.6 final and the only differences, excluding staging-stage vs ftp, in the snippets were 3.6 vs 3.6rc2 in the appv and bouncer URLs. This is OK because my test run emulates a "normal" MU. We can generate "final release" MUs with this too by flipping majorUpdateToVersion to eg, '3.6'. update verify logs all matched the 3.6 final build notes.

No changes other than to version and old version information - I needed to bump them to do a proper test of the 'updates' builder.

Comment on attachment 424855 [details] [diff] [review] tools updates needed for major update automation This is ready for review and bug free, as far as I know.

Similar to the last version, but actually makes use of getUpdateVerifyBumpCommand() and corrects the check-in comment for the patcher config commit in MajorUpdateFactory.

Sorry for the churn - screwed up my last attachment.

Comment on attachment 425259 [details] [diff] [review] mu configs - same as before, better staging config variables Nits only, r+ to fix on checkin. >diff --git a/mozilla2/release-firefox-mozilla-1.9.1.py b/mozilla2/release-firefox-mozilla-1.9.1.py >+majorUpdateRepoPath = None Leaving this disabled as 3.5.8 is in progress ? >diff --git a/mozilla2/release_master.py b/mozilla2/release_master.py >+if majorUpdateRepoPath: >+ major_update_factory = MajorUpdateFactory( ... >+ # We disable this on staging, because we don't have a CVS mirror to >+ # commit to >+ commitPatcherConfig=False, Should be True for production, and the comment can go.

Comment on attachment 425269 [details] [diff] [review] buildbotcustom patch, for real >diff --git a/process/factory.py b/process/factory.py > class ReleaseUpdatesFactory(ReleaseFactory): >+ def __init__(self, cvsroot, patcherToolsTag, verifyConfigs, appName, ... >+ hgSshKey, hgUsername, patcherConfig='%(patcher-config)s', I didn't follow this change to the default for patcherConfig, and then later use WithProperties(self.patcherConfigFile). What did you have in mind ?

(In reply to comment #25) > (From update of attachment 425259 [details] [diff] [review]) > Nits only, r+ to fix on checkin. > > >diff --git a/mozilla2/release-firefox-mozilla-1.9.1.py b/mozilla2/release-firefox-mozilla-1.9.1.py > >+majorUpdateRepoPath = None > > Leaving this disabled as 3.5.8 is in progress ? Yeah, I didn't want to mess with it while a release was in progress. I figured we could update it when we do 3.5.9. > >diff --git a/mozilla2/release_master.py b/mozilla2/release_master.py > >+if majorUpdateRepoPath: > >+ major_update_factory = MajorUpdateFactory( > ... > >+ # We disable this on staging, because we don't have a CVS mirror to > >+ # commit to > >+ commitPatcherConfig=False, > > Should be True for production, and the comment can go. Whoops, thanks for catching that! (In reply to comment #26) > (From update of attachment 425269 [details] [diff] [review]) > >diff --git a/process/factory.py b/process/factory.py > > class ReleaseUpdatesFactory(ReleaseFactory): > >+ def __init__(self, cvsroot, patcherToolsTag, verifyConfigs, appName, > ... > >+ hgSshKey, hgUsername, patcherConfig='%(patcher-config)s', > > I didn't follow this change to the default for patcherConfig, and then later > use WithProperties(self.patcherConfigFile). What did you have in mind ? Ah. When I originally got started I was looking at making it possible to run this builder without a reconfig, eg, by passing patcher-config as a property on the waterfall. I should remove this though, because it will just cause bustage if we try to do it (since versions, repos, et. al will still be pulled release_config, and not necessarily be correct for an overridden patcher config). Does that make sense?

unchanged, other than the patcherConfig arg.

Same as before, with your comment addressed.

Comment on attachment 426576 [details] [diff] [review] buildbot-configs patch, v2 Could you prepare a patch for mozilla2/release-firefox-mozilla-1.9.1.py to land after 3.5.8 completes ? We'll be less likely to forget that way, and it's unlikely to rot that far down in the config.

Comment on attachment 426575 [details] [diff] [review] buildbotcustom patch, v2 Ok, just a few nits then, r+ to fix on checkin. >diff --git a/process/factory.py b/process/factory.py > class ReleaseUpdatesFactory(ReleaseFactory): >+ def __init__(self, cvsroot, patcherToolsTag, verifyConfigs, appName, ... >+ brandName=None, buildSpace=14, triggerSchedulers=None, Please add a comment documenting triggerSchedulers. >+ def setup(self): ... >+ # Bump the patcher config >+ self.addStep(ShellCommand( >+ name='get_shipped_locales', Comment needs moving to the top of bumpPatcherConfig(). >+ '-c', WithProperties(self.patcherConfigFile), The WithProperties can be replaced with self.patcherConfigFile.

Comment on attachment 424855 [details] [diff] [review] tools updates needed for major update automation >diff --git a/release/patcher-config-creator.pl b/release/patcher-config-creator.pl >+use MozBuild::Util qw(GetBuildIDFromFTP); >+use Bootstrap::Util qw(GetBouncerPlatforms GetBouncerToPatcherPlatformMap >+ LoadLocaleManifest); Think you only need LoadLocaleManifest here. >+my $FORCE_FILES = ['freebl3.chk', 'softokn3.chk', 'nssdbm3.chk', Please comment that this doesn't have any effect for major updates, as they provide complete mars only. >+Usage: bump-patcher-config.pl [options] >+This script depends on the MozBuild::Util and Bootstrap::Util modules. Script name and deps need updating. >+sub CreatePatcherConfig { >+ if (not LoadLocaleManifest(localeHashRef => $localeInfo, >+ manifest => $shippedLocales)) { >+ die "Could not load locale manifest"; Please put the filename in two instances of this error messages so they're unique. >+sub RunUnitTests { Could we add a comment somewhere describing all the things you need to pull to run these tests, and the command to use ? Saves on reverse engineering it from the updates factory any time you want to work on it. >+ platforms => {'linux-i686' => '20091029152254', >+ win32 => '20091029171059', >+ mac => '20091029151354'}, >+ oldPlatforms => {'linux-i686' => '20090806155851', >+ win32 => '20090806165642', >+ mac => '20090806155510'}, I don't see where these blocks are used. >+sub ExecuteTest { >+ my $stagingServer = 'build.mozilla.org'; Looks like we have a bunch of duplicated content at /var/www/html/build/pub and update-bump-unit-tests/. Can we remove one ? update-bump-unit-tests has the advantage of not requiring any ldap auth. >diff --git a/release/updates/moz191-firefox-linux-major.cfg b/release/updates >-build_id="20091221152502" \ >-locales="af ar as be bg bn-BD ... >-ftp_server="stage-old.mozilla.org/pub/mozilla.org" \ >+release="3.5.7" ... build_id="20100108093207" locales="en-US ja ro ... Looks like some testing changes slipped into the diff by mistake (this patch doesn't apply with these changes in it). Go ahead and collapse the multi-line format though.

Comment on attachment 425063 [details] [diff] [review] patcher creator unit tests configs This looks fine to me, and if you need to tweak any completemarurl's in response to my comments on the tools patch then go ahead.

Potential followups * converting patcher-config-bump.pl to GetProductDetails(), GetReleaseBlock(), BumpFilePath() * after Bootstrap dies moving everything out of CVS (ie update-verify-bump.pl)

Nick, thanks for your in depth review. I'm going to finish this up now, I'll attach a new tools patch and production version of the buildbot-configs.

(In reply to comment #34) > * after Bootstrap dies moving everything out of CVS (ie update-verify-bump.pl) Do you mean moving the two Util.pm modules out of CVS? update-verify-bump.pl is already in HG

* Updated usage message and various comments * Removed superfluous 'use' statements * Improved some 'die' messages. * Removed unused testing variables (platforms, oldPlatforms) * Use update-bump-unit-tests dir, so we can share with the update verify script. * Unbitrotted and corrected update verify config updates. I've also done you one better re: documenting what needs to be pulled -- I've added a script that pulls everything needed for us.

Updated to use the update-bump-unit-tests dir. Checking in moz-major-patcher-unit-test.cfg; /cvsroot/mozilla/tools/patcher-configs/moz-major-patcher-unit-test.cfg,v <-- moz-major-patcher-unit-test.cfg initial revision: 1.1 done RCS file: /cvsroot/mozilla/tools/patcher-configs/moz-minor-patcher-unit-test.cfg,v done Checking in moz-minor-patcher-unit-test.cfg; /cvsroot/mozilla/tools/patcher-configs/moz-minor-patcher-unit-test.cfg,v <-- moz-minor-patcher-unit-test.cfg initial revision: 1.1 done

This is a speculative update to the 1.9.1 release config. Ideally, when we do 3.5.9 we'll have done or will be doing 3.6.2 and won't have to touch this at that time.

(In reply to comment #37) > I've also done you one better re: documenting what needs to be pulled -- I've > added a script that pulls everything needed for us. Everything else looks good, assuming a patch relative to the original one I looked at, but no sign of the setup script here ?

(In reply to comment #36) > (In reply to comment #34) > > * after Bootstrap dies moving everything out of CVS (ie update-verify-bump.pl) > > Do you mean moving the two Util.pm modules out of CVS? update-verify-bump.pl is > already in HG All the useful bits that we can share between these scripts, yeah. /me stabs CVS.

Sorry about that Nick, I diff'ed wrongly. Here's the full tools patch, the only practical difference here is the setup script. No other changes.

Comment on attachment 428201 [details] [diff] [review] full tools patch, with setup script >diff --git a/release/download-perl-modules.sh b/release/download-perl-modules.sh Had a bunch of problems running this on my mac, and changed it to http://pastebin.mozilla.org/704285 get it working. Any comments on it ? Drops the tools clone since I think it's meant to be run from build/tools/release anyway. I get PASSes for update-verify-bump.pl & patcher-config-creator.pl --run-tests but patcher-config-bump.pl fails. We can leave the latter to bug 521111 though. Everything else looks fine.

Nick, here's the changes we chatted about -- your modified version of the patch plus a DEFAULT_CVSROOT for better out-of-box experience.

Comment on attachment 428605 [details] [diff] [review] the right patch Great work Ben!

Comment on attachment 426575 [details] [diff] [review] buildbotcustom patch, v2 changeset: 623:199ff21f5212

Comment on attachment 426576 [details] [diff] [review] buildbot-configs patch, v2 changeset: 2111:129ab407121b

Comment on attachment 427799 [details] [diff] [review] update production 1.9.1 config w/ speculative major update changeset: 2112:1c9b2ab68008

Comment on attachment 428605 [details] [diff] [review] the right patch changeset: 513:77af5ab84268

Everything landed, masters reconfig'ed. Still need to write some docs.

landed, docs written. closing this.

Comment on attachment 426575 [details] [diff] [review] buildbotcustom patch, v2 >+ self.oldRepoPath = oldRepoPath or mozRepoPath This falling back to the Mozilla repo path instead of the normal repo path means that anyone actually specifying a mozRepoPath (which is different from the main repo), such as SeaMonkey and Thunderbird, need to specify an oldRepoPath as well. I just had bustage in the SeaMonkey 2.0.4 process due to not having done that. :(