[PATCH] D134454: [Driver][Distro] Fix ArchLinux sysroot detection

[Adrian Ratiu via Phabricator via cfe-commits]

10ne1 marked 5 inline comments as done.
10ne1 added a comment.

FYI: @MaskRay I think you will be very happy that after the simplifications Nick suggested the Distro::* additions are not necessary anymore.



================
Comment at: clang/lib/Driver/ToolChains/Linux.cpp:377-389
   const StringRef InstallDir = GCCInstallation.getInstallPath();
   const StringRef TripleStr = GCCInstallation.getTriple().str();
   const Multilib &Multilib = GCCInstallation.getMultilib();
+  std::string BasePath = (InstallDir + "/../../../../" + TripleStr).str();
 
-  std::string Path =
-      (InstallDir + "/../../../../" + TripleStr + "/libc" + Multilib.osSuffix())
-          .str();
-
-  if (getVFS().exists(Path))
-    return Path;
+  if (Distro.IsArchLinux() && getVFS().exists(BasePath))
+    return BasePath;
----------------
nickdesaulniers wrote:
> ...seems like some of this is duplicated with CSKY above. Can you please refactor the MIPS and CSKY checks to reuse more code?
> 
> I doubt arch-linux has a CSKY port; I suspect you might be able to check for arch-linux first, then do the CSKY and MIPS special casing after.
> 
> All this code is doing is figuring out a Path, then if it exists then return it.  Feel like is should be:
> 
> ```
> if android:
>   path = ...
> elif arch:
>   path = ...
> elif csky:
>   path = ...
> elif mips:
>   path = ...
> else:
>   return ""
> 
> if path.exists():
>   return path
> return ""
> ```
Thanks for this suggestion. Indeed after doing all the simplifications I have some great news: we do not need to test if Distro == ArchLinux because the sysroot path it uses is the implicit base path one common to all architectures!

So just doing the following is enough to also fix ArchLinux:

```
  std::string Path = (InstallDir + "/../../../../" + TripleStr).str();
  
  if (getTriple().isCSKY())
    Path = Path + "/libc";
  
  if (getTriple().isMIPS()) {
    ...
   }

  if (getVFS().exists(Path))
    return Path;

  return std::string();
```


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

https://reviews.llvm.org/D134454