Closed Bug 540833 Opened 15 years ago Closed 14 years ago

make update-packaging failed finding correct directory due to MOZ_PKG_PRETTYNAMES

Categories

(Firefox Build System :: General, defect)

1.9.1 Branch
All
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: standard8, Unassigned)

References

Details

(Whiteboard: [tbtrunkneeded])

Attachments

(1 file)

We've just tried getting build automation to do a build of Lanikai Alpha 1. Windows and Linux worked fine, Mac did not. To do this, we're setting: MOZ_APP_NAME=thunderbird MOZ_APP_DISPLAYNAME=Lanikai When the build automation runs: make -C objdir-tb/ppc/mozilla/tools/update-packaging we're then getting: mkdir -p ../../dist/update/mac/en-US/ MAR=/builds/slave/macosx_build/build/objdir-tb/ppc/mozilla/dist/host/bin/mar \ /builds/slave/macosx_build/build/mozilla/tools/update-packaging/make_full_update.sh \ "../../dist/update/mac/en-US//lanikai-3.1a1.complete.mar" \ "../../dist/universal/Lanikai/Lanikai.app" /builds/slave/macosx_build/build/mozilla/tools/update-packaging/make_full_update.sh: line 42: pushd: ../../dist/universal/Lanikai/Lanikai.app: No such file or directory make: *** [complete-patch] Error 1 That step is setting MOZ_PKG_PRETTYNAMES=1. I can reproduce this on my self build with MOZ_PKG_PRETTYNAMES=1. If I don't set MOZ_PKG_PRETTYNAMES then the build works fine but gives us thunderbird-3.1a1.en-US.mac.complete.mar rather than lanikai-3.1a1.en-US.mac.complete.mar.
From what I've worked out so far, the error is either: http://hg.mozilla.org/mozilla-central/annotate/b91227360383/toolkit/mozapps/installer/package-name.mk#l120 ifndef MOZ_PKG_APPNAME MOZ_PKG_APPNAME = $(MOZ_APP_DISPLAYNAME) endif or in one of the lines starting at http://hg.mozilla.org/mozilla-central/annotate/b91227360383/tools/update-packaging/Makefile.in#l57 One of those options is causing the directory naming to be different: ../../dist/universal/Lanikai/Lanikai.app versus ../../dist/universal/thunderbird/Lanikai.app I'm not convinced which one is right or expected.
The current thinking is that this is due to brandName being set to "Thunderbird" in release_config.py. Why it only affects Mac though.....
(In reply to comment #2) > The current thinking is that this is due to brandName being set to > "Thunderbird" in release_config.py. Why it only affects Mac though..... The reason sounds plausible, and why it affects only Mac is probably to be found somewhere in factory.py, where we probably are using the brandName to point to the right path, which is probably even the right thing to do when brandName is set correctly.
Given the hand-hack that we had to do here, do we still think this is a bug in core? gozer, are you the right person to own this bug?
(In reply to comment #3) > (In reply to comment #2) > > The current thinking is that this is due to brandName being set to > > "Thunderbird" in release_config.py. Why it only affects Mac though..... I tried a new build with Lanakai in brandName of release_config, and it failed in exactly the same way, so that's not it. > The reason sounds plausible, and why it affects only Mac is probably to be > found somewhere in factory.py, where we probably are using the brandName to > point to the right path, which is probably even the right thing to do when > brandName is set correctly. Actually, it looks like it's something that's broken in comm-central's build system, and that release automation/buildbot is not to blame.
Attached patch l10n repack mac bustage fix for branded builds (deleted) — — Splinter Review
When running the l10n repacks on Mac OS X for a branded build (Lanikai), I've run into bustage, attached patch fixes it, but not 100% certain it's a safe patch
Attachment #424791 - Flags: review?(bugzilla)
This patch sounds strange, you shouldn't need to touch mozilla/ in any way, IIRC, things work fine for Firefox branded as Namoroka, etc.
(In reply to comment #7) > This patch sounds strange, you shouldn't need to touch mozilla/ in any way, > IIRC, things work fine for Firefox branded as Namoroka, etc. I'm certainly suspicious of us needing to do patches like that, I just haven't had time to examine in detail. One thing to note is that AFAIK Namoroka didn't do l10n builds of its alpha releases so there may be issues there that weren't picked up.
The analysis Ben did in bug 544928 could be helpful in actually solving this.
Just to keep everything in one place, here's my comment from bug 544928: This used to work fine but broke sometime between the 3.6a1 and the 3.7a1 builds. MOZ_PKG_DIR is set to MOZ_APP_DISPLAYNAME for release builds (whereas it's normally set to MOZ_APP_NAME). For alphas, MOZ_APP_DISPLAYNAME gets changed to a code name. When the update-packaging target tried to run for 3.7a1, we had this error: mkdir -p ../../dist/update/mac/en-US/ MAR=/builds/slave/macosx_build/build/obj-firefox/ppc/dist/host/bin/mar \ /builds/slave/macosx_build/build/tools/update-packaging/make_full_update.sh \ "../../dist/update/mac/en-US//mozilladeveloperpreview-3.7a1.complete.mar" \ "../../dist/universal/MozillaDeveloperPreview/MozillaDeveloperPreview.app" /builds/slave/macosx_build/build/tools/update-packaging/make_full_update.sh: line 42: pushd: ../../dist/universal/MozillaDeveloperPreview/MozillaDeveloperPreview.app: No such file or directory make: *** [complete-patch] Error 1 *But*, in dist/universal/firefox we had a MozillaDeveloperPreview.app. So it seems like at some point 'make package' started putting the package in a dir based on MOZ_APP_NAME rather than MOZ_APP_DISPLAYNAME. The workaround used was to switch MOZ_APP_DISPLAYNAME with MOZ_PKG_DIR in tools/update-packaging/Makefile.in
(In reply to comment #8) > (In reply to comment #7) > > This patch sounds strange, you shouldn't need to touch mozilla/ in any way, > > IIRC, things work fine for Firefox branded as Namoroka, etc. > > I'm certainly suspicious of us needing to do patches like that, I just haven't > had time to examine in detail. One thing to note is that AFAIK Namoroka didn't > do l10n builds of its alpha releases so there may be issues there that weren't > picked up. This is correct, but l10n is a red herring. The specific problem occurs when MOZ_APP_NAME and MOZ_APP_DISPLAYNAME differ, and you attempt to create a MAR on Mac.
I'm going to bisect and figure out when this broke.
Hmm, was the patch for http://hg.mozilla.org/mozilla-central/rev/42d7cb939283 somehow wrong or incomplete? I just went through all patches since 3.6a1 that touched package-name.mk and that one is the only one that looked suspicious.
(In reply to comment #14) > Hmm, was the patch for http://hg.mozilla.org/mozilla-central/rev/42d7cb939283 > somehow wrong or incomplete? > I just went through all patches since 3.6a1 that touched package-name.mk and > that one is the only one that looked suspicious. That seems more likely than any of the other patches I've seen....I'll look around that changeset first.
hg bisect points to http://hg.mozilla.org/mozilla-central/rev/b375e5cab4dc -- which is the changeset in which MOZ_PKG_PRETTYNAMES landed. So, this has been broken for a long time. Ted suggested that getting flight.mk to pull in vars from package-names.mk instead of defining its own would be a good fix here. I'll give that a try.
Before I got a chance to really dive into this I was told that Alpha -> Alpha partials for Firefox have been deprioritized. I won't have a chance to dig into this anytime soon.
No longer blocks: 543794
Adding [tb31needs], because this is still going to be a problem for Tb 3.1b2 if we ship it as Lanikai. Ben, can I talk you into spending 5-10 mins to figure out how much work it's likely to be to do the right thing here?
Whiteboard: [tb31needs]
standard8: ping on this review? We think we'll need this for the upcoming FF3.7a3 in order to do alpha->alpha updates.
Needed before we can do alpha->alpha updates for Mac. (Does not block alpha->alpha updates for win32, linux).
blocking2.0: --- → ?
Does this block manual alpha->alpha updates or only automated?
(In reply to comment #22) > Does this block manual alpha->alpha updates or only automated? It blocks manual ones, too. We can't generate *any* MARs without this fixed
Any MARs on Mac, I mean.
Who actually needs to review, here? standard8? Is that the same person as in the review request? Just want to make sure the right person(s) are seeing the request for review.
Comment on attachment 424791 [details] [diff] [review] l10n repack mac bustage fix for branded builds setting nick as a reviewer at his request
Attachment #424791 - Flags: review?(nrthomas)
So I'm probably not the right person to review. However, some comments: 1) no-one seems to know the original intention of how these were meant to be packaged i.e. which directories to be created. 2) I'm not convinced the patch is complete (especially for Firefox)/will work. 3) Ben's workaround in comment 11 may be better. Unfortunately I've not got time to work on this at the moment, so it is probably better that somemone else looks at it.
Attachment #424791 - Flags: review?(bugzilla)
Comment on attachment 424791 [details] [diff] [review] l10n repack mac bustage fix for branded builds Hrm, currently MOZ_PKG_DIR = $(MOZ_APP_NAME) not $(MOZ_PKG_APPNAME)... is that difference consequential?
AIUI (and this is all for en-US), we call unify at the end of 'make build', and that puts the the universal app in $(DIST_UNI)/$(MOZ_PKG_APPNAME)/$(APPNAME) via flight.mk line 112. Neglecting the DIST_UNI part, that expands to $(MOZ_APP_NAME)/$(MOZ_APP_DISPLAYNAME).app That's using the definitions earlier in flight.mk, and I've added spaces to make it clear what comes each var. For nightly branding we set MOZ_APP_NAME = firefox MOZ_APP_DISPLAYNAME = Minefield in config/autoconf.mk, while for 3.7a2 we had MOZ_APP_NAME = firefox MOZ_APP_DISPLAYNAME = MozillaDeveloperPreview When we come to do 'make package' we end up at http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/packager.mk#172 and use $(PKG_DMG_SOURCE) as the location of the files for the dmg. A couple of lines earlier that's defined as $(STAGEPATH)$(MOZ_PKG_DIR), and that expands to universal/$(MOZ_APP_NAME) via package-name.mk. So that lines up, although not without adding some vars to make it less obvious what's going on. When creating the complete mar we expect the source files in PACKAGE_DIR = $(PACKAGE_BASE_DIR)/universal/$(MOZ_PKG_APPNAME)/$(MOZ_APP_DISPLAYNAME).app Here MOZ_PKG_APPNAME comes from package-name.mk and is MOZ_APP_NAME for nightlies and MOZ_APP_DISPLAYNAME for releases (where MOZ_PKG_PRETTYNAMES is set so that the binaries have the right name straight away). So using MOZ_PKG_DIR for update-packaging is correct. In a perfect world flight.mk would use the same variables too.
blocking2.0: ? → ---
blocking2.0: --- → ?
Attachment #424791 - Flags: review?(nrthomas) → review+
Comment on attachment 424791 [details] [diff] [review] l10n repack mac bustage fix for branded builds Checked in the m-c part to fix en-US builds: http://hg.mozilla.org/mozilla-central/rev/da1d4c2324a5 Locales are still outstanding.
Whiteboard: [tb31needs] → [tb32needs]
This bug has a blocking nomination but no owner, and I'm not sure what remains. Can we either close it or assign it to the person who cares?
(In reply to comment #30) > Comment on attachment 424791 [details] [diff] [review] > l10n repack mac bustage fix for branded builds > > Checked in the m-c part to fix en-US builds: > http://hg.mozilla.org/mozilla-central/rev/da1d4c2324a5 > > Locales are still outstanding. Do you recall what needs to be done, Nick?
Anything I knew in comment #29 has evaporated, but perhaps http://mxr.mozilla.org/mozilla-central/source/toolkit/locales/l10n.mk#147 just works because it calls tools/update-packaging/Makefile.in, which we fixed already. In Firefox nightlies we end up calling MAR=/builds/slave/mozilla-central-macosx-l10n-nightly/build/mozilla-central/dist/host/bin/mar \ ./make_full_update.sh \ "/builds/slave/mozilla-central-macosx-l10n-nightly/build/mozilla-central/browser/locales/../../dist/update//firefox-4.0b3pre.ku.mac.complete.mar" \ "/builds/slave/mozilla-central-macosx-l10n-nightly/build/mozilla-central/browser/locales/../../dist/l10n-stage/firefox/Minefield.app" vs this in a 4.0b2 release locale: MAR=/builds/moz2_slave/macosx_repack/build/mozilla-1.9.2/dist/host/bin/mar \ ./make_full_update.sh \ "/builds/moz2_slave/macosx_repack/build/mozilla-1.9.2/browser/locales/../../dist/update/mac/or//firefox-3.6.8.complete.mar" \ "/builds/moz2_slave/macosx_repack/build/mozilla-1.9.2/browser/locales/../../dist/l10n-stage/Firefox/Firefox.app" Thunderbird dudes, have you done a localized unofficial branding build since 2010-03-11 ?
(In reply to comment #33) > Thunderbird dudes, have you done a localized unofficial branding build since > 2010-03-11 ? Nope, and I believe our next alphas will be non-localized as well. I guess this no-longer blocks 2.0. We could possibly close it for now due to "not needed" and file a new one if the need for localized unofficial branding comes up again.
Marking this fixed, open a new bug if necessary.
Status: NEW → RESOLVED
blocking2.0: ? → ---
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [tb32needs] → [tbtrunkneeded]
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: