Closed Bug 370459 Opened 13 years ago Closed 13 years ago

make l10n metadiff work without human intervention


(Release Engineering :: General, defect)

Not set


(Not tracked)



(Reporter: rhelmer, Assigned: rhelmer)




(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'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 would be the input to the final "metadiff", which is not true. Also, due to the DE permissions problem during the 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 ./ 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 "./" 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


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/;
/cvsroot/mozilla/tools/release/Bootstrap/,v  <--
new revision: 1.9; previous revision: 1.8
Checking in Bootstrap/Step/;
/cvsroot/mozilla/tools/release/Bootstrap/Step/,v  <--
new revision: 1.7; previous revision: 1.6
Closed: 13 years ago
Resolution: --- → FIXED
Product: → Release Engineering
You need to log in before you can comment on or make changes to this bug.