Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1200833013:
Use third_party clang on Mac. (Closed)
|
Created:
5 years, 6 months ago by balazs.kilvady Modified:
5 years, 5 months ago CC:
v8-dev Base URL:
https://chromium.googlesource.com/v8/v8.git@master Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
DescriptionUse third_party clang on Mac.
BUG=
Committed: https://crrev.com/57b20413294940476ab2b2e71ed0802aaf8c223f
Cr-Commit-Position: refs/heads/master@{#29343}
Patch Set 1 #Patch Set 2 : Rebase and fix Xcode config. #
Total comments: 2
Patch Set 3 : Fix nit #Patch Set 4 : Rebased on Michael's CL. #Messages
Total messages: 31 (10 generated)
The CQ bit was checked by balazs.kilvady@imgtec.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1200833013/1
The CQ bit was unchecked by balazs.kilvady@imgtec.com
The CQ bit was checked by balazs.kilvady@imgtec.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1200833013/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
balazs.kilvady@imgtec.com changed reviewers: + akos.palfi@imgtec.com, danno@chromium.org, jkummerow@chromium.org, paul.lind@imgtec.com
This patch allows to use the same version of clang (third_party/llvm/clang) on Mac and Linux. Works from Xcode and command line build also. And with v8 buildbots: https://codereview.chromium.org/1214753002/
paul.lind@imgtec.com changed reviewers: + machenbach@chromium.org
+ machenbach
lgtm
The CQ bit was checked by balazs.kilvady@imgtec.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1200833013/20001
The CQ bit was unchecked by jkummerow@chromium.org
How about rebasing this onto https://codereview.chromium.org/1217783002/ which computes base_dir for all configurations? https://codereview.chromium.org/1200833013/diff/20001/build/standalone.gypi File build/standalone.gypi (right): https://codereview.chromium.org/1200833013/diff/20001/build/standalone.gypi#n... build/standalone.gypi:778: 'CLANG_CXX_LIBRARY': 'libc++' nit: trailing comma for consistency
On 2015/06/29 09:58:48, Jakob wrote: > How about rebasing this onto https://codereview.chromium.org/1217783002/ which > computes base_dir for all configurations? That is a better way. I also fixed the nit, thanks for catching it. Can I cancel the commit or should I make a followup CL?
On 2015/06/29 10:11:26, balazs.kilvady wrote: > Can I cancel the commit or should I make a followup CL? I have already un-checked the "commit" check box, so you can rebase and upload a new patch set.
Rebased on https://codereview.chromium.org/1217783002/ https://codereview.chromium.org/1200833013/diff/20001/build/standalone.gypi File build/standalone.gypi (right): https://codereview.chromium.org/1200833013/diff/20001/build/standalone.gypi#n... build/standalone.gypi:778: 'CLANG_CXX_LIBRARY': 'libc++' On 2015/06/29 09:58:48, Jakob wrote: > nit: trailing comma for consistency Done.
lgtm
The CQ bit was checked by balazs.kilvady@imgtec.com
The patchset sent to the CQ was uploaded after l-g-t-m from machenbach@chromium.org Link to the patchset: https://codereview.chromium.org/1200833013/#ps60001 (title: "Rebased on Michael's CL.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1200833013/60001
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/57b20413294940476ab2b2e71ed0802aaf8c223f Cr-Commit-Position: refs/heads/master@{#29343}
Message was sent while issue was closed.
A revert of this CL (patchset #4 id:60001) has been created in https://codereview.chromium.org/1217823003/ by machenbach@chromium.org. The reason for reverting is: [Sheriff] Breaks mac on the main waterfall (though not on the tryserver): http://build.chromium.org/p/client.v8/builders/V8%20Mac64/builds/3925.
Message was sent while issue was closed.
Michael, Jakob, Could you tell me what is the status of c++11 usage of V8 on mac? If we can’t use the third_party/…/clang on mac then why the gclient sync downloads it and why the buildbots are using it? Anyway it seems we don’t have any chance to use c++11: numeric_limits::lowest(), std::unordered_set and so on (while they can be used on Linux). So Chrom[e|ium] builds/bundles might contain the proper libc++ on macs to support versions back to 10.5. Can we use that?
Message was sent while issue was closed.
I don't know. I'd think that using clang on Mac is possible, but I haven't tried it. It looks like Chromium only sets CLANG_CXX_LIBRARY when building for iOS. Can you reproduce the failure that the bots hit locally? Does it go away if you drop the CLANG_CXX_LIBRARY definition? Or maybe if you change it from libc++ to libstdc++? (I'm just guessing wildly; there is a third_party/llvm-build/Release+Asserts/lib/libstdc++.so.6 but no libc++.)
Message was sent while issue was closed.
Jakob, thaks for the answer. On 2015/06/30 12:21:17, Jakob wrote: > I don't know. I'd think that using clang on Mac is possible, but I haven't tried > it. Yes, till now we used Xcode's clang (which is a modified 3.6) running current gyp. But we could not use c++11 with it as c++11 is only supported using libc++ with minimal target OS X 10.7 and up by Xcode. > > It looks like Chromium only sets CLANG_CXX_LIBRARY when building for iOS. Can > you reproduce the failure that the bots hit locally? Does it go away if you drop No, I have the latest 10.10.3 which has a proper libc++ in /usr/lib so the patch works fine on my mac. I think the bot is running OS X 10.6 without /usr/lib/libc++. > the CLANG_CXX_LIBRARY definition? Or maybe if you change it from libc++ to Without CLANG_CXX_LIBRARY def. Xcode uses an stdlibc++ from gcc 4.2.1 (the version where Apple stopped gcc support and it is from pre-c11 era). > libstdc++? (I'm just guessing wildly; there is a > third_party/llvm-build/Release+Asserts/lib/libstdc++.so.6 but no libc++.) third_party/llvm-build/Release+Asserts/lib/libstdc++.so.6 exists only on Linux, on mac gclient sync downloads third_party/llvm-build/Release+Asserts/bin/libc++.1.dylib. I tried to use it with an -rpath like solution but have 2 problems with it: 1) it is a 64 bit version so I can compile only mac64 build with it. (gclient sync might download a 32/64 fat dylib to make this solution work) 2) the build would not run on a non-v8-developer mac (at least on 10.5 or 10.6).
Message was sent while issue was closed.
On 2015/06/30 12:53:42, balazs.kilvady wrote: > Without CLANG_CXX_LIBRARY def. Xcode uses an stdlibc++ from gcc 4.2.1 (the > version where Apple stopped gcc support and it is from pre-c11 era). That may be fine. The Chromium style guide explicitly forbids C++11 features that require library support, because such libraries are not available on all platforms we're targeting. Only purely compiler-based C++11 features are allowed currently.
Message was sent while issue was closed.
On 2015/06/30 14:11:30, Jakob wrote: > On 2015/06/30 12:53:42, balazs.kilvady wrote: > > Without CLANG_CXX_LIBRARY def. Xcode uses an stdlibc++ from gcc 4.2.1 (the > > version where Apple stopped gcc support and it is from pre-c11 era). > > That may be fine. The Chromium style guide explicitly forbids C++11 features > that require library support, because such libraries are not available on all > platforms we're targeting. Only purely compiler-based C++11 features are allowed > currently. I understood so removing usage of std::unordered_set and std::numeric_limits<>::lowest() suits to Chromium style guide. Thank you very much for the explanation. |
