Closed
Bug 650217
Opened 14 years ago
Closed 13 years ago
L10n verification always failing on release builds
Categories
(Release Engineering :: General, defect, P2)
Release Engineering
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: standard8, Assigned: rail)
References
Details
(Whiteboard: [l10n][automation])
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
bhearsum
:
review+
rail
:
checked-in+
|
Details | Diff | Splinter Review |
I've noticed this for the last few releases that we've done. It appears that the L10n metadiff step of L10n verification is failing on the diff.
However in the evaluateCommand function of L10nVerifyMetaDiff, the code states this:
'''We ignore failures here on purpose, since diff will
return 1(FAILURE) if it actually finds anything to output.
'''
This is at: http://hg.mozilla.org/build/buildbotcustom/file/c99163a10cb8/steps/release.py#l49
See also: http://hg.mozilla.org/build/buildbotcustom/file/c99163a10cb8/steps/release.py#l33
I'm pretty sure this was originally ignoring failure, and somehow it is now broken.
Here's a log from the 3.6.17 builds that shows this happening:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.6-Release/1302807793.1302808578.7132.gz
created metadiff failed
=== Output ===
diff -r firefox-3.6.17-build2/diffs firefox-3.6.16-build1/diffs
in dir /builds/moz2_slave/rel-192-w32-l10n-verification/tools/release/l10n (timeout 1200 secs)
watching logfiles {}
argv: ['diff', '-r', 'firefox-3.6.17-build2/diffs', 'firefox-3.6.16-build1/diffs']
...
<lots of diffs>
...
< < # NOTE: The restartLaterButton string is also used in
< < # mozapps/extensions/content/blocklist.js
program finished with exit code 1
elapsedTime=3.346318
=== Output ended ===
======== BuildStep ended ========
Reporter | ||
Comment 1•14 years ago
|
||
Rail tells me on irc that the change was intentional:
rail: Standard8, it fails if you update l10n changesets
rail: initial purpose of it was to check if something changed in translation since prev stable release
I don't see the changeset where that was intentional though.
If it is really what we want, then I propose the following changes:
- Remove the comment in L10nVerifyMetaDiff as it is clearly not correct
- Change the build to warn if the l10n meta diff is different. Preferably printing that changesets have updated or somehow note that the result can generally be expected.
My reasoning for the change to a warning: Nothing in the l10n verification or the builds is actually broken. This step is really intended as a test and showing a test failure is more logical.
Comment 2•14 years ago
|
||
(In reply to comment #1)
> My reasoning for the change to a warning: Nothing in the l10n verification or
> the builds is actually broken. This step is really intended as a test and
> showing a test failure is more logical.
I wholeheartedly agree.
Priority: -- → P3
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → rail
Priority: P3 → P2
Assignee | ||
Comment 3•13 years ago
|
||
Not sure what happens here, but self.super_class.evaluateCommand in http://hg.mozilla.org/build/buildbotcustom/file/28ca612b4a2c/steps/release.py#l52
is resolved as <unbound method C.evaluateCommand>, so parent's .evaluateCommand is not called.
Calling it as TinderboxShellCommand.evaluateCommand fixes the problem, but I'm not sure if the solution is reconfig safe.
Attachment #550312 -
Flags: feedback?(dustin)
Comment 4•13 years ago
|
||
(In reply to comment #3)
Last time I got an error like that, I checked my reload() statements to make sure there were no duplicates. Found and removed a duplicate reload(), and the problem went away.
Comment 5•13 years ago
|
||
Your solution doesn't sound reconfig-safe, no. Recall the problem occurs on a *failed* reconfig, not a successful one.
I don't immediately see why the parent class would evaluate incorrectly, though. Is TinderboxShellCommand ever replaced with C?
Assignee | ||
Comment 6•13 years ago
|
||
This line http://hg.mozilla.org/build/buildbotcustom/file/d76595b99f4e/steps/misc.py#l250 assigns buildbotcustom.steps.base.C instead of ShellCommand which is replaced here: http://hg.mozilla.org/build/buildbotcustom/file/d76595b99f4e/steps/base.py#l24. addErrorCatching adds its own checks if log_eval_func is not set and uses the worst value. Faking log_eval_func for L10nVerifyMetaDiff resolves the problem so addErrorCatching doesn't run its own checks.
Attachment #550312 -
Attachment is obsolete: true
Attachment #550312 -
Flags: feedback?(dustin)
Attachment #550503 -
Flags: review?(bhearsum)
Updated•13 years ago
|
Attachment #550503 -
Flags: review?(bhearsum) → review+
Assignee | ||
Comment 7•13 years ago
|
||
Comment on attachment 550503 [details] [diff] [review]
Pass fake log_eval_func
http://hg.mozilla.org/build/buildbotcustom/rev/88bf9993006c
Attachment #550503 -
Flags: checked-in+
Assignee | ||
Comment 8•13 years ago
|
||
Will be available after merge/reconfig.
Status: NEW → RESOLVED
Closed: 13 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
•