1151755 - Updates failed at 38.0b2 because 'from' was set incorrectly in the patcher config for 38.0b1

Status: RESOLVED FIXED

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

Description

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

For 38.0b1 we had a partials set to 37.0b6 + 37.0b7 + 37.0 RC2, and the patcher bump was this call:

perl tools/release/patcher-config-bump.pl -p firefox -r Firefox -v 38.0b1 -a 38.0 -o 37.0b7 -b 1 -c tools/release/patcher-configs/mozBeta-branch-patcher2.cfg -t stage.mozilla.org -f ftp.mozilla.org -d download.mozilla.org -l shipped-locales --partial-version 37.0 --partial-version 37.0b6 --partial-version 37.0b7 --platform linux --platform linux64 --platform macosx64 --platform win32 --platform win64

Note that -o (old-version) is 37.0b7 instead of 37.0, and we end up with
            from   37.0b7

For 38.0b2 we bump again and get this in the patcher config:
         past-update   37.0b6 37.0b7 betatest releasetest beta
         past-update   37.0b7 37.0 betatest releasetest beta
+        past-update   37.0b7 38.0b1 betatest releasetest beta
         <release>
which the update verify bumper objects to with
release.updates.patcher.PatcherConfigError: Found multiple past-updates with duplicate to/from versions: ['37.0b7', '38.0b1', ['betatest', 'releasetest', 'beta']]

The immediate fix was changing from to '37.0', and this bug is to fix up the -o argument in the buildbot factory.

Comment 1

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

The -o is passed with previousVersion defined at http://hg.mozilla.org/build/buildbotcustom/file/default/process/factory.py#l3935 with
  3935         self.previousVersion = str(
  3936             max(LooseVersion(v) for v in self.partialUpdates))

and 'yay':
>>> from distutils.version import LooseVersion, StrictVersion
>>> V = ['37.0', '37.0b7']
>>> str(max(LooseVersion(v) for v in V))
'37.0b7'
>>> str(max(StrictVersion(v) for v in V))
'37.0'

We'd just swap to StrictVersion but
>>> StrictVersion('31.6.0esr')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/Cellar/python/2.7.9/Frameworks/Python.framework/Versions/2.7/lib/python2.7/distutils/version.py", line 40, in __init__
    self.parse(vstring)
  File "/usr/local/Cellar/python/2.7.9/Frameworks/Python.framework/Versions/2.7/lib/python2.7/distutils/version.py", line 107, in parse
    raise ValueError, "invalid version number '%s'" % vstring
ValueError: invalid version number '31.6.0esr'

Very tempted to just special case ESR to use LooseVersion, otherwise StrictVersion.

Comment 2

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

Attachment: [buildbotcustom] Fix version handling

I verified this worked with a little print debugging and make checkconfig, using the buildbot-configs for 38.0b1:

repo           patcher-config  version  prevVer     prevVer fixed

mozilla-release  mozRelease       37.0       37.0       37.0
mozilla-release     mozBeta       37.0     37.0b7     37.0b7
mozilla-esr31      mozEsr31  31.6.0esr  31.5.3esr  31.5.3esr
mozilla-beta        mozBeta     38.0b1     37.0b7       37.0

This happened to cover 37.0RC2 too, and caught an error which would have slipped in where the 2nd line would have changed to 37.0 as well. I think we'd have trouble with 'from' and 'to' being 37.0; ideally we'd have all the RC's in the beta config and name them separately.

Comment 3

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

As usual, thought of something just after submitting a patch - it might be better to test self.repoPath rather than a substring in the version. (ie if self.repoPath == 'releases/mozilla-beta': use StrictVersion).

Comment 4

[[e-mail] (:bhearsum)]

Comment on attachment 8591471 [details] [diff] [review]
[buildbotcustom] Fix version handling

Review of attachment 8591471 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to Nick Thomas [:nthomas] from comment #3)
> As usual, thought of something just after submitting a patch - it might be
> better to test self.repoPath rather than a substring in the version. (ie if
> self.repoPath == 'releases/mozilla-beta': use StrictVersion).

I'm fine with either, hopefully this will be relatively short lived if we can replace patcher configs this year?

According to your chart, it looks like version + prevVersion are both 37.0 still. I agree that that will probably be an issue, perhaps I'm misreading something?
> repo           patcher-config  version  prevVer     prevVer fixed
> 
> mozilla-release  mozRelease       37.0       37.0       37.0

Comment 5

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

(In reply to Ben Hearsum [:bhearsum] from comment #4)
> I'm fine with either, hopefully this will be relatively short lived if we
> can replace patcher configs this year?

They are likely to go away in favour of some other way of generating update verify configs, as part of release promotion. Probably using Balrog and ship-it as data sources  *waves hands a bit*
 
> According to your chart, it looks like version + prevVersion are both 37.0
> still. I agree that that will probably be an issue, perhaps I'm misreading
> something?
> > repo           patcher-config  version  prevVer     prevVer fixed
> > 
> > mozilla-release  mozRelease       37.0       37.0       37.0

I was mostly concerned with anything touching the beta patcher config and missed this. We actually did pass 37.0 as the old version for 37.0 build2, but because we'd already bumped for build1 that turned out to be ignored (doOnetimePatcherBumps is false at http://hg.mozilla.org/build/tools/file/default/release/patcher-config-bump.pl#l204). This is probably fine, unless we abandon a build1 before running the updates builder.

Comment 6

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

Attachment: [buildbotcustom] Fix version handling, v2

I've removed the logic into a separate function to simplify adding tests. Two of them fail before the logic gets fixed, testBetaFirstInCycle and testReleaseBuild2. The before/after table looks like his now:

mozilla-release mozRelease       37.0       37.0     36.0.4
mozilla-release    mozBeta       37.0     37.0b7     37.0b7
mozilla-esr31     mozEsr31  31.6.0esr  31.5.3esr  31.5.3esr
mozilla-beta       mozBeta     38.0b1     37.0b7       37.0

I've verified that redoing a 37.0 RC2 bump with -o 36.0.4 results in no changes to the patcher config, but we'd now handle the case where RC1 didn't already bump the config. Also checked that doing 38.0b1 with -o 37.0 only changes 'from' to '37.0'.

Comment 7

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

Comment on attachment 8592738 [details] [diff] [review]
[buildbotcustom] Fix version handling, v2

I also removed the LooseVersion import in factory.py.

https://hg.mozilla.org/build/buildbotcustom/rev/466c5a5a791c

Comment 8

[Release Engineering SlaveAPI Service]

In production: https://hg.mozilla.org/build/buildbotcustom/rev/466c5a5a791c