Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
14333010:
Use clang on mac if XCode >= 4.5 (Closed)
|
Created:
7 years, 8 months ago by kustermann Modified:
7 years, 8 months ago CC:
reviews_dartlang.org Visibility:
Public. |
DescriptionUse clang on mac if XCode >= 4.5
Committed: https://code.google.com/p/dart/source/detail?r=21950
Patch Set 1 #
Total comments: 3
Patch Set 2 : #
Total comments: 5
Patch Set 3 : #Messages
Total messages: 12 (0 generated)
https://codereview.chromium.org/14333010/diff/1/tools/gyp/find_mac_gcc_versio... File tools/gyp/find_mac_gcc_version.py (right): https://codereview.chromium.org/14333010/diff/1/tools/gyp/find_mac_gcc_versio... tools/gyp/find_mac_gcc_version.py:31: raise Exception('Unknown XCode Version "%s"' % version_match) you can't hit this raise
https://codereview.chromium.org/14333010/diff/1/tools/gyp/find_mac_gcc_versio... File tools/gyp/find_mac_gcc_version.py (right): https://codereview.chromium.org/14333010/diff/1/tools/gyp/find_mac_gcc_versio... tools/gyp/find_mac_gcc_version.py:31: raise Exception('Unknown XCode Version "%s"' % version_match) Under what condition will you raise this exception here?
https://codereview.chromium.org/14333010/diff/1/tools/gyp/find_mac_gcc_versio... File tools/gyp/find_mac_gcc_version.py (right): https://codereview.chromium.org/14333010/diff/1/tools/gyp/find_mac_gcc_versio... tools/gyp/find_mac_gcc_version.py:31: raise Exception('Unknown XCode Version "%s"' % version_match) On 2013/04/24 13:48:09, ricow1 wrote: > you can't hit this raise True.
LGTM, but you might want to check with somebody who has a Xcode 4.6 machine as well. -Ivan
LGTM with small suggestion https://codereview.chromium.org/14333010/diff/5001/tools/gyp/find_mac_gcc_ver... File tools/gyp/find_mac_gcc_version.py (right): https://codereview.chromium.org/14333010/diff/5001/tools/gyp/find_mac_gcc_ver... tools/gyp/find_mac_gcc_version.py:28: elif major == 4 and minor == 5: minor >= 5? if xcode is selecting it automatically anyway I don't see why we should disallow people that at some point upgrades to compile
I don't think I'll find somebody here with 4.6 (it has been released on 28th of January and it will probably take a bit until we get the update). https://codereview.chromium.org/14333010/diff/5001/tools/gyp/find_mac_gcc_ver... File tools/gyp/find_mac_gcc_version.py (right): https://codereview.chromium.org/14333010/diff/5001/tools/gyp/find_mac_gcc_ver... tools/gyp/find_mac_gcc_version.py:28: elif major == 4 and minor == 5: On 2013/04/24 14:01:47, ricow1 wrote: > minor >= 5? > if xcode is selecting it automatically anyway I don't see why we should disallow > people that at some point upgrades to compile I discussed this with ivan offline: we can't be sure that the next XCode will ship with 'com.apple.compilers.llvm.clang.1_0' (maybe they'll ship with 'com.apple.compilers.llvm.clang.2_0'). But since this case is very unlikely, I could change it?
I'll land it now. https://codereview.chromium.org/14333010/diff/5001/tools/gyp/find_mac_gcc_ver... File tools/gyp/find_mac_gcc_version.py (right): https://codereview.chromium.org/14333010/diff/5001/tools/gyp/find_mac_gcc_ver... tools/gyp/find_mac_gcc_version.py:28: elif major == 4 and minor == 5: On 2013/04/24 14:01:47, ricow1 wrote: > minor >= 5? > if xcode is selecting it automatically anyway I don't see why we should disallow > people that at some point upgrades to compile Ivan pointed out we can install XCode 4.6. I tried it and it works there as well. I've changed it now to 'minor >= 5'.
Message was sent while issue was closed.
Committed patchset #3 manually as r21950 (presubmit successful).
Message was sent while issue was closed.
https://codereview.chromium.org/14333010/diff/5001/tools/gyp/find_mac_gcc_ver... File tools/gyp/find_mac_gcc_version.py (right): https://codereview.chromium.org/14333010/diff/5001/tools/gyp/find_mac_gcc_ver... tools/gyp/find_mac_gcc_version.py:28: elif major == 4 and minor == 5: I have XCode 4.6.1 (4H512) on my new laptop that I installed last week. It uses clang now to compile, and fails while compiling v8. Interestingly, I have 4.6 (4H127) on the desktop. It uses gcc (after your change)! The V8 build passes on this machine. On 2013/04/24 14:42:59, kustermann wrote: > On 2013/04/24 14:01:47, ricow1 wrote: > > minor >= 5? > > if xcode is selecting it automatically anyway I don't see why we should > disallow > > people that at some point upgrades to compile > > Ivan pointed out we can install XCode 4.6. I tried it and it works there as > well. I've changed it now to 'minor >= 5'.
Message was sent while issue was closed.
https://codereview.chromium.org/14333010/diff/5001/tools/gyp/find_mac_gcc_ver... File tools/gyp/find_mac_gcc_version.py (right): https://codereview.chromium.org/14333010/diff/5001/tools/gyp/find_mac_gcc_ver... tools/gyp/find_mac_gcc_version.py:28: elif major == 4 and minor == 5: On 2013/04/24 20:33:20, hausner wrote: > I have XCode 4.6.1 (4H512) on my new laptop that I installed last week. It uses > clang now to compile, and fails while compiling v8. > > Interestingly, I have 4.6 (4H127) on the desktop. It uses gcc (after your > change)! The V8 build passes on this machine. This should not happen. I tried it on my macbook (with 4.6.1 [4H512]) and it failed because of the warning in v8/src/parser.cc. Could you check if it still uses gcc after doing - 'gclient runhooks' (-> generating project files with clang instead of gcc) - removing the build folder (-> no incremental build of v8)
Message was sent while issue was closed.
> > Interestingly, I have 4.6 (4H127) on the desktop. It uses gcc (after your > > change)! The V8 build passes on this machine. > > This should not happen. I tried it on my macbook (with 4.6.1 [4H512]) and it > failed because of the warning in v8/src/parser.cc. > Could you check if it still uses gcc after doing > - 'gclient runhooks' (-> generating project files with clang instead of gcc) > - removing the build folder (-> no incremental build of v8) I did both of those steps. Sync, runhooks, rm -rf xcodebuild. It still selects gcc on my desktop. I even verified that find_mac_gcc_version.p executes the expected else branch (major == 4 and minor >= 5) by inserting an exception. It fired when I ran runhooks. |
