Windows Clang build fails with DEPOT_TOOLS_WIN_TOOLCHAIN=0 |
|||||||||
Issue description
It seems the Windows Clang build doesn't work when not using the Visual Studio managed with depot_tools.
To reproduce, I ran:
set DEPOT_TOOLS_WIN_TOOLCHAIN=0
set WINDOWSSDKDIR=c:\Program Files (x86)\Microsoft SDKs\Windows\v7.0A
and gn'd with is_clang=true.
There are quotes missing around the -imsvc paths in toolchain.ninja:
rule cc
command = ../../../../llvm/build.release/bin/clang-cl.exe /nologo /showIncludes /FC @${out}.rsp /c ${in} /Fo${out} /Fd"${target_out_dir}/${label_name}_c.pdb"
description = CC ${out}
rspfile = ${out}.rsp
rspfile_content = -imsvcC:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\INCLUDE -imsvcC:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\ATLMFC\INCLUDE -imsvcC:\Program Files (x86)\Windows Kits\8.1\include\shared -imsvcC:\Program Files (x86)\Windows Kits\8.1\include\um -imsvcC:\Program Files (x86)\Windows Kits\8.1\include\winrt -imsvc ${defines} ${include_dirs} ${cflags} ${cflags_c}
deps = msvc
But more mysteriously, if one adds the quotes, the build fails with various errors, e.g.:
D:\src\chromium\src>ninja -C out\Debug obj/components/keyed_service/core/core/ke
yed_service.obj
ninja: Entering directory `out\Debug'
[13/13] CXX obj/components/keyed_service/core/core/keyed_service.obj
FAILED: obj/components/keyed_service/core/core/keyed_service.obj
../../../../llvm/build.release/bin/clang-cl.exe /nologo /showIncludes /FC @obj/c
omponents/keyed_service/core/core/keyed_service.obj.rsp /c ../../components/keye
d_service/core/keyed_service.cc /Foobj/components/keyed_service/core/core/keyed_
service.obj /Fd"obj/components/keyed_service/core/core_cc.pdb"
../../components/keyed_service/core/keyed_service.cc(7,15) : error: 'KeyedServi
ce::KeyedService' redeclared without 'dllimport' attribute: 'dllexport' attribut
e added [-Werror,-Winconsistent-dllimport]
KeyedService::KeyedService() {}
^
../..\components/keyed_service/core/keyed_service.h(18,3) : note: previous decl
aration is here
KeyedService();
^
../../components/keyed_service/core/keyed_service.cc(9,15) : error: 'KeyedServi
ce::~KeyedService' redeclared without 'dllimport' attribute: 'dllexport' attribu
te added [-Werror,-Winconsistent-dllimport]
KeyedService::~KeyedService() {}
^
../..\components/keyed_service/core/keyed_service.h(21,11) : note: previous dec
laration is here
virtual ~KeyedService();
^
../../components/keyed_service/core/keyed_service.cc(11,20) : error: 'KeyedServ
ice::Shutdown' redeclared without 'dllimport' attribute: 'dllexport' attribute a
dded [-Werror,-Winconsistent-dllimport]
void KeyedService::Shutdown() {}
^
../..\components/keyed_service/core/keyed_service.h(24,16) : note: previous dec
laration is here
virtual void Shutdown();
^
3 errors generated.
,
Aug 23 2016
I added the above mentioned argument and this time it only returned errors of the following type (I've enclosed the log of all errors): gen/third_party/iaccessible2/ia2_api_all_i.c(73,23): error: 'extern' variable has an initializer [-Werror,-Wextern-initializer] MIDL_DEFINE_GUID(IID, IID_IAccessibleAction,0xB70D9F59,0x3B5A,0x4dba,0xAB,0x9E,0x22,0x01,0x2F,0x60,0x7D,0xF5); Then I added "-Wno-extern-initializer" warning suppression command to toolchain.ninja and it compiled and built peacefully and the output runs. Is it safe to do it this way?
,
Aug 23 2016
It'll probably work (until it breaks), but it's not supported. The Right Way is to wait for this bug to be fixed (or fix it yourself, but we'll likely get to it today, so unless you're in a rush you can just wait). In general, things are supposed to just work without you having to hack stuff up locally -- and if things don't just work, it's a bug.
,
Aug 23 2016
rhalavati: Can you check if https://codereview.chromium.org/2276513002 helps?
,
Aug 23 2016
I've also had issue with unquoted paths.
I've tried the fix and that particular issue seems to be gone but still getting compilation errors mentioned above ('KeyedService::KeyedService' redeclared without 'dllimport' attribute: 'dllexport' attribute added [-Werror,-Winconsistent-dllimport]).
,
Aug 23 2016
Based on observation above, I've manually removed "-imsvc" snippets from toolchain.ninja and build seems to be progressing further. So maybe the correct fix would be to filter those out if there is no argument for it.
,
Aug 23 2016
,
Aug 23 2016
rchlodnicki: see patch set 2 on https://codereview.chromium.org/2276513002
,
Aug 23 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/42bff43d05a5272b3e697e52801c266c04b59bc8 commit 42bff43d05a5272b3e697e52801c266c04b59bc8 Author: Nico Weber <thakis@chromium.org> Date: Tue Aug 23 15:29:30 2016 clang/win: Support spaces in Visual Studio directory name The hermetic toolchain in depot_tools usually doesn't contain spaces, but system MSVC usually does ("Program Files"). Escape spaces once for the shell, and use gn_helpers to escape a second time for gn, instead of asserting that no " is in the path -- that's no longer true since the first level of quoting adds some. Also filter out empty include directories, since -msvc always combines with what's after it. BUG= 640000 R=hans@chromium.org, scottmg@chromium.org Review URL: https://codereview.chromium.org/2276513002 . Cr-Commit-Position: refs/heads/master@{#413742} [modify] https://crrev.com/42bff43d05a5272b3e697e52801c266c04b59bc8/build/toolchain/win/setup_toolchain.py
,
Aug 23 2016
,
Aug 23 2016
Everyone who ran into this, please try again after syncing past #413742 locally.
,
Aug 24 2016
I compiled again using https://codereview.chromium.org/2276513002, the initial errors are resolved, but the one stated in Comment 2 still happens.
,
Aug 24 2016
Are you using update 3?
,
Aug 24 2016
Msvc 2015 update 3 that is
,
Aug 24 2016
Yes, CL version is 19.00.24213.1
,
Aug 24 2016
Ah ok, then that's bug 637456 – let's mark this bug as Fixed again since things now work with 2015 update 2 (which is what all the bots use). I suppose we can land a workaround for folks using update 3 locally without using the hermetic toolchain until the real fix is deployed. I'll make a CL for that (on the other bug, so star that if you want to know when it lands -- should be in a few hours).
,
Aug 24 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/109db8b6fcdb52a55e40c44128a9cf36c00a5649 commit 109db8b6fcdb52a55e40c44128a9cf36c00a5649 Author: thakis <thakis@chromium.org> Date: Wed Aug 24 15:57:13 2016 clang/win: Make builds work with MSVC2015 update 3. The midl compiler now produces slightly questionable code that clang-cl warns on. The warning is no longer emitted in upstream clang-cl, but until we roll that in, suppress it so that people who locally use update 3 are able to build with clang. BUG= 637456 , 627216 , 640000 NOTRY=true Review-Url: https://codereview.chromium.org/2270693008 Cr-Commit-Position: refs/heads/master@{#414083} [modify] https://crrev.com/109db8b6fcdb52a55e40c44128a9cf36c00a5649/build/config/win/BUILD.gn
,
Jun 14 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b2293631d4e6450376aa4c8f21e593b4cc2824e1 commit b2293631d4e6450376aa4c8f21e593b4cc2824e1 Author: Nico Weber <thakis@chromium.org> Date: Thu Jun 14 20:53:23 2018 win: Only quote /I and -imsvc flags if needed. After https://chromium-review.googlesource.com/1075947, quoting is never needed with the hermetic msvc toolchain, so let's make the build command a tiny bit nicer in that scenario. Bug: 640000 ,439949 Change-Id: I22e3470057b43fb6a5df3f675bc95a7d84dce9c9 Reviewed-on: https://chromium-review.googlesource.com/1101557 Commit-Queue: Nico Weber <thakis@chromium.org> Reviewed-by: Scott Graham <scottmg@chromium.org> Cr-Commit-Position: refs/heads/master@{#567401} [modify] https://crrev.com/b2293631d4e6450376aa4c8f21e593b4cc2824e1/build/toolchain/win/setup_toolchain.py |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by h...@chromium.org
, Aug 22 2016