Closed Bug 460977 Opened 13 years ago Closed 13 years ago

make -C browser/locales langpack-AB_CD is not compatible with --enable-chrome-format=flat

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla3.1b3

People

(Reporter: dev-null, Assigned: dev-null)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

Attached patch Patch v1.0 (obsolete) — Splinter Review
Regression from Bug 458014.

Steps to Reproduce:
1. configure with --enable-chrome-format=flat
2. make -C browser/locales langpack-AB_CD

Actual Results:  
The langpack is broken.
Attachment #344106 - Flags: review?(l10n)
I recommend WONTFIX. Flat chrome is for debugging, and we shouldn't be packing it up for any reason I can think of.
(In reply to comment #1)
> I recommend WONTFIX. Flat chrome is for debugging, and we shouldn't be packing
> it up for any reason I can think of.

Localizers who don't have compile environment may want to make langpack, while flat chrome is better for debugging than jar chrome.
Comment on attachment 344106 [details] [diff] [review]
Patch v1.0

This does not work with installers-AB_CD.
Attachment #344106 - Attachment is obsolete: true
Attachment #344106 - Flags: review?(l10n)
Should not remove AB_CD.manifest, just not packaging.
Attachment #344229 - Flags: review?(l10n)
Comment on attachment 344229 [details] [diff] [review]
Patch v1.1
[Checkin: Comment 8+9]

r=me, putting this on ted's and bhearsum's radar, too.

Single line nod-off review should be good enough.
Attachment #344229 - Flags: review?(ted.mielczarek)
Attachment #344229 - Flags: review?(l10n)
Attachment #344229 - Flags: review?(bhearsum)
Attachment #344229 - Flags: review+
Attachment #344229 - Flags: review?(bhearsum) → review+
Comment on attachment 344229 [details] [diff] [review]
Patch v1.1
[Checkin: Comment 8+9]

I don't understand the details of constructing a langpack, but I don't see how this would break anything.
Comment on attachment 344229 [details] [diff] [review]
Patch v1.1
[Checkin: Comment 8+9]

Your word is good enough for me.
Attachment #344229 - Flags: review?(ted.mielczarek) → review+
Assignee: nobody → dev-null
Status: NEW → ASSIGNED
Keywords: checkin-needed
Comment on attachment 344229 [details] [diff] [review]
Patch v1.1
[Checkin: Comment 8+9]

http://hg.mozilla.org/mozilla-central/rev/6629bb8baf41
Attachment #344229 - Attachment description: Patch v1.1 → Patch v1.1 [Checkin: Comment 8]
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.2a1
Comment on attachment 344229 [details] [diff] [review]
Patch v1.1
[Checkin: Comment 8+9]

http://hg.mozilla.org/mozilla-central/rev/579407c55f2d
Attachment #344229 - Attachment description: Patch v1.1 [Checkin: Comment 8] → Patch v1.1 [Checkin: Comment 8+9]
Blocks: 471093
Carrying over review because it's just synced with 1.9.1.
Requesting approval for 1.9.1.
Attachment #354657 - Flags: review+
Attachment #354657 - Flags: approval1.9.1?
Comment on attachment 354657 [details] [diff] [review]
Patch v1.1 for 1.9.1
[Checkin: Comment 14]

As the patch shows, the 1.9.1 tree never regressed, so there's no reason to land this on 1.9.1
(In reply to comment #11)
> As the patch shows, the 1.9.1 tree never regressed, so there's no reason to
> land this on 1.9.1

The changed lines are same between the trunk patch and the branch patch.
trunk:
-	  $(ZIP) -r9D $(LANGPACK_FILE) install.rdf chrome/$(AB_CD).jar chrome.manifest
+	  $(ZIP) -r9D $(LANGPACK_FILE) install.rdf chrome chrome.manifest -x chrome/$(AB_CD).manifest

1.9.1 branch:
-	  $(ZIP) -r9D $(LANGPACK_FILE) install.rdf chrome/$(AB_CD).jar chrome.manifest
+	  $(ZIP) -r9D $(LANGPACK_FILE) install.rdf chrome chrome.manifest -x chrome/$(AB_CD).manifest

This was regressed on 2008-10-20, which was before branching on 2008-11-29.
Need to land also on 1.9.1 because the regression has been branched.
Attachment #354657 - Flags: approval1.9.1? → approval1.9.1+
Comment on attachment 354657 [details] [diff] [review]
Patch v1.1 for 1.9.1
[Checkin: Comment 14]

a191=beltzner
Keywords: checkin-needed
Whiteboard: checkin-needed for 1.9.1
Comment on attachment 354657 [details] [diff] [review]
Patch v1.1 for 1.9.1
[Checkin: Comment 14]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/9873114c0cb8
Attachment #354657 - Attachment description: Patch v1.1 for 1.9.1 → Patch v1.1 for 1.9.1 [Checkin: Comment 14]
Whiteboard: checkin-needed for 1.9.1
Target Milestone: Firefox 3.2a1 → Firefox 3.1b3
Component: Build Config → General
Product: Firefox → Firefox Build System
Target Milestone: Firefox 3.1b3 → mozilla3.1b3
You need to log in before you can comment on or make changes to this bug.