420947 - patcher creates directories with a pretty version when it should use the short version

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

[[e-mail] (:bhearsum)]

We hit this in Firefox 3 Beta 4, it appears to be fallout from bug 410006. When creating update snippets patcher creates a directory like this:
/opt/aus2/incoming/3/Firefox/3 Beta 3
When it should really be creating:
/opt/aus2/incoming/3/Firefox/3.0b3

Apparently, this only happens when the oldVersion, as the patch in bug 410006 was landed before the Beta 3 release and did not cause problems with it.

Patcher should be fixed to only use the 'version' in a <release> subsection when it needs something pretty. Any other time it should simply use the subsection name.
Here's an example
<app>
 <Firefox>
...
...
 &nbsp;&nbsp;<release>
 &nbsp;&nbsp;&nbsp;<3.0b3>
 &nbsp;&nbsp;&nbsp;&nbsp;version 3 Beta 3
 &nbsp;&nbsp;&nbsp;</3.0b3>
 &nbsp;&nbsp;</release>
...
...
 </Firefox>
</app>

In this case, '3 Beta 3' should be used as the pretty string, and '3.0b3' for everything else.

I'm working on a patch for this.

Comment 1

[[e-mail] (:bhearsum)]

Ugh, stupid Bugzilla.
<app>
 <Firefox>
...
...
  <release>
   <3.0b3>
    version 3 Beta 3
   </3.0b3>
  </release>
...
...
 </Firefox>
</app>

Comment 2

[[e-mail] (:bhearsum)]

From discussion on IRC:
Nick suggested using a 'prettyVersion' in the patcher config and leaving 'version' for what it was originally intended for. This will require patcher and Bootstrap patches. We should also go through the patcher configs and make sure there are no 'version' properties with a pretty version in them.

Comment 3

[[e-mail] (:bhearsum)]

For posterity, this bug seems to be caused by these two lines:
http://lxr.mozilla.org/mozilla/source/tools/patcher/patcher2.pl#937
http://lxr.mozilla.org/mozilla/source/tools/patcher/patcher2.pl#981

Comment 4

[[e-mail] (:bhearsum)]

I've got a fix in testing right now, I'll be posting patches tomorrow.

Comment 5

[[e-mail] (:bhearsum)]

Attachment: use prettyVersion in the patcher config for appv in a snippet

I looked through the MozAUS* and patcher2.pl code and this seems to be the only place this change is needed. I've run patcher locally and it appears to generate good snippets, I'll be attaching them in a moment. I will also be attaching updated patcher configs.

Comment 6

[[e-mail] (:bhearsum)]

Attachment: complete.txt on beta channel

The <rc> section of this config did not contain 'beta'

Comment 7

[[e-mail] (:bhearsum)]

Attachment: partial.txt on betatest channel

betatest _was_ in the <rc> section.

Comment 8

[[e-mail] (:bhearsum)]

Attachment: [checked in] 1.8 and 1.9 patcher configs, update with prettyVersion

I'm not 100% sure if this is needed or not, I believe prettyVersion will only be used on the current release, not the old one. But just in case, I added it to the latest releases on the 1.8 and 1.9 patcher configs. After reading MozAUSConfig.pm a bit more I'm quite certain we won't need these on even older releases.

I purposely did not touch major update configs.

Comment 9

[[e-mail] (:bhearsum)]

Attachment: [checked in] bootstrap changes to support patcher changes

This is pretty trivial. The 'version' is set to the be same as the 'extension-version', like before. And 'prettyVersion' has been added. Now that GetVersion() exists we can just use it to extrapolate a pretty version (which I probably should've done when I added that function in the first place).

Comment 10

[[e-mail] (:bhearsum)]

Attachment: [checked in] try it in a different way

This patch contains minor changes to MozAUSConfig.pm, which add a 'prettyAppv' to the data structure that patcher2.pl uses to generate updates.

There are larger changes to patcher2.pl that do the following (in a few places): Add a 'prettySnippetToAppVersion' wherever there is a 'snippetToAppVersion'. 'snippetToAppVersion' was previously used to set the appv in the snippet as well as provide progress on the command-line. In this version 'prettySnippetToAppVersion' performs the former, while snippetToAppVersion does the latter.

I was going to simply redefine 'snippetToAppVersion' as 'prettyVersion', but that made the command line output pretty ugly.

Comment 11

[[e-mail] (:bhearsum)]

Attachment: diffs of aus2 and aus2.test dirs with unpatched vs patched patcher2

This is a diff of the aus2 and aus2.test directories using both unmodified patcher code, and patcher patched with attachment 307546 [details] [diff] [review]. As far as I can tell, only the appv is changed.

Comment 12

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

Comment on attachment 307546 [details] [diff] [review]
[checked in] try it in a different way

r+, thanks for making the more involved patch. I did some extra testing with 2.0.0.x, adding prettyVersion's to the 2.0.0.11 and the 2.0.0.12 blocks in the moz18-branch-patcher.cfg. That induces no changes except where it should,  obviously a quick inspection rather than a complete one, 15000 files :-(

Comment 13

[[e-mail] (:bhearsum)]

Comment on attachment 307463 [details] [diff] [review]
[checked in] 1.8 and 1.9 patcher configs, update with prettyVersion

Checking in moz18-branch-patcher2.cfg;
/cvsroot/mozilla/tools/patcher-configs/moz18-branch-patcher2.cfg,v  <--  moz18-branch-patcher2.cfg
new revision: 1.9; previous revision: 1.8
done
Checking in moz19-branch-patcher2.cfg;
/cvsroot/mozilla/tools/patcher-configs/moz19-branch-patcher2.cfg,v  <--  moz19-branch-patcher2.cfg
new revision: 1.21; previous revision: 1.20
done

Comment 14

[[e-mail] (:bhearsum)]

Comment on attachment 307546 [details] [diff] [review]
[checked in] try it in a different way

Checking in MozAUSConfig.pm;
/cvsroot/mozilla/tools/patcher/MozAUSConfig.pm,v  <--  MozAUSConfig.pm
new revision: 1.17; previous revision: 1.16
done
Checking in patcher2.pl;
/cvsroot/mozilla/tools/patcher/patcher2.pl,v  <--  patcher2.pl
new revision: 1.30; previous revision: 1.29
done

I'm going to retag with UPDATE_PACKAGING_R1_1 and bump the staging machines to that. If that goes well, we can bump the production ones later.

Comment 15

[[e-mail] (:bhearsum)]

Quick correction: will be tagging with UPDATE_PACKAGING_R2

Comment 16

[[e-mail] (:bhearsum)]

Comment on attachment 307464 [details] [diff] [review]
[checked in] bootstrap changes to support patcher changes

Checking in Bootstrap/Step/PatcherConfig.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/PatcherConfig.pm,v  <--  PatcherConfig.pm
new revision: 1.19; previous revision: 1.18
done
Checking in configs/fx-moz19-bootstrap.cfg;
/cvsroot/mozilla/tools/release/configs/fx-moz19-bootstrap.cfg,v  <--  fx-moz19-bootstrap.cfg
new revision: 1.18; previous revision: 1.17
done
Checking in configs/fx-moz19-staging-bootstrap.cfg;
/cvsroot/mozilla/tools/release/configs/fx-moz19-staging-bootstrap.cfg,v  <--  fx-moz19-staging-bootstrap.cfg
new revision: 1.13; previous revision: 1.12
done

Comment 17

[[e-mail] (:bhearsum)]

Alright, I've created UPDATE_PACKAGING_R2 following the instructions here: https://bugzilla.mozilla.org/show_bug.cgi?id=411235#c10

cvs co -r UPDATE_PACKAGING_R1 mozilla/client.mk
cd mozilla
make -f client.mk MOZ_CO_PROJECT=all checkout
cvs up -A tools/update-packaging
cvs up -AdP tools/release
cvs up -AdP tools/patcher
cvs -q tag UPDATE_PACKAGING_R2 2>&1 | tee ../tag.log
grep -v '^T' ../tag.log
cvs tag -b UPDATE_PACKAGING_R2_minibranch client.mk
cvs up -r UPDATE_PACKAGING_R2_minibranch client.mk

edited client.mk, bumped to UPDATE_PACKAGING_R2
commited client.mk

Checking in client.mk;
/cvsroot/mozilla/client.mk,v  <--  client.mk
new revision: 1.314.2.1.2.1.2.1; previous revision: 1.314.2.1.2.1
done

re-tagged
cvs -q tag UPDATE_PACKAGING_R2 client.mk
W client.mk : UPDATE_PACKAGING_R2 already exists on version 1.314.2.1.2.1 : NOT MOVING tag to version 1.314.2.1.2.1.2.1
cvs -q tag -F UPDATE_PACKAGING_R2 client.mk