Omnijar ordering is not preserved by l10n repacking

RESOLVED FIXED

Status

defect
--
major
RESOLVED FIXED
8 years ago
Last year

People

(Reporter: taras.mozilla, Assigned: mwu)

Tracking

Trunk
All
Windows 7
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 -)

Details

Attachments

(1 attachment)

Reporter

Description

8 years ago
Work implemented in bug 559961 and bug 589368 did not get turned on in our RC1. I'm guessing this is because the build was done manually(according to https://bugzilla.mozilla.org/show_bug.cgi?id=641325#c2). Since there is no build log, I have no other leads. We shipped ordered omnijar in betas and it's working correctly in nightlies.
I hope FF4 release gets the omnijar ordering right.
"manually" just means "ran the steps that buildbot would run by hand", so I would be surprised if that's a culprit. Regardless, I'll help investigate this in a bit.
Assignee: nobody → bhearsum
Summary: Firefox 4 RC1 does have an ordered omnijar → Firefox 4 RC1 does not have an ordered omnijar

Comment 2

8 years ago
> Firefox 4 RC1 does not have an ordered omnijar

Not exactly, the windows en-us installer does, and has had for long time.  As you said, "Looks like our l10n repacks might be busted"

(In reply to comment #0)
> We shipped ordered omnijar in betas and it's
> working correctly in nightlies.

The windows German b12 does not have the ordered omnijar.  Possibly none of the localized installers do, or ever had.
Any idea if they were ordered at any point? Eg, an earlier beta?
cc'ing others who may have something to add.
Also, has anyone checked other platforms?
Reporter

Comment 6

8 years ago
(In reply to comment #5)
> Also, has anyone checked other platforms?

This only works on PGO-enabled platforms, ie windows.

> > Firefox 4 RC1 does not have an ordered omnijar
> 
> Not exactly, the windows en-us installer does, and has had for long time.  As
> you said, "Looks like our l10n repacks might be busted"
> 
> (In reply to comment #0)
> > We shipped ordered omnijar in betas and it's
> > working correctly in nightlies.
> 
> The windows German b12 does not have the ordered omnijar.  Possibly none of the
> localized installers do, or ever had.


Thanks for extensive testing.


I just confirmed that rc is fine with en-US and broken in german and others. This is disappointing as I developed this while testing with russian locale which is broken now.(In reply to comment #2)
Assignee: bhearsum → nobody
Component: Release Engineering → Build Config
Product: mozilla.org → Core
QA Contact: release → build-config
Version: other → Trunk
I did a quick look at b5 through b11 (all of the other releases that had omni.jar). None of their win32 de builds had an ordered omni.jar, which makes me strongly think that this never worked.
Reporter

Comment 8

8 years ago
To summarize: non en-US builds wont get ordered omnijar in ff4 :(
Summary: Firefox 4 RC1 does not have an ordered omnijar → Omnijar ordering is not preserved by l10n repacking
Marking as blocking2.0? to make drivers aware of the fact that we don't have optimized omni.jar's for any l10n repacks. So far, it looks like we've never had it for l10n repacks.
blocking2.0: --- → ?
Indeed, but it need not block.
blocking2.0: ? → -
I just did a couple more paranoid checks to confirm that this is l10n-only, and indeed, it is:
- Latest win32 nightly en-US build (zip and installer) -- ordered
- Latest l10n nightly de build (zip and installer) -- unordered
So, intuitively this makes sense if you think about it, because we optimize the omnijar using info in $(DIST)/jarlog:
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/packager.mk#327

Now that command is part of $(MAKE_PACKAGE):
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/packager.mk#337

which does get called to repack the l10n builds:
http://mxr.mozilla.org/mozilla-central/source/toolkit/locales/l10n.mk#151

...except $(DIST)/jarlog isn't going to exist on the machine that does the repacking.

Updated

8 years ago
Blocks: 641803
Assignee

Comment 14

8 years ago
The perl command that we use to do s/en-US/AB_CD/g stomps all over the logs instead of doing what it's suppose to. I have no idea why. I'll fix/replace it.
Assignee: nobody → mwu
Assignee

Comment 15

8 years ago
Posted patch FixSplinter Review
It seems like perl doesn't do inplace editing well in windows. Having it make a backup seems to make this work better.
Attachment #526167 - Flags: review?(ted.mielczarek)
Reporter

Comment 16

8 years ago
(In reply to comment #15)
> It seems like perl doesn't do inplace editing well in windows. Having it make a
> backup seems to make this work better.

ewww, thanks for catching this.
Comment on attachment 526167 [details] [diff] [review]
Fix

Boo, perl!
Attachment #526167 - Flags: review?(ted.mielczarek) → review+
Assignee

Comment 18

8 years ago
Optimistically resolving fixed.

http://hg.mozilla.org/mozilla-central/rev/9cf0970e50e5
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Assignee

Comment 19

8 years ago
Good news: The logs now exist and omni.jar in the ja repack does use an optimized jar.
Bad news: Localized files don't seem to be ordered. I can't reproduce it with my standard test setup - localized files are ordered fine there. Not sure what's going on.
Inline editing won't work in general, as we're unpacking once, and then doing multiple repacks on it. At which point you couldn't replace en-US anymore, it's already 'af'. We'll need to keep the en-US jarlog around, and create a "localized" one on each run.

Reopening.

I'm a bit confused by my local tests, I see

+ cd firefox/Nightly.app/Contents/MacOS
+ /usr/bin/python2.6 /Users/axelhecht/src/central/mozilla-central/config/optimizejars.py --deoptimize /Users/axelhecht/src/central/b-repack/browser/locales/../../dist/jarlog/ ./ ./
Deoptimized 0/1479 in ./omni.jar


similar thing happens on the nightlies, linux log says:

./omni.jar: startup data ends at byte 0
Deoptimized 0/1450 in ./omni.jar

wokbok:dist axelhecht$ ls -l jarlog/
total 0
-rw-r--r--  1 axelhecht  staff  0 22 Apr 11:52 omni.jar.log

shows that omni.jar.log is empty after unpacking.

Debugging shows that readahead is 0 in my case, and thus
  elif readahead >= old_entry_offset + len(data):
never triggers? But that's all magic to me, I don't know what it's supposed to do.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee

Comment 21

8 years ago
(In reply to comment #20)
> Inline editing won't work in general, as we're unpacking once, and then doing
> multiple repacks on it.

Why wouldn't it work? AFAIK, it actually does work.

> At which point you couldn't replace en-US anymore, it's
> already 'af'. We'll need to keep the en-US jarlog around, and create a
> "localized" one on each run.
> 

Why? Doing the replacement each time is harmless - it just does nothing.

> Reopening.
> 
> I'm a bit confused by my local tests, I see
> 
> + cd firefox/Nightly.app/Contents/MacOS
> + /usr/bin/python2.6
> /Users/axelhecht/src/central/mozilla-central/config/optimizejars.py
> --deoptimize
> /Users/axelhecht/src/central/b-repack/browser/locales/../../dist/jarlog/ ./ ./
> Deoptimized 0/1479 in ./omni.jar
> 

Ordering only happens if you either do PGO or go out of your way to generate jarlogs. Linux/OSX omnijars are not ordered, so this behavior is expected. Windows is the only platform that we currently reorder jars on.

There is an issue with certain entries not getting ordered but I want to address that in a separate bug.
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Assignee

Comment 22

8 years ago
(In reply to comment #21)
> > At which point you couldn't replace en-US anymore, it's
> > already 'af'. We'll need to keep the en-US jarlog around, and create a
> > "localized" one on each run.
> > 
> 
> Why? Doing the replacement each time is harmless - it just does nothing.
> 
Oh, are you saying we unpack once and do multiple language repacks? Then this other issue makes sense. But I still want to address that in another bug.
Assignee

Comment 23

8 years ago
Accidentally checked in the patch for bug 659379 with this bug's number. But, bug 659379 is the followup for this anyway, so good enough.

Updated

Last year
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.