Closed
Bug 978591
Opened 11 years ago
Closed 10 years ago
Remove MOZ_CHROME_FILE_FORMAT from Makefile.ins
Categories
(Firefox Build System :: General, defect)
Tracking
(firefox40 fixed)
RESOLVED
FIXED
mozilla40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: Fallen, Assigned: bokeefe)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
I have two Makefiles that use MOZ_CHROME_FILE_FORMAT. Does it make sense to add this to moz.build instead? I could probably remove 1-2 makefiles this way.
Comment 1•11 years ago
|
||
(In reply to Philipp Kewisch [:Fallen] from comment #0)
> I have two Makefiles that use MOZ_CHROME_FILE_FORMAT. Does it make sense to
> add this to moz.build instead? I could probably remove 1-2 makefiles this
> way.
It looks like it should be easy to make it a passthrough variable in moz.build. Is this for testing/specialpowers and testing/mochitest? Does anyone know if the explicit MOZ_CHROME_FILE_FORMATS are still necessary there? It's possible we can just remove them entirely...
Reporter | ||
Comment 2•11 years ago
|
||
No, its for packaging Lightning locales. We use MOZ_CHROME_FILE_FORMAT=jar, otherwise the files are extracted one by one when the xpi is installed. I don't know if its been fixed, but the extraction of the xpi is done synchronous and triggers a long running script warning. If the user clicks "stop script", then Lightning is left in some limbo state which is hard to recover from. Packing the locales as .jar files reduces file size a bit and makes the extraction a lot quicker.
http://mxr.mozilla.org/comm-central/search?string=MOZ_CHR&find=calendar&findi=&filter=^[^\0]*%24&hitlimit=&tree=comm-central
Comment 3•11 years ago
|
||
You know xpis don't have to be extracted when installed, right?
Reporter | ||
Comment 4•11 years ago
|
||
Yes, but we still have a binary xpcom component, so it must. Trying to get rid of that, but that won't happen for the next major release.
Comment 5•11 years ago
|
||
I'm not following. You said lightning locales. Don't tell me those have a binary xpcom component.
Reporter | ||
Comment 6•11 years ago
|
||
Sorry, I should have been more elaborate. Lightning itself contains a binary component. The locales are not packaged as langpacks, they are included in the main lightning.xpi. When we finally release there is one lightning.xpi per platform, that contains the extension itself and all locales. Example directory structure, abbreviated:
lightning.xpi
├── calendar-js
├── chrome
│ ├── calendar
│ │ ├── content
│ │ └── skin
│ ├── icons
│ ├── lightning
│ │ ├── content
│ │ └── skin
│ ├── calendar-en-US.jar
│ ├── lightning-en-US.jar
│ ├── ...
│ ├── calendar-zh-TW.jar
│ └── lightning-zh-TW.jar
├── components
│ ├── ...
│ └── libcalbasecomps.so
├── defaults
├── modules
├── chrome.manifest
└── install.rdf
We use MOZ_CHROME_FILE_FORMAT to make sure the locale files, calendar-AB-CD and lightning-AB-CD are jarred.
Assignee | ||
Comment 7•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Blocks: nomakefiles
Reporter | ||
Comment 8•10 years ago
|
||
Nice coincidence :-) I am just getting rid of MOZ_CHROME_FILE_FORMAT in calendar and now its being patched. Its good to see how easy it is to port variables, I might send a few patches myself in the future.
Comment 9•10 years ago
|
||
I think at this point we should just remove MOZ_CHROME_FILE_FORMAT.
Comment 10•10 years ago
|
||
Comment on attachment 8596770 [details] [diff] [review]
Move MOZ_CHROME_FILE_FORMAT to moz.build
Review of attachment 8596770 [details] [diff] [review]:
-----------------------------------------------------------------
::: python/mozbuild/mozbuild/frontend/context.py
@@ +1150,5 @@
> +
> + Setting MOZ_CHROME_FILE_FORMAT overrides the default value set during
> + configure with --enable-chrome-format.
> +
> + ``MOZ_CHROME_FILE_FORMAT`may be set to one of the following:
Drop one ` and add a space
@@ +1158,5 @@
> + There are two other possible values, which cannot be set per directory:
> + - symlink: Like ``flat``, but using symlinks instead of file copies.
> + This option is deprecated.
> + - omni: Like ``jar``, but files are packaged into the Omnijar.
> + This option can only be set at configure time.
So why mention them here?
Assignee | ||
Comment 11•10 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #9)
> I think at this point we should just remove MOZ_CHROME_FILE_FORMAT.
I pushed a patch to try that just removes the two occurrences in Makefile.ins, to get a feel for how hard that would be: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3358947a05bd
I'm pretty sure the specialpowers one is entirely unnecessary - flat packaging is already the default on Windows and Android, and symlink packaging didn't exist when it was added, but Linux and OSX didn't have a problem with that.
The one in mochitest I'm not sure about. That's building the mochikit.jar inside a mochijar extension, and was added in bug 543800. Evidently there are no Android API 9 debug tests on try, so I triggered API 11 tests, but those haven't finished yet.
(In reply to :Ms2ger from comment #10)
> @@ +1158,5 @@
> > + There are two other possible values, which cannot be set per directory:
> > + - symlink: Like ``flat``, but using symlinks instead of file copies.
> > + This option is deprecated.
> > + - omni: Like ``jar``, but files are packaged into the Omnijar.
> > + This option can only be set at configure time.
>
> So why mention them here?
Just for completeness. I can take them out if we end up keeping the variable; the configure option tells you what's allowed at configure time anyway.
Assignee | ||
Comment 12•10 years ago
|
||
(In reply to Brian O'Keefe [:bokeefe] from comment #11)
> (In reply to Mike Hommey [:glandium] from comment #9)
> > I think at this point we should just remove MOZ_CHROME_FILE_FORMAT.
>
> I pushed a patch to try that just removes the two occurrences in
> Makefile.ins, to get a feel for how hard that would be:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=3358947a05bd
It took a while to get earlier patches working, but try is okay with just removing them.
Properly working try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8148fcc121ec
Attachment #8596770 -
Attachment is obsolete: true
Attachment #8596770 -
Flags: review?(mshal)
Attachment #8597966 -
Flags: review?(mh+mozilla)
Updated•10 years ago
|
Attachment #8597966 -
Flags: review?(mh+mozilla) → review+
Comment 13•10 years ago
|
||
Note lightning/thunderbird likely needs changes too.
Reporter | ||
Comment 14•10 years ago
|
||
Lightning is now free of MOZ_CHROME_FILE_FORMAT, Thunderbird just has one in confvars.sh at http://mxr.mozilla.org/comm-central/source/mail/confvars.sh#11 Not sure if that needs to go in mail/moz.build or if it should stay in confvars.sh as is.
Comment 15•10 years ago
|
||
Those in confvars.sh can stay.
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Summary: Add MOZ_CHROME_FILE_FORMAT to moz.build → Remove MOZ_CHROME_FILE_FORMAT from Makefile.ins
Comment 16•10 years ago
|
||
Keywords: checkin-needed
Comment 17•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•