775535 - support for multiple partial updates in patcher config bumper

Status: RESOLVED FIXED

(Release Engineering :: Release Automation: Other, defect)

Description

[[e-mail] (:bhearsum)]

Attachment: full patcher config script, for marking up

Comment 1

[[e-mail] (:bhearsum)]

Attachment: full patcher config script, for marking up

Comment 2

[[e-mail] (:bhearsum)]

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.

Comment 3

[Rail Aliiev [:rail]]

Attachment: patcher-config-bump.pl patch

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 4

[[e-mail] (:bhearsum)]

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 5

[[e-mail] (:bhearsum)]

Comment on attachment 645435 [details] [diff] [review]
patcher-config-bump.pl patch

This patch worked well in my staging run.

Comment 6

[[e-mail] (:bhearsum)]

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?

Comment 7

[[e-mail] (:bhearsum)]

(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

Comment 8

[[e-mail] (:bhearsum)]

Attachment: don't rely on a <partial> block when bumping

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 9

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

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.

Comment 10

[[e-mail] (:bhearsum)]

(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

Comment 11

[[e-mail] (:bhearsum)]

Attachment: fix docs, indenting

Carrying forward r+.

Comment 12

[[e-mail] (:bhearsum)]

Worked fine in Firefox 15.0b6/Thunderbird 15.0b5.