|
|
Created:
7 years, 7 months ago by mistydemeo Modified:
6 years, 9 months ago CC:
gyp-developer_googlegroups.com Base URL:
http://gyp.googlecode.com/svn/trunk Visibility:
Public. |
Descriptionxcode_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=
Patch Set 1 #
Messages
Total messages: 6 (0 generated)
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.
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.)
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.
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.
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.
> 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… |