Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1886293002:
[build] Port options to support win-clang (Closed)
|
Created:
4 years, 8 months ago by Michael Achenbach Modified:
4 years, 8 months ago CC:
v8-reviews_googlegroups.com Base URL:
https://chromium.googlesource.com/v8/v8.git@master Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
Description[build] Port options to support win-clang
BUG=chromium:603011
LOG=n
NOTRY=true
Committed: https://crrev.com/a1fa6d0be514ea3dbeac04de8652e3a13e671418
Cr-Commit-Position: refs/heads/master@{#35495}
Patch Set 1 #
Total comments: 3
Messages
Total messages: 22 (8 generated)
The CQ bit was checked by machenbach@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1886293002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1886293002/1
Description was changed from ========== [build] Port options to support win-clang BUG= ========== to ========== [build] Port options to support win-clang BUG=chromium:603011 LOG=n ==========
machenbach@chromium.org changed reviewers: + jochen@chromium.org, thakis@chromium.org
PTAL. C/p from common.gypi
lgtm
Description was changed from ========== [build] Port options to support win-clang BUG=chromium:603011 LOG=n ========== to ========== [build] Port options to support win-clang BUG=chromium:603011 LOG=n NOTRY=true ==========
The CQ bit was unchecked by machenbach@chromium.org
The CQ bit was checked by machenbach@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1886293002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1886293002/1
Message was sent while issue was closed.
Description was changed from ========== [build] Port options to support win-clang BUG=chromium:603011 LOG=n NOTRY=true ========== to ========== [build] Port options to support win-clang BUG=chromium:603011 LOG=n NOTRY=true ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== [build] Port options to support win-clang BUG=chromium:603011 LOG=n NOTRY=true ========== to ========== [build] Port options to support win-clang BUG=chromium:603011 LOG=n NOTRY=true Committed: https://crrev.com/a1fa6d0be514ea3dbeac04de8652e3a13e671418 Cr-Commit-Position: refs/heads/master@{#35495} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/a1fa6d0be514ea3dbeac04de8652e3a13e671418 Cr-Commit-Position: refs/heads/master@{#35495}
Message was sent while issue was closed.
https://codereview.chromium.org/1886293002/diff/1/build/standalone.gypi File build/standalone.gypi (right): https://codereview.chromium.org/1886293002/diff/1/build/standalone.gypi#newco... build/standalone.gypi:969: '-fmsc-version=1900', 1900 means msvc2015. If you're still on 2013, you might want 1800 here
Message was sent while issue was closed.
https://codereview.chromium.org/1886293002/diff/1/build/standalone.gypi File build/standalone.gypi (right): https://codereview.chromium.org/1886293002/diff/1/build/standalone.gypi#newco... build/standalone.gypi:969: '-fmsc-version=1900', On 2016/04/14 13:54:12, Nico wrote: > 1900 means msvc2015. If you're still on 2013, you might want 1800 here Yea - but no point in supporting also 2013 with clang as we never supported win-clang...
Message was sent while issue was closed.
@Nico: Any hint for this one? https://build.chromium.org/p/client.v8/builders/V8%20Win64%20-%20clang/builds...
Message was sent while issue was closed.
For `error: argument unused during compilation: '/GS'`, you have two options: 1. Add -Qunused-arguments 2. Don't pass /GS and /GL if clang==1 See https://codereview.chromium.org/1828543003 for reference. I'd recommend 1 for you, and once we've implemented /GS you can just remove that. For Wparentheses in icu: third_party/icu/icu.gyp has a "clang_warning_flags" block that disables -Wparentheses for that project. If you copy https://code.google.com/p/chromium/codesearch#chromium/src/build/set_clang_wa... into your standalone build, then that should go away. (Looks like pdfium just copied that file too. See https://codereview.chromium.org/1532723003 for how this change looked in pdfium.
Message was sent while issue was closed.
https://codereview.chromium.org/1886293002/diff/1/build/standalone.gypi File build/standalone.gypi (right): https://codereview.chromium.org/1886293002/diff/1/build/standalone.gypi#newco... build/standalone.gypi:969: '-fmsc-version=1900', On 2016/04/14 14:03:42, Michael Achenbach wrote: > On 2016/04/14 13:54:12, Nico wrote: > > 1900 means msvc2015. If you're still on 2013, you might want 1800 here > > Yea - but no point in supporting also 2013 with clang as we never supported > win-clang... In general, I feel you'll have fewest problems if the version here matches the versions of the MSVC headers you're using. If you're using the 2015 headers, this _must_ be 1900 since clang-cl in 2013 mode can't parse the 2015 headers. If you're using clang-cl in 2015 mode with the 2013 headers, then that might work, but you're using a configuration that as far as I know nobody else uses.
Message was sent while issue was closed.
On 2016/04/14 14:48:07, Nico wrote: > https://codereview.chromium.org/1886293002/diff/1/build/standalone.gypi > File build/standalone.gypi (right): > > https://codereview.chromium.org/1886293002/diff/1/build/standalone.gypi#newco... > build/standalone.gypi:969: '-fmsc-version=1900', > On 2016/04/14 14:03:42, Michael Achenbach wrote: > > On 2016/04/14 13:54:12, Nico wrote: > > > 1900 means msvc2015. If you're still on 2013, you might want 1800 here > > > > Yea - but no point in supporting also 2013 with clang as we never supported > > win-clang... > > In general, I feel you'll have fewest problems if the version here matches the > versions of the MSVC headers you're using. If you're using the 2015 headers, > this _must_ be 1900 since clang-cl in 2013 mode can't parse the 2015 headers. If > you're using clang-cl in 2015 mode with the 2013 headers, then that might work, > but you're using a configuration that as far as I know nobody else uses. Thanks, will try. Note that before the msvs2015 switch attempt, we failed already in runhooks: https://build.chromium.org/p/client.v8/builders/V8%20Win64%20-%20clang/builds... So there's something else wrong to support msvs2013 and clang.
Message was sent while issue was closed.
On 2016/04/14 15:05:10, Michael Achenbach wrote: > On 2016/04/14 14:48:07, Nico wrote: > > https://codereview.chromium.org/1886293002/diff/1/build/standalone.gypi > > File build/standalone.gypi (right): > > > > > https://codereview.chromium.org/1886293002/diff/1/build/standalone.gypi#newco... > > build/standalone.gypi:969: '-fmsc-version=1900', > > On 2016/04/14 14:03:42, Michael Achenbach wrote: > > > On 2016/04/14 13:54:12, Nico wrote: > > > > 1900 means msvc2015. If you're still on 2013, you might want 1800 here > > > > > > Yea - but no point in supporting also 2013 with clang as we never supported > > > win-clang... > > > > In general, I feel you'll have fewest problems if the version here matches the > > versions of the MSVC headers you're using. If you're using the 2015 headers, > > this _must_ be 1900 since clang-cl in 2013 mode can't parse the 2015 headers. > If > > you're using clang-cl in 2015 mode with the 2013 headers, then that might > work, > > but you're using a configuration that as far as I know nobody else uses. > > Thanks, will try. Note that before the msvs2015 switch attempt, we failed > already in runhooks: > https://build.chromium.org/p/client.v8/builders/V8%20Win64%20-%20clang/builds... > > So there's something else wrong to support msvs2013 and clang. Heh, I broke that 2 days ago: https://codereview.chromium.org/1879753002/ It didn't occur to me that people will use older MSVCs still. It's easy to fix, just copy msdia120.dll instead of msdia140.dll with 2013, or just don't copy msdia140.dll if it doesn't exist (it's currently only needed for running llvm tests; eventually it'll be used for symbolizing asan stacks too). So if you think you'll stay for 2013 a bit longer, you can make that work.
Message was sent while issue was closed.
On 2016/04/14 15:17:58, Nico (hiding Wed-Thu) wrote: > On 2016/04/14 15:05:10, Michael Achenbach wrote: > > On 2016/04/14 14:48:07, Nico wrote: > > > https://codereview.chromium.org/1886293002/diff/1/build/standalone.gypi > > > File build/standalone.gypi (right): > > > > > > > > > https://codereview.chromium.org/1886293002/diff/1/build/standalone.gypi#newco... > > > build/standalone.gypi:969: '-fmsc-version=1900', > > > On 2016/04/14 14:03:42, Michael Achenbach wrote: > > > > On 2016/04/14 13:54:12, Nico wrote: > > > > > 1900 means msvc2015. If you're still on 2013, you might want 1800 here > > > > > > > > Yea - but no point in supporting also 2013 with clang as we never > supported > > > > win-clang... > > > > > > In general, I feel you'll have fewest problems if the version here matches > the > > > versions of the MSVC headers you're using. If you're using the 2015 headers, > > > this _must_ be 1900 since clang-cl in 2013 mode can't parse the 2015 > headers. > > If > > > you're using clang-cl in 2015 mode with the 2013 headers, then that might > > work, > > > but you're using a configuration that as far as I know nobody else uses. > > > > Thanks, will try. Note that before the msvs2015 switch attempt, we failed > > already in runhooks: > > > https://build.chromium.org/p/client.v8/builders/V8%20Win64%20-%20clang/builds... > > > > So there's something else wrong to support msvs2013 and clang. > > Heh, I broke that 2 days ago: https://codereview.chromium.org/1879753002/ It > didn't occur to me that people will use older MSVCs still. It's easy to fix, > just copy msdia120.dll instead of msdia140.dll with 2013, or just don't copy > msdia140.dll if it doesn't exist (it's currently only needed for running llvm > tests; eventually it'll be used for symbolizing asan stacks too). So if you > think you'll stay for 2013 a bit longer, you can make that work. This requires a chromium-side fix as we deps in the update.py script. Trying here: https://codereview.chromium.org/1888883004/ |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
