In bug 463605 I plan to make OS X use a packaging manifest like Windows and Linux. Once that's done, we should just combine all three packaging manifests into a single preprocessed file, so people don't have to go hunting for various packaging files. Filing this as a followup to keep the scope of bug 463605 focused.

From bug 463605 comment 3: ------- Comment #3 From Phil Ringnalda (:philor) 2008-11-23 23:47:15 PDT (-) [reply] ------- > Having a single one would be a lot more pleasant if we drop USE_SHORT_LIBNAME > for Windows, so we could have a single @DLL_PREFIX@browsercomps@DLL_SUFFIX@ > instead of having to #ifdef XP_WIN brwsrcmp.dll #else. I agree with this. I don't understand why we never dropped USE_SHORT_LIBNAME for Windows. I guess OS/2 still uses it, which is why we never dropped it completely.

Can't comment on Windows, but yes, OS/2 does still use USE_SHORT_LIBNAME. (The system cannot load DLLs with more than 8.3 chars in the filename, so no way around.)

This works on mac, I need to push-to-try to check lin/win.

This breaks the Windows installer. Oops.

I think we might be able to just remove all references to packages-static in this makefile: http://mxr.mozilla.org/mozilla-central/source/browser/installer/windows/Makefile.in#57 All the real magic happens in installer-stage, which is in packager.mk, so I don't think this makefile actually uses packages-static in a meaningful way.

Here's the log from testing on WinCE which you asked for Ted. The delta from a log without attachment 397123 [details] [diff] [review] is * add '-DBINPATH=bin' when processing packages-manifest.in & removed-files.in * s/packages-static/packages-manifest/g * charset changes * add to package res/charsetData.properties, res/charsetalias.properties * start trying to package res/maccharset.properties * no longer package res/wincharset.properties * stopped packaging LICENSE * no more warnings trying to package Accessibility that wasn't built (woo!) * failed to package components/proxyObjInst.xpt, should be components/proxyObject * failed to package components/xpcom_threads.xpt, should be components/xpcom_thread.xpt * Still warns about components/layout_printing.xpt, plugins/npnul32.dll, uninstall/helper.exe proxyObject and xpcom_threads have platform specific name. Dunno what the charset stuff does.

Dunno whether that ifdef SOLARIS is actually accurate for "32-bit OS_ARCH == SunOS, OS_TEST != 86" but I bet you'll be more successful at finding out by just landing it than I ever was by filing a bug and hoping for comments :)

Thanks for the info Nick! I'll clean that up in the next rev. Some of the platform differences here are bizarre. Definitely going to go through and fix most of those, perhaps after this patch. Phil: yeah, I know that SOLARIS is at least sufficient ( http://mxr.mozilla.org/mozilla-central/source/configure.in#2544 ), and if it covers too many cases, well they can live with the packaging warnings or fix it themselves.

This just removes some bits of the installer makefiles. Rob: if you can run this through l10n repackaging, that would be cool. I think these bits are purely vestigal.

Comment on attachment 397283 [details] [diff] [review] rev 2, remove some installer/windows makefile bits >diff --git a/browser/installer/windows/Makefile.in b/browser/installer/windows/Makefile.in >--- a/browser/installer/windows/Makefile.in >+++ b/browser/installer/windows/Makefile.in >@@ -54,10 +54,6 @@ > PRE_RELEASE_SUFFIX := $(shell $(PYTHON) $(topsrcdir)/config/printprereleasesuffix.py $(APP_VERSION)) > DEFINES += -DPRE_RELEASE_SUFFIX="$(PRE_RELEASE_SUFFIX)" > >-PP_LOCALIZED_FILES = \ >- packages-static \ >- $(NULL) >- > # All script and locale files used by the Unicode version of NSIS need to be > # converted from UTF-8 to UTF-16LE > INSTALLER_FILES_CONV = \ >@@ -112,9 +108,6 @@ > done > $(INSTALL) $(addprefix $(DIST)/branding/,$(BRANDING_FILES)) $(CONFIG_DIR) > $(EXIT_ON_ERROR) \ remove this as well >- for i in $(PP_LOCALIZED_FILES); do \ >- $(PYTHON) $(topsrcdir)/config/Preprocessor.py $(DEFINES) $(ACDEFINES) $(srcdir)/$$i > $(CONFIG_DIR)/$$i; \ >- done > $(PYTHON) $(topsrcdir)/config/Preprocessor.py -Fsubstitution $(DEFINES) $(ACDEFINES) \ > $(srcdir)/nsis/defines.nsi.in | iconv -f UTF-8 -t UTF-16LE | \ > cat $(MOZILLA_DIR)/toolkit/mozapps/installer/windows/nsis/utf16-le-bom.bin - > \ >@@ -138,9 +131,6 @@ > done > $(INSTALL) $(addprefix $(DIST)/branding/,$(BRANDING_FILES)) $(CONFIG_DIR) > $(EXIT_ON_ERROR) \ and this >- for i in $(PP_LOCALIZED_FILES); do \ >- $(PYTHON) $(topsrcdir)/config/Preprocessor.py $(DEFINES) $(ACDEFINES) $(srcdir)/$$i > $(CONFIG_DIR)/$$i; \ >- done > ifeq ($(CONFIG_DIR),instgen) > $(PERL) $(topsrcdir)/toolkit/mozapps/installer/windows/nsis/make-installremoves.pl \ > ../removed-files > $(CONFIG_DIR)/removed-files.log

I was able to create a localized de installer without any problems so besides comment #11 I think this is good to go.

Filed bug 514665 on fixing those stupid xpt filename differences.

Rob: I'm sorry, it's not completely clear to me which bits of code you're suggesting I remove. Can you quote a little more concisely?

This should fix all of the warnings from Nick's log, several of which were actually correctness problems. (Oops.) I'll update when Rob gets back to me.

The $(EXIT_ON_ERROR) \ was for the code you removed which followed immediately afterward. I believe if you put this through the try with that there it would fail there as it did for me

Ah, thanks! I totally missed that. I think this is good to go, but I'm pushing it to try just to double-check.

Comment on attachment 398748 [details] [diff] [review] rev 4, with makefile fixup I'm not sure how I can systematically review the actual packages-static content, so I'm going to trust you on that part.

I've been over it a number of times, and Nick's wince build log was pretty helpful, but there's still a possibility for error, of course. I'll check the package step in the build logs, and I'll download packaged builds and sanity check them against the previous builds once this cycles. Pushed to m-c: http://hg.mozilla.org/mozilla-central/rev/784f46967416

Downloaded and diffed nightly builds vs. hourly builds with the patch, and pushed a followup to fix Linux bustage: http://hg.mozilla.org/mozilla-central/rev/69f32850f635 I missed packaging the "firefox" script (oops), as well as chrome/icons/default. Otherwise the only difference is that with my patch, we now package the LICENSE file. WinCE builds are showing the following warnings: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1252418777.1252421718.29075.gz&fulltext=1 Warning: package error or possible missing or unnecessary file: bin/res/charsetalias.properties (package-manifest, 305). Warning: package error or possible missing or unnecessary file: bin/res/charsetData.properties (package-manifest, 306). Warning: package error or possible missing or unnecessary file: bin/softokn3.chk (package-manifest, 326). Warning: package error or possible missing or unnecessary file: bin/freebl3.chk (package-manifest, 328). Warning: package error or possible missing or unnecessary file: bin/nssdbm3.dll (package-manifest, 331). Warning: package error or possible missing or unnecessary file: bin/nssdbm3.chk (package-manifest, 332). but diffing the nightly vs. an hourly shows no differences, which is good. Linux builds also show a warning for the charsetalias.properties and charsetData.properties: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1252418777.1252422672.7456.gz&fulltext=1 Those two warnings are just a bad leftover from bug 508421, I'll fix that. OS X builds show those two warnings plus: Warning: package error or possible missing or unnecessary file: Minefield.app/Contents/MacOS/libsoftokn3.chk (package-manifest, 335). Warning: package error or possible missing or unnecessary file: Minefield.app/Contents/MacOS/libfreebl3.chk (package-manifest, 337). Warning: package error or possible missing or unnecessary file: Minefield.app/Contents/MacOS/libnssdbm3.chk (package-manifest, 341). but the nightly vs. hourly differ only in that the hourly contains LICENSE. Still waiting on a Win32 hourly to verify that.

Fixed taras' followup patch to m-c: http://hg.mozilla.org/mozilla-central/rev/dd26d29368ae That will get rid of the {charsetalias,charsetData}.properties warnings.

Ah, the .chk stuff is just because we don't generate those in a cross-compile. The WinCE .chk warnings are...obvious, and the OS X ones are from the PPC half of the compile (the universal packaging goop regenerates new .chk files from the universal binaries). The nssdbm stuff should be conditioned on NSS_DISABLE_DBM.

Followup fix to fix the .chk and nssdbm warnings: http://hg.mozilla.org/mozilla-central/rev/fcf4388393f7 Noticed a typo in the Win32 build log: Warning: package error or possible missing or unnecessary file: @BINPATH/AccessibleMarshal.dll (package-manifest, 53). Pushed a fix for that: http://hg.mozilla.org/mozilla-central/rev/cccf8041fc0b

(In reply to comment #23) > Noticed a typo in the Win32 build log: > Warning: package error or possible missing or unnecessary file: > @BINPATH/AccessibleMarshal.dll (package-manifest, 53). > Pushed a fix for that: > http://hg.mozilla.org/mozilla-central/rev/cccf8041fc0b Diffing the Win32 nightly and hourly builds shows that AccessibleMarshal.dll was the only difference, so they should be the same now with that fix.

Looking at recent build logs shows zero packager warnings on any platform. In addition, diffing the most recent hourlies vs. the nightlies from before landing my patch shows no unexpected differences.

Quick, let's make packager warnings fatal...

Rolled up the original patch + followup fixes, and pushed to 1.9.2: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/60dda235c428

I had to port one line of a patch from bug 402892 to make this work right on branch: configure.in + AC_DEFINE(MOZ_ENABLE_GNOMEVFS) (Could have worked around that in the makefile, but that was the simplest solution.)