Update packaging fails in comm-central when using official branding.

RESOLVED FIXED in Thunderbird 3.0b1

Status

MailNews Core
Build Config
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

Trunk
Thunderbird 3.0b1
All
Mac OS X

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

9 years ago
Created attachment 341106 [details] [diff] [review]
The fix

gozer tried a dry-run of Thunderbird with official branding, when doing the update packaging he got the following failure:

bm-xserve07:/builds/releases/3.0b1/1/objdir/ppc/mozilla/tools/update-packaging cltbld$ make
mkdir -p ../../dist/update/
MAR=../../dist/host/bin/mar \
  /builds/releases/3.0b1/1/comm-central/mozilla/tools/update-packaging/make_full_update.sh \
  "../../dist/update//thunderbird-3.0b1.en-US.mac.complete.mar" \
  "../../dist/universal/thunderbird/Shredder.app"
/builds/releases/3.0b1/1/comm-central/mozilla/tools/update-packaging/make_full_update.sh: line 42: pushd: ../../dist/universal/thunderbird/Shredder.app: No such file or directory
make: *** [complete-patch] Error 1

It turns out this is due to our c-c/m-c interaction when handling branding.

When we go to the mozilla-central configure, we currently pass --disable-offical-branding.

This means the m-c configure script attempts to its own default of $(MOZ_BUILD_APP)/branding/nightly. This is fine for unofficial builds.

We can't pass --enable-official-branding, because m-c configure doesn't know about us and will bail out. So the other option is to always pass the branding directory we have calculated with the --with-branding option.

We'll still need the --disable-official-branding option though to stop the m-c configure script bailing out.

This shouldn't affect SeaMonkey, because the m-c configure script will just detect the directory doesn't exist, and reverts to the defaults it uses now.
Attachment #341106 - Flags: review?(kairo)

Comment 1

9 years ago
Comment on attachment 341106 [details] [diff] [review]
The fix

looks good to me!
Attachment #341106 - Flags: review?(kairo) → review+
(Assignee)

Comment 2

9 years ago
Given a3 will now be on Shredder branding, I'll push this out and check it in once the tree reopens.
Target Milestone: Thunderbird 3.0a3 → Thunderbird 3.0b1
The MOZ_BRANDING_DIRECTORY variable can be empty so I think you probably want to use the REAL_BRANDING_DIRECTORY variable instead.
(Assignee)

Comment 4

9 years ago
(In reply to comment #3)
> The MOZ_BRANDING_DIRECTORY variable can be empty so I think you probably want
> to use the REAL_BRANDING_DIRECTORY variable instead.

It doesn't actually matter. If what we supply doesn't work, mozilla-central will fall back to using a pre-defined directory - and due to the way the system is set up - that fall back will work.
(In reply to comment #4)
> If what we supply doesn't work, mozilla-central
> will fall back to using a pre-defined directory - and due to the way the system
> is set up - that fall back will work.

I don't see the code doing this fallback in the configure.in of mozilla-central, but if you have tested this and it works for you, it doesn't matter.

In my case (Instantbird uses a build system which is basically a copy of the comm-central one), when launching the configure script of mozilla-central with a bogus branding directory, the update packaging failed because the MOZ_APP_DISPLAYNAME variable was empty (the configure.sh script of the branding directory wasn't called by configure).
(Assignee)

Comment 6

9 years ago
(In reply to comment #5)
> In my case (Instantbird uses a build system which is basically a copy of the
> comm-central one), when launching the configure script of mozilla-central with
> a bogus branding directory, the update packaging failed because the
> MOZ_APP_DISPLAYNAME variable was empty (the configure.sh script of the branding
> directory wasn't called by configure).

Have you got a configure.sh in <your-dir>/branding/nightly? That's what m-c would fall back to.
(Assignee)

Comment 7

9 years ago
Patch checked in, changeset id 540:02e0f61d2431
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
This has unexpected (and undesired) side-effects on OSX Universal.

MOZ_APP_DISPLAYNAME gets lost from the mozilla/ point of view.

before the change:
$ grep MOZ_APP_DISPLAY objdir-tb/ppc/mozilla/* objdir-tb/ppc/*
objdir-tb/ppc/mozilla/config.status:s%@MOZ_APP_DISPLAYNAME@%Shredder%g
objdir-tb/ppc/config.status:s%@MOZ_APP_DISPLAYNAME@%Shredder%g

after the change:
$ grep MOZ_APP_DISPLAY objdir-tb/ppc/mozilla/* objdir-tb/ppc/*
objdir-tb/ppc/mozilla/config.status:s%@MOZ_APP_DISPLAYNAME@%%g
objdir-tb/ppc/config.status:s%@MOZ_APP_DISPLAYNAME@%Shredder%g

I suspect it's caused by the '..' not dealing well with the fact that with universal builds, you end up with objdir/{ppc,i386}/mozilla
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Would also be nice to get this resolved (or backed out) before tonight nightlies kisk in (0400 PDT)
(Assignee)

Comment 10

9 years ago
(In reply to comment #9)
> Would also be nice to get this resolved (or backed out) before tonight
> nightlies kisk in (0400 PDT)

I'll be around from about 0001 PDT (i.e. tinderbox time), if the fix isn't obvious I'll back out before we get to nightly build time.
The issue in comment 8 is more or less what I described in comment 5. Replacing MOZ_BRANDING_DIRECTORY by REAL_BRANDING_DIRECTORY fixed it for me.
(Assignee)

Comment 12

9 years ago
(In reply to comment #11)
> The issue in comment 8 is more or less what I described in comment 5. Replacing
> MOZ_BRANDING_DIRECTORY by REAL_BRANDING_DIRECTORY fixed it for me.

I've checked in a fix with this change. From a couple of tests I did it looks like it should work. I'll leave this open until the nightlies have spun.
Closing as it looks like the nightlies have been green for a while, plus, we successfully released Aplha 3 with this.
Status: REOPENED → RESOLVED
Last Resolved: 9 years ago9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.