- Using LLVM_WINSYSROOT would pick up DIA SDK path automatically, otherwise llvm-pdbutil has no DIA support.
- Add MSVC_VER to specify VC tools version.
- Make MSVC_VER/WINSDK_VER optional. If not specified, use the highest version number like the driver does.
/winsysroot is somewhat new, but I suppose that's fine (?)
Looks fine to me (haven't tried it though). The only real question below is where and how winsysroot gets added as flag :)
clang does this internally if they're not defined. Maybe add -vctoolsversion only if MSVC_VER is defined, and -winsdkversion only if WINSDK_VER is defined?
MSVC_VER and WINSDK_VER are optional, so you probably should only check for the sysroot
given that this is only set to check for existence, maybe this variable (and the check against it) isn't needed?
Do you want to add /winsysroot down here too? Or how is that flag getting added?
FWIW in Chromium we build our lld-link against libxml2 (with almost everything disabled) these days, and it can do /manifest handling without mt.exe if build that way.
Thanks for the review!
yeah, considered that. Later I found out I still need MSVC_VER and WINSDK_VER to specify libpath for the linker. Guess we need to wait until lld-link accepts /winsysroot...
same. Optional for the user, still need it to specify linker libpath.
Thanks for the info. I'll try removing it locally.
Oh, okay. It seems LLVM Windows release build does not link libxml2 yet. Is it possible to upstream your code (https://chromium.googlesource.com/chromium/src/+/66ff126e39334b72ec6f23b6d28a60e7efbbc019/tools/clang/scripts/build.py#281) to llvm/utils/release/build_llvm_package.bat so the release lld-link works with /manifest?
Not all parts of LLVM's build call HandleLLVMOptions, e.g. compiler-rt doesn't. We should add the flag explicitly to handle those cases as well (and I don't think having it specified twice should cause any harm). I'll put up a patch.
I believe that was the case for earlier versions as well (at least 2015 and 2017), but this is intended for cross-compilation, so the idea was that you'd arrange the SDK and VC directories in the layout expected by /winsysroot.