Closed Bug 370459 Opened 13 years ago Closed 13 years ago

make l10n metadiff work without human intervention

Categories

(Release Engineering :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: rhelmer, Assigned: rhelmer)

References

Details

Attachments

(1 file, 2 obsolete files)

Attached patch auto-metadiff (obsolete) — Splinter Review
Nothing like a field test :)
Here are some issues that didn't come up when testing this :

The first two just make it go (much) faster by only downloading what we need : 

* need to exclude non-dmg/exe/tar.gz files, the way I did it didn't work with quoting semantics of RunShellCommand
* rsync should not be "-a"


* we expect the actual metadiff step to return a non-zero exit code, because there will almost always be some differences, even if they are just line offsets.
* look for the word 'FAIL' in the logs, that'll show us things like read-only files
* run verification in the dir it expects

To deal with the metadiff step, I've added an "ignoreExitValue" (boolean) param to Step.pm's Shell() method.
Attachment #255165 - Flags: review?(preed)
Assignee: build → rhelmer
Blocks: end2end-bld
We're expecting to get FAIL lines for the DE builds that shipped last time :(
Attachment #255165 - Attachment is obsolete: true
Attachment #255404 - Flags: review?(preed)
Attachment #255165 - Flags: review?(preed)
Comment on attachment 255404 [details] [diff] [review]
do not die when encountering FAIL

Sorry, missed a spot here :/ need to make sure to pull from oldVersion/oldRc, not oldVersion/rc

The latter will work in some situations, making this hard to test, but is of course a bug!

The _real_ fix here is to have a place to store this info that won't change out from under us; storing the diffs would be enough.
Attachment #255404 - Attachment is obsolete: true
Attachment #255404 - Flags: review?(preed)
Ok, this works as expected :) The last patch ran fine and did something useful, but I was making a bad assumption - that the output of verify_l10n.sh would be the input to the final "metadiff", which is not true. Also, due to the DE permissions problem during the 1.5.0.9/2.0.0.1 cycle and the new helper.exe signing requirement, we need to treat some formerly fatal conditions as warnings now.

The desired sequence of events is more like this:

* bootstrap downloads current release and previous release
* bootstrap runs ./verify_l10n.sh for each and logs output
* if logs contain "Only", "Binary" or "FAIL" then test has failed
** Note that "FAIL" is a warning right now because of the known DE problems
** Also, "Binary" is a warning because of the l10n helper.exe signing issue
* the metadiff should be run against the diff directories that "./verify_l10n.sh" created, NOT against the output logged above

This patch fixes the last item (which was a bug), and makes Binary and FAIL messages return a warning instead of halting. After this release, FAIL should be  made fatal again, and we should figure out a good way of including exceptions for Binary (maybe it's ok for anything in "localized/" ?)
Attachment #255534 - Flags: review?(preed)
Comment on attachment 255534 [details] [diff] [review]
produce expected metadiff

$ignoreExitValue

Is that so that it will basically ignore failures? (If the exitValue is true, then nothing happens, right?)

If so, I think this should be renamed "ignoreFailure," since that more accurately indicates what you're doing.

(Maybe I'm slow, but "ignoreExitValue" had me scratching my head for a second.)

The other thing that's going to come back to bite you is relying on the previous release's -candidates directory.

We should at least file a bug on finding a way around that. We shouldn't have to do that. :-/
Attachment #255534 - Flags: review?(preed) → review+
Landed as-is, as per our irc conversation ("ignoreExitValue" is a little obtuse but it means only ignore the $exitValue, not e.g. core dumps or other bad failures):

Checking in Bootstrap/Step.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step.pm,v  <--  Step.pm
new revision: 1.9; previous revision: 1.8
done
Checking in Bootstrap/Step/Repack.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/Repack.pm,v  <--  Repack.pm
new revision: 1.7; previous revision: 1.6
done
Status: NEW → RESOLVED
Closed: 13 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.