Re: r311391 - [Driver] Recognize DevDiv internal builds of MSVC, with a different directory structure.

[Hans Wennborg via cfe-commits]

Merged to 5.0 in r311500.

On Mon, Aug 21, 2017 at 3:19 PM, Stephan T. Lavavej via cfe-commits
 wrote:
> Author: stl_msft
> Date: Mon Aug 21 15:19:33 2017
> New Revision: 311391
>
> URL: http://llvm.org/viewvc/llvm-project?rev=311391=rev
> Log:
> [Driver] Recognize DevDiv internal builds of MSVC, with a different directory 
> structure.
>
> This is a reasonably non-intrusive change, which I've verified
> works for both x86 and x64 DevDiv-internal builds.
>
> The idea is to change `bool IsVS2017OrNewer` into a 3-state
> `ToolsetLayout VSLayout`. Either a build is DevDiv-internal,
> released VS 2017 or newer, or released VS 2015 or older. When looking at
> the directory structure, if instead of `"VC"` we see `"x86ret"`, `"x86chk"`,
> `"amd64ret"`, or `"amd64chk"`, we recognize this as a DevDiv-internal build.
>
> After we get past the directory structure validation, we use this knowledge
> to regenerate paths appropriately. `llvmArchToDevDivInternalArch()` knows how
> we use `"i386"` subdirectories, and `MSVCToolChain::getSubDirectoryPath()`
> uses that. It also knows that DevDiv-internal builds have an `"inc"`
> subdirectory instead of `"include"`.
>
> This may still not be the "right" fix in any sense, but I believe that it's
> non-intrusive in the sense that if the special directory names aren't found,
> no codepaths are affected. (`ToolsetLayout::OlderVS` and
> `ToolsetLayout::VS2017OrNewer` correspond to `IsVS2017OrNewer` being `false`
> or `true`, respectively.) I searched for all references to `IsVS2017OrNewer`,
> which are places where Clang cares about VS's directory structure, and the
> only one that isn't being patched is some logic to deal with
> cross-compilation. I'm fine with that not working for DevDiv-internal builds
> for the moment (we typically test the native compilers), so I added a comment.
>
> Fixes D36860.
>
> Modified:
> cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
> cfe/trunk/lib/Driver/ToolChains/MSVC.h
>
> Modified: cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/MSVC.cpp?rev=311391=311390=311391=diff
> ==
> --- cfe/trunk/lib/Driver/ToolChains/MSVC.cpp (original)
> +++ cfe/trunk/lib/Driver/ToolChains/MSVC.cpp Mon Aug 21 15:19:33 2017
> @@ -76,7 +76,7 @@ static bool getSystemRegistryString(cons
>
>  // Check various environment variables to try and find a toolchain.
>  static bool findVCToolChainViaEnvironment(std::string ,
> -  bool ) {
> +  MSVCToolChain::ToolsetLayout 
> ) {
>// These variables are typically set by vcvarsall.bat
>// when launching a developer command prompt.
>if (llvm::Optional VCToolsInstallDir =
> @@ -84,7 +84,7 @@ static bool findVCToolChainViaEnvironmen
>  // This is only set by newer Visual Studios, and it leads straight to
>  // the toolchain directory.
>  Path = std::move(*VCToolsInstallDir);
> -IsVS2017OrNewer = true;
> +VSLayout = MSVCToolChain::ToolsetLayout::VS2017OrNewer;
>  return true;
>}
>if (llvm::Optional VCInstallDir =
> @@ -94,7 +94,7 @@ static bool findVCToolChainViaEnvironmen
>  // so this check has to appear second.
>  // In older Visual Studios, the VC directory is the toolchain.
>  Path = std::move(*VCInstallDir);
> -IsVS2017OrNewer = false;
> +VSLayout = MSVCToolChain::ToolsetLayout::OlderVS;
>  return true;
>}
>
> @@ -134,9 +134,16 @@ static bool findVCToolChainViaEnvironmen
>}
>if (IsBin) {
>  llvm::StringRef ParentPath = llvm::sys::path::parent_path(TestPath);
> -if (llvm::sys::path::filename(ParentPath) == "VC") {
> +llvm::StringRef ParentFilename = 
> llvm::sys::path::filename(ParentPath);
> +if (ParentFilename == "VC") {
>Path = ParentPath;
> -  IsVS2017OrNewer = false;
> +  VSLayout = MSVCToolChain::ToolsetLayout::OlderVS;
> +  return true;
> +}
> +if (ParentFilename == "x86ret" || ParentFilename == "x86chk"
> +  || ParentFilename == "amd64ret" || ParentFilename == "amd64chk") {
> +  Path = ParentPath;
> +  VSLayout = MSVCToolChain::ToolsetLayout::DevDivInternal;
>return true;
>  }
>
> @@ -165,7 +172,7 @@ static bool findVCToolChainViaEnvironmen
>ToolChainPath = llvm::sys::path::parent_path(ToolChainPath);
>
>  Path = ToolChainPath;
> -IsVS2017OrNewer = true;
> +VSLayout = MSVCToolChain::ToolsetLayout::VS2017OrNewer;
>  return true;
>}
>
> @@ -181,7 +188,7 @@ static bool findVCToolChainViaEnvironmen
>  // This is the preferred way to discover new Visual Studios, as they're no
>  // longer listed in the registry.
>  static bool findVCToolChainViaSetupConfig(std::string ,
> -

r311391 - [Driver] Recognize DevDiv internal builds of MSVC, with a different directory structure.

[Stephan T. Lavavej via cfe-commits]

Author: stl_msft
Date: Mon Aug 21 15:19:33 2017
New Revision: 311391

URL: http://llvm.org/viewvc/llvm-project?rev=311391=rev
Log:
[Driver] Recognize DevDiv internal builds of MSVC, with a different directory 
structure.

This is a reasonably non-intrusive change, which I've verified
works for both x86 and x64 DevDiv-internal builds.

The idea is to change `bool IsVS2017OrNewer` into a 3-state
`ToolsetLayout VSLayout`. Either a build is DevDiv-internal,
released VS 2017 or newer, or released VS 2015 or older. When looking at
the directory structure, if instead of `"VC"` we see `"x86ret"`, `"x86chk"`,
`"amd64ret"`, or `"amd64chk"`, we recognize this as a DevDiv-internal build.

After we get past the directory structure validation, we use this knowledge
to regenerate paths appropriately. `llvmArchToDevDivInternalArch()` knows how
we use `"i386"` subdirectories, and `MSVCToolChain::getSubDirectoryPath()`
uses that. It also knows that DevDiv-internal builds have an `"inc"`
subdirectory instead of `"include"`.

This may still not be the "right" fix in any sense, but I believe that it's
non-intrusive in the sense that if the special directory names aren't found,
no codepaths are affected. (`ToolsetLayout::OlderVS` and
`ToolsetLayout::VS2017OrNewer` correspond to `IsVS2017OrNewer` being `false`
or `true`, respectively.) I searched for all references to `IsVS2017OrNewer`,
which are places where Clang cares about VS's directory structure, and the
only one that isn't being patched is some logic to deal with
cross-compilation. I'm fine with that not working for DevDiv-internal builds
for the moment (we typically test the native compilers), so I added a comment.

Fixes D36860.

Modified:
cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
cfe/trunk/lib/Driver/ToolChains/MSVC.h

Modified: cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/MSVC.cpp?rev=311391=311390=311391=diff
==
--- cfe/trunk/lib/Driver/ToolChains/MSVC.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/MSVC.cpp Mon Aug 21 15:19:33 2017
@@ -76,7 +76,7 @@ static bool getSystemRegistryString(cons
 
 // Check various environment variables to try and find a toolchain.
 static bool findVCToolChainViaEnvironment(std::string ,
-  bool ) {
+  MSVCToolChain::ToolsetLayout 
) {
   // These variables are typically set by vcvarsall.bat
   // when launching a developer command prompt.
   if (llvm::Optional VCToolsInstallDir =
@@ -84,7 +84,7 @@ static bool findVCToolChainViaEnvironmen
 // This is only set by newer Visual Studios, and it leads straight to
 // the toolchain directory.
 Path = std::move(*VCToolsInstallDir);
-IsVS2017OrNewer = true;
+VSLayout = MSVCToolChain::ToolsetLayout::VS2017OrNewer;
 return true;
   }
   if (llvm::Optional VCInstallDir =
@@ -94,7 +94,7 @@ static bool findVCToolChainViaEnvironmen
 // so this check has to appear second.
 // In older Visual Studios, the VC directory is the toolchain.
 Path = std::move(*VCInstallDir);
-IsVS2017OrNewer = false;
+VSLayout = MSVCToolChain::ToolsetLayout::OlderVS;
 return true;
   }
 
@@ -134,9 +134,16 @@ static bool findVCToolChainViaEnvironmen
   }
   if (IsBin) {
 llvm::StringRef ParentPath = llvm::sys::path::parent_path(TestPath);
-if (llvm::sys::path::filename(ParentPath) == "VC") {
+llvm::StringRef ParentFilename = llvm::sys::path::filename(ParentPath);
+if (ParentFilename == "VC") {
   Path = ParentPath;
-  IsVS2017OrNewer = false;
+  VSLayout = MSVCToolChain::ToolsetLayout::OlderVS;
+  return true;
+}
+if (ParentFilename == "x86ret" || ParentFilename == "x86chk"
+  || ParentFilename == "amd64ret" || ParentFilename == "amd64chk") {
+  Path = ParentPath;
+  VSLayout = MSVCToolChain::ToolsetLayout::DevDivInternal;
   return true;
 }
 
@@ -165,7 +172,7 @@ static bool findVCToolChainViaEnvironmen
   ToolChainPath = llvm::sys::path::parent_path(ToolChainPath);
 
 Path = ToolChainPath;
-IsVS2017OrNewer = true;
+VSLayout = MSVCToolChain::ToolsetLayout::VS2017OrNewer;
 return true;
   }
 
@@ -181,7 +188,7 @@ static bool findVCToolChainViaEnvironmen
 // This is the preferred way to discover new Visual Studios, as they're no
 // longer listed in the registry.
 static bool findVCToolChainViaSetupConfig(std::string ,
-  bool ) {
+  MSVCToolChain::ToolsetLayout 
) {
 #if !defined(USE_MSVC_SETUP_API)
   return false;
 #else
@@ -263,7 +270,7 @@ static bool findVCToolChainViaSetupConfi
 return false;
 
   Path = ToolchainPath.str();
-  IsVS2017OrNewer = true;
+  VSLayout =