Added ability to reverse jar optimization. --deoptimize mode takes a list of optimized jars and produces a bunch of boring jars + startup logs. Locale logic then does s/en-US/relevant-locale/ and reoptimizes jars. Kyle, asking for your review first for build stuff(perhaps there is a better way to do my perl oneliner?).

Comment on attachment 467984 [details] [diff] [review] magic patch Some python style nits: if inlog <> None: is commonly wrote as if inlog is not None: and the opposite as if inlog is None: On the makefile, use sed or python? No pearls for perl. I didn't do a thorough review, but didn't see anything obvious.

(In reply to comment #1) > On the makefile, use sed or python? No pearls for perl. I haven't looked at this closely, but yes, please use sed.

I don't understand the need to deoptimize the JAR. Can't we create a startup log (if we need to) from optimized JARs, instead of "boring" JARs? The actual code to read the JARs isn't going to change, is it?

(In reply to comment #3) > I don't understand the need to deoptimize the JAR. Can't we create a startup > log (if we need to) from optimized JARs, instead of "boring" JARs? The actual > code to read the JARs isn't going to change, is it? I do create a startup log while deoptimizing the jar. The main reason for deoptimizing is that unzip has braindamaged error checks. It unzips the jar successfully, but prints out bogus warnings about zip structure and returns an error code. I figured the easiest way to work around that was to in addition to extracting the log to deoptimize the jar too.

s/perl/sed/ + misc absolute path fix

Now with Pike's "is not None" suggestion

Missed the is not None changes in previous patch. I wish there was some way to abort uploading a patch if I got outstanding changes that weren't qrefed.

Comment on attachment 468729 [details] [diff] [review] l10n repacking support for jar ordering I don't think that I should offer more than a feedback+ on patches of this scope. A few more hints on what to watch out for: There are a bunch of files created in the working dir, if I read this right. The l10n repacks do src-dir builds, still, so watch out for that? Also, I haven't run across prefixing a command with -, what does that do to the sed line?

(In reply to comment #8) > > There are a bunch of files created in the working dir, if I read this right. > The l10n repacks do src-dir builds, still, so watch out for that? The only files created should be jar log files, those seem ok in my testing. > > Also, I haven't run across prefixing a command with -, what does that do to the > sed line? Makes it ignore errors from sed, so if there are no log files, repacking still works.

Comment on attachment 468729 [details] [diff] [review] l10n repacking support for jar ordering Going to wait for a way to specify the path/to/omnijar as a absolute path. Too fragile otherwise.

Is this needed before we do l10n repacks for beta5? If so, can we get a quick review, khuey?

No, it's not. The l10n builds might be slower than the en-US builds, but it shouldn't affect functionality.

Comment on attachment 470592 [details] [diff] [review] l10n repacking support for jar ordering Ted, perhaps you can review this sooner than Kyle who might be busy with school.

Yeah, I'm a little swamped with school and my own blockers right now, but if ted can't get to it I'll try to handle it.

Comment on attachment 470592 [details] [diff] [review] l10n repacking support for jar ordering ># HG changeset patch ># User Taras Glek <tglek@mozilla.com> ># Parent 64f6f59520c7b67b53260449347fe0b308edd25d >Bug 589368 - Locale repacking support for jar reordering > >diff --git a/config/optimizejars.py b/config/optimizejars.py >--- a/config/optimizejars.py >+++ b/config/optimizejars.py >@@ -183,14 +183,24 @@ class BinaryBlob: > assert_true(out_data == data, "Serialization fail %d !=%d"% (len(out_data), len(data))) > return retstruct > >-def optimizejar(log, jar, outjar): >- entries = open(log).read().rstrip().split("\n") >+def optimizejar(jar, outjar, inlog = None): >+ if inlog is not None: >+ inlog = open(inlog).read().rstrip() >+ # in the case of an empty log still move the index forward >+ if len(inlog) == 0: >+ inlog = [] >+ else: >+ inlog = inlog.split("\n") inlog = open(inlog).readlines() >+def optimize(JAR_LOG_DIR, IN_JAR_DIR, OUT_JAR_DIR): >+ if not os.path.exists(JAR_LOG_DIR): >+ print("No jar logs found in %s. No jars to optimize." % JAR_LOG_DIR) >+ exit(0) > >-if not os.path.exists(JAR_LOG_DIR): >- print "No jar logs found. No jars to optimize." >- exit(0) >+ ls = os.listdir(JAR_LOG_DIR) >+ for logfile in ls: >+ if logfile[-8:] != ".jar.log": if not logfile.endswith(".jar.log"): >+def deoptimize(JAR_LOG_DIR, IN_JAR_DIR, OUT_JAR_DIR): >+ if not os.path.exists(JAR_LOG_DIR): >+ os.makedirs(JAR_LOG_DIR) >+ >+ ls = os.listdir(IN_JAR_DIR) >+ for jarfile in ls: >+ if jarfile[-4:] != ".jar": if not jarfile.endswith(".jar"): >+MODE = sys.argv[1] >+JAR_LOG_DIR = sys.argv[2] >+IN_JAR_DIR = sys.argv[3] >+OUT_JAR_DIR = sys.argv[4] >+if MODE == "--optimize": >+ optimize(JAR_LOG_DIR, IN_JAR_DIR, OUT_JAR_DIR) >+elif MODE == "--deoptimize": >+ deoptimize(JAR_LOG_DIR, IN_JAR_DIR, OUT_JAR_DIR) >+else: >+ print("Unknown mode %s" % MODE) >+ exit(1) Could you fix this slightly to do: if __name__ == '__main__': main() and stick all the current top-level stuff in: def main(): ... Looks fine otherwise. r=me with those changes.

inlog = inlog.split("\n") and inlog = open(inlog).readlines() are not equiv, sadly, you'll need to strip the trailing newlines if you use readlines().

True. [line.rstrip() for line in open(inlog).readlines()] would work.

Comment on attachment 470592 [details] [diff] [review] l10n repacking support for jar ordering Clearing review since ted has this covered.

Addressed review comments(except for .readlines(), I like the existing code better).

Comment on attachment 473234 [details] [diff] [review] l10n repacking support for jar ordering a=beltzner