Bug 26537 – Builds from command-line fail if custom build product directory is set and ~/Library/Preferences/xcodebuild.plist exists

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Takeshi Yoshino]

When I set the directory to place build products by Xcode like this,
Xcode -> Preferences -> Building tab -> Place Build Products in -> Customized location -> e.g. /tmp/ramdisk
I got the following error. This happened because build-webkit script didn't set BUILT_PRODUCTS_DIR variable correctly. Output files were generated into each project directories separately, not into the specified directory together.

% time build-webkit --debug
...
    setenv BUILT_PRODUCTS_DIR /Users/tyoshino/wk2_svn/JavaScriptCore/build/Debug
...
CompileC build/JavaScriptGlue.build/Debug/JavaScriptGlue.build/Objects-normal/i386/JSBase.o /Users/tyoshino/wk2_svn/JavaScriptGlue/JSBase.cpp normal i386 c++ com.apple.compilers.gcc.4_2
    cd /Users/tyoshino/wk2_svn/JavaScriptGlue
    /Developer/usr/bin/gcc-4.2 -x c++ -arch i386 -fmessage-length=0 -pipe -Wno-trigraphs -fno-exceptions -fno-rtti -fpascal-strings -fasm-blocks -O0 -Werror -Wnon-virtual-dtor -Wnewline-eof -DWEBKIT_VERSION_MIN_REQUIRED=WEBKIT_VERSION_LATEST -fstrict-aliasing -fvisibility-inlines-hidden -fno-threadsafe-statics -mmacosx-version-min=10.5 -gdwarf-2 -I/Users/tyoshino/wk2_svn/JavaScriptGlue/build/JavaScriptGlue.build/Debug/JavaScriptGlue.build/JavaScriptGlue.hmap -Wall -W -Wcast-align -Wchar-subscripts -Wformat-security -Wmissing-format-attribute -Wpointer-arith -Wwrite-strings -Wno-format-y2k -Wno-unused-parameter -Wundef -Wno-strict-aliasing -Wshorten-64-to-32 -F/Users/tyoshino/wk2_svn/JavaScriptGlue/build/Debug -I/Users/tyoshino/wk2_svn/JavaScriptGlue/build/Debug/include -IForwardingHeaders -I. -Iicu -I/Users/tyoshino/wk2_svn/JavaScriptGlue/build/JavaScriptGlue.build/Debug/JavaScriptGlue.build/DerivedSources -Wno-deprecated-declarations -c /Users/tyoshino/wk2_svn/JavaScriptGlue/JSBase.cpp -o /Users/tyoshino/wk2_svn/JavaScriptGlue/build/JavaScriptGlue.build/Debug/JavaScriptGlue.build/Objects-normal/i386/JSBase.o
In file included from /Users/tyoshino/wk2_svn/JavaScriptGlue/config.h:5,
                 from /Users/tyoshino/wk2_svn/JavaScriptGlue/JSBase.cpp:29:
ForwardingHeaders/wtf/Platform.h:1:37: error: JavaScriptCore/Platform.h: No such file or directory

[Takeshi Yoshino]

Created attachment 31541 [details]
Proposed fix for 26537

[Takeshi Yoshino]

Created attachment 31542 [details]
Proposed fix for 26537

Oops! I included unrelated changes by mistake.

[Eric Seidel (no email)]

Comment on attachment 31542 [details]
Proposed fix for 26537

LGTM.

[Mark Rowe (bdash)]

I don't understand why this is necessary.  I'm aware of a number of people that build with a global built products directory configured in Xcode.  I'd like to understand what is special about your setup that requires this change?

[Takeshi Yoshino]

(In reply to comment #4)
> I don't understand why this is necessary.  I'm aware of a number of people that
> build with a global built products directory configured in Xcode.  I'd like to
> understand what is special about your setup that requires this change?
> 

Do they build using build-webkit script? I can build WebKit on Xcode IDE without this patch, but cannot with build-webkit script when I changed the global built products directory to somewhere else than <Checkout directory>/WebKitBuild.

I just followed the instruction on www.webkit.org. AFAIK, I didn't make any special changes on Xcode settings other than built product. One of my coworker also came across this failure when he tried to build.

Thanks

[Mark Rowe (bdash)]

Comment on attachment 31542 [details]
Proposed fix for 26537

Explicitly setting SYMROOT and OBJROOT shouldn't be necessary.  Xcode derives these values automatically relative to configured build products directory.  While the patch apparently fixes your problem, I think we should understand why you're running in to this when no-one else is.  It makes me suspect that something is just a little off in your Xcode preferences.  Until we understand what, I don't think this should be landed.

[Mark Rowe (bdash)]

(In reply to comment #5)
> (In reply to comment #4)
> > I don't understand why this is necessary.  I'm aware of a number of people that
> > build with a global built products directory configured in Xcode.  I'd like to
> > understand what is special about your setup that requires this change?
> > 
> 
> Do they build using build-webkit script? I can build WebKit on Xcode IDE
> without this patch, but cannot with build-webkit script when I changed the
> global built products directory to somewhere else than <Checkout
> directory>/WebKitBuild.

Almost all of my coworkers at Apple have a custom built product location and build from the command-line (using "make", which gets the arguments to xcodebuild from webkitdirs.pm in a similar fashion to build-webkit).

Do you happen to have a ~/Library/Preferences/xcodebuild.plist?

[Takeshi Yoshino]

(In reply to comment #7)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > I don't understand why this is necessary.  I'm aware of a number of people that
> > > build with a global built products directory configured in Xcode.  I'd like to
> > > understand what is special about your setup that requires this change?
> > > 
> > 
> > Do they build using build-webkit script? I can build WebKit on Xcode IDE
> > without this patch, but cannot with build-webkit script when I changed the
> > global built products directory to somewhere else than <Checkout
> > directory>/WebKitBuild.
> 
> Almost all of my coworkers at Apple have a custom built product location and
> build from the command-line (using "make", which gets the arguments to
> xcodebuild from webkitdirs.pm in a similar fashion to build-webkit).

Hmm, I see.

> 
> Do you happen to have a ~/Library/Preferences/xcodebuild.plist?
> 

Yes

% od -c xcodebuild.plist                                                                                        [~/Library/Preferences]
0000000    b   p   l   i   s   t   0   0 321 001 002   _ 020 037   P   B
0000020    X   A   p   p   l   i   c   a   t   i   o   n   w   i   d   e
0000040    B   u   i   l   d   S   e   t   t   i   n   g   s 320  \b  \v
0000060    -  \0  \0  \0  \0  \0  \0 001 001  \0  \0  \0  \0  \0  \0  \0
0000100  003  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0
0000120    .                                                            
0000121

[Takeshi Yoshino]

(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #5)
> > > (In reply to comment #4)
> > > > I don't understand why this is necessary.  I'm aware of a number of people that
> > > > build with a global built products directory configured in Xcode.  I'd like to
> > > > understand what is special about your setup that requires this change?
> > > > 
> > > 
> > > Do they build using build-webkit script? I can build WebKit on Xcode IDE
> > > without this patch, but cannot with build-webkit script when I changed the
> > > global built products directory to somewhere else than <Checkout
> > > directory>/WebKitBuild.
> > 
> > Almost all of my coworkers at Apple have a custom built product location and
> > build from the command-line (using "make", which gets the arguments to
> > xcodebuild from webkitdirs.pm in a similar fashion to build-webkit).
> 
> Hmm, I see.
> 
> > 
> > Do you happen to have a ~/Library/Preferences/xcodebuild.plist?
> > 
> 
> Yes
> 
> % od -c xcodebuild.plist                                                       
>                                 [~/Library/Preferences]
> 0000000    b   p   l   i   s   t   0   0 321 001 002   _ 020 037   P   B
> 0000020    X   A   p   p   l   i   c   a   t   i   o   n   w   i   d   e
> 0000040    B   u   i   l   d   S   e   t   t   i   n   g   s 320  \b  \v
> 0000060    -  \0  \0  \0  \0  \0  \0 001 001  \0  \0  \0  \0  \0  \0  \0
> 0000100  003  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0
> 0000120    .                                                            
> 0000121
> 

And, FYI,

% defaults read com.apple.Xcode PBXApplicationwideBuildSettings                                                 [~/Library/Preferences]
{
    SYMROOT = "/tmp/ramdisk/WebKitBuild";
}

[Takeshi Yoshino]

Deleting ~/Library/Preferences/xcodebuild.plist fixed this problem.
I see. This file gave bad parameter to xcodebuild.

[Mark Rowe (bdash)]

> > Do you happen to have a ~/Library/Preferences/xcodebuild.plist?
> > 
> 
> Yes

Delete it!  The problem you're seeing is that xcodebuild is reading preferences from that file when it should be pulling them from com.apple.Xcode.plist, the standard Xcode preferences.  The situations under which xcodebuild.plist gets created are something of a mystery, but appears to be the result of a bug in xcodebuild itself (<rdar://problem/5585899>).

The existence of this bogus preference file is likely to cause other subtle issues beyond the obvious build directory problem, so I don't think your patch is the appropriate workaround.  A more direct approach may be to look for the existence of this preference file and warn the user, or perhaps even automatically remove it.

[Takeshi Yoshino]

(In reply to comment #11)
> > > Do you happen to have a ~/Library/Preferences/xcodebuild.plist?
> > > 
> > 
> > Yes
> 
> Delete it!  The problem you're seeing is that xcodebuild is reading preferences
> from that file when it should be pulling them from com.apple.Xcode.plist, the
> standard Xcode preferences.  The situations under which xcodebuild.plist gets
> created are something of a mystery, but appears to be the result of a bug in
> xcodebuild itself (<rdar://problem/5585899>).
> 
> The existence of this bogus preference file is likely to cause other subtle
> issues beyond the obvious build directory problem, so I don't think your patch
> is the appropriate workaround.  A more direct approach may be to look for the

So, what's the best thing to do in this function? It's confusing that it has unnecessary code for setting baseProductionDirOption here.

> existence of this preference file and warn the user, or perhaps even
> automatically remove it.
> 

OK. I agree with you about this point. I'll send another patch.

[Mark Rowe (bdash)]

(In reply to comment #12)
> (In reply to comment #11)
> > > > Do you happen to have a ~/Library/Preferences/xcodebuild.plist?
> > > > 
> > > 
> > > Yes
> > 
> > Delete it!  The problem you're seeing is that xcodebuild is reading preferences
> > from that file when it should be pulling them from com.apple.Xcode.plist, the
> > standard Xcode preferences.  The situations under which xcodebuild.plist gets
> > created are something of a mystery, but appears to be the result of a bug in
> > xcodebuild itself (<rdar://problem/5585899>).
> > 
> > The existence of this bogus preference file is likely to cause other subtle
> > issues beyond the obvious build directory problem, so I don't think your patch
> > is the appropriate workaround.  A more direct approach may be to look for the
> 
> So, what's the best thing to do in this function? It's confusing that it has
> unnecessary code for setting baseProductionDirOption here.

What unnecessary code are you referring to?  baseProductDirOption is set in two places: one place deals with the situation where Xcode has a global build product directory configured (no extra options necessary), and the other deals with the default behavior of placing the build product directory alongside the project (we override SYMROOT and OBJROOT to force things in to a single directory for all projects). 

It may be that this logic could be cleaner or more explicit, but that is something that would be best done in a separate patch.

[Takeshi Yoshino]

> > So, what's the best thing to do in this function? It's confusing that it has
> > unnecessary code for setting baseProductionDirOption here.
> 
> What unnecessary code are you referring to?  baseProductDirOption is set in two
> places: one place deals with the situation where Xcode has a global build
> product directory configured (no extra options necessary), and the other deals
> with the default behavior of placing the build product directory alongside the
> project (we override SYMROOT and OBJROOT to force things in to a single
> directory for all projects). 
> 
Sorry, I was misunderstanding. I got it. The code is completely correct.

> It may be that this logic could be cleaner or more explicit, but that is
> something that would be best done in a separate patch.
> 

All right. Will do.

Thank you for review, Mark and Eric.

[Takeshi Yoshino]

Created attachment 31638 [details]
Proposed fix for 26537 (rev 3)

A new approach to fix this issue following Mark's suggestion.

[Darin Adler]

Comment on attachment 31638 [details]
Proposed fix for 26537 (rev 3)

I think we could go further and just remove that file!

r=me

[Mark Rowe (bdash)]

Comment on attachment 31638 [details]
Proposed fix for 26537 (rev 3)

This logic should go in webkitdirs.pm, near where we determine whether we need to add any custom options to xcodebuild in relation to the build product directory.  That code is also used by the Makefiles, so it would be good to have the workaround affect both build-webkit and the Makefiles.  I also think that removing the file silently if it exists is a better option, as we're just asking the user to do something we can trivially do ourselves.  In addition, it'd be great if you could update the comment to refer to the Radar number I mentioned, so that in the future we can quickly determine why the workaround exists and if it is still needed.  Something like the following should suffice:

   # The presence of ~/Library/Preferences/xcodebuild.plist can prevent xcodebuild from respecting global settings such as a custom build products directory (<rdar://problem/5585899>).

[Takeshi Yoshino]

Created attachment 31702 [details]
Proposed fix for 26537 (rev 4)

[Takeshi Yoshino]

Created attachment 31703 [details]
Proposed fix for 26537 (rev 5)

[Takeshi Yoshino]

(In reply to comment #16)
> (From update of attachment 31638 [details] [review])
> I think we could go further and just remove that file!
> 
> r=me
> 

OK. Done.

(In reply to comment #17)
> (From update of attachment 31638 [details] [review])
> This logic should go in webkitdirs.pm, near where we determine whether we need
> to add any custom options to xcodebuild in relation to the build product
> directory.  That code is also used by the Makefiles, so it would be good to
> have the workaround affect both build-webkit and the Makefiles.  I also think
> that removing the file silently if it exists is a better option, as we're just
> asking the user to do something we can trivially do ourselves.  In addition,
> it'd be great if you could update the comment to refer to the Radar number I
> mentioned, so that in the future we can quickly determine why the workaround
> exists and if it is still needed.  Something like the following should suffice:
> 
>    # The presence of ~/Library/Preferences/xcodebuild.plist can prevent
> xcodebuild from respecting global settings such as a custom build products
> directory (<rdar://problem/5585899>).
> 

Done. Please take a look again.

Thanks,

[Mark Rowe (bdash)]

Comment on attachment 31703 [details]
Proposed fix for 26537 (rev 5)

This looks fine to me.  Lines in the ChangeLog and containing comments appear to be wrapped at a very narrow width, making them hard to read.  Whomever lands the patch may want to reflow them at a more reasonable length.

[Eric Seidel (no email)]

Sigh.  Script failure r45032.

[Takeshi Yoshino]

(In reply to comment #22)
> Sigh.  Script failure r45032.
> 

Thank you.