Driver: fallback to parent directory of clang if no sysroot for mingw-w64 target

[martell]

Summary

Driver: fallback to the location of clang if no sysroot,
hard coding /usr makes little sense for mingw-w64.

If we have portable toolchains having /usr breaks that.
If the clang we use is in /usr/bin or /usr/sbin etc this will still detect as though it was hard coded to /usr

This makes the most sense going forward for mingw-w64 toolchains on both linux and mac

[yaron.keren]

The code could now be merged (untested). I disconnected the third part on purpose based on comment from Joerg Sonnenberger on the previous code that having else after #endif is confusing.

if (getDriver().SysRoot.size())
  Base = getDriver().SysRoot;
#ifdef LLVM_ON_WIN32
else if (llvm::ErrorOr<std::string> GPPName =
             llvm::sys::findProgramByName("gcc"))
  Base = llvm::sys::path::parent_path(
      llvm::sys::path::parent_path(GPPName.get()));
#endif

if (!Base.size())
  Base = llvm::sys::path::parent_path(getDriver().getInstalledDir());

[martell]

Yes I was thinking about that.
I just wanted to get the initial patch in for review incase you had a specific reason to keep /usr

If everything is good with replacing /usr I will update the patch merging to 2 code paths ?

[yaron.keren]

Yes, I think this is a better default than /usr.

[martell]

My code is a little different from yours as I wanted to preserve as much of the original code as possible.

Added llvm-commits as a subscriber.

[yaron.keren]

This code was my first thought too, but I had been commented on the structure of very similar code in this file:

http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20150713/133105.html

[martell]

Thanks for the tip.
That does make sense :)
Updated accordingly with the comment also

[yaron.keren]

We did not have cfe-commits as subscriber so I'm adding it now (this is a clang commit), see if someone would like to further comment.

[yaron.keren]

The formatting is wrong, aligned to the left, clang-format the new code.

Can we control getInstalledDir() when running under LIT so this could be tested? if possible, add a test before committing.

LGTM.

[martell]

I tried adding a test case but I couldn't see how to control getInstalledDir from LIT.
It is a bit redundant because this was already true for windows and there was no test case for that.
After trying to create one I saw why :)

I missed to formatting so I updated it in rL253899

cfe/trunk/lib/Driver/MinGWToolChain.cpp

for (StringRef CandidateLib : {"lib", "lib64"}) {
     }
   }
 }
 
 MinGW::MinGW(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
     : ToolChain(D, Triple, Args) {
   getProgramPaths().push_back(getDriver().getInstalledDir());
 
-// In Windows there aren't any standard install locations, we search
-// for gcc on the PATH. In Linux the base is always /usr.
-#ifdef LLVM_ON_WIN32
-  if (getDriver().SysRoot.size())
-    Base = getDriver().SysRoot;
-  else if (llvm::ErrorOr<std::string> GPPName =
-               llvm::sys::findProgramByName("gcc"))
-    Base = llvm::sys::path::parent_path(
-        llvm::sys::path::parent_path(GPPName.get()));
-  else
-    Base = llvm::sys::path::parent_path(getDriver().getInstalledDir());
-#else
-  if (getDriver().SysRoot.size())
-    Base = getDriver().SysRoot;
-  else
-    Base = "/usr";
+// On Windows if there is no sysroot we search for gcc on the PATH.
+
+#ifdef LLVM_ON_WIN32
 #endif
 
+if (!Base.size())
+  Base = llvm::sys::path::parent_path(getDriver().getInstalledDir());
+
   Base += llvm::sys::path::get_separator();
   findGccLibDir();
   // GccLibDir must precede Base/lib so that the
   // correct crtbegin.o ,cetend.o would be found.
   getFilePaths().push_back(GccLibDir);
   getFilePaths().push_back(
       (Base + Arch + llvm::sys::path::get_separator() + "lib").str());
   getFilePaths().push_back(Base + "lib");