Comment on attachment 643841 [details] [diff] [review] full patcher config script, for marking up Review of attachment 643841 [details] [diff] [review]: ----------------------------------------------------------------- ::: release/patcher-config-bump.pl @@ +29,5 @@ > + > +sub ProcessArgs { > + GetOptions( > + \%config, > + "product|p=s", "brand|r=s", "version|v=s", "old-version|o=s", Need to add --partial-version. I'm not 100% sure if Perl's Getopt supports arguments occurring more than once so we may have to find a different way to pass this data if it doesn't. We *do* need to continue passing old-version here, so that we can set the "from" property correctly. Might be better to rename it to fromVersion. @@ +213,5 @@ > + # doOnetimePatcherBump > + > + if ($doOnetimePatcherBumps) { > + $currentUpdateObj->{'to'} = $version; > + $currentUpdateObj->{'from'} = $oldVersion; Possibly s/oldVersion/fromVersion/ @@ +237,5 @@ > + } > + > + my $buildStr = 'build' . $build; > + > + my $partialUpdate = {}; If we end up using the new script to generate all of the partials, we can get rid of all of the $partialUpdate stuff. If we use the existing patcher to generate those we may need to s/oldVersion/fromVersion/ here. Either way we need to add some new code to generate a <partials> block, which will be another child of <current-update>. The format is described in depth here: https://bugzilla.mozilla.org/show_bug.cgi?id=773290#c3 and is basically the same as the existing <partial> block except with an intermediate level to support multiple versions. Most of this code can probably be reused. Not sure if bumpFilePath is going to work out of box, though. @@ +374,5 @@ > + $patcherConfigObj->save_file($patcherConfig); > +} > + > + > +sub RunUnitTests { If these still work, we need to update them.

A quick patcher-config-bump.pl patch which generates <partials> section for every version passed as --partial-version. The patch doesn't indent the code properly. I did this on purpose to let the patch live longer without being bitrotten. Example command: perl patcher-config-bump.pl -p firefox -r Firefox -v 14.0.1 -a 14.0.1 -b 1 -o 13.0.2 --partial-version 13.0.2 --partial-version 13.0.1 --partial-version 13.0 -c mozRelease-branch-patcher2.cfg -t stage.mozilla.org -f ftp.mozilla.org -d download.mozilla.org -l shipped-locales --platform linux --platform linux64 --platform macosx64 --platform win32 --marname firefox --oldmarname firefox

Comment on attachment 645435 [details] [diff] [review] patcher-config-bump.pl patch Review of attachment 645435 [details] [diff] [review]: ----------------------------------------------------------------- This looks pretty plausible to me. Will need to be tested in the context of release automation still, of course.

Comment on attachment 645435 [details] [diff] [review] patcher-config-bump.pl patch This patch worked well in my staging run.

Comment on attachment 645435 [details] [diff] [review] patcher-config-bump.pl patch Actually, sorry...I forgot that we wanted to get rid of the <partial> blocks completely. Is it possible to make BumpFilePath calls work without using stuff from <partial> in oldFilePath?

(In reply to Ben Hearsum [:bhearsum] from comment #6) > Comment on attachment 645435 [details] [diff] [review] > patcher-config-bump.pl patch > > Actually, sorry...I forgot that we wanted to get rid of the <partial> blocks > completely. Is it possible to make BumpFilePath calls work without using > stuff from <partial> in oldFilePath? I took a crack at this: https://github.com/bhearsum/tools/compare/mozilla:master...patcher-config-bump

Used this in my latest staging run and it worked well! This is basically the same as Rail's patch, except I look at one of the <partials> blocks instead of <partial> as the starting paths. The updates run is here, if you want to see the command + diff: http://dev-master01.build.scl1.mozilla.com:8018/builders/release-mozilla-beta-updates/builds/30 Probably good to have someone who didn't work on the patch review it, too....so +nthomas here.

Comment on attachment 652225 [details] [diff] [review] don't rely on a <partial> block when bumping >diff --git a/release/patcher-config-bump.pl b/release/patcher-config-bump.pl > sub ProcessArgs { > GetOptions( > \%config, > "product|p=s", "brand|r=s", "version|v=s", "old-version|o=s", >+ "partial-version=s@", Please add this new arg to the usage doc, in particular noting that you must pass a --partial-version the same as --old-version to get partials for the old version. >+ my $pBetatestPath = BumpFilePath( >+ oldFilePath => $oldPaths->{'betatest-url'}, >+ product => $product, Could we restore the indenting on landing ? Otherwise looks good.

(In reply to Nick Thomas [:nthomas] from comment #9) > Comment on attachment 652225 [details] [diff] [review] > don't rely on a <partial> block when bumping > > >diff --git a/release/patcher-config-bump.pl b/release/patcher-config-bump.pl > > sub ProcessArgs { > > GetOptions( > > \%config, > > "product|p=s", "brand|r=s", "version|v=s", "old-version|o=s", > >+ "partial-version=s@", > > Please add this new arg to the usage doc, in particular noting that you must > pass a --partial-version the same as --old-version to get partials for the > old version. > > >+ my $pBetatestPath = BumpFilePath( > >+ oldFilePath => $oldPaths->{'betatest-url'}, > >+ product => $product, > > Could we restore the indenting on landing ? Otherwise looks good. I addressed both of these in https://github.com/bhearsum/tools/commit/8ac6feb02c5c1a182f07432b5106004a6edf9c1c

Carrying forward r+.

Worked fine in Firefox 15.0b6/Thunderbird 15.0b5.