Closed
Bug 809798
Opened 12 years ago
Closed 12 years ago
Don't use BOTH_MANIFESTS
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: glandium, Assigned: glandium)
References
Details
Attachments
(2 files)
(deleted),
patch
|
Pike
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
standard8
:
review+
|
Details | Diff | Splinter Review |
Using BOTH_MANIFESTS=1, we end up with:
- chrome.manifest, containing all manifest entries, *and* a "manifest chrome/ab-CD.manifest" entry.
- chrome/ab-CD.manifest containing all manifest entries.
What we currently do in langpacks is including chrome.manifest and skip chrome/ab-CD.manifest, and we leave the spurious "manifest chrome/ab-CD.manifest" entry, which adds a useless file lookup at runtime.
*Not* using BOTH_MANIFESTS=1, we'd end up with:
- chrome.manifest, containing a "manifest chrome/ab-CD.manifest" entry.
- chrome/ab-CD.manifest containing all manifest entries.
which is just as fine, provided chrome/ab-CD.manifest is included in langpacks.
Switching to not using BOTH_MANIFESTS=1 also makes life easier in bug 780561.
Assignee | ||
Comment 1•12 years ago
|
||
Attachment #679610 -
Flags: review?(l10n)
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → mh+mozilla
Assignee | ||
Comment 2•12 years ago
|
||
Attachment #679611 -
Flags: review?(mbanner)
Comment 3•12 years ago
|
||
Comment on attachment 679610 [details] [diff] [review]
Don't use BOTH_MANIFESTS
Review of attachment 679610 [details] [diff] [review]:
-----------------------------------------------------------------
Makes sense, r=me.
I was contemplating on whether we should remove BOTH_MANIFESTS support, but we need to support both manifests for test extensions at least, so there's little code removal, and we still have tests for it outside of code, so things will likely continue to work, if anyone wanted both.
This is gonna introduce a minor merge conflict with bug 796051, but that's trivial and shouldn't hold us back in either bug.
Attachment #679610 -
Flags: review?(l10n) → review+
Comment 4•12 years ago
|
||
Comment on attachment 679611 [details] [diff] [review]
Don't use BOTH_MANIFESTS (for c-c)
Looks fine to me.
Attachment #679611 -
Flags: review?(mbanner) → review+
Assignee | ||
Comment 5•12 years ago
|
||
Whiteboard: [leave open]
Comment 6•12 years ago
|
||
Assignee | ||
Comment 7•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [leave open]
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
•