[PATCH] D145517: MSVC: support version preference with search

[Saleem Abdulrasool via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGaf5f46822847: MSVC: support version preference with search 
(authored by compnerd).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145517

Files:
  clang/lib/Driver/ToolChains/MSVC.cpp
  lld/COFF/Driver.cpp
  llvm/include/llvm/WindowsDriver/MSVCPaths.h
  llvm/lib/ExecutionEngine/Orc/COFFVCRuntimeSupport.cpp
  llvm/lib/WindowsDriver/MSVCPaths.cpp

Index: llvm/lib/WindowsDriver/MSVCPaths.cpp
===
--- llvm/lib/WindowsDriver/MSVCPaths.cpp
+++ llvm/lib/WindowsDriver/MSVCPaths.cpp
@@ -609,8 +609,9 @@
   return false;
 }
 
-bool findVCToolChainViaSetupConfig(vfs::FileSystem , std::string ,
-   ToolsetLayout ) {
+bool findVCToolChainViaSetupConfig(vfs::FileSystem ,
+   std::optional VCToolsVersion,
+   std::string , ToolsetLayout ) {
 #if !defined(USE_MSVC_SETUP_API)
   return false;
 #else
@@ -677,17 +678,24 @@
   std::string VCRootPath;
   convertWideToUTF8(std::wstring(VCPathWide), VCRootPath);
 
-  SmallString<256> ToolsVersionFilePath(VCRootPath);
-  sys::path::append(ToolsVersionFilePath, "Auxiliary", "Build",
-"Microsoft.VCToolsVersion.default.txt");
+  std::string ToolsVersion;
+  if (VCToolsVersion.has_value()) {
+ToolsVersion = *VCToolsVersion;
+  } else {
+SmallString<256> ToolsVersionFilePath(VCRootPath);
+sys::path::append(ToolsVersionFilePath, "Auxiliary", "Build",
+  "Microsoft.VCToolsVersion.default.txt");
+
+auto ToolsVersionFile = MemoryBuffer::getFile(ToolsVersionFilePath);
+if (!ToolsVersionFile)
+  return false;
+
+ToolsVersion = ToolsVersionFile->get()->getBuffer().rtrim();
+  }
 
-  auto ToolsVersionFile = MemoryBuffer::getFile(ToolsVersionFilePath);
-  if (!ToolsVersionFile)
-return false;
 
   SmallString<256> ToolchainPath(VCRootPath);
-  sys::path::append(ToolchainPath, "Tools", "MSVC",
-ToolsVersionFile->get()->getBuffer().rtrim());
+  sys::path::append(ToolchainPath, "Tools", "MSVC", ToolsVersion);
   auto Status = VFS.status(ToolchainPath);
   if (!Status || !Status->isDirectory())
 return false;
Index: llvm/lib/ExecutionEngine/Orc/COFFVCRuntimeSupport.cpp
===
--- llvm/lib/ExecutionEngine/Orc/COFFVCRuntimeSupport.cpp
+++ llvm/lib/ExecutionEngine/Orc/COFFVCRuntimeSupport.cpp
@@ -160,7 +160,7 @@
   if (!findVCToolChainViaCommandLine(*VFS, std::nullopt, std::nullopt,
  std::nullopt, VCToolChainPath, VSLayout) &&
   !findVCToolChainViaEnvironment(*VFS, VCToolChainPath, VSLayout) &&
-  !findVCToolChainViaSetupConfig(*VFS, VCToolChainPath, VSLayout) &&
+  !findVCToolChainViaSetupConfig(*VFS, {}, VCToolChainPath, VSLayout) &&
   !findVCToolChainViaRegistry(VCToolChainPath, VSLayout))
 return make_error("Couldn't find msvc toolchain.",
inconvertibleErrorCode());
Index: llvm/include/llvm/WindowsDriver/MSVCPaths.h
===
--- llvm/include/llvm/WindowsDriver/MSVCPaths.h
+++ llvm/include/llvm/WindowsDriver/MSVCPaths.h
@@ -90,11 +90,15 @@
ToolsetLayout );
 
 // Query the Setup Config server for installs, then pick the newest version
-// and find its default VC toolchain.
+// and find its default VC toolchain. If `VCToolsVersion` is specified, that
+// version is preferred over the latest version.
+//
 // This is the preferred way to discover new Visual Studios, as they're no
 // longer listed in the registry.
-bool findVCToolChainViaSetupConfig(vfs::FileSystem , std::string ,
-   ToolsetLayout );
+bool
+findVCToolChainViaSetupConfig(vfs::FileSystem ,
+  std::optional VCToolsVersion,
+  std::string , ToolsetLayout );
 
 // Look in the registry for Visual Studio installs, and use that to get
 // a toolchain path. VS2017 and newer don't get added to the registry.
Index: lld/COFF/Driver.cpp
===
--- lld/COFF/Driver.cpp
+++ lld/COFF/Driver.cpp
@@ -559,7 +559,7 @@
  WinSysRoot, vcToolChainPath, vsLayout) &&
   (Args.hasArg(OPT_lldignoreenv) ||
!findVCToolChainViaEnvironment(*VFS, vcToolChainPath, vsLayout)) &&
-  !findVCToolChainViaSetupConfig(*VFS, vcToolChainPath, vsLayout) &&
+  !findVCToolChainViaSetupConfig(*VFS, {}, vcToolChainPath, vsLayout) &&
   !findVCToolChainViaRegistry(vcToolChainPath, vsLayout))
 return;
 
Index: clang/lib/Driver/ToolChains/MSVC.cpp

[PATCH] D145517: MSVC: support version preference with search

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

Sure thing, I can try to write up some details about that @hans!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145517

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145517: MSVC: support version preference with search

[Hans Wennborg via Phabricator via cfe-commits]

hans accepted this revision.
hans added a comment.
This revision is now accepted and ready to land.

lgtm

I wish the way clang-cl finds system libraries was better documented though. 
Since you've been digging through this code, would you be up for writing 
something in the https://clang.llvm.org/docs/UsersManual.html#clang-cl section?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145517

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145517: MSVC: support version preference with search

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd created this revision.
compnerd added reviewers: mstorsjo, rnk.
Herald added a subscriber: hiraditya.
Herald added a project: All.
compnerd requested review of this revision.
Herald added subscribers: llvm-commits, MaskRay.
Herald added projects: clang, LLVM.

Extend the logic for the WinSDK and UCRT handling to prefer a user
specified version of the VisualC++ tools and Windows SDK.  This allows
us to now perform the regular search for the installation but select the
exact version of the SDK or VC++ tools to override the latest version.
Similar to the other flags controlling this behaviour, if the user
specifies a value, we will not perform validation on the input and will
attempt to prefer that, particularly in the case of VisualC++ tools
where no fallback occurs.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145517

Files:
  clang/lib/Driver/ToolChains/MSVC.cpp
  lld/COFF/Driver.cpp
  llvm/include/llvm/WindowsDriver/MSVCPaths.h
  llvm/lib/ExecutionEngine/Orc/COFFVCRuntimeSupport.cpp
  llvm/lib/WindowsDriver/MSVCPaths.cpp

Index: llvm/lib/WindowsDriver/MSVCPaths.cpp
===
--- llvm/lib/WindowsDriver/MSVCPaths.cpp
+++ llvm/lib/WindowsDriver/MSVCPaths.cpp
@@ -609,8 +609,9 @@
   return false;
 }
 
-bool findVCToolChainViaSetupConfig(vfs::FileSystem , std::string ,
-   ToolsetLayout ) {
+bool findVCToolChainViaSetupConfig(vfs::FileSystem ,
+   std::optional VCToolsVersion,
+   std::string , ToolsetLayout ) {
 #if !defined(USE_MSVC_SETUP_API)
   return false;
 #else
@@ -677,17 +678,24 @@
   std::string VCRootPath;
   convertWideToUTF8(std::wstring(VCPathWide), VCRootPath);
 
-  SmallString<256> ToolsVersionFilePath(VCRootPath);
-  sys::path::append(ToolsVersionFilePath, "Auxiliary", "Build",
-"Microsoft.VCToolsVersion.default.txt");
+  std::string ToolsVersion;
+  if (VCToolsVersion.has_value()) {
+ToolsVersion = *VCToolsVersion;
+  } else {
+SmallString<256> ToolsVersionFilePath(VCRootPath);
+sys::path::append(ToolsVersionFilePath, "Auxiliary", "Build",
+  "Microsoft.VCToolsVersion.default.txt");
+
+auto ToolsVersionFile = MemoryBuffer::getFile(ToolsVersionFilePath);
+if (!ToolsVersionFile)
+  return false;
+
+ToolsVersion = ToolsVersionFile->get()->getBuffer().rtrim();
+  }
 
-  auto ToolsVersionFile = MemoryBuffer::getFile(ToolsVersionFilePath);
-  if (!ToolsVersionFile)
-return false;
 
   SmallString<256> ToolchainPath(VCRootPath);
-  sys::path::append(ToolchainPath, "Tools", "MSVC",
-ToolsVersionFile->get()->getBuffer().rtrim());
+  sys::path::append(ToolchainPath, "Tools", "MSVC", ToolsVersion);
   auto Status = VFS.status(ToolchainPath);
   if (!Status || !Status->isDirectory())
 return false;
Index: llvm/lib/ExecutionEngine/Orc/COFFVCRuntimeSupport.cpp
===
--- llvm/lib/ExecutionEngine/Orc/COFFVCRuntimeSupport.cpp
+++ llvm/lib/ExecutionEngine/Orc/COFFVCRuntimeSupport.cpp
@@ -160,7 +160,7 @@
   if (!findVCToolChainViaCommandLine(*VFS, std::nullopt, std::nullopt,
  std::nullopt, VCToolChainPath, VSLayout) &&
   !findVCToolChainViaEnvironment(*VFS, VCToolChainPath, VSLayout) &&
-  !findVCToolChainViaSetupConfig(*VFS, VCToolChainPath, VSLayout) &&
+  !findVCToolChainViaSetupConfig(*VFS, {}, VCToolChainPath, VSLayout) &&
   !findVCToolChainViaRegistry(VCToolChainPath, VSLayout))
 return make_error("Couldn't find msvc toolchain.",
inconvertibleErrorCode());
Index: llvm/include/llvm/WindowsDriver/MSVCPaths.h
===
--- llvm/include/llvm/WindowsDriver/MSVCPaths.h
+++ llvm/include/llvm/WindowsDriver/MSVCPaths.h
@@ -90,11 +90,15 @@
ToolsetLayout );
 
 // Query the Setup Config server for installs, then pick the newest version
-// and find its default VC toolchain.
+// and find its default VC toolchain. If `VCToolsVersion` is specified, that
+// version is preferred over the latest version.
+//
 // This is the preferred way to discover new Visual Studios, as they're no
 // longer listed in the registry.
-bool findVCToolChainViaSetupConfig(vfs::FileSystem , std::string ,
-   ToolsetLayout );
+bool
+findVCToolChainViaSetupConfig(vfs::FileSystem ,
+  std::optional VCToolsVersion,
+  std::string , ToolsetLayout );
 
 // Look in the registry for Visual Studio installs, and use that to get
 // a toolchain path. VS2017 and newer don't get added to the registry.
Index: lld/COFF/Driver.cpp
===
--- lld/COFF/Driver.cpp
+++ lld/COFF/Driver.cpp
@@