552668 - l10n verify should be unpacking jar files

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

[[e-mail] (:bhearsum)]

During the 3.5.9 release I noticed that l10n verify doesn't diff anything contained in jar files. Since so many strings are packed into jar files we should be unpacking them before running l10n verify. We miss a lot of string changes by ignoring these, which could potentially make us miss issues.

Comment 1

[Rail Aliiev [:rail]]

Attachment: Unpack JAR files

Something like this?

Comment 2

[[e-mail] (:bhearsum)]

Comment on attachment 458301 [details] [diff] [review]
Unpack JAR files

This seems like it would work. It'd be great to have l10n verify re-run for something like 3.6b1 - 3.6b2, or something else guaranteed to have string changes as a test for this.

Comment 3

[Rail Aliiev [:rail]]

Attachment: Don't rsync txt files

during testing the patch in staging I found that verify_l10n.sh calls unpack_build() and passes wildcard here: http://hg.mozilla.org/build/tools/file/6c1a3110a600/release/l10n/verify_l10n.sh#l28

As a result for locale (luckily txt comes after the installers) becomes firefox-3.6.8-build1/linux-i686/en-US/firefox-3.6.8.txt instead of "en-US".

To prevent this we shouldn't rsync *.txt files.

Comment 4

[Rail Aliiev [:rail]]

Attachment: Unpack JAR files

* Use -o (override) parameter to prevent console interaction
* Redirect stdout to /dev/null as in other unzip commands in the same file
* Add -N parameter to diff, so it creates diffs for new files. Otherwise it doesn't produce diffs for localized content (en-US.jar.dir vs ab_CD.jar.dir).

Comment 5

[[e-mail] (:bhearsum)]

Comment on attachment 462071 [details] [diff] [review]
Unpack JAR files

Seems reasonable. I'd be interested in seeing the output from a staging run, though.

Comment 6

[Rail Aliiev [:rail]]

(In reply to comment #5)
> I'd be interested in seeing the output from a staging run, though.

Sure, on moz2-darwin9-slave03:
/builds/slave/*_l10n_verification/tools/release/l10n/firefox*/diffs/*

Buildbot output is almost the same, on sm:
/builds/buildbot/moz2-master/*_l10n_verification/

Comment 7

[[e-mail] (:bhearsum)]

(In reply to comment #6)
> (In reply to comment #5)
> > I'd be interested in seeing the output from a staging run, though.
> 
> Sure, on moz2-darwin9-slave03:
> /builds/slave/*_l10n_verification/tools/release/l10n/firefox*/diffs/*
> 
> Buildbot output is almost the same, on sm:
> /builds/buildbot/moz2-master/*_l10n_verification/

Looks good. Land at your convenience!

Comment 8

[Rail Aliiev [:rail]]

Comment on attachment 462064 [details] [diff] [review]
Don't rsync txt files

http://hg.mozilla.org/build/buildbotcustom/rev/050b3c4f87f6

Comment 9

[Rail Aliiev [:rail]]

Comment on attachment 462071 [details] [diff] [review]
Unpack JAR files

http://hg.mozilla.org/build/tools/rev/483d22dfacf8

Comment 10

[Rail Aliiev [:rail]]

The patch affects update verify factory which uses the same function. JAR files should be unpacked only for l10n verification.

Comment 11

[Rail Aliiev [:rail]]

Attachment: Don't unpack JAR file by default

Comment 12

[Rail Aliiev [:rail]]

Comment on attachment 467776 [details] [diff] [review]
Don't unpack JAR file by default

http://hg.mozilla.org/build/tools/rev/c7ecffe0c0a1