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)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: standard8, Unassigned)
References
Details
(Whiteboard: [tbtrunkneeded])
Attachments
(1 file)
(deleted),
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•15 years ago
|
||
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.
Reporter | ||
Comment 2•15 years ago
|
||
The current thinking is that this is due to brandName being set to "Thunderbird" in release_config.py. Why it only affects Mac though.....
Comment 3•15 years ago
|
||
(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.
Comment 4•15 years ago
|
||
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?
Comment 5•15 years ago
|
||
(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.
Comment 6•15 years ago
|
||
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)
Comment 7•15 years ago
|
||
This patch sounds strange, you shouldn't need to touch mozilla/ in any way, IIRC, things work fine for Firefox branded as Namoroka, etc.
Reporter | ||
Comment 8•15 years ago
|
||
(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.
Comment 10•15 years ago
|
||
The analysis Ben did in bug 544928 could be helpful in actually solving this.
Comment 11•15 years ago
|
||
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
Comment 12•15 years ago
|
||
(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.
Comment 13•15 years ago
|
||
I'm going to bisect and figure out when this broke.
Comment 14•15 years ago
|
||
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.
Comment 15•15 years ago
|
||
(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.
Comment 16•15 years ago
|
||
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.
Comment 17•15 years ago
|
||
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.
Comment 18•15 years ago
|
||
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]
Comment 20•15 years ago
|
||
standard8: ping on this review? We think we'll need this for the upcoming FF3.7a3 in order to do alpha->alpha updates.
Comment 21•15 years ago
|
||
Needed before we can do alpha->alpha updates for Mac. (Does not block alpha->alpha updates for win32, linux).
blocking2.0: --- → ?
Comment 22•15 years ago
|
||
Does this block manual alpha->alpha updates or only automated?
Comment 23•15 years ago
|
||
(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
Comment 24•15 years ago
|
||
Any MARs on Mac, I mean.
Comment 25•15 years ago
|
||
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 26•15 years ago
|
||
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)
Reporter | ||
Comment 27•15 years ago
|
||
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.
Reporter | ||
Updated•15 years ago
|
Attachment #424791 -
Flags: review?(bugzilla)
Comment 28•15 years ago
|
||
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?
Comment 29•15 years ago
|
||
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: ? → ---
Updated•15 years ago
|
blocking2.0: --- → ?
Updated•15 years ago
|
Attachment #424791 -
Flags: review?(nrthomas) → review+
Comment 30•15 years ago
|
||
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.
Reporter | ||
Updated•15 years ago
|
Whiteboard: [tb31needs] → [tb32needs]
Comment 31•14 years ago
|
||
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?
Comment 32•14 years ago
|
||
(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?
Comment 33•14 years ago
|
||
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 ?
Reporter | ||
Comment 34•14 years ago
|
||
(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.
Comment 35•14 years ago
|
||
Marking this fixed, open a new bug if necessary.
Status: NEW → RESOLVED
blocking2.0: ? → ---
Closed: 14 years ago
Resolution: --- → FIXED
Reporter | ||
Updated•14 years ago
|
Whiteboard: [tb32needs] → [tbtrunkneeded]
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•