The l10n.mk that I used successfully on mozilla-central for fennec should be good to use for Firefox, too. We might want to land the status quo on the 1.9.1 branch before that, though, if Fennec decides to build off of 1.9.1 in the short term.

We're planning on shipping Fennec off of 1.9.1 so this will be needed for Fennec 1.0 l10n.

Taking. I kept myself from fixing the pattern rules in Makefile.in to the firstword usecases in fennec's one, for the most part. That makes the patch a lot easier to read I hope. I might come up with a follow up to fix that later.

run_for_effects_too is the part of run_for_effects that is not in l10n.mk already, aka the branding part. The defines get changed to just amend those in l10n.mk. I couldn't resist the one edit of using firstword for firefox-l10n.js already. The rest is just removing rules that are literally the same in l10n.mk, and using l10n.mk. Tested on a french mac build. The windows installer files are not touched at all, so all should be good.

Comment on attachment 377154 [details] [diff] [review] Use l10n.mk, drop duplicate rules run_for_effects_too := if ! test -d $(DIST)/branding; then $(NSINSTALL) -D $(DIST)/branding; fi) It'd be nice if we could make these explicit dependencies instead of evaluating for side effects, something like: http://mxr.mozilla.org/mozilla-central/source/config/rules.mk#1571 +PREF_JS_EXPORTS = $(firstword $(wildcard $(LOCALE_SRCDIR)/firefox-l10n.js) \ + @srcdir@/en-US/firefox-l10n.js ) I'd rather you used $(srcdir) instead of @srcdir@, since the former is actual make syntax.

Fixed on mozilla-central, http://hg.mozilla.org/mozilla-central/rev/d2a23f2c0020. We need this for fennec l10n, but I'll need a branch patch including l10n.mk for that. I'll flag the 1.9.1 flags once I have the patch.

Doh, I did something so stupid, it doesn't even break anything. Ran across that while trying to do the 1.9.1 patch. run_for_effects_too := if ! test -d $(DIST)/branding; then $(NSINSTALL) -D $(DIST)/branding; fi) doesn't contain an initial $(shell So, if I have to fix that again anyway, let's do it right. Patch coming up.

Follow up patch, turning the $(shell) side effect thingie into a dependency. I'm not doing this for the one in l10n.mk slightly on purpose, I need ABSDIST to be right early on, not sure if there's a good way around that. Tested on a mac without the deps and the run_for_effects, which breaks on unpack, fixed by adding just the deps and unpacked and created an installer successfully.

PS: Note to self and the approval committee, I'll be taking the $(CURDIR) piece of bug 483856 when doing the 1.9.1 patch, too, so the only difference between trunk and branch will be core_abspath = $(if $(findstring :,$(1)),$(1),$(if $(filter /%,$(1)),$(1),$(PWD)/$(1))) which moved to config.mk in that patch. :bs said that he's not driving that over to 1.9.1, so I'm leaving core_abspath as it is.

Comment on attachment 383452 [details] [diff] [review] Use dependency for $(DIST)/branding instead of run_for_effects Mac l10n nightly repacks are currently broken on m-c until this patch goes in.

Comment on attachment 383452 [details] [diff] [review] Use dependency for $(DIST)/branding instead of run_for_effects +$(DIST)/branding: + $(NSINSTALL) -D $(DIST)/branding Might just use $@ here to keep from duplicating.

http://hg.mozilla.org/mozilla-central/rev/a437dbe2197e, FIXED. Still need to write a patch for 1.9.1.

This updates browser/locales/Makefile.in mostly to the status of mozilla-central. It includes parts of bug 483856 and all of bug 496196. The only remaining diff is the one mentioned in comment 8, --- ../../mozilla-central/browser/locales/Makefile.in 2009-07-02 10:11:00.000000000 +0200 +++ browser/locales/Makefile.in 2009-07-06 13:14:38.000000000 +0200 @@ -77,6 +77,7 @@ APP_VERSION := $(shell cat $(srcdir)/../config/version.txt) PWD := $(CURDIR) +core_abspath = $(if $(findstring :,$(1)),$(1),$(if $(filter /%,$(1)),$(1),$(PWD)/$(1))) # These are defaulted to be compatible with the files the wget-en-US target # pulls. You may override them if you provide your own files. You _must_ Carrying over reviews.

Axel: Is this only a 1.9.2 thing now or do we still need it on 1.9.1?

Axel: dunno if you saw Sam's question there, but it's a large patch so we're wondering if it's needed on mozilla-1.9.1 or if it can wait for the mozilla-1.9.2 branch.

Yes, we should take this on 1.9.1. This isn't such a big deal in terms of patches, but it makes porting of build fixes from trunk to 1.9.1 easier and it's going to make it easier to port build changes for the stable branch onto 1.9.2 once 3.6 ships.

Comment on attachment 386972 [details] [diff] [review] update browser/locales/Makefile.in on 1.9.1, add l10n.mk Approved for 1.9.1.2. a=ss for release-drivers Please land on mozilla-1.9.1 and use the ".2-fixed" option of the "status1.9.1" flag.

Landed, http://hg.mozilla.org/releases/mozilla-1.9.1/rev/562e14bff99f. Gonna do the .2-fixed status as soon as the boxens cycle and some l10n builds came through.

Fixed .2. Unrequesting fennec, as I think that's not coming from 1.9.1 anymore.

Comment on attachment 386972 [details] [diff] [review] update browser/locales/Makefile.in on 1.9.1, add l10n.mk >diff --git a/toolkit/locales/l10n.mk b/toolkit/locales/l10n.mk >+ifdef MOZ_MAKE_COMPLETE_MAR >+ $(MAKE) -C $(DEPTH)/tools/update-packaging full-update AB_CD=$(AB_CD) \ >+ MOZ_PKG_PRETTYNAMES=$(MOZ_PKG_PRETTYNAMES) \ >+ PACKAGE_BASE_DIR="$(_ABS_DIST)/l10n-stage" \ >+ DIST="$(_ABS_DIST)" >+ $(MAKE) generate-snippet-$(AB_CD) >+endif Adding the call to generate-snippet target without actually landing tools/update-packaging/generatesnippet.py on the branch broke 3.5.1 release builds. Congratulations.

3.5.2 even.

Raising priority because this has broken FF3.5.2 production. nthomas is trying a workaround for now, but this should be fixed asap.

Ouch, sorry. I lost track of what landed and what did not. Guess the safest way to cut this is to comment out the generate-snippet call until we actually do something on the branch. Nick?

Comment on attachment 391542 [details] [diff] [review] not call generate-snippets until we actually land that On GECKO1912_20090729_RELBRANCH: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/d57f05bcdbe1 On default: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/a83d5a98cdc6

Comment on attachment 386972 [details] [diff] [review] update browser/locales/Makefile.in on 1.9.1, add l10n.mk This has been backed out of 1.9.1 because it breaks Mac DMG packaging.

Comment on attachment 391542 [details] [diff] [review] not call generate-snippets until we actually land that This has been backed out of 1.9.1 because it breaks Mac DMG packaging.

Such a looser, apparently the variable defines for the mac image stuff need to be before packager.mk. That's coming in through l10n.mk, so in front of that.

Turns out this broke win32 repacks for 3.5.2 too - the win32 MARs got the en-US helper.exe instead of the localized one. The installers were OK.

This is not fixed .2, nor FIXED on trunk. Reopening. Putting back on the triage for 1.9.1, I think we should start with a backout there. So far we only backed it out on the relbranch. I have a grip on the mac branding stuff by now, but not on the uninstaller.

Comment on attachment 391610 [details] [diff] [review] put the branding vars before l10n.mk and thus packager.mk Found the helper.exe issue, I think. Working on a new patch.

This should do it. I moved the vars for the mac branding up, and added a UNINSTALLER_PACKAGE_HOOK, which I apparently added to l10n.mk just to have that helper.exe logic. I'm not able to actually compile on windows, so I haven't been able to test the MAR code path. I expect this to be fine, but I could use some help in testing.

(In reply to comment #30) > I'm not able to actually compile on windows, so I haven't been able to test the > MAR code path. I expect this to be fine, but I could use some help in testing. Can you verify using TryServer?

Nope, tryserver doesn't do l10n repacks, and not mars either.

We need to get this fixed on 1.9.1 one way or another (backing out is acceptable) before code freeze on August 11. Who's on point to get this fixed? Axel?

Backed out on 1.9.1, http://hg.mozilla.org/releases/mozilla-1.9.1/rev/e03876b4e587.

Comment on attachment 391859 [details] [diff] [review] move variables for mac up, add UNINSTALLER_PACKAGE_HOOK +ifneq (,$(filter mac cocoa,$(MOZ_WIDGET_TOOLKIT))) While you're here, just make this ifeq (cocoa,$(MOZ_WIDGET_TOOLKIT)).

Comment on attachment 386972 [details] [diff] [review] update browser/locales/Makefile.in on 1.9.1, add l10n.mk Clearing this approval flag since the patch was backed out.

This bug is no longer blocking 1.9.1.3 since the patch has been backed out. This bug is now "wanted" on 1.9.1, meaning we'll consider a well-tested patch. Can we get tests for any such patch so we don't break packaging in the future?

http://hg.mozilla.org/mozilla-central/rev/20fe6fe0d592, marking FIXED again. Now we need to get the bustage fixes on to 1.9.2.

(In reply to comment #37) > Can we get tests for any such patch so we don't break packaging in the future? Axel: Not sure how easy tests would be for this patch. However, if you tell us when this patch is landed on mozilla1.9.1, and again when landed on mozilla-1.9.2, we can do a fake release in staging for both branches to verify all is ok.

beltzner/ss: gentle ping about approval192?

Comment on attachment 391859 [details] [diff] [review] move variables for mac up, add UNINSTALLER_PACKAGE_HOOK a192=beltzner

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/a6db2a107e5c, landed on 1.9.2.

Per discussion with Axel in irc, this needs to also be landed in mozilla-1.9.1. Axel, when you have reviewed patch approved and landed, please let us know.

bug 489313 will test that the patch does not affect releases. Once tested it we can land it on 1.9.1