297227 - Depend build fails to build PrintPDE.plugin

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[jhp (no longer active)]

Didn't see a bug open for this.  Feel free to dupe.

I get the following error when updating my tree (building OBJDIR):
mkdir -p ../../../../../../dist/package
cp -R build/UninstalledProducts/PrintPDE.plugin ../../../../../../dist/package/
cp: cannot overwrite directory
../../../../../../dist/package/PrintPDE.plugin/PrintPDE.plugin with
non-directory build/UninstalledProducts/PrintPDE.plugin/PrintPDE.plugin
make[8]: *** [libs] Error 1

It seems that 'build/UninstalledProducts/PrintPDE.plugin' has a symlink to
itself inside it.  I see the following line in the Xcode build:
    /bin/ln -sf
/Users/pedemonte/builds/foxtrunk/mozilla/objdbg/embedding/components/printingui/src/mac/printpde/build/UninstalledProducts/PrintPDE.plugin
/Users/pedemonte/builds/foxtrunk/mozilla/objdbg/embedding/components/printingui/src/mac/printpde/build/PrintPDE.plugin

Not sure why.  This is on OS X 10.3, building with OBJDIR.

Comment 1

[Mark Mentovai]

I've seen this too, infrequently.  Some (but not all) of the Firefox and
Thunderbird tinderboxen started experiencing it after bug 292530 landed. 
cadence (fox/trunk) still may.  Does it clear up if you rm -rf
/path/to/build/embedding/components/printingui/src/mac/printpde/build
/path/to/build/dist/DeerPark.app/Contents/Plug-Ins/PrintPDE.plugin ?

Probably the best fix is to do $(INSTALL) build/PrintPDE.plugin $(DIST)/package
in PrintPDE's Makefile instead of the cp -R that's there now, and let it always
be a symbolic link.  That'll break current depend builds, though, because
nsinstall won't replace a directory with a symlink.  I suppose it wouldn't be
awful to rm -rf $(DIST)/package/PrintPDE.plugin before doing $(INSTALL).

Comment 2

[Mark Mentovai]

Attachment: like this...

...but then we'd have the same problem when copying PrintPDE.plugin from
dist/package to *.app/Contents/Plug-Ins.  That's done by the Makefiles in
browser/app, mail/app, and xpfe/bootstrap.  May also have the same problem
during the Camino build.

I guess that rm could be added in those cases too...

Comment 3

[jhp (no longer active)]

You've got no idea why Xcode is doing this?

Comment 4

[Mark Mentovai]

The problem is the word "install" in the PrintPDE Makefile.  It causes
xcodebuild (or its suitable forerunner) to move the PrintPDE.plugin build
product into build/UninstalledProducts and make a symbolic link from
build/PrintPDE.plugin to the real plugin.

Xcode "native targets," which we just converted to, aren't able to figure out
that the symbolic link already exists when the build had been managed in the
past by "legacy targets," which we just switched from.  So even though it's a
depend build, Xcode always makes a new symbolic link.  We wind up with
build/PrintPDE.plugin as a link to build/UninstalledProducts/PrintPDE.plugin
and, Xcode makes a new link anyway using "ln -s
build/UninstalledProducts/PrintPDE.plugin build/PrintPDE.plugin".  Guess what
happens next?

If we didn't have "install" in the xcodebuild invocation, the build product
would always wind up as build/PrintPDE.plugin.  No links.  Clean.  We don't have
"install" in any other call to xcodebuild on the critical path, DefaultPlugin or
Camino.  It doesn't belong in PrintPDE either.  It's always been there, and it
was harmless until I started poking it with updated targets.

That's really the best way to do it.  It won't clean out any builds that have
already been stuffed full of symbolic links.  It WILL fix it for sure, once and
for all.

It WON'T fix builds that have already gotten filled with unwanted symbolic
links, and it might break existing depend builds that haven't broken yet.

If there's overwhelming support for that path, we can just get rid of the
"install" and tell everyone using a Mac to clobber (or at least trash a few
files and directories).  The other option is to test for a possible problem
within the build system (probably by looking for the existence of
build/UnistalledProducts) and do the necessary housekeeping in that case.  It
can be short-lived, with a comment that says "whoever edits me after date X,
remove these lines."  I hate bloat almost as much as I hate broken stuff.

I'll roll these ideas into a patch.

Comment 5

[Mark Mentovai]

Attachment: Fix the problem and repair bollocksed-up depend builds

Get rid of "install" passed to xcodebuild, so that there aren't any weird
UninstalledProducts directories or symbolic links, and so that xcodebuild won't
be tempted to make new ones.

Do "make clean" in printpde and remove dist/package/PrintPDE.plugin if an
UninstalledProducts directory exists, to make sure that old recursive links and
ridiculously deep folder structures get the treatment they deserve.  This part
only needs to stick around long enough to repair old depend builds, then it's
safe to remove.

Ship the PrintPDE plugin out with $(INSTALL) instead of cp -R.	This is better.


In the three Makefiles, switch from cp -R to "rsync -a --copy-unsafe-links
--delete" when copying into the .app.  The --deletes can be removed if desired
after a reasonable time, as above.

Tested with depend builds of Firefox and Camino.  Did not test with Thunderbird
or Seamonkey, but the changes specific to those builds in their Makefiles are
identical to the changes made in the Firefox Makefile.

Comment 6

[jhp (no longer active)]

Got the following error:

rsync -a --copy-unsafe-links --delete ../../dist/package/PrintPDE.plugin
../../dist/Firefox.app/Contents/Plug-Ins
file has vanished:
"/Users/pedemonte/builds/foxtrunk/mozilla/objdbg/browser/app/../../dist/Firefox.app/Contents/Plug-Ins/PrintPDE.plugin/PrintPDE.plugin"
rsync warning: some files vanished before they could be transfered (code 24) at
/SourceCache/rsync/rsync-14/rsync/main.c(633)


Plugins directory looks like this:

10239954    0 drwxr-xr-x    3 pedemont pedemont      102 Jun  9 14:40
/Users/pedemonte/builds/foxtrunk/mozilla/objdbg/dist/Firefox.app/Contents/Plug-Ins/
11608890    0 drwxr-xr-x    4 pedemont pedemont      136 Jun 10 12:53
/Users/pedemonte/builds/foxtrunk/mozilla/objdbg/dist/Firefox.app/Contents/Plug-Ins//PrintPDE.plugin
11608891    0 drwxr-xr-x    5 pedemont pedemont      170 Jun 10 12:53
/Users/pedemonte/builds/foxtrunk/mozilla/objdbg/dist/Firefox.app/Contents/Plug-Ins//PrintPDE.plugin/Contents
11639087    8 -rw-r--r--    1 pedemont pedemont     1090 Jun 10 12:53
/Users/pedemonte/builds/foxtrunk/mozilla/objdbg/dist/Firefox.app/Contents/Plug-Ins//PrintPDE.plugin/Contents/Info.plist
11608892    0 drwxr-xr-x    3 pedemont pedemont      102 Jun 10 12:53
/Users/pedemonte/builds/foxtrunk/mozilla/objdbg/dist/Firefox.app/Contents/Plug-Ins//PrintPDE.plugin/Contents/MacOS
11639088   96 -rwxr-xr-x    1 pedemont pedemont    48268 Jun 10 12:53
/Users/pedemonte/builds/foxtrunk/mozilla/objdbg/dist/Firefox.app/Contents/Plug-Ins//PrintPDE.plugin/Contents/MacOS/PrintPDE
11608894    0 drwxr-xr-x    3 pedemont pedemont      102 Jun 10 12:53
/Users/pedemonte/builds/foxtrunk/mozilla/objdbg/dist/Firefox.app/Contents/Plug-Ins//PrintPDE.plugin/Contents/Resources
11608895    0 drwxr-xr-x    4 pedemont pedemont      136 Jun 10 12:53
/Users/pedemonte/builds/foxtrunk/mozilla/objdbg/dist/Firefox.app/Contents/Plug-Ins//PrintPDE.plugin/Contents/Resources/English.lproj
11639089    8 -rwxr-xr-x    1 pedemont pedemont     1978 Jun 10 12:53
/Users/pedemonte/builds/foxtrunk/mozilla/objdbg/dist/Firefox.app/Contents/Plug-Ins//PrintPDE.plugin/Contents/Resources/English.lproj/Localizable.strings
11608896    0 drwxr-xr-x    5 pedemont pedemont      170 Jun 10 12:53
/Users/pedemonte/builds/foxtrunk/mozilla/objdbg/dist/Firefox.app/Contents/Plug-Ins//PrintPDE.plugin/Contents/Resources/English.lproj/PrintPDE.nib
11639090    8 -rw-r--r--    1 pedemont pedemont       35 Jun 10 12:53
/Users/pedemonte/builds/foxtrunk/mozilla/objdbg/dist/Firefox.app/Contents/Plug-Ins//PrintPDE.plugin/Contents/Resources/English.lproj/PrintPDE.nib/classes.nib
11639091    8 -rw-r--r--    1 pedemont pedemont      486 Jun 10 12:53
/Users/pedemonte/builds/foxtrunk/mozilla/objdbg/dist/Firefox.app/Contents/Plug-Ins//PrintPDE.plugin/Contents/Resources/English.lproj/PrintPDE.nib/info.nib
11639092   16 -rw-r--r--    1 pedemont pedemont     4715 Jun 10 12:53
/Users/pedemonte/builds/foxtrunk/mozilla/objdbg/dist/Firefox.app/Contents/Plug-Ins//PrintPDE.plugin/Contents/Resources/English.lproj/PrintPDE.nib/objects.xib
11608902    8 lrwxr-xr-x    1 pedemont pedemont      138 Jun  9 14:40
/Users/pedemonte/builds/foxtrunk/mozilla/objdbg/dist/Firefox.app/Contents/Plug-Ins//PrintPDE.plugin/PrintPDE.plugin
->
/Users/pedemonte/builds/foxtrunk/mozilla/objdbg/embedding/components/printingui/src/mac/printpde/build/UninstalledProducts/PrintPDE.plugin

Comment 7

[Mark Mentovai]

Attachment: v3: symlinks: smoke 'em out

Even rsync --delete isn't strong enough.  We need to harness the power of rm
-rf.

Comment 8

[jhp (no longer active)]

Comment on attachment 185882 [details] [diff] [review]
v3: symlinks: smoke 'em out

Great, this one works.	Thanks.

Comment 9

[Mark Mentovai]

Ready for check-in.

Comment 10

[Scott MacGregor]

Josh asked me to check this in for him.