472431 - Localized builds have chrome.manifest & install.rdf at root level

Status: RESOLVED FIXED

(Firefox Build System :: General, defect, P1)

Description

[Nick Thomas [:nthomas] (UTC+12)]

If you unpack a latest-mozilla-central-l10n/ or latest-mozilla-1.9.1-l10n/ build for a locale then you'll get a chrome.manifest and install.rdf, which appear to be from the associated langpack. These aren't present in en-US nightlies or 3.0.5 builds, but can also be found in 3.1b2. Even if this doesn't break the app, I think we ought to fix it because it introduces a bogus difference between en-US and l10n builds.

Fallout from bug 458014 ?

Comment 1

[Axel Hecht [:Pike]]

Found the bug, taking.

Comment 2

[Axel Hecht [:Pike]]

Attachment: don't copy over install.rdf and chrome.manifest for installers and packages

Simple patch, once I found where we actually copy over. Asking Nick for review to not make Ted cry.

Comment 3

[Nick Thomas [:nthomas] (UTC+12)]

I'm not familiar with this code. Ben I think you might know more, would you mind taking the review ?

Does this cover the Mac case too ? All three platforms are affected.

Comment 4

[Axel Hecht [:Pike]]

Actually, all four binaries are affected. All three packages (tar, zip, dmg) are covered by the repackage-zip tweak, and the windows installer by the repackage-win32-installer tweak.

PS: When testing this, you need to clobber l10n-stage. Local builds don't do that on purpose, but that happens anyway when a new en-US binary comes in to get repackaged by the deps.

Comment 5

[[e-mail] (:bhearsum)]

Comment on attachment 357285 [details] [diff] [review]
don't copy over install.rdf and chrome.manifest for installers and packages

>diff --git a/browser/locales/Makefile.in b/browser/locales/Makefile.in
>--- a/browser/locales/Makefile.in
>+++ b/browser/locales/Makefile.in
>@@ -226,7 +226,10 @@
> 	$(CYGWIN_WRAPPER) 7z x -ol10n-stage $(WIN32_INSTALLER_IN)
> 	$(RM) -r l10n-stage/localized
> 	$(RM) l10n-stage/setup.exe
>+# copy xpi-stage over, but not the langpack files
>+# install.rdf and chrome.manifest

This comment is a little unclear to me. Maybe you can drop 'the langpack files' and just leave the names ?

>+# copy xpi-stage over, but not the langpack files
>+# install.rdf and chrome.manifest

same here.


Looks fine otherwise.

Comment 6

[Nick Thomas [:nthomas] (UTC+12)]

*nudge* Please land this fix before 3.1b3 comes along.

Comment 7

[Nick Thomas [:nthomas] (UTC+12)]

Looks like this landed in m-c:
  http://hg.mozilla.org/mozilla-central/rev/8fe5bc1ec5e9
so please take comment #6 for mozilla-1.9.1

Of course it's hard to tell if it's working because we're not pulling m-c properly when building locales - bug 474898.

Comment 8

[Axel Hecht [:Pike]]

Attachment: patch that landed, with better comments

This is the patch that I landed. I reworded the comment to be clearer which files we don't copy over, but kept referencing langpack as the reason why.

Verified the fix against my l10n builds. Those just finished the first build with the new nightlies, which I needed as I compile with the nightly en-US sources in the end.

Requesting approval1.9.1 as we want this for Beta 3.

Comment 9

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

Comment on attachment 358381 [details] [diff] [review]
patch that landed, with better comments

a191=beltzner

Comment 10

[Axel Hecht [:Pike]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/eb7e484c485f, fixed1.9.1.

Comment 11

[Nick Thomas [:nthomas] (UTC+12)]

I'd like to remove the two files on update to 3.1b3 too, it's adding noise to our update verifications and we should keep everyone in sync.

Comment 12

[Nick Thomas [:nthomas] (UTC+12)]

Attachment: Update removed-files.in

Comment 13

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 360837 [details] [diff] [review]
Update removed-files.in

http://hg.mozilla.org/mozilla-central/rev/688c44602a55

Comment 14

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 360837 [details] [diff] [review]
Update removed-files.in

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/b42b29ee809b