[PATCH] gnu: clang: Fix search path detection with --sysroot.

OpenSubmitted by Carl Dong.
Details
2 participants
  • Carl Dong
  • Mathieu Othacehe
Owner
unassigned
Severity
normal
C
C
Carl Dong wrote on 25 Nov 2019 19:01
(address . guix-patches@gnu.org)(name . Carl Dong)(address . contact@carldong.me)
20191125180144.1060289-1-contact@carldong.me
Prior to this commit, we had

1. Patched out a lot of search path detection in clang's codebase which
didn't make sense in Guix
2. Used -DC_INCLUDE_DIRS= to point clang to our libc, which, as a side
effect, causes the search path detection code in clang to return early,
and skip some search path detection

This was fine when running clang normally, but when a user supplies
clang with --sysroot, clang _should_ do the search path detection. This
commit fixes runing clang with --sysroot.

More details included as comments in the
gnu/packages/patches/clang-*-libc-search-path.patch files.

* gnu/packages/patches/clang-3.5-libc-search-path.patch: Restore search
path detection code relative to $SYSROOT. Add @C_EXTRA_INCLUDE_DIRS@, a
C_INCLUDE_DIRS replacement which does not return early.
* gnu/packages/patches/clang-3.8-libc-search-path.patch: Same as above.
* gnu/packages/patches/clang-6.0-libc-search-path.patch: Same as above.
* gnu/packages/patches/clang-7.0-libc-search-path.patch: Same as above.
* gnu/packages/llvm.scm: Substitute @C_EXTRA_INCLUDE_DIRS@ instead of
using -DC_INCLUDE_DIRS=.
---
gnu/packages/llvm.scm | 13 ++-
.../patches/clang-3.5-libc-search-path.patch | 74 ++++++++-------
.../patches/clang-3.8-libc-search-path.patch | 70 ++++++++-------
.../patches/clang-6.0-libc-search-path.patch | 77 ++++++++--------
.../patches/clang-7.0-libc-search-path.patch | 89 +++++++++----------
5 files changed, 169 insertions(+), 154 deletions(-)

Toggle diff (73 lines)
diff --git a/gnu/packages/llvm.scm b/gnu/packages/llvm.scm
index 082e6e96ca..383180e140 100644
--- a/gnu/packages/llvm.scm
+++ b/gnu/packages/llvm.scm
@@ -190,12 +190,7 @@ compiler.  In LLVM this library is called \"compiler-rt\".")
 
              ;; Find libgcc_s, crtbegin.o, and crtend.o.
              (string-append "-DGCC_INSTALL_PREFIX="
-                            (assoc-ref %build-inputs "gcc-lib"))
-
-             ;; Use a sane default include directory.
-             (string-append "-DC_INCLUDE_DIRS="
-                            (assoc-ref %build-inputs "libc")
-                            "/include"))
+                            (assoc-ref %build-inputs "gcc-lib")))
 
        ;; Don't use '-g' during the build to save space.
        #:build-type "Release"
@@ -207,6 +202,7 @@ compiler.  In LLVM this library is called \"compiler-rt\".")
                      (let ((libc (assoc-ref inputs "libc"))
                            (compiler-rt (assoc-ref inputs "clang-runtime"))
                            (gcc (assoc-ref inputs "gcc"))
+                           (kernel-headers (assoc-ref inputs "kernel-headers"))
                            (version
                             (string->number
                              ,(version-major (package-version clang-runtime)))))
@@ -232,7 +228,10 @@ compiler.  In LLVM this library is called \"compiler-rt\".")
                             ;; Make sure libc's libdir is on the search path, to
                             ;; allow crt1.o & co. to be found.
                             (("@GLIBC_LIBDIR@")
-                             (string-append libc "/lib"))))
+                             (string-append libc "/lib"))
+                            (("@C_EXTRA_INCLUDE_DIRS@")
+                             (string-append libc "/include" ":"
+                                            kernel-headers "/include"))))
                          (else
                           (substitute* "lib/Driver/Tools.cpp"
                             ;; Patch the 'getLinuxDynamicLinker' function so that
diff --git a/gnu/packages/patches/clang-3.5-libc-search-path.patch b/gnu/packages/patches/clang-3.5-libc-search-path.patch
index 50e4480239..54fb9be7d8 100644
--- a/gnu/packages/patches/clang-3.5-libc-search-path.patch
+++ b/gnu/packages/patches/clang-3.5-libc-search-path.patch
@@ -8,7 +8,7 @@ to make sure Clang also works on non-GuixSD systems.
 
 --- cfe-3.6.0.src/lib/Driver/ToolChains.cpp	2015-02-18 22:03:07.000000000 +0100
 +++ cfe-3.6.0.src/lib/Driver/ToolChains.cpp	2015-06-19 16:37:20.459701044 +0200
-@@ -2931,6 +2931,9 @@ Linux::Linux(const Driver &D, const llvm
+@@ -3040,6 +3040,9 @@
  
    Linker = GetLinkerPath();
  
@@ -18,7 +18,7 @@ to make sure Clang also works on non-GuixSD systems.
    Distro Distro = DetectDistro(Arch);
  
    if (IsOpenSUSE(Distro) || IsUbuntu(Distro)) {
-@@ -2973,6 +2976,7 @@ Linux::Linux(const Driver &D, const llvm
+@@ -3082,6 +3085,7 @@
  
    if (IsOpenSUSE(Distro))
      ExtraOpts.push_back("--enable-new-dtags");
@@ -26,41 +26,49 @@ to make sure Clang also works on non-GuixSD systems.
  
    // The selection of paths to try here is designed to match the patterns which
    // the GCC driver itself uses, as this is part of the GCC-compatible driver.
-@@ -3043,14 +3047,12 @@ Linux::Linux(const Driver &D, const llvm
-     addPathIfExists(D.Dir + "/../" + OSLibDir, Paths);
-   }
+@@ -3194,6 +3198,10 @@
  
--  addPathIfExists(SysRoot + "/lib/" + MultiarchTriple, Paths);
--  addPathIfExists(SysRoot + "/lib/../" + OSLibDir, Paths);
--  addPathIfExists(SysRoot + "/usr/lib/" + MultiarchTriple, Paths);
--  addPathIfExists(SysRoot + "/usr/lib/../" + OSLibDir, Paths);
--
- // Try walking via the GCC triple path in case of biarch or multiarch GCC
- // installations with strange symlinks.
- if (GCCInstallation.isValid()) {
-+ // The following code would end up adding things like
-+ // "/usr/lib/x86_64-unknown-linux-gnu/../../lib64" to the search path.
-+#if 0
- addPathIfExists(SysRoot + "/usr/lib/" + GCCInstallation.getTriple().str() +
- "/../../" + OSLibDir, Paths);
-
-@@ -3060,6 +3062,7 @@ Linux::Linux(const Driver &D, const llvm
- addPathIfExists(GCCInstallation.getInstallPath() +
- BiarchSibling.gccSuffix(), Paths);
- }
-+#endif
-
- // See comments above on the multilib variant for details of why this is
- // included even from outside the sysroot.
-@@ -3083,8 +3086,9 @@ Linux::Linux(const Driver &D, const llvm
- if (StringRef(D.Dir).startswith(SysRoot))
- addPathIfExists(D.Dir + "/../lib", Paths);
-
-- addPathIfExists(SysRoot + "/lib", Paths);
-- addPathIfExists(SysRoot + "/usr/lib", Paths);
+ addPathIfExists(SysRoot + "/lib", Paths);
+ addPathIfExists(SysRoot + "/usr/lib", Paths);
++
+ // Add libc's lib/ directory to the search path, so that crt1.o, crti.o,
+ // and friends can be found.
+ addPathIfExists("@GLIBC_LIBDIR@", Paths);
}
bool Linux::HasNativeLLVMSupport() const {
+@@ -3384,6 +3392,34 @@
+ addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/include");
+
+ addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/usr/include");
++
++ // Check for configure-time "extra" C include directories. When constructing a
++ // toolchain, @C_EXTRA_INCLUDE_DIRS@ should be replaced with something like a
++ // colon-separated string of the include dirs of libc and kernel headers.
++ //
++ // The reason why we use this mechanism instead of C_INCLUDE_DIRS above is
++ // because when a user supplies clang with --sysroot, the normal expectation
++ // is that clang will detect and add the proper $SYSROOT/$MULTIARCHINCL,
++ // $SYSROOT/include, and $SYSROOT/usr/include to its list of search paths.
++ // However, if C_INCLUDE_DIRS is not empty, this function will return early
++ // and not attempt to add the aforementioned search paths, which is not
++ // desirable.
++ //
++ // By adding our configure-time "extra" C include directories here, after
++ // we've added $SYSROOT/include and $SYSROOT/usr/include, we make sure that IF
++ // --sysroot is supplied on the command line, we pick up the expected search
++ // paths in the $SYSROOT, and that they come before our configure-time "extra"
++ // C include directories.
++ StringRef CExtraIncludeDirs("@C_EXTRA_INCLUDE_DIRS@");
++ if (CExtraIncludeDirs != "") {
++ SmallVector<StringRef, 5> dirs;
++ CExtraIncludeDirs.split(dirs, ":");
++ for (StringRef dir : dirs) {
++ StringRef Prefix =
++ llvm::sys::path::is_absolute(dir) ? StringRef(SysRoot) : "";
++ addExternCSystemInclude(DriverArgs, CC1Args, Prefix + dir);
++ }
++ }
+ }
+
+ /// \brief Helper to add the variant paths of a libstdc++ installation.
Toggle diff (17 lines)
diff --git a/gnu/packages/patches/clang-3.8-libc-search-path.patch b/gnu/packages/patches/clang-3.8-libc-search-path.patch
index 0f7d0a4add..243d167755 100644
--- a/gnu/packages/patches/clang-3.8-libc-search-path.patch
+++ b/gnu/packages/patches/clang-3.8-libc-search-path.patch
@@ -29,41 +29,49 @@ changes in clang 3.8.
  
    // The selection of paths to try here is designed to match the patterns which
    // the GCC driver itself uses, as this is part of the GCC-compatible driver.
-@@ -3771,14 +3775,12 @@
-     addPathIfExists(D, D.Dir + "/../" + OSLibDir, Paths);
-   }
+@@ -3817,6 +3821,10 @@
  
--  addPathIfExists(D, SysRoot + "/lib/" + MultiarchTriple, Paths);
--  addPathIfExists(D, SysRoot + "/lib/../" + OSLibDir, Paths);
--  addPathIfExists(D, SysRoot + "/usr/lib/" + MultiarchTriple, Paths);
--  addPathIfExists(D, SysRoot + "/usr/lib/../" + OSLibDir, Paths);
--
- // Try walking via the GCC triple path in case of biarch or multiarch GCC
- // installations with strange symlinks.
- if (GCCInstallation.isValid()) {
-+ // The following code would end up adding things like
-+ // "/usr/lib/x86_64-unknown-linux-gnu/../../lib64" to the search path.
-+#if 0
- addPathIfExists(D,
- SysRoot + "/usr/lib/" + GCCInstallation.getTriple().str() +
- "/../../" + OSLibDir,
-@@ -3791,6 +3793,7 @@
- BiarchSibling.gccSuffix(),
- Paths);
- }
-+#endif
-
- // See comments above on the multilib variant for details of why this is
- // included even from outside the sysroot.
-@@ -3815,8 +3818,9 @@
- if (StringRef(D.Dir).startswith(SysRoot))
- addPathIfExists(D, D.Dir + "/../lib", Paths);
-
-- addPathIfExists(D, SysRoot + "/lib", Paths);
-- addPathIfExists(D, SysRoot + "/usr/lib", Paths);
+ addPathIfExists(D, SysRoot + "/lib", Paths);
+ addPathIfExists(D, SysRoot + "/usr/lib", Paths);
++
+ // Add libc's lib/ directory to the search path, so that crt1.o, crti.o,
+ // and friends can be found.
+ addPathIfExists(D, "@GLIBC_LIBDIR@", Paths);
}
bool Linux::HasNativeLLVMSupport() const { return true; }
+@@ -4026,6 +4034,34 @@
+ addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/include");
+
+ addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/usr/include");
++
++ // Check for configure-time "extra" C include directories. When constructing a
++ // toolchain, @C_EXTRA_INCLUDE_DIRS@ should be replaced with something like a
++ // colon-separated string of the include dirs of libc and kernel headers.
++ //
++ // The reason why we use this mechanism instead of C_INCLUDE_DIRS above is
++ // because when a user supplies clang with --sysroot, the normal expectation
++ // is that clang will detect and add the proper $SYSROOT/$MULTIARCHINCL,
++ // $SYSROOT/include, and $SYSROOT/usr/include to its list of search paths.
++ // However, if C_INCLUDE_DIRS is not empty, this function will return early
++ // and not attempt to add the aforementioned search paths, which is not
++ // desirable.
++ //
++ // By adding our configure-time "extra" C include directories here, after
++ // we've added $SYSROOT/include and $SYSROOT/usr/include, we make sure that IF
++ // --sysroot is supplied on the command line, we pick up the expected search
++ // paths in the $SYSROOT, and that they come before our configure-time "extra"
++ // C include directories.
++ StringRef CExtraIncludeDirs("@C_EXTRA_INCLUDE_DIRS@");
++ if (CExtraIncludeDirs != "") {
++ SmallVector<StringRef, 5> dirs;
++ CExtraIncludeDirs.split(dirs, ":");
++ for (StringRef dir : dirs) {
++ StringRef Prefix =
++ llvm::sys::path::is_absolute(dir) ? StringRef(SysRoot) : "";
++ addExternCSystemInclude(DriverArgs, CC1Args, Prefix + dir);
++ }
++ }
+ }
+
+
Toggle diff (13 lines)
diff --git a/gnu/packages/patches/clang-6.0-libc-search-path.patch b/gnu/packages/patches/clang-6.0-libc-search-path.patch
index a62e8063c2..d80c798467 100644
--- a/gnu/packages/patches/clang-6.0-libc-search-path.patch
+++ b/gnu/packages/patches/clang-6.0-libc-search-path.patch
@@ -8,18 +8,17 @@ to make sure Clang also works on non-GuixSD systems.
 
 --- cfe-6.0.0.src/lib/Driver/ToolChains/Linux.cpp
 +++ cfe-6.0.0.src/lib/Driver/ToolChains/Linux.cpp
-@@ -207,7 +207,9 @@
-   PPaths.push_back(Twine(GCCInstallation.getParentLibPath() + "/../" +
+@@ -208,6 +208,9 @@
                           GCCInstallation.getTriple().str() + "/bin")
                         .str());
--
+
+ // Comment out the distro-specific tweaks so that they don't bite when
+ // using Guix on a foreign distro.
+#if 0
Distro Distro(D.getVFS());
if (Distro.IsAlpineLinux()) {
-@@ -255,6 +257,7 @@
+@@ -255,6 +258,7 @@
if (IsAndroid || Distro.IsOpenSUSE())
ExtraOpts.push_back("--enable-new-dtags");
@@ -27,41 +26,49 @@ to make sure Clang also works on non-GuixSD systems.
// The selection of paths to try here is designed to match the patterns which
// the GCC driver itself uses, as this is part of the GCC-compatible driver.
-@@ -329,14 +332,12 @@
- addPathIfExists(D, D.Dir + "/../" + OSLibDir, Paths);
- }
-
-- addPathIfExists(D, SysRoot + "/lib/" + MultiarchTriple, Paths);
-- addPathIfExists(D, SysRoot + "/lib/../" + OSLibDir, Paths);
-- addPathIfExists(D, SysRoot + "/usr/lib/" + MultiarchTriple, Paths);
-- addPathIfExists(D, SysRoot + "/usr/lib/../" + OSLibDir, Paths);
--
- // Try walking via the GCC triple path in case of biarch or multiarch GCC
- // installations with strange symlinks.
- if (GCCInstallation.isValid()) {
-+ // The following code would end up adding things like
-+ // "/usr/lib/x86_64-unknown-linux-gnu/../../lib64" to the search path.
-+#if 0
- addPathIfExists(D,
- SysRoot + "/usr/lib/" + GCCInstallation.getTriple().str() +
- "/../../" + OSLibDir,
-@@ -349,6 +350,7 @@
- BiarchSibling.gccSuffix(),
- Paths);
- }
-+#endif
+@@ -375,6 +379,10 @@
- // See comments above on the multilib variant for details of why this is
- // included even from outside the sysroot.
-@@ -373,8 +375,9 @@
- if (StringRef(D.Dir).startswith(SysRoot))
- addPathIfExists(D, D.Dir + "/../lib", Paths);
-
-- addPathIfExists(D, SysRoot + "/lib", Paths);
-- addPathIfExists(D, SysRoot + "/usr/lib", Paths);
+ addPathIfExists(D, SysRoot + "/lib", Paths);
+ addPathIfExists(D, SysRoot + "/usr/lib", Paths);
++
+ // Add libc's lib/ directory to the search path, so that crt1.o, crti.o,
+ // and friends can be found.
+ addPathIfExists(D, "@GLIBC_LIBDIR@", Paths);
}
bool Linux::HasNativeLLVMSupport() const { return true; }
+@@ -710,6 +718,34 @@
+ addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/include");
+
+ addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/usr/include");
++
++ // Check for configure-time "extra" C include directories. When constructing a
++ // toolchain, @C_EXTRA_INCLUDE_DIRS@ should be replaced with something like a
++ // colon-separated string of the include dirs of libc and kernel headers.
++ //
++ // The reason why we use this mechanism instead of C_INCLUDE_DIRS above is
++ // because when a user supplies clang with --sysroot, the normal expectation
++ // is that clang will detect and add the proper $SYSROOT/$MULTIARCHINCL,
++ // $SYSROOT/include, and $SYSROOT/usr/include to its list of search paths.
++ // However, if C_INCLUDE_DIRS is not empty, this function will return early
++ // and not attempt to add the aforementioned search paths, which is not
++ // desirable.
++ //
++ // By adding our configure-time "extra" C include directories here, after
++ // we've added $SYSROOT/include and $SYSROOT/usr/include, we make sure that IF
++ // --sysroot is supplied on the command line, we pick up the expected search
++ // paths in the $SYSROOT, and that they come before our configure-time "extra"
++ // C include directories.
++ StringRef CExtraIncludeDirs("@C_EXTRA_INCLUDE_DIRS@");
++ if (CExtraIncludeDirs != "") {
++ SmallVector<StringRef, 5> dirs;
++ CExtraIncludeDirs.split(dirs, ":");
++ for (StringRef dir : dirs) {
++ StringRef Prefix =
++ llvm::sys::path::is_absolute(dir) ? StringRef(SysRoot) : "";
++ addExternCSystemInclude(DriverArgs, CC1Args, Prefix + dir);
++ }
++ }
+ }
+
+ static std::string DetectLibcxxIncludePath(StringRef base) {
Toggle diff (120 lines)
diff --git a/gnu/packages/patches/clang-7.0-libc-search-path.patch b/gnu/packages/patches/clang-7.0-libc-search-path.patch
index 07ff8c90bd..445b5d17ac 100644
--- a/gnu/packages/patches/clang-7.0-libc-search-path.patch
+++ b/gnu/packages/patches/clang-7.0-libc-search-path.patch
@@ -8,7 +8,7 @@ to make sure Clang also works on non-GuixSD systems.
 
 --- a/lib/Driver/ToolChains/Linux.cpp
 +++ b/lib/Driver/ToolChains/Linux.cpp
-@@ -225,7 +225,9 @@ Linux::Linux(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
+@@ -225,7 +225,9 @@
    PPaths.push_back(Twine(GCCInstallation.getParentLibPath() + "/../" +
                           GCCInstallation.getTriple().str() + "/bin")
                         .str());
@@ -19,7 +19,7 @@ to make sure Clang also works on non-GuixSD systems.
    Distro Distro(D.getVFS());
  
    if (Distro.IsAlpineLinux()) {
-@@ -284,6 +286,7 @@ Linux::Linux(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
+@@ -284,6 +286,7 @@
  
    if (IsAndroid || Distro.IsOpenSUSE())
      ExtraOpts.push_back("--enable-new-dtags");
@@ -27,56 +27,49 @@ to make sure Clang also works on non-GuixSD systems.
  
    // The selection of paths to try here is designed to match the patterns which
    // the GCC driver itself uses, as this is part of the GCC-compatible driver.
-@@ -342,7 +345,7 @@ Linux::Linux(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
-     // the cross. Note that GCC does include some of these directories in some
-     // configurations but this seems somewhere between questionable and simply
-     // a bug.
--    if (StringRef(LibPath).startswith(SysRoot)) {
-+    if (0) {
-       addPathIfExists(D, LibPath + "/" + MultiarchTriple, Paths);
-       addPathIfExists(D, LibPath + "/../" + OSLibDir, Paths);
-     }
-@@ -361,6 +364,8 @@ Linux::Linux(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
-   addPathIfExists(D, SysRoot + "/lib/" + MultiarchTriple, Paths);
-   addPathIfExists(D, SysRoot + "/lib/../" + OSLibDir, Paths);
+@@ -431,6 +434,10 @@
  
-+  // This requires the commented distro tweaks above.
-+#if 0
-   if (IsAndroid) {
-     // Android sysroots contain a library directory for each supported OS
-     // version as well as some unversioned libraries in the usual multiarch
-@@ -389,10 +394,14 @@ Linux::Linux(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
-     addPathIfExists(D, SysRoot + "/" + OSLibDir + "/" + ABIName, Paths);
-     addPathIfExists(D, SysRoot + "/usr/" + OSLibDir + "/" + ABIName, Paths);
-   }
-+#endif
- 
-   // Try walking via the GCC triple path in case of biarch or multiarch GCC
-   // installations with strange symlinks.
-   if (GCCInstallation.isValid()) {
-+    // The following code would end up adding things like
-+    // "/usr/lib/x86_64-unknown-linux-gnu/../../lib64" to the search path.
-+#if 0
-     addPathIfExists(D,
-                     SysRoot + "/usr/lib/" + GCCInstallation.getTriple().str() +
-                         "/../../" + OSLibDir,
-@@ -405,6 +414,7 @@ Linux::Linux(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
-                              BiarchSibling.gccSuffix(),
-                       Paths);
-     }
-+#endif
- 
-     // See comments above on the multilib variant for details of why this is
-     // included even from outside the sysroot.
-@@ -429,8 +439,9 @@ Linux::Linux(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
-   if (StringRef(D.Dir).startswith(SysRoot))
-     addPathIfExists(D, D.Dir + "/../lib", Paths);
- 
--  addPathIfExists(D, SysRoot + "/lib", Paths);
--  addPathIfExists(D, SysRoot + "/usr/lib", Paths);
+   addPathIfExists(D, SysRoot + "/lib", Paths);
+   addPathIfExists(D, SysRoot + "/usr/lib", Paths);
++
 +  // Add libc's lib/ directory to the search path, so that crt1.o, crti.o,
 +  // and friends can be found.
 +  addPathIfExists(D, "@GLIBC_LIBDIR@", Paths);
  }
  
  bool Linux::HasNativeLLVMSupport() const { return true; }
+@@ -794,6 +801,34 @@
+   addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/include");
+ 
+   addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/usr/include");
++
++  // Check for configure-time "extra" C include directories. When constructing a
++  // toolchain, @C_EXTRA_INCLUDE_DIRS@ should be replaced with something like a
++  // colon-separated string of the include dirs of libc and kernel headers.
++  //
++  // The reason why we use this mechanism instead of C_INCLUDE_DIRS above is
++  // because when a user supplies clang with --sysroot, the normal expectation
++  // is that clang will detect and add the proper $SYSROOT/$MULTIARCHINCL,
++  // $SYSROOT/include, and $SYSROOT/usr/include to its list of search paths.
++  // However, if C_INCLUDE_DIRS is not empty, this function will return early
++  // and not attempt to add the aforementioned search paths, which is not
++  // desirable.
++  //
++  // By adding our configure-time "extra" C include directories here, after
++  // we've added $SYSROOT/include and $SYSROOT/usr/include, we make sure that IF
++  // --sysroot is supplied on the command line, we pick up the expected search
++  // paths in the $SYSROOT, and that they come before our configure-time "extra"
++  // C include directories.
++  StringRef CExtraIncludeDirs("@C_EXTRA_INCLUDE_DIRS@");
++  if (CExtraIncludeDirs != "") {
++      SmallVector<StringRef, 5> dirs;
++      CExtraIncludeDirs.split(dirs, ":");
++      for (StringRef dir : dirs) {
++          StringRef Prefix =
++              llvm::sys::path::is_absolute(dir) ? StringRef(SysRoot) : "";
++          addExternCSystemInclude(DriverArgs, CC1Args, Prefix + dir);
++      }
++  }
+ }
+ 
+ static std::string DetectLibcxxIncludePath(StringRef base) {
-- 
2.24.0
M
M
Mathieu Othacehe wrote on 26 Nov 2019 08:15
(name . Carl Dong)(address . contact@carldong.me)(address . 38376@debbugs.gnu.org)
8736ebt8fr.fsf@gmail.com
Hello Carl,

Toggle quote (4 lines)
> This was fine when running clang normally, but when a user supplies
> clang with --sysroot, clang _should_ do the search path detection. This
> commit fixes runing clang with --sysroot.

Thanks for your patch. Do you think this could be done in a way that can
be upstreamed? In lib/Driver/ToolChains/Gnu.cpp, there are some Gentoo
specific stuff, maybe they would agree to integrate some Guix/nix specific
stuff too.

WDYT?

Mathieu
C
C
Carl Dong wrote on 26 Nov 2019 18:05
(name . Mathieu Othacehe)(address . m.othacehe@gmail.com)(address . 38376@debbugs.gnu.org)
379EB7C5-A387-4331-A48A-91C9083F3A73@carldong.me
Hi Mathieu!

I think that’s an excellent idea but unfortunately I don’t have the bandwidth for it currently… I’ll put it on my TODO list though :-)
Please do let me know if there’s anything amiss in the patch as it stands right now!

Cheers,
Carl Dong


Toggle quote (21 lines)
> On Nov 26, 2019, at 2:15 AM, Mathieu Othacehe <m.othacehe@gmail.com> wrote:
>
>
>
> Hello Carl,
>
>> This was fine when running clang normally, but when a user supplies
>> clang with --sysroot, clang _should_ do the search path detection. This
>> commit fixes runing clang with --sysroot.
>
> Thanks for your patch. Do you think this could be done in a way that can
> be upstreamed? In lib/Driver/ToolChains/Gnu.cpp, there are some Gentoo
> specific stuff, maybe they would agree to integrate some Guix/nix specific
> stuff too.
>
> WDYT?
>
> Mathieu
>
>
>
M
M
Mathieu Othacehe wrote on 2 Dec 2019 15:03
(name . Carl Dong)(address . contact@carldong.me)(address . 38376@debbugs.gnu.org)
8736e2lt8f.fsf@gmail.com
Hello Carl,

Toggle quote (2 lines)
> Please do let me know if there’s anything amiss in the patch as it stands right now!

Sorry for the delay! Well in the current form we end up with 4 big
patches to clang + 2 that are missing for clang-8 and clang-9.

This makes 6 patches, it will keep increasing. So maintaing those
patches will be very hard. I'd like to find another way to proceed.

Could we instead patch "clang-from-llvm" procedure to fix sysroot
support?

Mathieu
C
C
Carl Dong wrote on 2 Dec 2019 19:10
(name . Mathieu Othacehe)(address . m.othacehe@gmail.com)(address . 38376@debbugs.gnu.org)
A72CBE3E-CB34-4694-BC3F-07517168B693@carldong.me
Toggle quote (2 lines)
> This makes 6 patches, it will keep increasing. So maintaing those
> patches will be very hard. I'd like to find another way to proceed.
I agree it would be very nice if we can avoid maintaining these patches…

Toggle quote (2 lines)
> Could we instead patch "clang-from-llvm" procedure to fix sysroot
> support?
Do you have anything particular in mind? I can’t think of any clean ways to do this off the top of my head right now unfortunately :-(

Cheers,
Carl Dong
M
M
Mathieu Othacehe wrote on 2 Dec 2019 20:38
(name . Carl Dong)(address . contact@carldong.me)(address . 38376@debbugs.gnu.org)
877e3ewmau.fsf@gmail.com
Toggle quote (4 lines)
>> Could we instead patch "clang-from-llvm" procedure to fix sysroot
>> support?
> Do you have anything particular in mind? I can’t think of any clean ways to do this off the top of my head right now unfortunately :-(

No sadly, nothing in mind at the moment, but I'll have a look tomorrow!

Thanks,

Mathieu
C
C
Carl Dong wrote on 5 Dec 2019 19:26
(name . Mathieu Othacehe)(address . m.othacehe@gmail.com)(address . 38376@debbugs.gnu.org)
33E84CBF-D451-4512-A515-1DA6A5E26F20@carldong.me
Hey Mathieu,

Did you get a chance to think of something? If not, what do you think about going with this right now and leaving an issue open for a cleaner way to patch in the future?
Thanks again for all your help and insight.

Cheers,
Carl Dong

Toggle quote (16 lines)
> On Dec 2, 2019, at 2:38 PM, Mathieu Othacehe <m.othacehe@gmail.com> wrote:
>
>
>
>>> Could we instead patch "clang-from-llvm" procedure to fix sysroot
>>> support?
>> Do you have anything particular in mind? I can’t think of any clean ways to do this off the top of my head right now unfortunately :-(
>
> No sadly, nothing in mind at the moment, but I'll have a look tomorrow!
>
> Thanks,
>
> Mathieu
>
>
>
M
M
Mathieu Othacehe wrote on 8 Dec 2019 16:31
(name . Carl Dong)(address . contact@carldong.me)(address . 38376@debbugs.gnu.org)
8736duj0lf.fsf@gmail.com
Hello Carl,

Toggle quote (3 lines)
> Did you get a chance to think of something? If not, what do you think about going with this right now and leaving an issue open for a cleaner way to patch in the future?
> Thanks again for all your help and insight.

No sadly I didn't get time :(. I would prefer we at least wait to find a way to
factorize the different patches.

Thank you,

Mathieu
?