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 ========

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.

(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.

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.

(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.

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?

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.

Comment on attachment 550503 [details] [diff] [review] Pass fake log_eval_func http://hg.mozilla.org/build/buildbotcustom/rev/88bf9993006c

Will be available after merge/reconfig.