193164 - macho lib*.dylib components copying should be enhanced

Status: RESOLVED FIXED

(SeaMonkey :: Build Config, defect, P3)

Description

[Kai Engert (:KaiE:)]

1) do a full build with the unix build system on Mac OS X
2) FYI only: running from dist/bin/mozilla does not work
3) confirm mozilla runs by starting it from dist/Mozilla.app/Contents/MacOS/mozilla
4) change c++ files in one component only, e.g. security/manager/ssl/src
5) execute make in security/manager/ssl
6) find out that a new lib*.dylib file was created below security/manager/ssl/src

Expected behaviour (as seen on Windows and Linux):
The dist directory should contain a fully up to date runtime environment that
can be used to start Mozilla directly

Actual behaviour:
dist/Mozilla.app/Contents/MacOS/mozilla does not contain the new lib
In addition to building in the component directory, it is necessary to build in
xpfe/bootstrap

Possible fix (suggested by dbaron):
make install targets in component directories should copy the lib*.dylib file to
the Mozilla.app directory, too

Comment 1

[Conrad Carlen (not reading bugmail)]

The make in xpfe/bootstrap uses rsync to update
Mozilla[Debug].app/Contents/MacOS - so at least it happens pretty quickly. It
used to use cp which was painful. Since that change was made, doing a make in
xpfe/bootstrap hasn't seemed so bad to me. I just keep a terminal open which is
in xpfe/bootstrap or a ProjectBuilder project for debugging which is set to make
in xpfe/bootstrap as the build command.

But, it still could be improved :-) 

Comment 2

[hacker formerly known as [e-mail]]

Hrrmmm.  The expected behavior is incorrect because the behavior of the platform
is different than the other platforms listed as examples.  The only time you're
guaranteed a build that's runnable from dist/ is when you build from the top. 
Building in select directories works in most cases but it's not guaranteed to
always work.

To fulfull the premise that building in any directory should allow you to use
those changes in a previously built .app, you'd have to change more than just
the .dylib targets.  Every common rule in rules.mk would need the appropriate
rsync / cp call.  This would include the chrome rules.  Due to the nature of the
chrome tree, you'd have to rsync the entire bin/chrome tree every time you
processed a jar.mn file.  That seems a bit wasteful.  Those are the easy (read:
"one-time" changes).  The hard part (read: the part that's has high probability
of breaking in the future) is all of the local changes to makefiles.  You'd have
to make sure that you have a Darwin ifdef for each local rule that touches dist/
and that when people make changes in the future that they also add those ifdefs.
Then there's the embedding scenarios where they don't need yet another copy of
the dist/bin in the form of the .app directories. 

Yes, it can be improved but I'm not sure if it's worth it.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

But how does 'make install' (recently implemented) work?

I was thinking that Mach-O could do the copying by having the install pass on by
default for Mach-O, with the default install target of dist/Mozilla[Debug].app/.
 That would be slower for complete builds, though...

Comment 4

[Conrad Carlen (not reading bugmail)]

The reason the contents of dist/Mozilla[Debug].app/Contents/MacOS need to be
updated from dist/bin is because the destination items can't be the same
symlinks as they are in the src. This is because the symlinks made by nsinstall
are relative, by default. For example, readlink on dist/bin/libxpcom.dylib gives
 ../../xpcom/build/libxpcom.dylib Obviously, that symlink can't be copied as-is
to the destination, so we have to resolve the link on the copy. I think we could
get around this by making nsinstall make absolute symlinks. If the items in
dist/Mozilla[Debug].app/Contents/MacOS were symlinks, it would be the same as
Linux: build a component and the symlink in dist will point to the updated
component, no need to repackage or update anything. Then, when doing 'make
install' in xpfe/bootstrap, we could resolve the links.

Isn't it possible to set a build flag which would force nsinstall to make
absolute links instead of relative?

Comment 5

[hacker formerly known as [e-mail]]

make install (implemented almost a year ago) has the same problem and has broken
several times since it landed. 

Oh!  I thought that the files under .app couldn't be symlinks at all.  Well, in
that case, we could just set NSDISTMODE=absolute_symlink for OSX builds.

Comment 6

[hacker formerly known as [e-mail]]

Attachment: use absolute symlinks for nsinstall

So this patch makes us use absolute symlinks by default on OSX and to not
resolve symlinks when creating Mozilla.app.  However, it doesn't work.	I'm
still on 10.1.5 if that matters. If I go to .app/Contents/MacOS/ and run
./mozilla, it gets far enough to lock the profile but no windows show up.

Comment 7

[hacker formerly known as [e-mail]]

Strange.  When running inside of gdb, a window popped up but I can't bring it to
front and it's not registering any of the clicks on any of the buttons.  It does
underline links in the personal toolbar though and show tooltips.

Comment 8

[Conrad Carlen (not reading bugmail)]

If it gets that far, it seems like loading of symlinked components is working. 
There must be some (hopefully small) number of items in the bundle that need to
be actual and not symlinked? This patch gets us closer, anyway.

Comment 9

[hacker formerly known as [e-mail]]

Attachment: don't symlink mozilla-bin

Apparently, when using absolute symlinks, everything can be a symlink except
the actual binary itself.

Comment 10

[Conrad Carlen (not reading bugmail)]

Comment on attachment 114937 [details] [diff] [review]
don't symlink mozilla-bin

Works great. Also, the resulting app is still debuggable from Project Builder -
the symlinks cause no problems there.

Comment 11

[Mark Smith [:mcs]]

Comment on attachment 114937 [details] [diff] [review]
don't symlink mozilla-bin

This may be a naive question but can someone explain what nfspwd does and why
you no longer want to use it?

Comment 12

[hacker formerly known as [e-mail]]

nfspwd is a perl script that looks like it was designed to cope with building a
particular tree on a nfs mount from multiple machines.  If the path returned by
fastcwd.pl does not contain a nfsmount point (/u or /h), then it prepends
/h/<hostname> or /u/<user> to the path.  

Since most OSX boxes aren't tied into the internal NIS/NFS setup, those paths do
not work.  I could fix the nfspwd script to avoid prepending those paths (or at
least verify that they return a real directory) but calling a perl script to do
a `pwd` is overkill.

Comment 13

[hacker formerly known as [e-mail]]

The patch has been checked in on the mozilla, nspr & ldap trunks.

Comment 14

[Conrad Carlen (not reading bugmail)]

I assumed that when the mozilla.org nightlies were made, the symlinks would be
resolved. Apparently not. I just downloaded the nightly from mozilla.org,
noticed it was very small, looked inside, and - nothing but symlinks. The
commercial build on sweetlou is OK.

Comment 15

[J.J. Enser]

I guess the best place to fix this is in the nightly packaging automation:

- cp -R ../mozilla/dist/Mozilla.app .
+ cp -pRL ../mozilla/dist/Mozilla.app .

Note that I intend to apply the same change to the Netscape packaging
automation, although it doesn't seem to be affected by this problem.... Chris,
should both behave the same way?

Comment 16

[hacker formerly known as [e-mail]]

jj, yes, they should.

Comment 17

[Wan-Teh Chang]

Comment on attachment 114937 [details] [diff] [review]
don't symlink mozilla-bin

>Index: nsprpub/config/config.mk
[...]
>+ifeq ($(OS_ARCH),Darwin)
>+ifndef NSDISTMODE
>+NSDISTMODE=absolute_symlink
>+endif
>+PWD := $(shell pwd)
>+endif

The NSDISTMODE change should be moved to nsprpub/Makefile.in
inside ifdef MOZILLA_CLIENT or PR_CLIENT_BUILD because it
solves a Mozilla client build problem.

The PWD change is a good change to all NSPR customers.	It
seems that the nfspwd.pl script is no good outside Netscape.

Comment 18

[hacker formerly known as [e-mail]]

We can wrap the NSPR change inside the MOZILLA_CLIENT ifdef but moving that
change to nsprpub/Makefile.in won't work because we need NSDISTMODE to be set
for each sub-make.  

nfspwd doesn't work outside of Netscape but I didn't want to touch that bit
because I don't know who relies upon that old behavior.  Wan-Teh, if you're
okaying the complete removal for NSPR, then the rest can follow suit since we
all depend upon NSPR. :) 

Comment 19

[hacker formerly known as [e-mail]]

Attachment: move NSDISTMODE check into toplevel Makefile for NSPR

Comment 20

[Wan-Teh Chang]

Comment on attachment 115569 [details] [diff] [review]
move NSDISTMODE check into toplevel Makefile for NSPR

r=wtc.	Would be nice if you could add a comment
explaining why NSDISTMODE default to absolute_symlink
(for example, "solve a Mozilla client build problem")
and referring to this bug.

Comment 21

[hacker formerly known as [e-mail]]

The patch has been checked into the NSPR trunk & client branch with the
requested comment.  The nightly build problem appears to be resolved as well.

Comment 22

[Jon Granrose]

hmm, looks like security didn't get updated since I have shlibsign and mangle
showing in Mozilla.app/Contents/MacOS as relative links which is breaking the
packaging (bug 270881).

I'm looking at security/coreconf/Darwin.mk but don't know nss at all.

Comment 23

[Wan-Teh Chang]

Attachment: Patch for mozilla/security

Please give this patch a try.

As I mentioned in bug 270881, the Mozilla package
should not include 'mangle' and 'shlibsign'.  But
we should still update security with this change.

Comment 24

[Chase Phillips]

*** Bug 280722 has been marked as a duplicate of this bug. ***

Comment 25

[Chase Phillips]

This problem is still (!) biting us.  Reopening this bug to make sure we get it
fixed.

Comment 26

[Chase Phillips]

Comment on attachment 166523 [details] [diff] [review]
Patch for mozilla/security

>Index: security/manager/Makefile.in

We need to double check that modifying this file is really necessary.

>Index: security/coreconf/UNIX.mk

I applied this patch without looking too closely at it and later found my
patched trunk build had failed.  The reason is due to a small error in the
code.

First pwd is run here..

>+		ifeq ($(OS_ARCH),Darwin)
>+			PWD     := $(shell pwd)

..then the output of pwd is executed again to gain one of the arguments for the
nsinstall invocation.

>+			INSTALL += -L `$(PWD)`

One or the other, but not both. :)

I worry about special casing UNIX.mk with Darwin rules when we have a Darwin.mk
for what I thought was that purpose.  If Darwin.mk is around for another
reason, though, then let's clean this patch up and get it in on the trunk while
we've got a window available.

Comment 27

[Wan-Teh Chang]

Attachment: Patch for mozilla/security v2

Chase, you're right.  That should be just
-L $(PWD).

Comment 12 confirmed my suspicion that the nfspwd.pl
script is only useful on the old Netscape intranet.
So I think we should eliminate nfspwd.pl from the
tree altogether.

For your immediate need, I suggest this patch.
Please give it a try.  And see if the change to
security/manager/Makefile.in is still necessary.

Comment 28

[Chase Phillips]

Comment on attachment 173332 [details] [diff] [review]
Patch for mozilla/security v2

This patch works for me.  Can we get this on the trunk and the 1.7 branch?

Comment 29

[Wan-Teh Chang]

Comment on attachment 173332 [details] [diff] [review]
Patch for mozilla/security v2

I've checked in this patch on the Mozilla
trunk/NSS_CLIENT_TAG (Mozilla 1.8 Beta).

The NSS change has been checked in on the
NSS trunk (NSS 3.10) and NSS_3_9_BRANCH
(NSS 3.9.6).

Comment 30

[Nelson Bolyard (seldom reads bugmail)]

The patch above was checked in on Friday late afternoon, Feb 4.
Our nightly 64 bit AIX trunk builds have been failing QA continuously 
since then.

We do nightly builds on AIX 5.1, for all combinations of 
   trunk / 3.9 branch
   debug / optimized
   32 bit/ 64 bit.  

Beginning with Saturday morning's build, the 64-bit AIX trunk builds
have been failing QA.  The FIPS mode tests all fail.  

The 32-bit builds are OK, and the 3.9 branch builds are OK.  

Comment 31

[Chase Phillips]

(In reply to comment #30)
In the build output, is the failure obviously due to this change?  Can you
attach a build log of a failed build?  Thanks.

Comment 32

[Nelson Bolyard (seldom reads bugmail)]

A few clarifications for my comment 30.
"The patch above" refers to the "Patch for mozilla/security v2", which 
is a patch to NSS.
The nightly builds and failing QA tests are for NSS only.  
"We" means Sun's NSS group.

I'm not 100% certain that this patch caused the failure.  OTOH, it was the
only change to NSS sources in several days, and the nightly builds all 
passed QA until that night, and have all failed since.

The failing commands from the nightly QA runs include:

  modutil -dbdir . -fips true

Using database directory ....
An I/O error occurred during security authorization.
ERROR: Unable to switch FIPS modes.
cert.sh ERROR: Enable FIPS mode on database for FIPS PUB 140-1 Test Certificate
failed 11  

Thereafter many tests failed.  

Comment 33

[Nelson Bolyard (seldom reads bugmail)]

Hmm.  Also on Friday, a checkin was made to NSPR for bug 280984.  
That may be the culprit.
https://bugzilla.mozilla.org/show_bug.cgi?id=280984#c7

Comment 34

[Nelson Bolyard (seldom reads bugmail)]

The patch for bug 280984, not the patch for this bug, has positively been
identified as the cause of the NSS QA regression.  Sorry for the alarm in
the wrong bug.

Comment 35

[Chase Phillips]

Still would like to get this patch on the 1.7 branch.  Doesn't need to happen
for 1.7.6 IMO, though.

Comment 36

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 173332 [details] [diff] [review]
Patch for mozilla/security v2

Too late for 1.7.6

Comment 37

[Chase Phillips]

Comment on attachment 173332 [details] [diff] [review]
Patch for mozilla/security v2

a=approval-aviary1.0.3?

Need this patch on the aviary1.0.1 branch to build Thunderbird via Tinderbox
scripts.

Comment 38

[Chase Phillips]

Comment on attachment 173332 [details] [diff] [review]
Patch for mozilla/security v2

Asa gives approval-aviary1.0.3+ via rl (part of the build process)

Comment 39

[Chase Phillips]

Landed on Aviary1.0.1 branch:

Checking in security/coreconf/UNIX.mk;
/cvsroot/mozilla/security/coreconf/UNIX.mk,v  <--  UNIX.mk
new revision: 1.2.220.1; previous revision: 1.2
done
Checking in security/manager/Makefile.in;
/cvsroot/mozilla/security/manager/Makefile.in,v  <--  Makefile.in
new revision: 1.53.16.1; previous revision: 1.53
done

Comment 40

[Daniel Veditz [:dveditz]]

Adding fixed-aviary1.0.3 based on comment 39, adding 1.7 blocking flags based on
comment 35

Comment 41

[Chase Phillips]

blocking1.7.7-

Back in January I implemented a workaround for this on the 1.7 branch that's
still in place.  I want to get this in for parity's sake since it's on the trunk
and the aviary1.0.1 branch, but this bug shouldn't hold up the 1.7.7 release.

Comment 42

[Chase Phillips]

This bug was fixed on the trunk after wtchang landed his patch 2005/02/04.  The
patch should land on the 1.7 branch at some point to sync up build config
between the code area trifecta, but that work is not urgent.

Resolving fixed.

Comment 43

[Daniel Veditz [:dveditz]]

Missed 1.7.8, moving request to 1.7.9 per comment 41 and 42

Comment 44

[Daniel Veditz [:dveditz]]

Per chase's comment 42 ("not urgent") not blocking 1.7.9