Closed Bug 372759 Opened 18 years ago Closed 18 years ago

Bootstrap - polish milestone bumping logic

Categories

(Release Engineering :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: preed, Assigned: rhelmer)

References

Details

Attachments

(1 file, 2 obsolete files)

Since we now bump versions on the _MINIBRANCH (bug 369825), we need a way to specify either pulling the milestone version from another _RELEASE tag (in the case of Thunderbird 1.5.0.n builds needing the same version as Firefox 1.5.0.n builds), or a way to not bump it at all (in the case of Thunderbird 2.0bN builds needing whatever's currently on the branch).
Assignee: build → rhelmer
Status: NEW → ASSIGNED
Comment on attachment 259760 [details] [diff] [review] if milestone is set in config, use it; otherwise skip that file client.mk needs to be moved into this loop; otherwise it'll try to s/$versionPre/$version/ when it should s/$branchTag/$version/
Attachment #259760 - Attachment is obsolete: true
Attachment #259760 - Flags: review?(preed)
Attached patch only bump milestone if specified (obsolete) — Splinter Review
This works on the staging environment. The bump logic is generalized and (hopefully) easier to understand now.
Attachment #260363 - Flags: review?(preed)
Comment on attachment 260363 [details] [diff] [review] only bump milestone if specified >+ my $bumpVersion = undef; >+ my $preVersion = undef; >+ >+ if ($fileName eq $milestoneTxt) { >+ $bumpVersion = $milestone; >+ $preVersion = $bumpVersion . 'pre'; >+ } elsif ($fileName eq 'client.mk') { >+ $bumpVersion = $version; >+ $preVersion = $branchTag; Shouldn't $bumpVersion be $productTag in the client.mk case? Wouldn't this just check in a file with a tag of "2.0.0.3" or whatever? >+ } else { >+ $bumpVersion = $version; >+ $preVersion = $bumpVersion . 'pre'; >+ } For this, I think: if ($milestoneTxt) { ... } elsif ($clientMk) { ... } elsif ($bar) { ... } else { die "ASSERT"; } Is more appropriate; it would be way too easy to add something to @bumpFiles, but never create the proper logic for it. The test of the substitution is could be tightened up, too; we know, for instance, that if the file is named something specific, what exactly to bump. We could constrain the regexes to be safer. Did this patch get run through the test gauntlet?
Attachment #260363 - Flags: review?(preed) → review-
(In reply to comment #4) > (From update of attachment 260363 [details] [diff] [review]) > > >+ my $bumpVersion = undef; > >+ my $preVersion = undef; > >+ > >+ if ($fileName eq $milestoneTxt) { > >+ $bumpVersion = $milestone; > >+ $preVersion = $bumpVersion . 'pre'; > >+ } elsif ($fileName eq 'client.mk') { > >+ $bumpVersion = $version; > >+ $preVersion = $branchTag; > > Shouldn't $bumpVersion be $productTag in the client.mk case? Wouldn't this just > check in a file with a tag of "2.0.0.3" or whatever? Ack! Excellent catch, that's true. > >+ } else { > >+ $bumpVersion = $version; > >+ $preVersion = $bumpVersion . 'pre'; > >+ } > > For this, I think: > > if ($milestoneTxt) { > ... > } elsif ($clientMk) { > ... > } elsif ($bar) { > ... > } else { > die "ASSERT"; > } > > Is more appropriate; it would be way too easy to add something to @bumpFiles, > but never create the proper logic for it. Ok, will do. > The test of the substitution is could be tightened up, too; we know, for > instance, that if the file is named something specific, what exactly to bump. > We could constrain the regexes to be safer. True, I'll see what I can do. > Did this patch get run through the test gauntlet? It did, but just the Tag step on the staging server; I didn't try a build off of it (waiting on bug 371325 to be able to do it for real).
Blocks: 375788
Ok, same as last time, plus your suggestions. I tested this and went over the output more carefully, looks like everything is bumped properly.
Attachment #260363 - Attachment is obsolete: true
Attachment #260658 - Flags: review?(preed)
Comment on attachment 260658 [details] [diff] [review] address preed's comments >+ foreach my $fileName (@bumpFiles) { > my $found = 0; >+ >+ my $file = catfile($parentDir, $fileName); >+ >+ my $bumpVersion = undef; >+ my $preVersion = undef; >+ my $search = undef; >+ my $replace = undef; >+ >+ if ($fileName eq 'client.mk') { >+ $search = '^MOZ_CO_TAG\s+=\s+' . $branchTag . '$'; >+ $replace = 'MOZ_CO_TAG = ' . $releaseTag; >+ } elsif ($fileName eq $moduleVer) { >+ $preVersion = $version . 'pre'; >+ $search = '^WIN32_MODULE_PRODUCTVERSION_STRING=' . $preVersion . '$'; >+ $replace = 'WIN32_MODULE_PRODUCTVERSION_STRING=' . $version; >+ } elsif ($fileName eq $versionTxt) { >+ $preVersion = $version . 'pre'; >+ $search = '^' . $preVersion . '$'; >+ $replace = $version; >+ } elsif ($fileName eq $milestoneTxt) { >+ $preVersion = $milestone . 'pre'; >+ $search = '^' . $preVersion . '$'; >+ $replace = $milestone; >+ } else { >+ die("ASSERT: do not know how to bump file $fileName"); >+ } This won't accidentally clobber the \n on the end of each of these lines, will it? I don't think matching against end of line will clobber it in a search/replace, but you'll want to check the automated checkins that were done here to make sure. Other than that, r=preed.
Attachment #260658 - Flags: review?(preed) → review+
Made sure that the newline is left alone, and landed: Checking in Bootstrap/Step/Tag/Bump.pm; /cvsroot/mozilla/tools/release/Bootstrap/Step/Tag/Bump.pm,v <-- Bump.pm new revision: 1.6; previous revision: 1.5 done
Status: ASSIGNED → RESOLVED
Closed: 18 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.

Attachment

General

Created:
Updated:
Size: