Re: r311391 - [Driver] Recognize DevDiv internal builds of MSVC, with a different directory structure.
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&view=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&r1=311390&r2=311391&view=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 &Path, > - bool &IsVS2017OrNewer) { > + MSVCToolChain::ToolsetLayout > &VSLayout) { >// 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 &Path, > -
r311391 - [Driver] Recognize DevDiv internal builds of MSVC, with a different directory structure.
Author: stl_msft Date: Mon Aug 21 15:19:33 2017 New Revision: 311391 URL: http://llvm.org/viewvc/llvm-project?rev=311391&view=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&r1=311390&r2=311391&view=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 &Path, - bool &IsVS2017OrNewer) { + MSVCToolChain::ToolsetLayout &VSLayout) { // 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 &Path, - bool &IsVS2017OrNewer) { + MSVCToolChain::ToolsetLayout &VSLayout) { #if !defined(USE_MSVC_SETUP_API) return false; #else @@ -263,7 +270,7 @@ static bool findVCToolChainViaSetupConfi return false; Path = ToolchainPath.st