[PATCH] D118070: Add /winsysroot support to lld

[Nico Weber via Phabricator via llvm-commits]

thakis added a subscriber: ychen.
thakis added a comment.

Very very cool.

This needs a test. Maybe you just forgot to `git add` it?

It'd be nice if the knowledge what the sysroot layout looks like (a `Windows\ Kits/10/...` dir, a `VC/Tools/MSVC/*/` dir, optionally a `DIA SDK` dir) wasn't duplicated between here and clang. Could we pull that out into some common code in llvm/lib/Support and call that in both places?

(@ychen fyi wrt D117852 <https://reviews.llvm.org/D117852>)



================
Comment at: lld/COFF/Driver.cpp:257
+
+  path::append(SDKPath, "Include");
+  Version = getHighestNumericTupleInDirectory(SDKPath);
----------------
`Include` seems like the wrong directory to look in in the linker (…i think?)


================
Comment at: lld/COFF/Driver.cpp:654
     case OPT_out:
-      os << arg->getSpelling() << sys::path::filename(arg->getValue()) << "\n";
       break;
----------------
there's a bunch of these unrelated `sys::` removals sprinkled through the patch. send them in a separate patch, they're unrelated.


================
Comment at: lld/COFF/Driver.cpp:687
 void LinkerDriver::addLibSearchPaths() {
   Optional<std::string> envOpt = Process::GetEnv("LIB");
   if (!envOpt.hasValue())
----------------
We shouldn't look at `%LIB%` here when `/winsysroot:` is passed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118070/new/

https://reviews.llvm.org/D118070