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

  • Open
  • quality assurance status badge
Details
2 participants
  • Carl Dong
  • Mathieu Othacehe
Owner
unassigned
Submitted by
Carl Dong
Severity
normal

Debbugs page

Carl Dong wrote 5 years ago
(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 (62 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)
- Bia
This message was truncated. Download the full message here.
Mathieu Othacehe wrote 5 years ago
(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
Carl Dong wrote 5 years ago
(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
>
>
>
Mathieu Othacehe wrote 5 years ago
(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
Carl Dong wrote 5 years ago
(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
Mathieu Othacehe wrote 5 years ago
(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
Carl Dong wrote 5 years ago
(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
>
>
>
Mathieu Othacehe wrote 5 years ago
(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
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 38376@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 38376
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch
You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags
mumi command -t +confirmed -t +easy
Or, remove the moreinfo tag and set the help tag
mumi command -t -moreinfo -t +help