Closed Bug 692743 Opened 8 years ago Closed 8 years ago

include hyphenation dictionaries in omnijar

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla10

People

(Reporter: jfkthame, Assigned: jfkthame)

References

Details

Attachments

(1 file, 2 obsolete files)

Now that bug 655337 has landed, we no longer need to provide the bundled hyphenation dictionaries as separate files on disk in order for Firefox to find and use them; they can be loaded directly from omnijar. This will reduce our installed footprint, which is a good thing.
AFAICS, this is trivial - we can add the hyphenation directory to OMNIJAR_FILES and it should Just Work.
Assignee: nobody → jfkthame
Attachment #565472 - Flags: review?(benjamin)
Attachment #565472 - Flags: review?(benjamin) → review?(mh+mozilla)
Comment on attachment 565472 [details] [diff] [review]
patch, include hyphenation files in omnijar

Sorry I didn't come to this earlier.
Attachment #565472 - Flags: review?(mh+mozilla) → review+
To clarify, you absolutely want to also update removed-files.in. Place the entries in the MOZ_OMNIJAR section.
(In reply to Michael Wu [:mwu] from comment #4)
> To clarify, you absolutely want to also update removed-files.in. Place the
> entries in the MOZ_OMNIJAR section.

So like this, yes? Also removed from package-manifest.in as per comment #2.
Attachment #569981 - Flags: review?(mwu)
(In reply to Jonathan Kew (:jfkthame) from comment #5)
> Created attachment 569981 [details] [diff] [review] [diff] [details] [review]
> v2, include hyphenation files in omnijar, and remove the separate files

I think you also need to add a line for just "hyphenation/" to removed-files.in to get rid of the then empty directory.
(In reply to Jonathan Kew (:jfkthame) from comment #5)
> Created attachment 569981 [details] [diff] [review] [diff] [details] [review]
> v2, include hyphenation files in omnijar, and remove the separate files
> 
> (In reply to Michael Wu [:mwu] from comment #4)
> > To clarify, you absolutely want to also update removed-files.in. Place the
> > entries in the MOZ_OMNIJAR section.
> 
> So like this, yes? Also removed from package-manifest.in as per comment #2.

package-manifest.in shouldn't be updated AFAIK. Without it in the package-manifest, the hyphenation files won't get copied to the staging directory, and the omnijar code only takes files out of the staging directory.

(In reply to Dave Garrett from comment #6)
> (In reply to Jonathan Kew (:jfkthame) from comment #5)
> > Created attachment 569981 [details] [diff] [review] [diff] [details] [review] [diff] [details] [review]
> > v2, include hyphenation files in omnijar, and remove the separate files
> 
> I think you also need to add a line for just "hyphenation/" to
> removed-files.in to get rid of the then empty directory.

Yup.
(In reply to Michael Wu [:mwu] from comment #7)
> package-manifest.in shouldn't be updated AFAIK. Without it in the
> package-manifest, the hyphenation files won't get copied to the staging
> directory, and the omnijar code only takes files out of the staging
> directory.

Right - I had just pushed it to tryserver, which confirmed this by failing the hyphenation tests. Hence dropping that from the patch.
Restored the required package-manifest.in entries.
Attachment #569981 - Attachment is obsolete: true
Attachment #569981 - Flags: review?(mwu)
Attachment #570044 - Flags: review?(mwu)
Comment on attachment 570044 [details] [diff] [review]
v2.1, include hyphenation files in omnijar, and remove the separate files

Looks good.
Attachment #570044 - Flags: review?(mwu) → review+
Attachment #565472 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/782f5b321370
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Blocks: 698451
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.