Omnijar ordering is not preserved by l10n repacking

RESOLVED FIXED

Status

()

Core
Build Config
--
major
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: (dormant account), Assigned: mwu)

Tracking

Trunk
All
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 -)

Details

Attachments

(1 attachment)

(Reporter)

Description

7 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

7 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

7 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

7 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: --- → ?

Comment 10

7 years ago
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.
(Reporter)

Comment 13

7 years ago
UNPACK_OMNIJAR recreates the logdir, see http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/packager.mk#331
Blocks: 641803
(Assignee)

Comment 14

6 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

6 years ago
Created attachment 526167 [details] [diff] [review]
Fix

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

6 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

6 years ago
Optimistically resolving fixed.

http://hg.mozilla.org/mozilla-central/rev/9cf0970e50e5
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Comment 19

6 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.

Comment 20

6 years ago
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

6 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
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
(Assignee)

Comment 22

6 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

6 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.
You need to log in before you can comment on or make changes to this bug.