Closed Bug 816398 Opened 12 years ago Closed 11 years ago

Thunderbird 18.0b1 updates failed to bump patcher config

Categories

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

x86
All
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: nthomas, Assigned: jhopkins)

References

Details

Attachments

(8 files)

The error was:
Traceback (most recent call last):
  File "/builds/buildbot/build1/lib/python2.6/site-packages/buildbot-0.8.2_hg_bd4812420e63_production_0.8-py2.6.egg/buildbot/process/buildstep.py", line 728, in startStep
    d.addCallback(self._startStep_2)
  File "/builds/buildbot/build1/lib/python2.6/site-packages/twisted/internet/defer.py", line 260, in addCallback
    callbackKeywords=kw)
  File "/builds/buildbot/build1/lib/python2.6/site-packages/twisted/internet/defer.py", line 249, in addCallbacks
    self._runCallbacks()
  File "/builds/buildbot/build1/lib/python2.6/site-packages/twisted/internet/defer.py", line 441, in _runCallbacks
    self.result = callback(self.result, *args, **kw)
--- <exception caught here> ---
  File "/builds/buildbot/build1/lib/python2.6/site-packages/buildbot-0.8.2_hg_bd4812420e63_production_0.8-py2.6.egg/buildbot/process/buildstep.py", line 769, in _startStep_2
    skip = self.start()
  File "/builds/buildbot/build1/lib/python2.6/site-packages/buildbotcustom/steps/mock.py", line 108, in start
    self.super_class.start(self)
  File "/builds/buildbot/build1/lib/python2.6/site-packages/buildbot-0.8.2_hg_bd4812420e63_production_0.8-py2.6.egg/buildbot/steps/shell.py", line 212, in start
    command = properties.render(self.command)
  File "/builds/buildbot/build1/lib/python2.6/site-packages/buildbot-0.8.2_hg_bd4812420e63_production_0.8-py2.6.egg/buildbot/process/properties.py", line 108, in render
    return [ self.render(e) for e in value ]
  File "/builds/buildbot/build1/lib/python2.6/site-packages/buildbot-0.8.2_hg_bd4812420e63_production_0.8-py2.6.egg/buildbot/process/properties.py", line 106, in render
    return value.render(self.pmap)
  File "/builds/buildbot/build1/lib/python2.6/site-packages/buildbot-0.8.2_hg_bd4812420e63_production_0.8-py2.6.egg/buildbot/process/properties.py", line 194, in render
    s = self.fmtstring % pmap
exceptions.TypeError: %o format: a number is required, not instance

in the bump step (patcher config). At a guess, this is from having a release notes url of 
  http://live.mozillamessaging.com/thunderbird/releasenotes?locale=%locale%&platform=%platform%&version=%version%'
and http://hg.mozilla.org/build/buildbotcustom/file/production-0.8/steps/mock.py#l64 not escaping the %'s, so that we fail when WithProperties is used on the command.

jhopkins, did you see anything like this when you were working on mock release support ?
John see question above :-)
No, I didn't see anything like that before.
Attached file proof of concept fix
This proof of concept replaces only the %(property)s style properties.

I think this is a decent approach - we just need to integrate it with buildbotcustom/steps/mock.py.  My assumption is that we don't use simple '%s' style replacements with WithProperties.
Attachment #686609 - Flags: feedback?(nthomas)
Attachment #686609 - Flags: feedback?(bhearsum)
Attachment #686609 - Attachment is patch: false
Comment on attachment 686609 [details]
proof of concept fix

feedback+ based on your explanation and running this script, but whatever the patch to MockCommand ends up being, I'd like to see it run through various builds on staging -- it's a very risky thing to be modifying. If we don't have time to do that before fixing up the release we should probably fix up the release some other way (even it's just a temporary hack).
Attachment #686609 - Flags: feedback?(bhearsum) → feedback+
Assignee: nobody → jhopkins
Short term, I propose we drop lines like this in the release configs
releaseConfig['releaseNotesUrl']     = 'http://live.mozillamessaging.com/thunderbird/releasenotes?locale=%locale%&platform=%platform%&version=%version%'
so that we don't attempt to pass % in the command strings, and modify the default at
 http://hg.mozilla.org/build/tools/file/d95dec15fd34/lib/perl/Release/Patcher/Config.pm#l29
to be 
 https://www.mozilla.org/%locale%/thunderbird/%version%/releasenotes/

That's where live.mozillamessaging.com redirects to, and with the 
  ?uri=/thunderbird/releasenotes&locale=en-US&platform=win32&version=17.0
suffix trimmed off.
Attachment #686729 - Flags: review?(nthomas)
... but let mozilla.org figure out the locale.
Attachment #686809 - Flags: review?(jhopkins)
I noticed this was missing during testing
Attachment #686812 - Flags: review?(nthomas)
Comment on attachment 686809 [details] [diff] [review]
[buildbot-configs] Use actual url

Looks ok to me (last hunk is optional), but let's be sure to track fixing this permanently.
Attachment #686809 - Flags: review?(jhopkins) → review+
Attachment #686812 - Flags: review?(nthomas) → review+
Comment on attachment 686809 [details] [diff] [review]
[buildbot-configs] Use actual url

http://hg.mozilla.org/build/buildbot-configs/rev/d3448164d783

Need to follow up and do the template too.
Attachment #686809 - Flags: checked-in+
Comment on attachment 686812 [details] [diff] [review]
[buildbot-configs] add perl-Config-General to match Firefox

http://hg.mozilla.org/build/buildbot-configs/rev/b44d1c09752d
Attachment #686812 - Flags: checked-in+
If you need anything else from me please let me know.
Assignee: jhopkins → nthomas
Attachment #686609 - Flags: feedback?(nthomas)
Attachment #687115 - Flags: review?(nthomas)
Comment on attachment 686729 [details] [diff] [review]
[buildbotcustom] use subclass of WithProperties for mock string_command and env.

I spoke with catlee about this patch and a more constrained fix is possible here:
- find where releaseNotesUrl is being used in the factories
- escape the '%' symbols at that point with '%%' (thus the config files themselves don't need to include escaping)
- WithProperties (and Python) will change '%%' back to '%' so the URL is not changed
Attachment #686729 - Flags: review?(nthomas)
Assignee: nthomas → jhopkins
I'm on buildduty this week so not actively working on this.

per nthomas: we don't need to fix that for the next beta, but for the next release
Assignee: jhopkins → nobody
(In reply to John Hopkins (:jhopkins) from comment #15)
> I'm on buildduty this week so not actively working on this.
> 
> per nthomas: we don't need to fix that for the next beta, but for the next
> release

Updating deps to reflect this.
Blocks: 810326
This needs to be done for Thunderbird 17.0.1.
Assignee: nobody → jhopkins
Any update here? We're less than 48 hours from go to build.
Standard8, this is what we discussed on IRC. With this we'll get 
  https://www.mozilla.org/thunderbird/17.0.1/releasenotes
for 17.0.1 esr. We should swap the variable to releaseConfig['version'] if you want that to be 17.0.1esr. Also fixes up the template for beta so we don't lose the fix there.

jhopkins, this is the same work around we did for beta, only for esr17 & release. bhearsum said you also had something in the pipeline so I'll leave it up to you and he to decide if you want to take this patch or not.
Attachment #697217 - Flags: review?(mbanner)
Comment on attachment 697217 [details] [diff] [review]
[buildbot-confifgs] Use actual url for other branches

This looks fine, but please use releaseConfig['version'] rather than the app version for the comm-esr17 files. The others (especially the beta), should remain as appVersion.
Attachment #697217 - Flags: review?(mbanner) → review+
This patch escapes releaseNotesUrl if use_mock is set.  I haven't tested it yet.
This was catlee's preferred approach.
Comment on attachment 697436 [details] [diff] [review]
[buildbotcustom] escape releaseNotesUrl if use_mock

Passed testing in staging
Attachment #697436 - Flags: review?(bhearsum)
Comment on attachment 697436 [details] [diff] [review]
[buildbotcustom] escape releaseNotesUrl if use_mock

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

Sucks to have to special case for mock in a factory, but looks reasonable. I think we can revert the URL in https://github.com/mozilla/build-buildbot-configs/blob/master/mozilla/release-thunderbird-comm-beta.py back to the mozillamessaging.com form now, but should probably check with Mark on that.
Attachment #697436 - Flags: review?(bhearsum) → review+
With the 'escape releaseNotesUrl if use_mock' patch, we can use the live.mozillamessaging.com format again.
Attachment #697553 - Flags: review?(mbanner)
Comment on attachment 697436 [details] [diff] [review]
[buildbotcustom] escape releaseNotesUrl if use_mock

Landed in http://hg.mozilla.org/build/buildbotcustom/rev/906281a69644
Attachment #697436 - Flags: checked-in+
in production
Attachment #697553 - Flags: review?(mbanner) → review+
Comment on attachment 697553 [details] [diff] [review]
[buildbot-configs] point releaseNotesUrl at live.mozillamessaging.com

Landed in https://hg.mozilla.org/build/buildbot-configs/rev/5e38dfdfb3b8
Attachment #697553 - Flags: checked-in+
Status: NEW → RESOLVED
Closed: 11 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: