Closed Bug 472431 Opened 11 years ago Closed 11 years ago

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


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



(Not tracked)



(Reporter: nthomas, Assigned: Pike)



(Keywords: regression)


(2 files, 1 obsolete file)

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 ?
Summary: Locale builds have chrome.manifest & install.rdf at root level → Localized builds have chrome.manifest & install.rdf at root level
Found the bug, taking.
Assignee: nobody → l10n
Keywords: regression
Simple patch, once I found where we actually copy over. Asking Nick for review to not make Ted cry.
Attachment #357285 - Flags: review?(nthomas)
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.
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.
Attachment #357285 - Flags: review?(nthomas) → review?(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/ b/browser/locales/
>--- a/browser/locales/
>+++ b/browser/locales/
>@@ -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.
Attachment #357285 - Flags: review?(bhearsum) → review+
*nudge* Please land this fix before 3.1b3 comes along.
Looks like this landed in m-c:
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.
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.
Attachment #357285 - Attachment is obsolete: true
Attachment #358381 - Flags: review+
Attachment #358381 - Flags: approval1.9.1?
Closed: 11 years ago
Flags: blocking-firefox3.1?
Resolution: --- → FIXED
Priority: -- → P1
Target Milestone: --- → Firefox 3.1b3
Blocks: 475120
Comment on attachment 358381 [details] [diff] [review]
patch that landed, with better comments

Attachment #358381 - Flags: approval1.9.1? → approval1.9.1+
Flags: blocking-firefox3.1? → blocking-firefox3.1+
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.
Resolution: FIXED → ---
Attachment #360837 - Flags: review? → review?(ted.mielczarek)
Attachment #360837 - Flags: review?(ted.mielczarek) → review+
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
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.