There's no technical reason why we need to block the start of non-l10n builds/source packages on l10n repositories being tagged. We should parallelize en-US and l10n tagging, and start en-US builds, source packages, and xulrunner builds as soon as en-US tagging finishes.

bug 708656 is adding a builder that runs prior to tagging. I think it will be easier to do this once it lands.

The bulk of this work is in tag-release.py. There's a couple ways we can approach it: 1) Add --tag-sourceRepo, --tag-l10n, --tag-other flags to the script, and make the associated blocks conditional on those flags. 2) Pass in a list of repositories as arguments, and check all repos against that list, skipping those that don't exist. I'm not sure which is better, etiher is probably OK.

This is the attempt at doing the tagging script change, still have the necessary buildbotcustom change to do.

Comment on attachment 592974 [details] [diff] [review] [tools] v1 Review of attachment 592974 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me. This can probably land right away ,considering it defaults to the old behaviour.

This should do it, I don't have a Firefox buildmaster test setup anywhere yet to verify that it doesn't cause any problems (even with checkconfig) though.

Comment on attachment 592974 [details] [diff] [review] [tools] v1 http://hg.mozilla.org/build/tools/rev/8107cd2e61af

Comment on attachment 594332 [details] [diff] [review] [buildbotcustom] v1 Review of attachment 594332 [details] [diff] [review]: ----------------------------------------------------------------- All the builder/scheduler changes look OK to me, but this code is fresh in Rail's mind so I'd like him to double check it. r- because of the small issue noted below. ::: process/release.py @@ +390,3 @@ > scriptRepo=tools_repo, > scriptName='scripts/release/tagging.sh', > + extra_args=("--tag-source",), This won't quite do it...these extra args are passed to tagging.sh, not tag-release.py. You'll need to find some way to pass them through.

I realized we have extra_data as an option to ScriptFactory, and this looks perfect for our use here.

This should adjust it as requested. Rail, ben wanted you to do a final review of it since this stuff is fresh in your mind.

Comment on attachment 594818 [details] [diff] [review] [buildbotcustom] v2 This is not a real review for now. Can you add a check if we really want to tag l10n repos? For example Fennec 11.0b1 was en-US only, so there will be no need to tag nor run repacks. Something like "if releaseConfig['l10nPlatforms']".

(In reply to Rail Aliiev [:rail] from comment #11) > Comment on attachment 594818 [details] [diff] [review] > [buildbotcustom] v2 > > This is not a real review for now. Can you add a check if we really want to > tag l10n repos? For example Fennec 11.0b1 was en-US only, so there will be > no need to tag nor run repacks. Something like "if > releaseConfig['l10nPlatforms']". The tag_l10n also does the tagging of "other" repos. I think the optional "don't do l10n repacks/tag l10n" should be a different bug. If you disagree I can try and fold that here, but I'd rather keep the logic separate to different bug.

Sounds good in this case. Fennec handles fine en-US only releases (with {} in the l10n json file). Not sure if en-US only Firefox should be a blocker here (another bug?). I'll review the patch in in a couple of days. Thanks for the patch, btw. It should reduce end-to-end time of the release procedure for another 10-20 minutes!

(In reply to Justin Wood (:Callek) from comment #12) > (In reply to Rail Aliiev [:rail] from comment #11) > > Comment on attachment 594818 [details] [diff] [review] > > [buildbotcustom] v2 > > > > This is not a real review for now. Can you add a check if we really want to > > tag l10n repos? For example Fennec 11.0b1 was en-US only, so there will be > > no need to tag nor run repacks. Something like "if > > releaseConfig['l10nPlatforms']". > > The tag_l10n also does the tagging of "other" repos. I think the optional > "don't do l10n repacks/tag l10n" should be a different bug. If you disagree > I can try and fold that here, but I'd rather keep the logic separate to > different bug. I think it makes sense to handle the tagging portion of this here. It should just modify which flags you pass in extra_data, no?

Comment on attachment 594818 [details] [diff] [review] [buildbotcustom] v2 Review of attachment 594818 [details] [diff] [review]: ----------------------------------------------------------------- ::: process/release.py @@ +1500,5 @@ > + repack_upstream = [ > + builderPrefix('%s_build' % platform), > + builderPrefix('%s_tag_l10n' % releaseConfig['productName']), > + ] > + repack_scheduler = AggregatingScheduler( Hmmm, if you're switching to AggregatingScheduler here you should remove the Trigger steps from the en-US builds, too. Even though its based on Triggerable, AggregatingScheduler functions like the Dependent Scheduler in terms of how builds are actually triggered. When you trigger() an AggergatingScheduler you actually reset its state, which would probably be bad here! Rail, any other gotchas to watch out for?

Comment on attachment 594818 [details] [diff] [review] [buildbotcustom] v2 > Hmmm, if you're switching to AggregatingScheduler here you should remove the > Trigger steps from the en-US builds, too. Even though its based on > Triggerable, AggregatingScheduler functions like the Dependent Scheduler in > terms of how builds are actually triggered. When you trigger() an > AggergatingScheduler you actually reset its state, which would probably be > bad here! +1 > Rail, any other gotchas to watch out for? I think that --tag-other should be done as a part of source tagging. Mobile builds would fail since they require tagged mozharness.

This should address all review comments: * Don't do tagging unless we have l10nPlatforms (instead do a Dummy* for tagging) * Do --tag-other in the source tagging. (Added a comment why) * Removed the Trigger directive from the Build.

Comment on attachment 595163 [details] [diff] [review] [buildbotcustom] v3 Review of attachment 595163 [details] [diff] [review]: ----------------------------------------------------------------- Yay! Thanks a lot for the patch. There are just 2 nits. Otherwise it looks fine and passes test_masters.sh. Let's land it once we're done with the 10.0.1 release. ::: process/release.py @@ +419,5 @@ > + scriptRepo=tools_repo, > + scriptName='scripts/release/tagging.sh', > + extra_data={"tag_args": "--tag-l10n"}, > + ) > + Please remove trailing spaces from this line. @@ +609,4 @@ > unittestBranch=unittestBranch, > clobberURL=branchConfig['base_clobber_url'], > triggerBuilds=True, > + triggeredSchedulers=None, You can drop this parameter completely since it's None by default.

Just pushed these, and had to do a followup for c#19 nits that I forgot to qref.

Didn't work as expected for 11.0b3. Tagging ran twice for all repos. Since tagging.sh runs without "set -x" it's harder to debug it. Some additional information: * download_extra step worked, what indicates that extra_data was passed * there is no data.json file on the slave

I suspect clobberer, adding "-s data.json" to our boilerplate script wrappers should help here...

Yes, I blame clobberer: rel-m-beta-firefox-tag-source:Clobbering... Skipping scripts Removing buildbot-configs/ Skipping last-clobber Skipping buildprops.json Removing mozilla-beta/ Removing data.json

I currently suspect this bug for breaking linux repacks (script_repo_revision not set anymore.)

(In reply to Aki Sasaki [:aki] from comment #24) > I currently suspect this bug for breaking linux repacks > (script_repo_revision not set anymore.) s,linux,all, script_repo_revision isn't set and branch is incorrect.

Not having the branch set also breaks clobberer: Checking clobber URL: http://build.mozilla.org/clobberer/index.php?master=http%3A%2F%2Fbuildbot-master08.build.sjc1.mozilla.com%3A8001%2F&slave=mv-moz2-linux-ix-slave02&builddir=rel-m-beta-lnx-rpk-2&branch=None&buildername=release-mozilla-beta-linux_repack_2%2F6 ... rel-m-beta-lnx-rpk-2:Our last clobber date: 2012-02-14 20:52:40 rel-m-beta-lnx-rpk-2:Server clobber date: 2012-01-27 16:05:40 vs Checking clobber URL: http://build.mozilla.org/clobberer/index.php?master=http%3A%2F%2Fbuildbot-master08.build.sjc1.mozilla.com%3A8001%2F&slave=mv-moz2-linux-ix-slave10&builddir=rel-m-beta-lnx-bld&branch=mozilla-beta&buildername=release-mozilla-beta-linux_build ... rel-m-beta-lnx-bld:Our last clobber date: 2012-02-14 10:26:49 rel-m-beta-lnx-bld:Server clobber date: 2012-02-14 20:52:40 rel-m-beta-lnx-bld:Server is forcing a clobber, initiated by asasaki@mozilla.com

Not sure which one of you needs to look, but this *should* fix my bustage here. I did not test since I have no staging area to use

(this time without typo)

Comment on attachment 597369 [details] [diff] [review] [buildbotcustom] fix bustage branch is a required parameter. On IRC we agreed to backout the patches and land them when they are ready and tested.

Comment on attachment 595163 [details] [diff] [review] [buildbotcustom] v3 Backout: http://hg.mozilla.org/build/buildbotcustom/rev/73437b4c315b

Comment on attachment 592974 [details] [diff] [review] [tools] v1 backout http://hg.mozilla.org/build/tools/rev/34ee54fea8e4

Comment on attachment 594814 [details] [diff] [review] [tools] Part 2, use EXTRA_DATA backout http://hg.mozilla.org/build/tools/rev/34ee54fea8e4

This replaces all previous patches on this bug, and [hopefully] corrects all issues. This is applied on top of bug #727489 and uses that design as branch=

This patch incorporates all previous patches in this bug, and does the following: * Fixes -z to -n * makes the use of EXTRA_DATA add to clobberer excludes * set -ex instead of set -e

(add in nits from previous review here)

I got to get better at saving and qref-ing before attaching lately.

Comment on attachment 598713 [details] [diff] [review] [tools] v3 Review of attachment 598713 [details] [diff] [review]: ----------------------------------------------------------------- ::: scripts/release/tag-release.py @@ +183,4 @@ > if err: > sys.exit(1) > + > + # Non-fatal warnings only after this point Please remove trailing spaces above. @@ +214,5 @@ > + help="Tag the source repo(s).") > + parser.add_option("--tag-l10n", dest="tag_l10n", > + help="Tag the L10n repo(s).") > + parser.add_option("--tag-other", dest="tag_other", > + help="Tag the other repo(s).") The options above expect argumets, if you try to run the script using command line you get: tag-release.py: error: --tag-l10n option requires an argument You need to use action="store_false/true" here. "tag-release.py ... --tag-source --tag-other" works, but it doesn't tag "other" repos, since optparse interprets --tag-other as an argument of --tag-source.

Callek, could you also update mozilla/release_templates to reflect the changes in the builder names? Please, also remove trailing spaces from the tools patch. I'm running a staging release right now. Tagging worked fine. Waiting for other builders to be triggered properly (l10n).

Mass move of bugs to Release Automation component.

Comment on attachment 599148 [details] [diff] [review] [tools] v4 r+ if you remove trailing spaces from the patch.

I'll be happy to review unbitrotten builbotcustom and buildbot-configs patches with template changes. ;)

Sorry it got lost in my queue.

This was re-created by hand based on the previous diff here, since I couldn't quickly remember how to rebase properly since my local repo was also newer enough to make the last copy of the patch fail to apply. A re-run of staging release is probably worth it

Last patch was old though had review, just re-requesting for the trio

Testing the patches in staging...

Interdiff: https://gist.github.com/2284520 * We still need tag_change, it used by http://hg.mozilla.org/build/buildbotcustom/file/309e0a7cddbd/process/release.py#l1818, then hardcoded here http://hg.mozilla.org/build/buildbotcustom/file/309e0a7cddbd/process/release.py#l219 * No need for *_tag_{source,l10n}_change templates, *_change templates are used only by ChangeNotifier, which uses hardcoded "tag" prefix. Ship it once you r+ this one!

Comment on attachment 611473 [details] [diff] [review] [buildbot-configs] v2 - Updates templates r+ for the changes, but f- since this didn't preserve the hg rename/copies I explicitly did in my patch. If you don't beat me to it I'll land in my unpaid time later today though.

(In reply to Justin Wood (:Callek) from comment #48) > Comment on attachment 611473 [details] [diff] [review] > [buildbot-configs] v2 - Updates templates > > r+ for the changes, but f- since this didn't preserve the hg rename/copies I > explicitly did in my patch. Ouch... I use git to generate the patches, which doesn't handle explicit file renames... The current patch should be fine. > If you don't beat me to it I'll land in my unpaid time later today though. Sure thing, no rush.

Comment on attachment 611361 [details] [diff] [review] [buildbotcustom] v4 - Unbitrotted http://hg.mozilla.org/build/buildbotcustom/rev/cdda75585a84 <-- Pushed with rail as author instead of me, not sure how that happened, but not worth the backout and reland imo.

Comment on attachment 611525 [details] [diff] [review] [buildbot-configs] v3 - Updates templates http://hg.mozilla.org/build/buildbot-configs/rev/906d96021b0c

Comment on attachment 611362 [details] [diff] [review] [tools] v4.1 - Updated (no trailing whitespace) http://hg.mozilla.org/build/tools/rev/bf37aabfd936

Sending a mail to release@ for a heads up, and this is now fixed.

Made it to production today.

I believe this broke Fennec l10n tagging.

(In reply to Aki Sasaki [:aki] from comment #55) > I believe this broke Fennec l10n tagging. Could you elaborate on this?

Since there are no l10nPlatforms (single locale) but we require l10n tagging for multilocale, we can't go by len(l10nPlatforms) as the only check for whether to tag. Testing on staging.

Comment on attachment 619157 [details] [diff] [review] (untested) fix fennec split tagging, with typo fix This patch gets the Fennec l10n tag builder to be a non-Dummy builder, but I'm hitting issues where the Fennec l10n mozilla-beta tag builder is trying to tag mozilla-beta over and over (instead of noticing there are no locales to tag, and skipping). I'm leaning towards backing out split tagging to fix for now.

Comment on attachment 611361 [details] [diff] [review] [buildbotcustom] v4 - Unbitrotted Backed Out in http://hg.mozilla.org/build/buildbotcustom/rev/9a5fc1d73fa6 and Merged (complex merge) in http://hg.mozilla.org/build/buildbotcustom/rev/5a0ad7d77453

Comment on attachment 611525 [details] [diff] [review] [buildbot-configs] v3 - Updates templates Backed out in http://hg.mozilla.org/build/buildbot-configs/rev/e8b171b117dd Merge in http://hg.mozilla.org/build/buildbot-configs/rev/cc59d4835ed9

Comment on attachment 611362 [details] [diff] [review] [tools] v4.1 - Updated (no trailing whitespace) Backed out in http://hg.mozilla.org/build/tools/rev/30a2aa656d8e Merged in http://hg.mozilla.org/build/tools/rev/195281b37ed7

There are a few gotchas here in terms of mobile: * The locales are specified in a json changeset file, not the l10n-changesets text file. ** This means that get_l10n_repositories() is not necessarily an accurate list for Fennec; get_locales_from_json() would be correct. ** This bug existed before your work * Multilocale is currently enabled, even though l10nPlatforms isn't. I tried to account for this in https://bugzilla.mozilla.org/attachment.cgi?id=619157 . * However, I hit issues while trying to tag Fennec after applying this patch (see comment 59). So that patch does create an l10n tagging builder instead of a dummy builder, but the builder seems to be doing the wrong thing. ** This may be a side effect of an empty json changeset file on beta?

I know this isn't actively being worked on, but if/when it gets done please make sure to update the Tag section of the documentation to talk about it: https://wiki.mozilla.org/Release:Release_Automation_on_Mercurial:Tagging_through_Signing#Tag

(In reply to Aki Sasaki [:aki] from comment #57) > Since there are no l10nPlatforms (single locale) but we require l10n tagging > for multilocale, we can't go by len(l10nPlatforms) as the only check for > whether to tag. Now that: 1) android native, with single locale repacks, is on mozilla-release, (l10nPlatforms is not empty), and 2) we don't have to support linux-android multilocale (no single locale repacks) on esr10, we may be able to land this again without breaking mobile.

I haven't worked on this in a while now. But could further reduce end-to-end time for release jobs if someone wanted to prioritize it.

Ported Callek's patch to current version of buildbotcustom and updated tag_l10n_factory to use mock.

Comment on attachment 8532606 [details] [diff] [review] [buildbotcustom] Split tagging into en-US and other.patch Review of attachment 8532606 [details] [diff] [review]: ----------------------------------------------------------------- In overall it looks good, just some minor changes. ::: process/release.py @@ +68,4 @@ > mozharness_repo_path = releaseConfig.get('mozharness_repo_path', > branchConfig['mozharness_repo_path']) > mozharness_repo = '%s%s' % (branchConfig['hgurl'], mozharness_repo_path) > + with_l10n = False This line is redundant. @@ +423,4 @@ > ) > > builders.append({ > + 'name': builderPrefix('%s_tag_source' % releaseConfig['productName']), This requires template file name changes. Can you rename the templates as well? @@ +457,5 @@ > + extra_data={"tag_args": "--tag-l10n"}, > + ) > + > + builders.append({ > + 'name': builderPrefix('%s_tag_l10n' % releaseConfig['productName']), This requires an extra template. @@ +464,5 @@ > + 'builddir': builderPrefix('%s_tag_l10n' % releaseConfig['productName']), > + 'slavebuilddir': normalizeName( > + builderPrefix('%s_tag_l10n' % releaseConfig['productName'])), > + 'factory': tag_l10n_factory, > + 'nextSlave': _nextSlave_skip_spot, nextSlave will be going away with bug 1106922, the same for the source tagging.

Added new templates for Thunderbird

Updated with changes requested in comment #68. Tools patch following soon.

Updated tagging scripts in tools.

BTW, there are 3 dangling symlinks: $ find -L . -type l ./mozilla/release_templates/fennec_ready_for_release ./mozilla/release_templates/firefox_ready_for_release ./mozilla/release_templates/thunderbird_ready_for_release All of them point to ready_for_release, which doesn't exist. Can you fix them too.

(In reply to Rail Aliiev [:rail] from comment #72) > BTW, there are 3 dangling symlinks: > > $ find -L . -type l > ./mozilla/release_templates/fennec_ready_for_release > ./mozilla/release_templates/firefox_ready_for_release > ./mozilla/release_templates/thunderbird_ready_for_release > > All of them point to ready_for_release, which doesn't exist. Can you fix > them too. Actually never mind, I'll forward this to the original bug.

Temporary fix for: ValueError: Cannot shorten "release-mozilla-release-34.1-firefox_tag_source" to maximum length (30). Got to: rel-m-rel-34.1-firefox_tag_source

In production: https://hg.mozilla.org/build/buildbot-configs/rev/a8299de1330d

In production: https://hg.mozilla.org/build/buildbotcustom/rev/4ee1407fb461

In production: https://hg.mozilla.org/build/buildbotcustom/rev/b163ccef8c0a

This patch worked fine for Fennec 35.0b4. On Firefox 35.0b4, instead, all repacks failed because there 'buildid' and 'script_repo_revision' were not set. I am creating a new patch to add the missing properties

I've talked with catlee about this issue. It's not easy to get the buildid in the Aggregating scheduler and propagate it to the down stream builders. A more sane approach would be fix the create-release-repacks.py script to figure out the buildid by itself. I am going to backout, for a while, the changes made in buildbotcustom/buildbot-configs/tools and fix the create-release-repacks.py script.

create-release-repacks.py now uses getBuildID() to find the right buildid. There is no need to get this parameter from the builders.

The buildid patch from comment #80, has worked fine (Firefox 35.0b5). I have re-landed all the backed out patches.

Firefox and Fennec 35.0b6 build 1 completed without any tagging/repacks problems.

\o/