Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1926633004:
Use third_party clang on Mac. (Closed)
|
Created:
4 years, 7 months ago by balazs.kilvady Modified:
4 years, 7 months ago Reviewers:
ivica.bogosavljevic, jochen (gone - plz use gerrit), Michael Achenbach, Jakob Kummerow, gergely.kis.imgtec, akos.palfi.imgtec CC:
jochen (gone - plz use gerrit), v8-reviews_googlegroups.com 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.
-Wno-undefined-var-template flag is unknown by Xcode's clang. And it is
better to use the same clang version that is used on linux.
BUG=
Committed: https://crrev.com/683730b1d0bd679e42fbfa106328ffdf14886171
Cr-Commit-Position: refs/heads/master@{#35867}
Patch Set 1 #
Total comments: 1
Patch Set 2 : Tie it to clang_xcode. #Messages
Total messages: 25 (11 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/1926633004/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1926633004/1
Description was changed from ========== MIPS: Use third_party clang on Mac. -Wno-undefined-var-template flag is unknown by Xcode's clang. And it is better to use the same clang version that is used on linux. BUG= ========== to ========== MIPS: Use third_party clang on Mac. -Wno-undefined-var-template flag is unknown by Xcode's clang. And it is better to use the same clang version that is used on linux. BUG= ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== MIPS: Use third_party clang on Mac. -Wno-undefined-var-template flag is unknown by Xcode's clang. And it is better to use the same clang version that is used on linux. BUG= ========== to ========== Use third_party clang on Mac. -Wno-undefined-var-template flag is unknown by Xcode's clang. And it is better to use the same clang version that is used on linux. BUG= ==========
PTAL. Also a question: Since the minimum required mac OS version raised from 10.5 to 10.7 we could use libc++ and c++11 STL (like std::function<>). Is c++11 STL still not allowed for v8?
We allow the same as chromium does unless it is maybe blocked on MSVS right now. Maybe jochen knows more. https://codereview.chromium.org/1926633004/diff/1/build/standalone.gypi File build/standalone.gypi (right): https://codereview.chromium.org/1926633004/diff/1/build/standalone.gypi#newco... build/standalone.gypi:1037: 'xcode_settings': { I'm hesitant doing things different than in chromium's setup. I thought this: https://code.google.com/p/chromium/codesearch#chromium/src/v8/build/standalon... would be enough. clang_xcode==0 is the default. Can you check chromium's config in regards to this? https://code.google.com/p/chromium/codesearch#chromium/src/build/common.gypi&... There are a bunch of differences, but I don't want to get too much out of sync.
jochen@chromium.org changed reviewers: + jochen@chromium.org
afaik it's allowed. http://chromium-cpp.appspot.com/ lists the allowed and disallowed features. For the exact config, I'd ask thakis@
On 2016/04/28 12:08:00, Michael Achenbach wrote: > https://codereview.chromium.org/1926633004/diff/1/build/standalone.gypi > File build/standalone.gypi (right): > > https://codereview.chromium.org/1926633004/diff/1/build/standalone.gypi#newco... > build/standalone.gypi:1037: 'xcode_settings': { > I'm hesitant doing things different than in chromium's setup. I thought this: > https://code.google.com/p/chromium/codesearch#chromium/src/v8/build/standalon... > would be enough. clang_xcode==0 is the default. I think GYP on Mac always uses xcode_settings rather than make_global_settings, even when using the Make generator. > Can you check chromium's config in regards to this? > https://code.google.com/p/chromium/codesearch#chromium/src/build/common.gypi&... AFAICS Chromium does the exact same thing: https://code.google.com/p/chromium/codesearch#chromium/src/build/common.gypi&... So this CL looks good to me.
On 2016/04/28 12:13:25, Jakob wrote: > On 2016/04/28 12:08:00, Michael Achenbach wrote: > > https://codereview.chromium.org/1926633004/diff/1/build/standalone.gypi > > File build/standalone.gypi (right): > > > > > https://codereview.chromium.org/1926633004/diff/1/build/standalone.gypi#newco... > > build/standalone.gypi:1037: 'xcode_settings': { > > I'm hesitant doing things different than in chromium's setup. I thought this: > > > https://code.google.com/p/chromium/codesearch#chromium/src/v8/build/standalon... > > would be enough. clang_xcode==0 is the default. > > I think GYP on Mac always uses xcode_settings rather than make_global_settings, > even when using the Make generator. > > > Can you check chromium's config in regards to this? > > > https://code.google.com/p/chromium/codesearch#chromium/src/build/common.gypi&... > > AFAICS Chromium does the exact same thing: > https://code.google.com/p/chromium/codesearch#chromium/src/build/common.gypi&... > > So this CL looks good to me. lgtm with comment: Alright, but could you tie it to clang_xcode just like chromium?
On 2016/04/28 12:20:40, Michael Achenbach wrote: > On 2016/04/28 12:13:25, Jakob wrote: > > On 2016/04/28 12:08:00, Michael Achenbach wrote: > > > https://codereview.chromium.org/1926633004/diff/1/build/standalone.gypi > > > File build/standalone.gypi (right): > > > > > > > > > https://codereview.chromium.org/1926633004/diff/1/build/standalone.gypi#newco... > > > build/standalone.gypi:1037: 'xcode_settings': { > > > I'm hesitant doing things different than in chromium's setup. I thought > this: > > > > > > https://code.google.com/p/chromium/codesearch#chromium/src/v8/build/standalon... > > > would be enough. clang_xcode==0 is the default. > > > > I think GYP on Mac always uses xcode_settings rather than > make_global_settings, > > even when using the Make generator. > > > > > Can you check chromium's config in regards to this? > > > > > > https://code.google.com/p/chromium/codesearch#chromium/src/build/common.gypi&... > > > > AFAICS Chromium does the exact same thing: > > > https://code.google.com/p/chromium/codesearch#chromium/src/build/common.gypi&... > > > > So this CL looks good to me. > > lgtm with comment: > > Alright, but could you tie it to clang_xcode just like chromium? Thank you guys for the reviews/comments. I added the extra clang_xcode condition but am not so strong in gyp :) Could you take one more look if my modification is good enough for landing? My test shows it works as expected.
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/1926633004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1926633004/20001
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
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/1926633004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1926633004/20001
Message was sent while issue was closed.
Description was changed from ========== Use third_party clang on Mac. -Wno-undefined-var-template flag is unknown by Xcode's clang. And it is better to use the same clang version that is used on linux. BUG= ========== to ========== Use third_party clang on Mac. -Wno-undefined-var-template flag is unknown by Xcode's clang. And it is better to use the same clang version that is used on linux. BUG= ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Use third_party clang on Mac. -Wno-undefined-var-template flag is unknown by Xcode's clang. And it is better to use the same clang version that is used on linux. BUG= ========== to ========== Use third_party clang on Mac. -Wno-undefined-var-template flag is unknown by Xcode's clang. And it is better to use the same clang version that is used on linux. BUG= Committed: https://crrev.com/683730b1d0bd679e42fbfa106328ffdf14886171 Cr-Commit-Position: refs/heads/master@{#35867} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/683730b1d0bd679e42fbfa106328ffdf14886171 Cr-Commit-Position: refs/heads/master@{#35867} |
