<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Jul 20, 2015 at 11:07 AM, Yaron Keren <span dir="ltr"><<a href="mailto:yaron.keren@gmail.com" target="_blank">yaron.keren@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="rtl"><div dir="ltr">If we input the root externally then we are not testing the root-discovering code (MinGW constructor)... and that's where the last "space" bug was. </div></div></blockquote><div><br></div><div>But it could still cover the code changes in this revision here, right? Having some coverage is probably better than none at all?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="rtl"><div dir="ltr">gcc driver does not have this problem as it knows where it's configured, but clang has to check several possible directories.</div><div dir="ltr">Maybe this could be simulated with the VirtualFileSystem if we make the mingw toolchain work with an abstract file system instead of the regular one? it will probably slow the driver and not sure it's worth the complexity.</div><div dir="ltr"><br></div><div dir="ltr"><br></div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><div dir="ltr"><br><div class="gmail_quote">2015-07-20 20:48 GMT+03:00 Nico Weber <span dir="ltr"><<a href="mailto:thakis@google.com" target="_blank">thakis@google.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 .8ex;border-left:1px #ccc solid;border-right:1px #ccc solid;padding-left:1ex;padding-right:1ex"><div class="gmail_extra"><div class="gmail_quote"><span>On Mon, Jul 20, 2015 at 12:20 AM, Yaron Keren <span dir="ltr"><<a href="mailto:yaron.keren@gmail.com" target="_blank">yaron.keren@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="rtl"><div dir="ltr">To make the toolchain "just work", clang gathers information - gcc installed version and existance of various directories - from the directory structure itself instead of requiring the user to supply them. So clang requires the mingw directory structure (at least) to really exist at the right locations, all of which are outside the llvm tree.</div></div></blockquote><div><br></div></span><div>Maybe there could be some test-only flag to set the root of the mingw tree to another place? Then driver tests could write a mingw-like tree in some temp dir and that could be used for testing (similar to how the gcc driver tests work).</div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="rtl"><div dir="ltr"><br></div><div dir="ltr">The real-world test would be to compile an small C++ test program that includes both C and C++ headers using the mingw toolchain on every target platform to see that all includes and libraries are found and the program correctly compiled and runs. That requires setting up mingw toolchains on the various bots. Currently we have a bot running mingw toolchain on windows only:</div><div dir="ltr"><br></div><div dir="ltr"> <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__bb.pgr.jp_grid&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=4nIHTywP6fLmiDMgQhTpYL7qyeyYstth1CEf67taeZw&s=1qCF1W4Yz2ZV2xYOxeNKCzX-mQAyyJ_DTKp7pVSXS88&e=" target="_blank">http://bb.pgr.jp/grid</a><br></div><div dir="ltr"><br></div><div dir="ltr">I'm not sure what could be tested otherwise.</div><div dir="ltr"><br></div></div><div><div><div class="gmail_extra"><br><div class="gmail_quote"><div dir="ltr">2015-07-20 9:46 GMT+03:00 Nico Weber <span dir="ltr"><<a href="mailto:thakis@google.com" target="_blank">thakis@google.com</a>></span>:</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><p dir="ltr">Is it possible to test this?</p>
<div class="gmail_quote"><span>On Jul 19, 2015 11:39 PM, "Yaron Keren" <<a href="mailto:yaron.keren@gmail.com" target="_blank">yaron.keren@gmail.com</a>> wrote:<br type="attribution"></span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>Author: yrnkrn<br>
Date: Mon Jul 20 01:38:39 2015<br>
New Revision: 242660<br>
<br></span>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D242660-26view-3Drev&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=4nIHTywP6fLmiDMgQhTpYL7qyeyYstth1CEf67taeZw&s=1CNrNJWF_jN5S1mvuq3jG_P_XJVUv3OFGDr4KqV67EY&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=242660&view=rev</a><span><br>
Log:<br>
Support mingw toolchain include and lib directories on Arch Linux.<br>
Thanks to Thomas Pochtrager for testing this!<br>
<br>
<br>
Modified:<br>
    cfe/trunk/lib/Driver/MinGWToolChain.cpp<br>
<br>
Modified: cfe/trunk/lib/Driver/MinGWToolChain.cpp<br></span>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_lib_Driver_MinGWToolChain.cpp-3Frev-3D242660-26r1-3D242659-26r2-3D242660-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=4nIHTywP6fLmiDMgQhTpYL7qyeyYstth1CEf67taeZw&s=P3h0Ed3TEuLuOUwnYnX0_XOdmHYb8NEqLKPbiHf-Wfw&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/MinGWToolChain.cpp?rev=242660&r1=242659&r2=242660&view=diff</a><div><div><br>
==============================================================================<br>
--- cfe/trunk/lib/Driver/MinGWToolChain.cpp (original)<br>
+++ cfe/trunk/lib/Driver/MinGWToolChain.cpp Mon Jul 20 01:38:39 2015<br>
@@ -26,6 +26,8 @@ MinGW::MinGW(const Driver &D, const llvm<br>
<br>
   llvm::SmallString<1024> LibDir;<br>
<br>
+  // In Windows there aren't any standard install locations, we search<br>
+  // for gcc on the PATH. In Liunx the base is always /usr.<br>
 #ifdef LLVM_ON_WIN32<br>
   if (getDriver().SysRoot.size())<br>
     Base = getDriver().SysRoot;<br>
@@ -36,42 +38,48 @@ MinGW::MinGW(const Driver &D, const llvm<br>
   else<br>
     Base = llvm::sys::path::parent_path(getDriver().getInstalledDir());<br>
   Base += llvm::sys::path::get_separator();<br>
-  LibDir = Base;<br>
-  llvm::sys::path::append(LibDir, "lib", "gcc");<br>
 #else<br>
   if (getDriver().SysRoot.size())<br>
     Base = getDriver().SysRoot;<br>
   else<br>
     Base = "/usr/";<br>
-  LibDir = Base;<br>
-  llvm::sys::path::append(LibDir, "lib64", "gcc");<br>
 #endif<br>
<br>
-  LibDir += llvm::sys::path::get_separator();<br>
-<br>
-  // First look for mingw-w64.<br>
-  Arch = getTriple().getArchName();<br>
-  Arch += "-w64-mingw32";<br>
-  std::error_code EC;<br>
-  llvm::sys::fs::directory_iterator MingW64Entry(LibDir + Arch, EC);<br>
-  if (!EC) {<br>
-    GccLibDir = MingW64Entry->path();<br>
-    Ver = llvm::sys::path::filename(GccLibDir);<br>
-  } else {<br>
+  // By default Arch is for mingw-w64.<br>
+  Arch = (getTriple().getArchName() + "-w64-mingw32").str();<br>
+  // lib: Arch Linux, Ubuntu, Windows<br>
+  // lib64: openSUSE Linux<br>
+  for (StringRef Lib : {"lib", "lib64 "}) {<br>
+    LibDir = Base;<br>
+    llvm::sys::path::append(LibDir, Lib, "gcc");<br>
+    LibDir += llvm::sys::path::get_separator();<br>
+    std::error_code EC;<br>
+    // First look for mingw-w64.<br>
+    llvm::sys::fs::directory_iterator MingW64Entry(LibDir + Arch, EC);<br>
+    if (!EC) {<br>
+      GccLibDir = MingW64Entry->path();<br>
+      Ver = llvm::sys::path::filename(GccLibDir);<br>
+      break;<br>
+    }<br></div></div>
     // If mingw-w64 not found, try looking for <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__mingw.org&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=4nIHTywP6fLmiDMgQhTpYL7qyeyYstth1CEf67taeZw&s=ksxDNnMg0W4s9dTxvOqxp1CgaInX1BXGJpkAjVw_VI8&e=" rel="noreferrer" target="_blank">mingw.org</a>.<div><div><br>
-    Arch = "mingw32";<br>
-    llvm::sys::fs::directory_iterator MingwOrgEntry(LibDir + Arch, EC);<br>
-    if (!EC)<br>
+    llvm::sys::fs::directory_iterator MingwOrgEntry(LibDir + "mingw32", EC);<br>
+    if (!EC) {<br>
       GccLibDir = MingwOrgEntry->path();<br>
+      // Replace Arch with mingw32 arch.<br>
+      Arch = "mingw32";<br>
+      break;<br>
+    }<br>
   }<br>
+<br>
   Arch += llvm::sys::path::get_separator();<br>
   // GccLibDir must precede Base/lib so that the<br>
   // correct crtbegin.o ,cetend.o would be found.<br>
   getFilePaths().push_back(GccLibDir);<br>
-  getFilePaths().push_back(Base + "lib");<br>
   getFilePaths().push_back(Base + Arch + "lib");<br>
-#ifdef LLVM_ON_UNIX<br>
-  // For openSUSE.<br>
+#ifdef LLVM_ON_WIN32<br>
+  getFilePaths().push_back(Base + "lib");<br>
+#else<br>
+  // openSUSE<br>
   getFilePaths().push_back(Base + Arch + "sys-root/mingw/lib");<br>
 #endif<br>
 }<br>
@@ -134,7 +142,7 @@ void MinGW::AddClangSystemIncludeArgs(co<br>
   addSystemInclude(DriverArgs, CC1Args, IncludeDir.c_str());<br>
   IncludeDir += "-fixed";<br>
 #ifdef LLVM_ON_UNIX<br>
-  // For openSUSE.<br>
+  // openSUSE<br>
   addSystemInclude(DriverArgs, CC1Args,<br>
                    "/usr/x86_64-w64-mingw32/sys-root/mingw/include");<br>
 #endif<br>
@@ -149,23 +157,28 @@ void MinGW::AddClangCXXStdlibIncludeArgs<br>
       DriverArgs.hasArg(options::OPT_nostdincxx))<br>
     return;<br>
<br>
-  // C++ includes may be found in several locations depending on distribution.<br>
+  // C++ includes locations are different with almost every mingw distribution.<br>
+  //<br>
   // Windows<br>
   // -------<br>
-  // mingw-w64 mingw-builds: $sysroot/i686-w64-mingw32/include/c++.<br>
+  // mingw-w64 mingw-builds: $sysroot/i686-w64-mingw32/include/c++<br>
   // mingw-w64 msys2:        $sysroot/include/c++/4.9.2<br></div></div>
   // <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__mingw.org&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=4nIHTywP6fLmiDMgQhTpYL7qyeyYstth1CEf67taeZw&s=ksxDNnMg0W4s9dTxvOqxp1CgaInX1BXGJpkAjVw_VI8&e=" rel="noreferrer" target="_blank">mingw.org</a>:              GccLibDir/include/c++<span><br>
   //<br>
   // Linux<br>
   // -----<br>
   // openSUSE:               GccLibDir/include/c++<br>
-  llvm::SmallVector<llvm::SmallString<1024>, 3> CppIncludeBases;<br>
+  // Arch:                   $sysroot/i686-w64-mingw32/include/c++/5.1.0<br>
+  //<br>
+  llvm::SmallVector<llvm::SmallString<1024>, 4> CppIncludeBases;<br>
   CppIncludeBases.emplace_back(Base);<br>
   llvm::sys::path::append(CppIncludeBases[0], Arch, "include", "c++");<br>
   CppIncludeBases.emplace_back(Base);<br>
-  llvm::sys::path::append(CppIncludeBases[1], "include", "c++", Ver);<br>
+  llvm::sys::path::append(CppIncludeBases[1], Arch, "include", "c++", Ver);<br>
+  CppIncludeBases.emplace_back(Base);<br>
+  llvm::sys::path::append(CppIncludeBases[2], "include", "c++", Ver);<br>
   CppIncludeBases.emplace_back(GccLibDir);<br>
-  llvm::sys::path::append(CppIncludeBases[2], "include", "c++");<br>
+  llvm::sys::path::append(CppIncludeBases[3], "include", "c++");<br>
   for (auto &CppIncludeBase : CppIncludeBases) {<br>
     CppIncludeBase += llvm::sys::path::get_separator();<br>
     addSystemInclude(DriverArgs, CC1Args, CppIncludeBase);<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
</span><a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>
</blockquote></div><br></div>
</div></div></blockquote></div></div></div><br></div></blockquote></div></div></div>
</div></div></blockquote></div><br></div></div>