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)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: nthomas, Assigned: jhopkins)
References
Details
Attachments
(8 files)
4.25 KB,
text/plain
|
bhearsum
:
feedback+
|
Details |
3.20 KB,
patch
|
Details | Diff | Splinter Review | |
1.80 KB,
patch
|
jhopkins
:
review+
nthomas
:
checked-in+
|
Details | Diff | Splinter Review |
4.72 KB,
patch
|
nthomas
:
review+
nthomas
:
checked-in+
|
Details | Diff | Splinter Review |
6.08 KB,
patch
|
Details | Diff | Splinter Review | |
11.10 KB,
patch
|
standard8
:
review+
|
Details | Diff | Splinter Review |
1.22 KB,
patch
|
bhearsum
:
review+
jhopkins
:
checked-in+
|
Details | Diff | Splinter Review |
13.63 KB,
patch
|
standard8
:
review+
jhopkins
:
checked-in+
|
Details | Diff | Splinter Review |
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 ?
Comment 1•12 years ago
|
||
John see question above :-)
Assignee | ||
Comment 2•12 years ago
|
||
No, I didn't see anything like that before.
Assignee | ||
Comment 3•12 years ago
|
||
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)
Assignee | ||
Updated•12 years ago
|
Attachment #686609 -
Attachment is patch: false
Comment 4•12 years ago
|
||
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 | ||
Updated•12 years ago
|
Assignee: nobody → jhopkins
Reporter | ||
Comment 5•12 years ago
|
||
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.
Assignee | ||
Comment 6•12 years ago
|
||
currently testing this patch
Assignee | ||
Updated•12 years ago
|
Attachment #686729 -
Flags: review?(nthomas)
Reporter | ||
Comment 7•12 years ago
|
||
... but let mozilla.org figure out the locale.
Attachment #686809 -
Flags: review?(jhopkins)
Assignee | ||
Comment 8•12 years ago
|
||
I noticed this was missing during testing
Attachment #686812 -
Flags: review?(nthomas)
Assignee | ||
Comment 9•12 years ago
|
||
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+
Reporter | ||
Updated•12 years ago
|
Attachment #686812 -
Flags: review?(nthomas) → review+
Reporter | ||
Comment 10•12 years ago
|
||
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+
Reporter | ||
Comment 11•12 years ago
|
||
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+
Assignee | ||
Comment 12•12 years ago
|
||
Attachment #687115 -
Flags: review?(nthomas)
Assignee | ||
Comment 13•12 years ago
|
||
If you need anything else from me please let me know.
Assignee: jhopkins → nthomas
Assignee | ||
Updated•12 years ago
|
Attachment #686609 -
Flags: feedback?(nthomas)
Assignee | ||
Updated•12 years ago
|
Attachment #687115 -
Flags: review?(nthomas)
Assignee | ||
Comment 14•12 years ago
|
||
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 | ||
Updated•12 years ago
|
Assignee: nthomas → jhopkins
Assignee | ||
Comment 15•12 years ago
|
||
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
Comment 16•11 years ago
|
||
(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
Assignee | ||
Comment 17•11 years ago
|
||
This needs to be done for Thunderbird 17.0.1.
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → jhopkins
Comment 18•11 years ago
|
||
Any update here? We're less than 48 hours from go to build.
Reporter | ||
Comment 19•11 years ago
|
||
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 20•11 years ago
|
||
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+
Assignee | ||
Comment 21•11 years ago
|
||
This patch escapes releaseNotesUrl if use_mock is set. I haven't tested it yet. This was catlee's preferred approach.
Assignee | ||
Comment 22•11 years ago
|
||
Comment on attachment 697436 [details] [diff] [review] [buildbotcustom] escape releaseNotesUrl if use_mock Passed testing in staging
Attachment #697436 -
Flags: review?(bhearsum)
Comment 23•11 years ago
|
||
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+
Assignee | ||
Comment 24•11 years ago
|
||
With the 'escape releaseNotesUrl if use_mock' patch, we can use the live.mozillamessaging.com format again.
Attachment #697553 -
Flags: review?(mbanner)
Assignee | ||
Comment 25•11 years ago
|
||
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+
Comment 26•11 years ago
|
||
in production
Updated•11 years ago
|
Attachment #697553 -
Flags: review?(mbanner) → review+
Assignee | ||
Comment 27•11 years ago
|
||
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+
Assignee | ||
Updated•11 years ago
|
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Product: mozilla.org → Release Engineering
You need to log in
before you can comment on or make changes to this bug.
Description
•