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

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Atsushi Sakai]

Attachment: Patch v1.0

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.

Comment 1

[Benjamin Smedberg]

I recommend WONTFIX. Flat chrome is for debugging, and we shouldn't be packing it up for any reason I can think of.

Comment 2

[Atsushi Sakai]

(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 3

[Atsushi Sakai]

Comment on attachment 344106 [details] [diff] [review]
Patch v1.0

This does not work with installers-AB_CD.

Comment 4

[Atsushi Sakai]

Attachment: Patch v1.1 [Checkin: Comment 8+9]

Should not remove AB_CD.manifest, just not packaging.

Comment 5

[Axel Hecht [:Pike]]

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.

Comment 6

[[e-mail] (:bhearsum)]

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 7

[(not currently active) Ted Mielczarek]

Comment on attachment 344229 [details] [diff] [review]
Patch v1.1
[Checkin: Comment 8+9]

Your word is good enough for me.

Comment 8

[Serge Gautherie (:sgautherie)]

Comment on attachment 344229 [details] [diff] [review]
Patch v1.1
[Checkin: Comment 8+9]

http://hg.mozilla.org/mozilla-central/rev/6629bb8baf41

Comment 9

[Serge Gautherie (:sgautherie)]

Comment on attachment 344229 [details] [diff] [review]
Patch v1.1
[Checkin: Comment 8+9]

http://hg.mozilla.org/mozilla-central/rev/579407c55f2d

Comment 10

[Atsushi Sakai]

Attachment: Patch v1.1 for 1.9.1 [Checkin: Comment 14]

Carrying over review because it's just synced with 1.9.1.
Requesting approval for 1.9.1.

Comment 11

[Axel Hecht [:Pike]]

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

Comment 12

[Atsushi Sakai]

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

Comment 13

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 354657 [details] [diff] [review]
Patch v1.1 for 1.9.1
[Checkin: Comment 14]

a191=beltzner

Comment 14

[Serge Gautherie (:sgautherie)]

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