xcode_emulation: work in the absence of xcodebuild

xcode_emulation: work in the absence of xcodebuild

OS X systems running only the Command Line Tools for Xcode package,
without the full Xcode, don't have a functioning xcodebuild, but this
isn't mandatory for building many gyp projects (e.g. node.js, v8).
This commit handles xcodebuild failures and avoids populating
Xcode-specific CFLAGS/LDFLAGS when xcodebuild can't be run.

This has been tested on both Xcode and CLT-only systems by successfully
building node.js. The behaviour can be simulated on systems with Xcode
by setting the xcode-select path to something nonsensical, e.g.
xcode-select -switch /usr/bin

This fixes issue https://code.google.com/p/gyp/issues/detail?id=292

BUG=

[mistydemeo, 2013-05-03 03:52:07]

This patch allows gyp builds to work in the absence of a working xcodebuild, for
example if the user only has the Command Line Tools for Xcode installed.

[Nico, 2013-05-03 04:02:57]

Thank you for your patch!

I'm somewhat afraid that the blanket "catch" might mask problems when someone
just hasn't set their path right though.

Can you instead change the three places that call _SdkPath() to only do so if
"$(SDKROOT)" is in the string? i.e. instead of

  s.replace("$(SDKROOT)", self._SdkPath())

write

  if "$(SDKROOT)" in s:
    s = s.replace("$(SDKROOT)", self._SdkPath())

I think this should fix your issue as well as long as your gyp file doesn't use
$(SDKROOT) (and if it does, your patch doesn't do the right thing anyways).

Can you think of a way to write a test for this? I can't, but without a test
it's near certain that this will regress again. (If you can live with that, I
can live with this going in without a test.)

[mistydemeo, 2013-05-03 04:23:09]

Thank you for the quick review!

On 2013/05/03 04:02:57, Nico wrote:
> Thank you for your patch!
> 
> I'm somewhat afraid that the blanket "catch" might mask problems when someone
> just hasn't set their path right though.

The way it's currently written, unfortunately, the exception will halt the gyp
configure process. That's what prevents CLT-only users from building anything
with a gyp buildsystem.

There isn't a reliable simple way to distinguish between the cases "user with
Xcode but path is unset" and "user with no Xcode", since in either case
xcodebuild and xcrun are unusable.

In Homebrew, we do the following to try and find the developer tools:
https://github.com/mxcl/homebrew/blob/master/Library/Homebrew/macos.rb#L63-L75
We assume the CLT is installed if the developer tools path is /usr/bin, and we
assume Xcode is installed if one of various conditions is met:
https://github.com/mxcl/homebrew/blob/master/Library/Homebrew/os/mac/xcode.rb...

I'm not sure how much detail you're interested in integrating to determine
whether Xcode/CLT are available at all.

I suppose a simpler option here could be to assume a) if xcodebuild works, Xcode
is installed; b) if xcodebuild doesn't work but a standard /usr/bin tool exists
(e.g. make), the CLT is installed; c) if xcodebuild is installed and there are
no dev tools in /usr/bin, the user has Xcode only without having run
xcode-select, or has no dev tools at all.

> Can you instead change the three places that call _SdkPath() to only do so if
> "$(SDKROOT)" is in the string? i.e. instead of
> 
>   s.replace("$(SDKROOT)", self._SdkPath())
> 
> write
> 
>   if "$(SDKROOT)" in s:
>     s = s.replace("$(SDKROOT)", self._SdkPath())
> 
> I think this should fix your issue as well as long as your gyp file doesn't
use
> $(SDKROOT) (and if it does, your patch doesn't do the right thing anyways).

True, returning a path with "$(SDKROOT)" wouldn't help. I haven't actually seen
any gyp files with "$(SDKROOT)", do you have an example?

> Can you think of a way to write a test for this? I can't, but without a test
> it's near certain that this will regress again. (If you can live with that, I
> can live with this going in without a test.)

It is a tricky thing to test. I haven't looked into how gyp's tests are set up
too specifically, but I suppose mocking xcodebuild's behaviour would work.

[Nico, 2013-05-03 04:40:14]

On 2013/05/03 04:23:09, mistydemeo wrote:
> Thank you for the quick review!
> 
> On 2013/05/03 04:02:57, Nico wrote:
> > Thank you for your patch!
> > 
> > I'm somewhat afraid that the blanket "catch" might mask problems when
someone
> > just hasn't set their path right though.
> 
> The way it's currently written, unfortunately, the exception will halt the gyp
> configure process. That's what prevents CLT-only users from building anything
> with a gyp buildsystem.
> 
> There isn't a reliable simple way to distinguish between the cases "user with
> Xcode but path is unset" and "user with no Xcode", since in either case
> xcodebuild and xcrun are unusable.

Oh, interesting. I was planning on using xcrun more, so that gyp works with just
Xcode and no command line tools. I wasn't aware that just command line tools and
not Xcode is also an interesting use case for some.

> In Homebrew, we do the following to try and find the developer tools:
> https://github.com/mxcl/homebrew/blob/master/Library/Homebrew/macos.rb#L63-L75
> We assume the CLT is installed if the developer tools path is /usr/bin, and we
> assume Xcode is installed if one of various conditions is met:
>
https://github.com/mxcl/homebrew/blob/master/Library/Homebrew/os/mac/xcode.rb...
> 
> I'm not sure how much detail you're interested in integrating to determine
> whether Xcode/CLT are available at all.

If we move to more xcrun in the future, I need to tackle this (to not break your
use case).

For this specific case, just not calling _SdkPath() if no string contains
"$(SDKROOT)" should be good enough to get things going again though, so I think
I'd prefer to do this for now.

> 
> I suppose a simpler option here could be to assume a) if xcodebuild works,
Xcode
> is installed; b) if xcodebuild doesn't work but a standard /usr/bin tool
exists
> (e.g. make), the CLT is installed; c) if xcodebuild is installed and there are
> no dev tools in /usr/bin, the user has Xcode only without having run
> xcode-select, or has no dev tools at all.
> 
> > Can you instead change the three places that call _SdkPath() to only do so
if
> > "$(SDKROOT)" is in the string? i.e. instead of
> > 
> >   s.replace("$(SDKROOT)", self._SdkPath())
> > 
> > write
> > 
> >   if "$(SDKROOT)" in s:
> >     s = s.replace("$(SDKROOT)", self._SdkPath())
> > 
> > I think this should fix your issue as well as long as your gyp file doesn't
> use
> > $(SDKROOT) (and if it does, your patch doesn't do the right thing anyways).
> 
> True, returning a path with "$(SDKROOT)" wouldn't help. I haven't actually
seen
> any gyp files with "$(SDKROOT)", do you have an example?

We use it fairly heavily in chromium:
https://code.google.com/p/chromium/codesearch#search/&q=%5C$%5C(SDKROOT%5C)&s...

> 
> > Can you think of a way to write a test for this? I can't, but without a test
> > it's near certain that this will regress again. (If you can live with that,
I
> > can live with this going in without a test.)
> 
> It is a tricky thing to test. I haven't looked into how gyp's tests are set up
> too specifically, but I suppose mocking xcodebuild's behaviour would work.

The existing tests are in the test/ directory; mac-specific tests are mostly i
test/mac. You can run the tests with `python gyptest.py -a`. You can pass `-f
ninja` to run only the ninja tests, `-f make,xcodebuild` to run make and
xcodebuild, and you can pass `test/mac` instead of `-a` to run only the tests
below test/mac instead of all of them. The test runner logic is implemented in
test/lib.

[mistydemeo, 2013-05-03 05:03:19]

On 2013/05/03 04:40:14, Nico wrote:
> On 2013/05/03 04:23:09, mistydemeo wrote:
> > Thank you for the quick review!
> > 
> > On 2013/05/03 04:02:57, Nico wrote:
> > > Thank you for your patch!
> > > 
> > > I'm somewhat afraid that the blanket "catch" might mask problems when
> someone
> > > just hasn't set their path right though.
> > 
> > The way it's currently written, unfortunately, the exception will halt the
gyp
> > configure process. That's what prevents CLT-only users from building
anything
> > with a gyp buildsystem.
> > 
> > There isn't a reliable simple way to distinguish between the cases "user
with
> > Xcode but path is unset" and "user with no Xcode", since in either case
> > xcodebuild and xcrun are unusable.
> 
> Oh, interesting. I was planning on using xcrun more, so that gyp works with
just
> Xcode and no command line tools. I wasn't aware that just command line tools
and
> not Xcode is also an interesting use case for some.

Yes, there are developers who mainly build command-line Unix software who prefer
to only install the ~150MB CLT instead of the multi-gig Xcode.

The xcrun script in /usr/bin is just a shim for the real xcrun that only comes
in Xcode, and is completely nonfunctional without Xcode and a correct
xcode-select path. (In fact, there's an infinite loop condition if the
xcode-select path is set to '/' that you should keep in mind if you're going to
be using it more heavily!)

> > In Homebrew, we do the following to try and find the developer tools:
> >
https://github.com/mxcl/homebrew/blob/master/Library/Homebrew/macos.rb#L63-L75
> > We assume the CLT is installed if the developer tools path is /usr/bin, and
we
> > assume Xcode is installed if one of various conditions is met:
> >
>
https://github.com/mxcl/homebrew/blob/master/Library/Homebrew/os/mac/xcode.rb...
> > 
> > I'm not sure how much detail you're interested in integrating to determine
> > whether Xcode/CLT are available at all.
> 
> If we move to more xcrun in the future, I need to tackle this (to not break
your
> use case).

You might want to consider wrapping xcrun with a function that can also find CLT
tools. Homebrew's looks like this:
https://github.com/mxcl/homebrew/blob/master/Library/Homebrew/macos.rb#L37-L61

> For this specific case, just not calling _SdkPath() if no string contains
> "$(SDKROOT)" should be good enough to get things going again though, so I
think
> I'd prefer to do this for now.

It would still need to avoid _SdkPath() raising and taking the build down with
it.

> > I suppose a simpler option here could be to assume a) if xcodebuild works,
> Xcode
> > is installed; b) if xcodebuild doesn't work but a standard /usr/bin tool
> exists
> > (e.g. make), the CLT is installed; c) if xcodebuild is installed and there
are
> > no dev tools in /usr/bin, the user has Xcode only without having run
> > xcode-select, or has no dev tools at all.
> > 
> > > Can you instead change the three places that call _SdkPath() to only do so
> if
> > > "$(SDKROOT)" is in the string? i.e. instead of
> > > 
> > >   s.replace("$(SDKROOT)", self._SdkPath())
> > > 
> > > write
> > > 
> > >   if "$(SDKROOT)" in s:
> > >     s = s.replace("$(SDKROOT)", self._SdkPath())
> > > 
> > > I think this should fix your issue as well as long as your gyp file
doesn't
> > use
> > > $(SDKROOT) (and if it does, your patch doesn't do the right thing
anyways).
> > 
> > True, returning a path with "$(SDKROOT)" wouldn't help. I haven't actually
> seen
> > any gyp files with "$(SDKROOT)", do you have an example?
> 
> We use it fairly heavily in chromium:
>
https://code.google.com/p/chromium/codesearch#search/&q=%255C%24%255C%28SDKRO...

Thanks! In that case, deleting "$(SDKROOT)" should result in valid paths, so
rather than the example you gave, this should work:

replacement = self._SdkPath() or ''
s = s.replace("$(SDKROOT)", replacement)

> > > Can you think of a way to write a test for this? I can't, but without a
test
> > > it's near certain that this will regress again. (If you can live with
that,
> I
> > > can live with this going in without a test.)
> > 
> > It is a tricky thing to test. I haven't looked into how gyp's tests are set
up
> > too specifically, but I suppose mocking xcodebuild's behaviour would work.
> 
> The existing tests are in the test/ directory; mac-specific tests are mostly i
> test/mac. You can run the tests with `python gyptest.py -a`. You can pass `-f
> ninja` to run only the ninja tests, `-f make,xcodebuild` to run make and
> xcodebuild, and you can pass `test/mac` instead of `-a` to run only the tests
> below test/mac instead of all of them. The test runner logic is implemented in
> test/lib.

Thanks, I'll take a look into it.

Incidentally, what do you recommend to hide stderr using subprocess.popen()? I
don't write python too frequently.

[Nico, 2013-05-03 05:07:38]

> You might want to consider wrapping xcrun with a function that can also find
CLT
> tools. Homebrew's looks like this:
> https://github.com/mxcl/homebrew/blob/master/Library/Homebrew/macos.rb#L37-L61

Thanks, I'll give that a look!

> > We use it fairly heavily in chromium:
> >
>
https://code.google.com/p/chromium/codesearch#search/&q=%25255C%2524%25255C%2...
> 
> Thanks! In that case, deleting "$(SDKROOT)" should result in valid paths, so
> rather than the example you gave, this should work:
> 
> replacement = self._SdkPath() or ''
> s = s.replace("$(SDKROOT)", replacement)

Right, but then you still need to call _SdkPath(). I'm trying to prevent the
addition of the try:/catch: in that function.

> > The existing tests are in the test/ directory; mac-specific tests are mostly
i
> > test/mac. You can run the tests with `python gyptest.py -a`. You can pass
`-f
> > ninja` to run only the ninja tests, `-f make,xcodebuild` to run make and
> > xcodebuild, and you can pass `test/mac` instead of `-a` to run only the
tests
> > below test/mac instead of all of them. The test runner logic is implemented
in
> > test/lib.
> 
> Thanks, I'll take a look into it.
> 
> Incidentally, what do you recommend to hide stderr using subprocess.popen()? I
> don't write python too frequently.

I think the usual way is

  devnull = open(os.devnull, 'w')
  subprocess.Popen(or call or whatevs)(..., stderr=devnull)

If you want to be tidy, you can then close devnull afterwards, or do

  with open(os.devnull, "w") as devnull:
    subprocess…