I'll look into this once the omnijar bug is fixed.

(In reply to comment #1) > I'll look into this once the omnijar bug is fixed. mwu hasn't resolved the bug, but he pushed a couple pieces last week - are we blocked on all of his patches here, or just certain among them?

I don't think there's enough landed in that bug to do anything with yet.

Once this lands I think we can do stat()s to invalidate caches in bug 531886, it should be cheap.

The android pieces have now landed, but Ted's not sure when he can get to the remaining desktop bits. I'm secretly hoping that "soon" is the answer; I'd suggest someone else if I could think of someone else to pick it up. bsmedberg's out this week, he'd be my obvious alternate, I think?

I suspect bsmedberg has too much other stuff on his plate as well. Once I finish up some Breakpad work I'll be free to work on this. I was planning on working on bug 561842, which is also a startup-related bug, but taras says he'd rather have me work on this. So I can probably get on this in a few days.

BTW, there's one more thing that needs to be done to get omnijar builds parity with normal builds - support for loading window icons from bitmaps instead of files. I'm also not sure how l10n and tests fit into the picture with omnijar - that needs to be investigated. Beyond that, there are things that are needed to cram absolutely everything into the omnijar/app binary: 1. Compiling application.ini into the binary. (platform.ini already got this treatment) 2. Convert spellcheck dictionary loading to be uri based so it can load things from the omnijar. 3. Read the default blocklist from the omnijar. 4. Statically link libxul, application binary components, and everything else into the app binary. 5. (Firefox specific) Move search plugins into the jar. Mobile already does this. 6. (Firefox specific) Unpack default profile files from the omnijar. 7. (maybe optional) Support autoconfig 8. Deal with plugin-container, possibly by making the main app do the job with the right flags. Not familiar enough with e10s to know if this is possible. 9. Deal with the updater - no idea how to handle this though we can probably avoid a ton of the complexity in the current updater. If we can fix these issues, we can have a single executable firefox. I might work on 1-3 since mobile needs it.

(In reply to comment #7) > 4. Statically link libxul, application binary components, and everything else > into the app binary. Ow. Do we really need to do this? This makes life harder for us in a number of other ways. Our current plan was instead to just get everything into libxul (bug 561842), which doesn't give us quite as many problems.

(In reply to comment #8) > (In reply to comment #7) > > 4. Statically link libxul, application binary components, and everything else > > into the app binary. > > Ow. Do we really need to do this? This makes life harder for us in a number of > other ways. Our current plan was instead to just get everything into libxul > (bug 561842), which doesn't give us quite as many problems. If we want firefox in a single executable. I don't know if that's where we want to end up - just listed the requirements for getting there.

In the interest of keeping this bug reasonably scoped, I propose that the end result of fixing this bug should be that we produce one single jar file containing all the non-code files for Firefox. bug 561842 will cover reducing the number of binaries we ship down to a firefox executable + libxul. If we want to go beyond that (single static binary or append the omnijar to libxul or something) we can file followup bugs.

Was there ever a plan for repackaging the localized parts of the omnijar? Unless that work has already been planned out, I think this is way out of scope for Firefox 4.

Now compatible with locale repacks. Remaining issues (known to me): 1. Only works on unix right now. 2. Locale specific prefs aren't concatenated to the omnipref yet.

This change allows xpcshell to work with omnijar. It also moves the omnijar setup to after locale setup. Setting the omnijar too early initialized the native charset conversion before the locale was set properly. This made the code think ascii was the native codeset instead of utf8, causing a number of test failures.

We need to set OMNIJAR_PATH to get things setup properly since the default omnijar file name is omni.jar now, not argv[0].

Generates a omni.jar when make package is run. Locale repacks on non-windows works.

This moves the extract function to nsZipArchive and makes the extract function automatically generate any parent directories necessary to extract a file. This is necessary for the next patch..

This initializes profiles from defaults/profile/ in the omnijar if we're using omnijar.

Two more patches to fix universal builds on OSX and tests on all platforms to come after they prove themselves on try.

Move OMNIJAR_PATH setting to java wrapper.

Fix all tests but Talos and tests on OSX.

Comment on attachment 458860 [details] [diff] [review] 4. Move extract function from nsJAR to nsZipArchive >- NS_ENSURE_TRUE(item, NS_ERROR_FILE_TARGET_DOES_NOT_EXIST); Use explicit returns instead of hiding them in macros. >- if (NS_FAILED(rv)) return rv; Keep return rv on separate line. r+ with those fixed. The single-directory backoff is cute, but wont work for files nested 2 directories deep without corresponding directories in the zip index. Don't have to fix this since it's inherited from old code.

Works pretty well. Has two remaining issues which I'll fix in followup bugs/patches - 1. Pref combining breaks locale repackaging stuff. 2. For some reason the empty chrome directory doesn't get preserved on windows so talos fails.

Please ignore talos, I'm fixing that in bug 580698.

(In reply to comment #25) > Please ignore talos, I'm fixing that in bug 580698. Nice! I can drop the empty chrome directory workaround then.

Address review comments.

This makes flight.mk and fix-buildconfig work on flat/omnijar builds.

Comment on attachment 458855 [details] [diff] [review] 1. Move omnijar setup to NS_InitXPCOM and use omni.jar by default I really don't like the OMNIJAR_PATH environment variable: if you need something for child processes, we should pass the omnijar path in the IPC launching code (as a commandline flag), or just figure it out automatically. Definitely not in the environment. Everything else look ok, though. Do you want to actually check that omni.jar exists?

Comment on attachment 459236 [details] [diff] [review] 7. Fix tests For test_bug292789.html, please try to use cr.convertChromeURI, instead of forking the file. In test_import, just remove the two lines you commented out. runtests.py.in doesn't look like it will work on Windows, which doesn't have a command named 'cp'. What is it you're actually copying there? And I think the installChromeFile stuff will be obsoleted by bug 579178.

(In reply to comment #30) > Comment on attachment 459236 [details] [diff] [review] > 7. Fix tests > > For test_bug292789.html, please try to use cr.convertChromeURI, instead of > forking the file. > Can't seem to access Components.classes. Any alternative? > In test_import, just remove the two lines you commented out. > Done. > runtests.py.in doesn't look like it will work on Windows, which doesn't have a > command named 'cp'. What is it you're actually copying there? > We're copying the contents of bin/ to firefox/. This helps xpcshell find the omnijar and also copies components over. 'cp' seems to work fine on try, actually. Mochitests wouldn't run otherwise. Another option is to specify the omnijar on the command line.

That cp is gross. xpcshell supports a -g option to pass the GRE dir (and we use it). Can we just make that work instead?

Added -omnijar. Omnijar code changed to setup nsZipArchive lazily to avoid initializing the native charset code too early.

Comment on attachment 459882 [details] [diff] [review] 1. Move omnijar setup to NS_InitXPCOM and use omni.jar by default, v2 Assuming you want omnijar to work on Windows, GetNativePath is not a good choice. It is possible to install Firefox to a unicode directory that cannot be represented by a native path. It looks like you may have to either use platform ifdefs or make childArgv a CommandLine instead of a vector of std::string. You need to remove the ifdef around mozilla::SetOmijar(nsnull): that ifdef should only be used for malloced data, not refcounted objects.

Avoid GetNativePath for windows, look for omnijar in gre directory instead of app directory.

Avoid cp and remove those two lines in test_import. Also, preprocess test_bug292789.html to avoid an omnijar fork of the file.

Do you have any data on how much this would improve start-up time for desktop builds?

(In reply to comment #37) > Do you have any data on how much this would improve start-up time for desktop > builds? I expect about 10% on cold start. From my runs on try server, I usually see a bit less than 10% improvement on linux and 10-20% on windows and osx. On first start, the improvement is greater (and noticeable) due to faster IO while loading js components. There are additional optimizations available that omnijar makes possible. It also makes it possible to fix bug 531886 which I expect to cause significant extension developer frustration if not fixed.

At this point I don't think this blocks, although we'll take this in a beta soon if the pieces come together. I probably wouldn't take this past beta4 (ships 20-Aug).

I think that this must block, actually -- Fx4 is supposed to be a release focused around major improvements in performance, of which startup time is one component. We know this has a significant impact on that, and so we should prioritize getting it in.

Comment on attachment 458864 [details] [diff] [review] 6. Let the browser reset bookmarks from the omnijar For more detailed review comments with context, please see http://reviews.visophyte.org/r/458864/. on file: browser/components/nsBrowserGlue.js line 823 > bookmarksURI = Services.io.newURI("resource:///defaults/profile/bookmarks.html", null, null); We should just import (locally in this method for now) NetUtil.jsm, and then use NetUtil.newURI. You don't have to pass the two null params. on file: browser/components/nsBrowserGlue.js line 825 > else > { nit: brace goes after the else on file: browser/components/nsBrowserGlue.js line 829 > bookmarksURI = Services.io.newFileURI(bookmarksFile); ...and then just use NetUtil.newURI(bookmarksFile) here. on file: toolkit/components/places/public/nsIPlacesImportExportService.idl line 71 > void importHTMLFromUri(in nsIURI aURI, in boolean aIsInitialImport); nit: URI on file: toolkit/components/places/src/nsPlacesImportExportService.h line 53 > nsresult ImportHTMLFromUriInternal(nsIURI* aURI, PRBool aAllowRootChanges, nit: URI on file: toolkit/components/places/src/nsPlacesImportExportService.cpp line 2185 > nsPlacesImportExportService::ImportHTMLFromUri(nsIURI* aURI, > PRBool aIsInitialImport) nit: spacing is off here r=sdwilsh This is an API change, so it's going to need sr too.

Comment on attachment 459579 [details] [diff] [review] 8. Bring universal binaries into the world of omnijar I'm sure I don't understand how this works. By the time we get around to flight.mk, haven't we already created the omnijar? Why are we modifying the flat version in chrome/toolkit? I really don't understand how flight.mk would get MOZ_CHROME_FILE_FORMAT_JAR set. I wish I weren't the reviewer for this patch: I didn't write the code in question, nor do I understand how it works.

Comment on attachment 459579 [details] [diff] [review] 8. Bring universal binaries into the world of omnijar >diff --git a/build/macosx/universal/flight.mk b/build/macosx/universal/flight.mk >--- a/build/macosx/universal/flight.mk >+++ b/build/macosx/universal/flight.mk >@@ -67,7 +67,7 @@ ifeq ($(MOZ_BUILD_APP),camino) # { >+ifdef MOZ_CHROME_FILE_FORMAT_JAR >+BUILDCONFIG = $(BUILDCONFIG_BASE)/toolkit.jar >+FIX_MODE = jar >+else >+BUILDCONFIG = $(BUILDCONFIG_BASE)/toolkit/ >+FIX_MODE = file >+endif I'm pretty sure this won't work, because flight.mk is invoked from client.mk, which doesn't know much about the build (it doesn't know any of the stuff from autoconf.mk, for example).

Comment on attachment 459579 [details] [diff] [review] 8. Bring universal binaries into the world of omnijar Didn't really mean to clear that review flag, but let's just do this.

Comment on attachment 459965 [details] [diff] [review] 7. Fix tests, v2 runtests.py.in should just use if not os.path.isdir(chromeDir): os.mkdir(chromeDir) I don't quite understand the nsComponentManager.cpp change. Under what condition would we have a MOZ_OMNIJAR build with no mozilla::OmnijarPath? Isn't that an error condition that should fail early, instead of getting all the way here? Or are you trying to support MOZ_OMNIJAR without actually building the omnijar?

(In reply to comment #45) > Comment on attachment 459965 [details] [diff] [review] > 7. Fix tests, v2 > > runtests.py.in should just use > > if not os.path.isdir(chromeDir): > os.mkdir(chromeDir) > Ok. > I don't quite understand the nsComponentManager.cpp change. Under what > condition would we have a MOZ_OMNIJAR build with no mozilla::OmnijarPath? Isn't > that an error condition that should fail early, instead of getting all the way > here? > > Or are you trying to support MOZ_OMNIJAR without actually building the omnijar? Yeah, this is for running xpcshell tests without packaging.

Address review comment about runtests.py.in .

Comment on attachment 458862 [details] [diff] [review] 5. Teach nsToolkitProfileService to initialize the profile from the omnijar Hate to pile another review on, but dietrich doesn't seem to be around.

Added a few #ifdef MOZ_OMNIJAR to make this build on non-omnijar.

Comment on attachment 459566 [details] [diff] [review] 3. Add support for packaging omnijar, v2 ># HG changeset patch ># Parent c4c18d1a7840ce97f9a3216d9f3088463f096470 > >diff --git a/browser/installer/package-manifest.in b/browser/installer/package-manifest.in >--- a/browser/installer/package-manifest.in >+++ b/browser/installer/package-manifest.in >@@ -10,6 +10,12 @@ > > #filter substitution > >+#ifdef MOZ_CHROME_FILE_FORMAT_JAR >+#define JAREXT .jar >+#else >+#define JAREXT >+#endif Can you do this in the Makefile instead? I'd rather keep all the DEFINES out in the Makefile in the same place. >diff --git a/toolkit/locales/l10n.mk b/toolkit/locales/l10n.mk >--- a/toolkit/locales/l10n.mk >+++ b/toolkit/locales/l10n.mk >@@ -95,8 +95,6 @@ clobber-%: > > > PACKAGER_NO_LIBS = 1 >-include $(topsrcdir)/toolkit/mozapps/installer/packager.mk >- > > ifeq (cocoa,$(MOZ_WIDGET_TOOLKIT)) > STAGEDIST = $(_ABS_DIST)/l10n-stage/$(MOZ_PKG_APPNAME)/$(_APPNAME)/Contents/MacOS >@@ -104,6 +102,8 @@ else > STAGEDIST = $(_ABS_DIST)/l10n-stage/$(MOZ_PKG_DIR) > endif > >+include $(topsrcdir)/toolkit/mozapps/installer/packager.mk What's this change for? >diff --git a/toolkit/mozapps/installer/packager.mk b/toolkit/mozapps/installer/packager.mk >--- a/toolkit/mozapps/installer/packager.mk >+++ b/toolkit/mozapps/installer/packager.mk >@@ -107,32 +107,32 @@ UNPACK_TAR = tar -xf- > > ifeq ($(MOZ_PKG_FORMAT),TAR) > PKG_SUFFIX = .tar >-MAKE_PACKAGE = $(CREATE_FINAL_TAR) - $(MOZ_PKG_DIR) > $(PACKAGE) >-UNMAKE_PACKAGE = $(UNPACK_TAR) < $(UNPACKAGE) >+_MAKE_PACKAGE = $(CREATE_FINAL_TAR) - $(MOZ_PKG_DIR) > $(PACKAGE) >+_UNMAKE_PACKAGE = $(UNPACK_TAR) < $(UNPACKAGE) > MAKE_SDK = $(CREATE_FINAL_TAR) - $(MOZ_APP_NAME)-sdk > $(SDK) > endif > ifeq ($(MOZ_PKG_FORMAT),TGZ) > PKG_SUFFIX = .tar.gz >-MAKE_PACKAGE = $(CREATE_FINAL_TAR) - $(MOZ_PKG_DIR) | gzip -vf9 > $(PACKAGE) >-UNMAKE_PACKAGE = gunzip -c $(UNPACKAGE) | $(UNPACK_TAR) >+_MAKE_PACKAGE = $(CREATE_FINAL_TAR) - $(MOZ_PKG_DIR) | gzip -vf9 > $(PACKAGE) >+_UNMAKE_PACKAGE = gunzip -c $(UNPACKAGE) | $(UNPACK_TAR) > MAKE_SDK = $(CREATE_FINAL_TAR) - $(MOZ_APP_NAME)-sdk | gzip -vf9 > $(SDK) > endif > ifeq ($(MOZ_PKG_FORMAT),BZ2) > PKG_SUFFIX = .tar.bz2 >-MAKE_PACKAGE = $(CREATE_FINAL_TAR) - $(MOZ_PKG_DIR) | bzip2 -vf > $(PACKAGE) >-UNMAKE_PACKAGE = bunzip2 -c $(UNPACKAGE) | $(UNPACK_TAR) >+_MAKE_PACKAGE = $(CREATE_FINAL_TAR) - $(MOZ_PKG_DIR) | bzip2 -vf > $(PACKAGE) >+_UNMAKE_PACKAGE = bunzip2 -c $(UNPACKAGE) | $(UNPACK_TAR) > MAKE_SDK = $(CREATE_FINAL_TAR) - $(MOZ_APP_NAME)-sdk | bzip2 -vf > $(SDK) > endif > ifeq ($(MOZ_PKG_FORMAT),ZIP) > PKG_SUFFIX = .zip >-MAKE_PACKAGE = $(ZIP) -r9D $(PACKAGE) $(MOZ_PKG_DIR) >-UNMAKE_PACKAGE = $(UNZIP) $(UNPACKAGE) >+_MAKE_PACKAGE = $(ZIP) -r9D $(PACKAGE) $(MOZ_PKG_DIR) >+_UNMAKE_PACKAGE = $(UNZIP) $(UNPACKAGE) > MAKE_SDK = $(ZIP) -r9D $(SDK) $(MOZ_APP_NAME)-sdk > endif > ifeq ($(MOZ_PKG_FORMAT),CAB) > PKG_SUFFIX = .cab >-MAKE_PACKAGE = $(MAKE_CAB) >-UNMAKE_PACKAGE = $(error Unpacking CAB files is not supported) >+_MAKE_PACKAGE = $(MAKE_CAB) >+_UNMAKE_PACKAGE = $(error Unpacking CAB files is not supported) > endif > ifeq ($(MOZ_PKG_FORMAT),DMG) > ifndef _APPNAME >@@ -169,11 +169,11 @@ endif > ifndef PKG_DMG_SOURCE > PKG_DMG_SOURCE = $(STAGEPATH)$(MOZ_PKG_DIR) > endif >-MAKE_PACKAGE = $(_ABS_MOZSRCDIR)/build/package/mac_osx/pkg-dmg \ >+_MAKE_PACKAGE = $(_ABS_MOZSRCDIR)/build/package/mac_osx/pkg-dmg \ > --source "$(PKG_DMG_SOURCE)" --target "$(PACKAGE)" \ > --volname "$(MOZ_APP_DISPLAYNAME)" $(PKG_DMG_FLAGS) > _ABS_DIST = $(call core_abspath,$(DIST)) >-UNMAKE_PACKAGE = \ >+_UNMAKE_PACKAGE = \ > set -ex; \ > rm -rf $(_ABS_DIST)/unpack.tmp; \ > mkdir -p $(_ABS_DIST)/unpack.tmp; \ >@@ -203,6 +203,36 @@ endif > MAKE_SDK = $(CREATE_FINAL_TAR) - $(MOZ_APP_NAME)-sdk | bzip2 -vf > $(SDK) > endif > >+ifdef MOZ_OMNIJAR >+OMNIJAR_FILES = \ >+ chrome \ >+ components/*.js \ >+ components/*.xpt \ >+ components/omnijar.manifest \ >+ modules \ >+ res \ >+ defaults \ >+ greprefs.js \ >+ $(NULL) >+ >+NON_OMNIJAR_FILES = \ >+ chrome/icons/\* \ >+ res/cursors/\* \ >+ res/MainMenu.nib/\* \ >+ $(NULL) It feels a little icky to have this list here. I get that it makes your MAKE_PACKAGE bit easier, but it doesn't feel very good. >+ifdef MOZ_OMNIJAR >+ @echo "Preparing files for omnijar" >+# create manifest for components containing only omnijarable files >+# also creates a new manifest for remaining binary components >+ @(for MANIFEST in $(DIST)/$(STAGEPATH)$(MOZ_PKG_DIR)$(_BINPATH)/components/*.manifest ; do grep -v '^binary-component' "$$MANIFEST" >> omnijar.manifest ; grep '^binary-component' "$$MANIFEST" >> binary.manifest ; echo >> omnijar.manifest ; echo >> binary.manifest ; rm "$$MANIFEST" ; done ) >+ mv omnijar.manifest $(DIST)/$(STAGEPATH)$(MOZ_PKG_DIR)$(_BINPATH)/components/ >+ mv binary.manifest $(DIST)/$(STAGEPATH)$(MOZ_PKG_DIR)$(_BINPATH)/components/ >+# combine prefs into the omnipref >+ @(for PREF in $(DIST)/$(STAGEPATH)$(MOZ_PKG_DIR)$(_BINPATH)/defaults/pref/*.js ; do cat "$$PREF" >> prefs.js ; echo >> prefs.js ; rm "$$PREF" ; done ) >+ mv prefs.js $(DIST)/$(STAGEPATH)$(MOZ_PKG_DIR)$(_BINPATH)/defaults/ >+endif # MOZ_OMNIJAR Seriously, can you make this a little Python script? This is super hard to read, and I don't want to have to maintain this much shell goop. If you did that, you could have the Python script generate omni.jar instead of hiding it in MAKE_PACKAGE, right? (Which has the nice side benefit that you can `make stage-package` and get an omnijar build in dist/firefox.) Although I guess you need the MAKE_PACKAGE / UNMAKE_PACKAGE to work for the l10n repackaging bit, eh? Hrm.

Use -omnijar instead of OMNIJAR_PATH.

Comment on attachment 458862 [details] [diff] [review] 5. Teach nsToolkitProfileService to initialize the profile from the omnijar This might be ok (although I hate it, and wish that we didn't have defaults/profile at all!). But I need a couple things in order to review: 1) move this extracting bit into a static function on its own: ::CreateProfile is already long and indented enough. 2) give the patch lots more context: -u12 or more. 3) Is there no better way to do the string-fu? RFindChar, substring-comparisons, Last() all come to mind so we can avoid doing low-level char* magic.

(In reply to comment #50) > Can you do this in the Makefile instead? I'd rather keep all the DEFINES out in > the Makefile in the same place. > Ok. > >diff --git a/toolkit/locales/l10n.mk b/toolkit/locales/l10n.mk > >--- a/toolkit/locales/l10n.mk > >+++ b/toolkit/locales/l10n.mk > >@@ -95,8 +95,6 @@ clobber-%: > > > > > > PACKAGER_NO_LIBS = 1 > >-include $(topsrcdir)/toolkit/mozapps/installer/packager.mk > >- > > > > ifeq (cocoa,$(MOZ_WIDGET_TOOLKIT)) > > STAGEDIST = $(_ABS_DIST)/l10n-stage/$(MOZ_PKG_APPNAME)/$(_APPNAME)/Contents/MacOS > >@@ -104,6 +102,8 @@ else > > STAGEDIST = $(_ABS_DIST)/l10n-stage/$(MOZ_PKG_DIR) > > endif > > > >+include $(topsrcdir)/toolkit/mozapps/installer/packager.mk > > What's this change for? > Hmm.. I thought I had a reason for this.. Guess not. I'll move it back. > >+ifdef MOZ_OMNIJAR > >+OMNIJAR_FILES = \ > >+ chrome \ > >+ components/*.js \ > >+ components/*.xpt \ > >+ components/omnijar.manifest \ > >+ modules \ > >+ res \ > >+ defaults \ > >+ greprefs.js \ > >+ $(NULL) > >+ > >+NON_OMNIJAR_FILES = \ > >+ chrome/icons/\* \ > >+ res/cursors/\* \ > >+ res/MainMenu.nib/\* \ > >+ $(NULL) > > It feels a little icky to have this list here. I get that it makes your > MAKE_PACKAGE bit easier, but it doesn't feel very good. I'm not sure where else to put this.. > >+ifdef MOZ_OMNIJAR > >+ @echo "Preparing files for omnijar" > >+# create manifest for components containing only omnijarable files > >+# also creates a new manifest for remaining binary components > >+ @(for MANIFEST in $(DIST)/$(STAGEPATH)$(MOZ_PKG_DIR)$(_BINPATH)/components/*.manifest ; do grep -v '^binary-component' "$$MANIFEST" >> omnijar.manifest ; grep '^binary-component' "$$MANIFEST" >> binary.manifest ; echo >> omnijar.manifest ; echo >> binary.manifest ; rm "$$MANIFEST" ; done ) > >+ mv omnijar.manifest $(DIST)/$(STAGEPATH)$(MOZ_PKG_DIR)$(_BINPATH)/components/ > >+ mv binary.manifest $(DIST)/$(STAGEPATH)$(MOZ_PKG_DIR)$(_BINPATH)/components/ > >+# combine prefs into the omnipref > >+ @(for PREF in $(DIST)/$(STAGEPATH)$(MOZ_PKG_DIR)$(_BINPATH)/defaults/pref/*.js ; do cat "$$PREF" >> prefs.js ; echo >> prefs.js ; rm "$$PREF" ; done ) > >+ mv prefs.js $(DIST)/$(STAGEPATH)$(MOZ_PKG_DIR)$(_BINPATH)/defaults/ > >+endif # MOZ_OMNIJAR > > Seriously, can you make this a little Python script? This is super hard to > read, and I don't want to have to maintain this much shell goop. If you did > that, you could have the Python script generate omni.jar instead of hiding it > in MAKE_PACKAGE, right? (Which has the nice side benefit that you can `make > stage-package` and get an omnijar build in dist/firefox.) > > Although I guess you need the MAKE_PACKAGE / UNMAKE_PACKAGE to work for the > l10n repackaging bit, eh? Hrm. Yeah I can put this part in a python script. I usually prefer shell scripting since it's more straightforward than python for simple things like this.

This one also builds on non-omnijar.

Comment on attachment 458862 [details] [diff] [review] 5. Teach nsToolkitProfileService to initialize the profile from the omnijar Tests seem to pass fine without this patch. Goodbye defaults/profile.

I took out the messy parts that are gonna end up changing after bug 579178.

Yeah, bug 579178 should land first: I'll mark approvals for this afterwards.

This is the last piece. browser/installer/Makefile.in | 5 -- browser/installer/windows/nsis/installer.nsi | 26 ++++-------- browser/locales/Makefile.in | 26 +----------- testing/release/common/unpack.sh | 7 +-- toolkit/mozapps/installer/packager.mk | 46 +++++++++------------- toolkit/mozapps/installer/windows/nsis/common.nsh | 3 - 6 files changed, 39 insertions(+), 74 deletions(-) It eliminates the nonlocalized/localized distinction in the win32 installer and makes the l10n repacking for the win32 installer use the same code that osx(dmg)/linux(tgz)/windows(zip) use.

Looks like I accidentally eliminated the use of MOZ_(LOCALIZED|NONLOCALIZED|OPTIONAL)_PKG_LIST. Can be put back or eliminated completely.

Comment on attachment 463765 [details] [diff] [review] 9. Make windows installer locale repacks use the standard repacking path If I read this patch correctly, if I take a completely clean tree and make repackage-win32-installer it will fail because you've removed the branding export which is necessary to build setup.exe. r-ing for this, though everything else looks pretty good. Can we ditch the core subdirectory? The optional components can live in the optional/ subdirectory, but it'd be nice if for the common case no subdirectory was needed at all. Since the browser doesn't use optional components http://mxr.mozilla.org/mozilla-central/source/browser/installer/windows/nsis/installer.nsi#238 can change to just "Installing Files". Other than that I didn't look at the NSIS bits too closely. When you upload a new version please use a diff with 8 lines of context.

It has come to my attention that we're talking about repacking locales in FF4 again. I really don't think we should be taking patches at this point which muck with repacking locales. It's extremely fragile, impossible to test on tryserver, and regressions aren't going to show up until very late in the cycle. We should either leave locales out of the omnijar, or punt on omnijar.

(In reply to comment #61) > It has come to my attention that we're talking about repacking locales in FF4 > again. I really don't think we should be taking patches at this point which > muck with repacking locales. It's extremely fragile, impossible to test on > tryserver, and regressions aren't going to show up until very late in the > cycle. We should either leave locales out of the omnijar, or punt on omnijar. Leaving locales out of the omnijar is a pain because it's not what it's designed to do. These patches are designed to be compatible as possible by mimicking flat packaged locale repacks. It's compatible with our current locale repacking code to the point that nearly all but the last patch do not actually touch any locale related code. To special case locale repacks would actually require more mucking with locale repacking code. These patches also do not turn omnijar on by default. We can land these patches, have releng see how well omnijar builds/repacks/etc. work, and have it on or off depending on this data. No patches need to be backed out to turn off omnijar.

a=me to land reviewed patches here which do not change the default behavior

Per today's meeting, we're going to push to get this in beta4, which codefreezes on Monday. Ted, can you do or delegate the remaining two reviews?

FWIW, here's a doc that describes pretty exactly how we do repacks, https://developer.mozilla.org/en/Creating_a_Language_Pack. Other than that, I'd suggest to get someone from releng to give you the exact list of commands we do for depends, nightlies, and release builds. Testing should work fine with, say, french, and l10n-merge on, which is an option to https://developer.mozilla.org/En/Compare-locales, a python tool we use in our build system.

It would be better not to enumerate, but trying to combine all the prefs causes more trouble than it's worth right now.

http://hg.mozilla.org/mozilla-central/rev/f80832cefeb5 http://hg.mozilla.org/mozilla-central/rev/f2bffbc6fee7 http://hg.mozilla.org/mozilla-central/rev/b57d52fc217e http://hg.mozilla.org/mozilla-central/rev/d0d46d0fc5d9

Not actually fixed, but a bunch of patches did land. I think there's about 4 more patches to go.

I dropped the MOZ_CORE_PKG_LIST variable change since the localized/nonlocalized separation is necessary after the manifest enumeration changes. Also made an attempt to support optional but it'll need more work if someone wants to use it. "Installing main files" wasn't changed.. not too comfortable with changing strings here..

Have you tested the Windows installer, l10n repackaging for the Windows installer, and / or update repackaging? Are all of the patches to get this working locally attached to this bug? Will the only option be omnijar? SeaMonkey uses optional components and I see that callek and kairo are both cc'd.

(In reply to comment #70) > Have you tested the Windows installer, l10n repackaging for the Windows > installer, and / or update repackaging? Windows installer and l10n repackaging are both tested. Update repackaging not tested - need to check that. > Are all of the patches to get this > working locally attached to this bug? I think so, though it's a pain. I have a patch queue available at http://hg.mozilla.org/users/mwu_mozilla.com/patchpile/ and can also provide rolled up patches. > Will the only option be omnijar? > Nope - this patch makes the windows installer code use more of the same code that's used by other platforms for packaging. We'll be able to test these changes independently of omnijar. > SeaMonkey uses optional components and I see that callek and kairo are both > cc'd. Ah ok. I'll test seamonkey then.

I recall that that I tried to factor as much as I could when I revamped the l10n repacks not too long ago. Doing the windows installer differently broke something rather subtle. Not sure if it was mar files, or zips? Something like that.

Comment on attachment 464724 [details] [diff] [review] 9. Make windows installer locale repacks use the standard repacking path, v2 Er, attached the wrong patch.

Fallout from bug 579178. Omnijar code doesn't need the / -> \ fix since zips always use forward slash. Also adds a check and logging so missing manifest problems are easier to debug.

Updated for the changes from bug 579178.

We get to remove a little over half the files we'd normally install.

I'm not very good at attaching patches.

http://hg.mozilla.org/mozilla-central/rev/11a41ea3cb79 Slash fixup fix. (patch 11) http://hg.mozilla.org/mozilla-central/rev/a186d256fbef Pref enum. (patch 10)

This ensures the child process always frees the omnijar data. Tried doing it in XRE_DeinitCommandLine but the leak keeps getting reported, possibly due to the NS_LogTerm.

Added ifdef MOZ_OMNIJAR to a line that needed it. Managed to build seamonkey okay with a port of the firefox packaging changes, though further fixes will be needed to get its optional package installation working properly again.

Couple of quick comments from testing. When repackaging l10n the installer is placed in obj-dir/dist instead of obj-dir/dist/install/sea and the installer name has changed as well (French example) From: firefox-4.0b4pre.fr.win32.installer.exe To: firefox-4.0b4pre.fr.win32.exe

(In reply to comment #78) > http://hg.mozilla.org/mozilla-central/rev/11a41ea3cb79 Slash fixup fix. (patch > 11) > http://hg.mozilla.org/mozilla-central/rev/a186d256fbef Pref enum. (patch 10) These were caught up in a mass backout.

Kev, heads up that this bug will move the distribution directory in the installer self extractive archive from the nonlocalized directory to a directory named core.

Kev ^^

(In reply to comment #82) > Couple of quick comments from testing. When repackaging l10n the installer is > placed in obj-dir/dist instead of obj-dir/dist/install/sea and the installer > name has changed as well (French example) > From: > firefox-4.0b4pre.fr.win32.installer.exe > > To: > firefox-4.0b4pre.fr.win32.exe Hm, odd. Looks like package-name.mk isn't being included properly, though I can't tell how it wouldn't be included.

Relevent output make[2]: Leaving directory `/d/moz/_omnijar_test/ff-opt/browser/installer/windows' make repackage-zip AB_CD=fr MOZ_PKG_FORMAT=SFX7Z ZIP_IN=/d/moz/_omnijar_test/ff-opt/dist/install/sea/firefox-4.0b4pre.en-US.win32.installer.exe ZIP_OUT="/d/moz/_omnijar_test/ff-opt/dist/install/sea/firefox-4.0b4pre.fr.win32.installer.exe" SFX_HEADER="/d/moz/_omnijar_test/ff-opt/browser/locales/../installer/windows/l10ngen/7zSD.sfx /d/moz/_omnijar_test/mozilla/browser/installer/windows/app.tag" make[2]: Entering directory `/d/moz/_omnijar_test/ff-opt/browser/locales' rm -f -r /d/moz/_omnijar_test/ff-opt/dist/l10n-stage/firefox/uninstall; d:/moz/_omnijar_test/ff-opt/config/nsinstall.exe -D /d/moz/_omnijar_test/ff-opt/dist/l10n-stage/firefox/uninstall; cp ../installer/windows/l10ngen/helper.exe /d/moz/_omnijar_test/ff-opt/dist/l10n-stage/firefox/uninstall; rm -f /d/moz/_omnijar_test/ff-opt/dist/l10n-stage/setup.exe; cp ../installer/windows/l10ngen/setup.exe /d/moz/_omnijar_test/ff-opt/dist/l10n-stage; cd ../../dist/xpi-stage/locale-fr && \ tar --exclude=install.rdf --exclude=chrome.manifest -cvhf - * | ( cd /d/moz/_omnijar_test/ff-opt/dist/l10n-stage/firefox && tar -xf - ) mv /d/moz/_omnijar_test/ff-opt/dist/l10n-stage/firefox/chrome/fr.manifest /d/moz/_omnijar_test/ff-opt/dist/l10n-stage/firefox/chrome/localized.manifest d:/moz/_omnijar_test/ff-opt/config/nsinstall.exe -D ../../dist/l10n-stage/ cd ../../dist/l10n-stage; \ (cd firefox && rm -f omni.jar components/binary.manifest && grep -h '^binary-component' components/*.manifest > binary.manifest ; zip -r9m omni.jar chrome chrome.manifest components/*.js components/*.xpt components/*.manifest modules res defaults greprefs.js -x chrome/icons/\* res/cursors/\* res/MainMenu.nib/\* && mv binary.manifest components && echo "manifest components/binary.manifest" > chrome.manifest) && rm -f app.7z && mv firefox core && 7z a -r -t7z app.7z -mx -m0=BCJ2 -m1=LZMA:d24 -m2=LZMA:d19 -m3=LZMA:d19 -mb0:1 -mb0s1:2 -mb0s2:3 && mv core firefox && cat /d/moz/_omnijar_test/ff-opt/browser/locales/../installer/windows/l10ngen/7zSD.sfx /d/moz/_omnijar_test/mozilla/browser/installer/windows/app.tag app.7z > firefox-4.0b4pre.fr.win32.exe && chmod 0755 firefox-4.0b4pre.fr.win32.exe <snip> make[3]: Leaving directory `/d/moz/_omnijar_test/ff-opt/browser/locales' d:/moz/_omnijar_test/ff-opt/config/nsinstall.exe -D ../../dist/ mv -f "../../dist/l10n-stage/firefox-4.0b4pre.fr.win32.exe" "../../dist/firefox-4.0b4pre.fr.win32.exe"

Comment on attachment 465132 [details] [diff] [review] 9. Make windows installer locale repacks use the standard repacking path, v4 >diff --git a/browser/installer/windows/nsis/installer.nsi b/browser/installer/windows/nsis/installer.nsi >--- a/browser/installer/windows/nsis/installer.nsi >+++ b/browser/installer/windows/nsis/installer.nsi >@@ -231,17 +231,17 @@ SectionEnd > Section "-Application" APP_IDX > ${StartUninstallLog} > > SetDetailsPrint both > DetailPrint $(STATUS_INSTALL_APP) > SetDetailsPrint none > > ${LogHeader} "Installing Main Files" >- ${CopyFilesFromDir} "$EXEDIR\nonlocalized" "$INSTDIR" \ >+ ${CopyFilesFromDir} "$EXEDIR\core" "$INSTDIR" \ > "$(ERROR_CREATE_DIRECTORY_PREFIX)" \ > "$(ERROR_CREATE_DIRECTORY_SUFFIX)" > > ; Register DLLs > ; XXXrstrong - AccessibleMarshal.dll can be used by multiple applications but > ; is only registered for the last application installed. When the last > ; application installed is uninstalled AccessibleMarshal.dll will no longer be > ; registered. bug 338878 >@@ -266,21 +266,16 @@ Section "-Application" APP_IDX > ${LogUninstall} "File: \install_status.log" > ${LogUninstall} "File: \install_wizard.log" > ${LogUninstall} "File: \updates.xml" > > SetDetailsPrint both > DetailPrint $(STATUS_INSTALL_LANG) > SetDetailsPrint none > Remove the previous four lines >- ${LogHeader} "Installing Localized Files" >- ${CopyFilesFromDir} "$EXEDIR\localized" "$INSTDIR" \ >- "$(ERROR_CREATE_DIRECTORY_PREFIX)" \ >- "$(ERROR_CREATE_DIRECTORY_SUFFIX)" >- I'm going to go check a few more things but overall it looks good... would like to see the patch that fixes the name of the l10n installer and where it ends up as well.

This does the trick. The repacking code resists moving the file so we just move the file after it's done.

(In reply to comment #89) > Created attachment 465166 [details] [diff] [review] > 9a. Move the repacked installer to the right path > > This does the trick. The repacking code resists moving the file so we just move > the file after it's done. This fails for me. make[3]: Entering directory `/d/moz/_omnijar_test/ff-opt/browser/locales' rm -f /d/moz/_omnijar_test/ff-opt/dist/l10n-stage/firefox/chrome/fr.jar \ /d/moz/_omnijar_test/ff-opt/dist/l10n-stage/firefox/chrome/fr.manifest \ /d/moz/_omnijar_test/ff-opt/dist/l10n-stage/firefox/defaults/pref/fire fox-l10n.js rm -f -rf /d/moz/_omnijar_test/ff-opt/dist/l10n-stage/firefox/searchplugins \ /d/moz/_omnijar_test/ff-opt/dist/l10n-stage/firefox/dictionaries \ /d/moz/_omnijar_test/ff-opt/dist/l10n-stage/firefox/defaults/profile \ /d/moz/_omnijar_test/ff-opt/dist/l10n-stage/firefox/chrome/fr make[3]: Leaving directory `/d/moz/_omnijar_test/ff-opt/browser/locales' d:/moz/_omnijar_test/ff-opt/config/nsinstall.exe -D ../../dist/ mv -f "../../dist/l10n-stage/firefox-4.0b4pre.fr.win32.exe" "../../dist/firefox- 4.0b4pre.fr.win32.exe" make[2]: Leaving directory `/d/moz/_omnijar_test/ff-opt/browser/locales' mv -f "/d/moz/_omnijar_test/ff-opt/dist/firefox-4.0b4pre.fr.win32.zip" ""/d/moz/ _omnijar_test/ff-opt/dist/install/sea/firefox-4.0b4pre.fr.win32.installer.exe"" mv: cannot stat `/d/moz/_omnijar_test/ff-opt/dist/firefox-4.0b4pre.fr.win32.zip' : No such file or directory make[1]: *** [repackage-win32-installer] Error 1 make[1]: Leaving directory `/d/moz/_omnijar_test/ff-opt/browser/locales' make: *** [repackage-win32-installer-fr] Error 2

Comment on attachment 464921 [details] [diff] [review] 3. Add support for packaging omnijar, v4 * Please rename _MAKE_PACKAGE and _UNMAKE_PACKAGE to INNER_MAKE_PACKAGE... leading underscores are hard to read. * please use printf instead of echo and add a trailing newline * is zip -m portable?

Relanded those two patches: http://hg.mozilla.org/mozilla-central/rev/48dfa2f5d62e http://hg.mozilla.org/mozilla-central/rev/3d9f5f454c1a

(In reply to comment #91) > Comment on attachment 464921 [details] [diff] [review] > 3. Add support for packaging omnijar, v4 > > * Please rename _MAKE_PACKAGE and _UNMAKE_PACKAGE to INNER_MAKE_PACKAGE... > leading underscores are hard to read. > Ok. > * please use printf instead of echo and add a trailing newline > Ok. (curious to know the reasoning..) > * is zip -m portable? It works on windows and manpages indicate it exists on OSX. At the very least, the builds work on try.

echo proved itself to be a portability pain, fwiw.

Yeah, echo doesn't behave the same way in various shells, and printf is highly portable.

I have greater confidence in this one. Started the installer to be sure.

Comment on attachment 461065 [details] [diff] [review] 8. Bring universal binaries into the world of omnijar, v2 ># HG changeset patch ># Parent f37600e6f77e1738b9fc15d4655c9a9e80951f4e > >diff --git a/build/macosx/universal/fix-buildconfig b/build/macosx/universal/fix-buildconfig >--- a/build/macosx/universal/fix-buildconfig >+++ b/build/macosx/universal/fix-buildconfig >@@ -42,46 +42,66 @@ use Archive::Zip(':ERROR_CODES'); > > my ($BUILDCONFIG); > >-sub fixBuildconfig($$); >+sub fixBuildconfig($$$); > > $BUILDCONFIG = 'content/global/buildconfig.html'; > >-if (scalar(@ARGV) != 2) { >- print STDERR ("usage: fix-buildconfig <zipfile1> <zipfile2>\n"); >+if (scalar(@ARGV) != 3) { >+ print STDERR ("usage: fix-buildconfig <jar or file mode> <zipfile1> <zipfile2>\n"); I'd write that as "usage: fix-buildconfig <jar|file> <file1> <file2>\n" > exit(1); > } > >-if (!fixBuildconfig($ARGV[0], $ARGV[1])) { >+if (!fixBuildconfig($ARGV[0], $ARGV[1], $ARGV[2])) { > exit(1); > } > > exit(0); > >-sub fixBuildconfig($$) { >- my ($zipPath1, $zipPath2); >- ($zipPath1, $zipPath2) = @_; >+sub fixBuildconfig($$$) { >+ my ($mode, $path1, $path2); >+ ($mode, $path1, $path2) = @_; You should probably just check that $mode == 'jar' or 'file' up top here and return, so you don't have to check it each time below. r=me with those two nits fixed.

All review comments addressed.

wrt comment #85: thanks for the heads-up. ccooper: we'll need to make sure we track for the repack script(s).

Comment on attachment 465366 [details] [diff] [review] 9. Make windows installer locale repacks use the standard repacking path, v5 Did you test all three results, that is, exe, zip, and mar, to yield the right thing, on a virgin build dir? That is, create a new build dir, copy the zip and exe over from the old into dist..., build mar, and then repack. I'm really sorry that I don't remember why I needed this, I only remember that I figured out something broke way late last time around. Also, did you get info from releng how they're calling this code in release factories to get the prettified binary names? Again, sadly I only know it's different, and it was too cumbersome for me to remember. IIRC, I had to have someone from releng jump in and fix my regressions.

(In reply to comment #101) > Comment on attachment 465366 [details] [diff] [review] > 9. Make windows installer locale repacks use the standard repacking path, v5 > > Did you test all three results, that is, exe, zip, and mar, to yield the right > thing, on a virgin build dir? That is, create a new build dir, copy the zip and > exe over from the old into dist..., build mar, and then repack. > I did copy to a new build dir and ensure repacking with a --disable-compile-environment objdir works correctly. Some fixes were applied after that. I didn't test mars though I think robstrong may have. (mostly because I don't know how to test mars) > I'm really sorry that I don't remember why I needed this, I only remember that > I figured out something broke way late last time around. > Understandable. It's pretty complicated. > Also, did you get info from releng how they're calling this code in release > factories to get the prettified binary names? Again, sadly I only know it's > different, and it was too cumbersome for me to remember. IIRC, I had to have > someone from releng jump in and fix my regressions. I saw the code that makes the pretty release build names. As far as I can tell, this correctly accounts for that. The final repacked filenames and locations should be the same as before. This code still uses the logic from package-name.mk to name files.

This is the scariest one line patch I've written all year.

mwu, I haven't checked the installer end result when MOZ_OMNIJAR isn't defined and unless I'm mistaken (though I haven't checked) you haven't made it a requirement when building with the installer. Could you provide details to this affect?

(In reply to comment #104) > mwu, I haven't checked the installer end result when MOZ_OMNIJAR isn't defined > and unless I'm mistaken (though I haven't checked) you haven't made it a > requirement when building with the installer. Could you provide details to this > affect? The omnijar repacking is handled somewhere inside unpack/repackage-zip and is automatically turned off when MOZ_OMNIJAR isn't defined. I've successfully done a regular jar build for seamonkey to test this case. See patch 3 for details. Basically, UNMAKE/MAKE_PACKAGE is modified when MOZ_OMNIJAR is defined to pack up the omnijar before packing everything else up, and to unpack the omnijar after unpacking the main package.

Comment on attachment 465452 [details] [diff] [review] 14. Turn on omnijar via confvars.sh r=me, but please don't land this part until I coordinate some l10n testing

en_US.manifest -> @AB_CD@.manifest and remove all the manifest files that are already specified. Also remove classic.jar. Also adds omni.jar to the list when MOZ_OMNIJAR isn't defined to allow switching back to normal packaging if we ever need it.

Comment on attachment 465366 [details] [diff] [review] 9. Make windows installer locale repacks use the standard repacking path, v5 This looks great, and between :rs and the bug :bs filed I'm confident it'll get enough testing before landing. Also, unifying code paths is awesome!

Comment on attachment 465498 [details] [diff] [review] 12. Add 60+ files to the removed-files.in, v3 diff --git a/browser/installer/removed-files.in b/browser/installer/removed-files.in --- a/browser/installer/removed-files.in >+>+>+ b/browser/installer/removed-files.in @@ -21,7 >+21,7 @@ chrome/classic.manifest chrome/comm.jar chrome/comm.manifest chrome/en-win.jar -chrome/en-US.manifest >+chrome/@AB_CD@.manifest Thanks! chrome/help.jar chrome/installed-chrome.txt chrome/m3ffxtbr.jar @@ -545,6 >+545,171 @@ uninstall/UninstallFirefox.exe uninstall/uninst.exe uninstall/uninstall.exe xpicleanup@BIN_SUFFIX@ >+#ifdef MOZ_OMNIJAR >+chrome/@AB_CD@.jar these should be indented just like the contents of the other ifdefs to make it obvious where they start and end >... >+modules/AddonLogging.jsm >+modules/AddonManager.jsm >+modules/AddonRepository.jsm >+modules/AddonUpdateChecker.jsm >+modules/CertUtils.jsm >+modules/CrashSubmit.jsm >+modules/CSPUtils.jsm >+modules/ctypes.jsm >+modules/debug.js >+modules/distribution.js >+modules/DownloadLastDir.jsm >+modules/DownloadPaths.jsm >+modules/DownloadUtils.jsm >+modules/FileUtils.jsm Looks like Geometry.jsm is missing >+modules/HUDService.jsm >+modules/InlineSpellChecker.jsm >+modules/ISO8601DateUtils.jsm >+modules/LightweightThemeConsumer.jsm >+modules/LightweightThemeManager.jsm >+modules/Microformats.js >+modules/NetUtil.jsm >+modules/NetworkPrioritizer.jsm >+modules/openLocationLastURL.jsm >+modules/PerfMeasurement.jsm >+modules/PlacesDBUtils.jsm >+modules/PlacesUIUtils.jsm >+modules/PlacesUtils.jsm >+modules/PluginProvider.jsm >+modules/PluralForm.jsm >+modules/PopupNotifications.jsm >+modules/Services.jsm >+modules/SpatialNavigation.js >+modules/stylePanel.jsm >+modules/utils.js >+modules/WindowDraggingUtils.jsm Looks like WindowsJumpLists.jsm and WindowsPreviewPerTab.jsm are missing. Should be #ifdef XP_WIN Still need to go through components

Comment on attachment 465498 [details] [diff] [review] 12. Add 60+ files to the removed-files.in, v3 >+components/nsDefaultCLH.js >+components/nsDownloadManagerUI.js >+components/nsFilePicker.js I believe this is XP_UNIX only and should be #ifdef XP_UNIX #ifndef XP_MACOSX components/nsFilePicker.js #endif #endif

All new files will need to be added to removed-files.in unless / until we make the decision that we won't allow updating from omnijar to jar or jar to omnijar builds. I think we should make the decision that we won't support anything besides going from jar to omnijar.

Review comments addressed and build verified working on try server.

Review comments addressed.

(In reply to comment #111) > All new files will need to be added to removed-files.in unless / until we make > the decision that we won't allow updating from omnijar to jar or jar to omnijar > builds. I think we should make the decision that we won't support anything > besides going from jar to omnijar. Once we're omnijar has stuck, I think we can stop caring about adding new files to removed-files.in. (and if there's an emergency need to switch back to jar, we could add the files then..)

Universal binaries patch: (8) http://hg.mozilla.org/mozilla-central/rev/810105bfc749 Fix leak in child process: (13) http://hg.mozilla.org/mozilla-central/rev/1bdb6586b0a0 Add support for packaging omnijar: (3) http://hg.mozilla.org/mozilla-central/rev/d37b6b337227 Make windows installer locale repacks use the standard repacking path: (9) http://hg.mozilla.org/mozilla-central/rev/7cf1fba2029a Add omnijar removed files: (12) http://hg.mozilla.org/mozilla-central/rev/b799524f7810

Opps. Forgot to update the windows locale repacks patch for the changes in the latest omnijar packaging patch.

jst checked in this fix for me. http://hg.mozilla.org/mozilla-central/rev/e63e4c634f75

At this point, we're planning on turning on desktop omnijar by default right after beta4 branches, so that we can get a full two weeks of bake time and test a dry-run localized beta.

(In reply to comment #47) > Created attachment 460956 [details] [diff] [review] > 7. Fix tests, v3 Nice to see the tests actually pass with flat chrome :-)

I believe this broke non-libxul builds on mac. Thoughts?

(In reply to comment #121) > I believe this broke non-libxul builds on mac. Thoughts? Filed bug 588075 for this.

After landing this in trunk, I can't no longer start Firefox.

Peter Henkel, please file a bug, with sufficient information (mozconfig, platform, anything else relevant for your build) to reproduce, and make it block this bug.

Filed bug 588386 on broken l10n repacks on the mac.

Backed out because it blew up update packaging on win32.

That might be the reason why I can't start Firefox at all.

It appears to be just a packaging issue.

/e/builds/moz2_slave/mozilla-central-win32-nightly/build/tools/update-packaging/make_full_update.sh: line 75: e:/builds/moz2_slave/mozilla-central-win32-nightly/build/obj-firefox/dist/host/bin/mar.exe: Bad file number mv: cannot stat `../../dist/firefox.work/output.mar': No such file or directory make: Leaving directory `/e/builds/moz2_slave/mozilla-central-win32-nightly/build/obj-firefox/tools/update-packaging' program finished with exit code 0 khuey, is that from the nightly build log?

Yes.

The updater code expects that dist/firefox contains what we ship. make package does this but make installer does not. The updater code runs right after make installer, so mar generation for windows fails. This patch ensures dist/firefox is left omnijar'd after make installer.

bah... I was able to create a full mar but didn't try to after running make installer. Sorry for not catching that. :(

Comment on attachment 467247 [details] [diff] [review] 9c. Do omnijar packing in dist/firefox instead of installer staging Nice.

cannot start. with new/clean profile. http://forums.mozillazine.org/viewtopic.php?p=9773093#p9773093

mwu, looks like Kyle also backed out the updated removed-files.in patch http://hg.mozilla.org/mozilla-central/rev/5d36870125a7

(In reply to comment #137) > mwu, looks like Kyle also backed out the updated removed-files.in patch > http://hg.mozilla.org/mozilla-central/rev/5d36870125a7 Thanks for the heads up. I've asked cjones to check that patch back in with his push.

seems to be fine with next released Hourly, http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-win32/1282197742/ changeset, http://hg.mozilla.org/mozilla-central/rev/9897ac78e6e3

At least one bustage in the l10n repacks is /tools/buildbot/bin/python2.6: can't open file '../../config/optimizejars.py': [Errno 2] No such file or directory I've also seen mv /builds/slave/mozilla-central-macosx64-l10n-nightly/build/mozilla-central/browser/locales/../../dist/l10n-stage/firefox/Minefield.app/Contents/MacOS/chrome/cy.manifest /builds/slave/mozilla-central-macosx64-l10n-nightly/build/mozilla-central/browser/locales/../../dist/l10n-stage/firefox/Minefield.app/Contents/MacOS/chrome/localized.manifest mv /builds/slave/mozilla-central-macosx64-l10n-nightly/build/mozilla-central/browser/locales/../../dist/l10n-stage/firefox/Minefield.app/Contents/Resources/en.lproj /builds/slave/mozilla-central-macosx64-l10n-nightly/build/mozilla-central/browser/locales/../../dist/l10n-stage/firefox/Minefield.app/Contents/Resources/cy.lproj mv: rename /builds/slave/mozilla-central-macosx64-l10n-nightly/build/mozilla-central/browser/locales/../../dist/l10n-stage/firefox/Minefield.app/Contents/Resources/en.lproj to /builds/slave/mozilla-central-macosx64-l10n-nightly/build/mozilla-central/browser/locales/../../dist/l10n-stage/firefox/Minefield.app/Contents/Resources/cy.lproj: No such file or directory

(In reply to comment #140) > At least one bustage in the l10n repacks is > > /tools/buildbot/bin/python2.6: can't open file '../../config/optimizejars.py': > [Errno 2] No such file or directory That's from bug 559961, actually.

Looks like DownloadTaskbarProgress.jsm needs to be added to removed-files.in

I just noticed that we're leaving directories behind. Does adding directories to removed-files.in remove them? Would be nice to clean those up too.

Comment on attachment 467470 [details] [diff] [review] 12b. Yet another removed-files.in update That should be windows only. Fix that and r=me. Would be nice but app update doesn't support directory removal - bug 386760.

http://hg.mozilla.org/mozilla-central/rev/6eead86e13dd (Leaving RESO->FIXED for mwu; not sure if this is the last.)

Optimistically resolving as fixed. Received my OSX nightly update fine. Also seems that l10n is ok, though other things broke l10n for unrelated reasons. Will fix any other issues in followups.

sorry, if wrong place [STR] create new profile no "chrome" folder (also userChrome-example.css etc.) in new profile caused by omnijar ?

(In reply to comment #146) > http://hg.mozilla.org/mozilla-central/rev/6eead86e13dd > > (Leaving RESO->FIXED for mwu; not sure if this is the last.) Mozilla/5.0 (Windows NT 6.1; rv:2.0b5pre) Gecko/20100822 Minefield/4.0b5pre FWIW on the above nightly the following files are found both inside and outside omni.jar: modules/PropertyPanel.jsm res/fonts/mathfontSymbol.properties

(In reply to comment #149) filed bug 589738

same for this file Warning: Cannot load binary components from the omnijar. Source File: C:\Program Files\Firefox\omni.jar:components/components.manifest Line: 109 the file is outside of the jar

I think this bug here might have broken MOZ_OPTIONAL_PKG_LIST, see Bug 590575 (non-omnijar build). It looks like everything from optional\ is also packaged in the core\ folder.

(In reply to comment #152) > I think this bug here might have broken MOZ_OPTIONAL_PKG_LIST, see Bug 590575 > (non-omnijar build). It looks like everything from optional\ is also packaged > in the core\ folder. If I see this correctly, Attachment 465366 [details] [diff] is to blame for this. Now everything from dist/bin/ just gets copied into the core/ folder: @cp -av $(DIST)/$(STAGEPATH)$(MOZ_PKG_DIR)$(_BINPATH)/. $(DEPTH)/installer-stage/core

FTR, we should have had a separate bug for turning omnijar support on for desktop builds so that it could have more clearly tracked its dependency on bug 586837

I'm not sure what the last comment means. I really don't think another bug would have bought us extra clarity, overall.

OK, then I suppose my issue is that bug 586837 didn't actually block this bug, and should have done, since it's marked that way. If we didn't believe that issue to be serious enough to stop omnijar from being turned on on desktops, then it should have been filed as dependent on this one, not blocking.

For lack of better "related to" relationships in bugzilla, I don't think nitpicking the depends-on relationships is helpful. We had hoped to get it done first, but I decided that it was more important to get it landed early in the b5 cycle than to wait on dry-run testing.

(In reply to comment #155) > I really don't think another bug would have bought us extra clarity, overall. Landing multiple independent patches days apart (including across a beta freeze) and trying to track them all in one bug is painful and leads to all sorts of coordination issues. This work should have been split across more than one bug.

Moving all file and directories from defaults/pref to omni.jar has made it impossible or unclear on how to specify and use mozilla.cfg to configure and lock prefs. Specifying a mozilla.cfg via a pref in a file in omni.jar gives "Failed to read the configuration file. Please contact your system administrator." I've created bug 595522 about that problem. Can someone look at it and make changes if needed to make sure that this gets attention in one of the coming beta releases. This needs to be fixed in a beta so mozilla.cfg will work in the release candidate and the final release. Works: 2010-08-17 : SourceStamp=116f2046b9ef Don't work: 2010-08-18 : SourceStamp=f4c97baa0e51 http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=116f2046b9ef&tochange=f4c97baa0e51