500471 - release automation needs to support buildNumber == 1 w/ relbranchOverride

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

[[e-mail] (:bhearsum)]

Bootstrap was able to do this fine, but the new Mercurial automation (specifically, ReleaseTaggingFactory) has issues with it. We've been known to require this for firedrill point releases, and it would've been useful for 3.5rc2.

Comment 1

[Nick Thomas [:nthomas] (UTC+12)]

Do you remember what those issues are Ben ? One problem we have with Bootstrap is that it can't bump the version on a reused relbranch, so it'd be great to fix version-bump.pl here too.

Comment 2

[[e-mail] (:bhearsum)]

(In reply to comment #1)
> Do you remember what those issues are Ben ? One problem we have with Bootstrap
> is that it can't bump the version on a reused relbranch, so it'd be great to
> fix version-bump.pl here too.

That's the main problem here - we're calling version-bump.pl and it fails to find,eg, 1.9.1.2pre on the 1.9.1.1 branch. I'm sure we can fix version-bump.pl and/or our calling of it.

Comment 3

[[e-mail] (:bhearsum)]

Attachment: [wip] make version bumps work on build1 + relbranchOverride

I'm still in the middle of testing this but I wouldn't mind some feedback on the approach. I'm a bit worried that ReleaseTaggingFactory is getting (even more) unreadable. I spent a little bit of time trying to find a better way to do, but there's just so many different cases too handle I couldn't improve it.

I'm not 100% happy with the addition of oldMilestone, just from a maintenance standpoint. The other way to do it would be to not explicitly check for the oldVersion in version-bump.pl, and change it to the new one regardless of the contents. That would avoid the need for oldMilestone. But....I *think* that's its a good thing to be explicit, as it might catch an incorrect branch or changeset early. What do you guys think? Is there a case where this explicitness would not work when it should?

Comment 4

[[e-mail] (:bhearsum)]

Attachment: fully tested, working build1 + relbranch patch

Alright, so this is basically the same as the first patch, but without bugs :-). I'm more happy with the explicit regexes now. Specifically, they will catch a case where we want to ship off of an existing relbranch, but use the wrong one in the config. It seems unlikely to happen, but...stranger things, etc, etc. If we have a situation where these regexes are making wrong assumptions we can revisit then.

Comment 5

[[e-mail] (:bhearsum)]

Comment on attachment 390462 [details] [diff] [review]
fully tested, working build1 + relbranch patch

Kairo, I'm sure you hit these codepaths too, I assume this will work the same for you but you should probably have a look anyways.

Comment 6

[Robert Kaiser]

Comment on attachment 390462 [details] [diff] [review]
fully tested, working build1 + relbranch patch

No changes to hand those to the tagging factory in release_master.py?

Fore the configs, we probably want the change in seamonkey/release_config.py as well (even though we actually never bump it, but the class would fail if we hand one but not the other), but I can do that when this has landed.

What I wonder a bit about is that we're using oldAppVersion in two slightly different ways, one time for the latest version to update to this one and one time for the version in the source pre-tagging. They probably will usually fall together in the relbranchOverride case at least for Firefox, but it makes it impossible for SeaMonkey and Thunderbird to ride on the same relbranch in comm-central, as one of us needs to set a relbranchOverride and still bump from 'pre' while the old version for updates is something else.

Comment 7

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 390462 [details] [diff] [review]
fully tested, working build1 + relbranch patch

What KaiRo said about mozilla2{,-staging}/release_master.py never passing the new arguments to ReleaseTaggingFactory. Otherwise looks fine for Firefox.

Comment 8

[[e-mail] (:bhearsum)]

Comment on attachment 390462 [details] [diff] [review]
fully tested, working build1 + relbranch patch

I'm going to think this through a bit more, but I suspect the only reasonable solution may be to replace anything that matches any version number.

Comment 9

[[e-mail] (:bhearsum)]

Attachment: try #2 - be less picky about the versions

I tested this in all the scenarios mentioned in the block comment. The output is a bit nasty now, but it works.

Comment 10

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 391182 [details] [diff] [review]
try #2 - be less picky about the versions

Lets me careful out there.

Comment 11

[Robert Kaiser]

Comment on attachment 391182 [details] [diff] [review]
try #2 - be less picky about the versions

Looks good to me, this is a reasonable approach, IMHO.

Comment 12

[[e-mail] (:bhearsum)]

Comment on attachment 391182 [details] [diff] [review]
try #2 - be less picky about the versions

changeset:   333:8afb25990ce6

Comment 13

[[e-mail] (:bhearsum)]

This broke 3.6a1 tagging. I think the problem is: ((a|b|(rc))\d+)|(pre)) - which doesn't appear to work for alphas or betas at all (doesn't match a|b *and* pre)

Comment 14

[[e-mail] (:bhearsum)]

Comment on attachment 391182 [details] [diff] [review]
try #2 - be less picky about the versions

backed out due to bustage in changeset:   337:f64f338d9e16

Comment 15

[[e-mail] (:bhearsum)]

I actually backed it out in changeset:   339:d3cf47d835d5

Comment 16

[[e-mail] (:bhearsum)]

Attachment: fixed version bumping

While fixing this up I realized that we actually never have 'rc' in the version files, so I removed that part of the regex.

I tested the regex separately with a bunch of scenarios:
bitters-2:tmp bhearsum$ cat test.pl
#!/usr/bin/perl

my $VERSION_REGEXP = '[\d\.]+' # A version number 
                   . '((a|b)\d+)?' # Might be an alpha or beta
                   . '(pre)?'; # Might be pre

if ($ARGV[0] =~ m/^$VERSION_REGEXP$/) {
    print "$ARGV[0]: matched!\n";
}
bitters-2:tmp bhearsum$ perl test.pl 3
3: matched!
bitters-2:tmp bhearsum$ perl test.pl 3.6
3.6: matched!
bitters-2:tmp bhearsum$ perl test.pl 3.6a1
3.6a1: matched!
bitters-2:tmp bhearsum$ perl test.pl 3.6b532
3.6b532: matched!
bitters-2:tmp bhearsum$ perl test.pl 3.6rc
bitters-2:tmp bhearsum$ perl test.pl 3.6rc3
bitters-2:tmp bhearsum$ perl test.pl 3.6pre
3.6pre: matched!
bitters-2:tmp bhearsum$ perl test.pl 3.6.9.2
3.6.9.2: matched!
bitters-2:tmp bhearsum$ perl test.pl 3.6.9.2pre
3.6.9.2pre: matched!
bitters-2:tmp bhearsum$ perl test.pl 3.6.9.2b34
3.6.9.2b34: matched!
bitters-2:tmp bhearsum$ perl test.pl 3.6.9.2b34pre
3.6.9.2b34pre: matched!

And I re-bumped mozilla-central with it, with success:
bitters-2:checkouts bhearsum$ perl ~/Mozilla/checkouts/tools/release/version-bump.pl -w mozilla-central -t FIREFOX_3_6a1_RELEASE -a browser -v 3.6a1 -m 1.9.2a1 config/milestone.txt js/src/config/milestone.txt browser/config/version.txt
mozilla-central/config/milestone.txt: ^[\d\.]+((a|b)\d+)?(pre)?$ found
mozilla-central/config/milestone.txt: ^[\d\.]+((a|b)\d+)?(pre)?$ replaced with 1.9.2a1
mozilla-central/js/src/config/milestone.txt: ^[\d\.]+((a|b)\d+)?(pre)?$ found
mozilla-central/js/src/config/milestone.txt: ^[\d\.]+((a|b)\d+)?(pre)?$ replaced with 1.9.2a1
mozilla-central/browser/config/version.txt: ^[\d\.]+((a|b)\d+)?(pre)?$ found
mozilla-central/browser/config/version.txt: ^[\d\.]+((a|b)\d+)?(pre)?$ replaced with 3.6a1

Comment 17

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 393170 [details] [diff] [review]
fixed version bumping

>diff -r 3bdeff59b68e release/version-bump.pl
>+my $VERSION_REGEXP = '[\d\.]+' # A version number 
>+                   . '((a|b)\d+)?' # Might be an alpha or beta
>+                   . '(pre)?'; # Might be pre
>+

Have we ever needed to match a single digit like '3' ? Can't recall a case of that, usually it's at least '3.0' in length. Changing the first part to 
   '\d\.\d[\.\d]*'
still matches all the other input. Would be nice to line up the # characters too.

Comment 18

[[e-mail] (:bhearsum)]

(In reply to comment #17)
> (From update of attachment 393170 [details] [diff] [review])
> >diff -r 3bdeff59b68e release/version-bump.pl
> >+my $VERSION_REGEXP = '[\d\.]+' # A version number 
> >+                   . '((a|b)\d+)?' # Might be an alpha or beta
> >+                   . '(pre)?'; # Might be pre
> >+
> 
> Have we ever needed to match a single digit like '3' ? Can't recall a case of
> that, usually it's at least '3.0' in length.

Yeah, I just confirmed this going back to 1.0.

> Changing the first part to 
>    '\d\.\d[\.\d]*'
> still matches all the other input. Would be nice to line up the # characters
> too.

That sounds fine, I'll do those.

Comment 19

[[e-mail] (:bhearsum)]

Attachment: slightly better regex, comments aligned

Comment 20

[[e-mail] (:bhearsum)]

Comment on attachment 394510 [details] [diff] [review]
slightly better regex, comments aligned

changeset:   353:c24c3c80e0ed

Hopefully we don't need to back this out again :S.

Comment 21

[[e-mail] (:bhearsum)]

(In reply to comment #20)
> (From update of attachment 394510 [details] [diff] [review])
> changeset:   353:c24c3c80e0ed
> 
> Hopefully we don't need to back this out again :S.

I didn't actually push this until today, apparently.