bug#32773: [PATCH] Fix clang libstdc++ header location search

bug#32773: [PATCH] Fix clang libstdc++ header location search

[Mathieu Othacehe]

Hello David,

This patch is a real progress, thank you! However, I still have a
linking error when compiling a trivial test program with clang:

mathieu@elbruz ~/guix [env]$ ./pre-inst-env guix environment --ad-hoc clang --pure
mathieu@elbruz ~/guix [env]$ clang ~/tmp/t.cpp 
clang-8: error: unable to execute command: Executable "ld" doesn't exist!
clang-8: error: linker command failed with exit code 1 (use -v to see invocation)

This is because the linker provided by "binutils" is not accessible. I
guess we need to make something analog to make-gcc-toolchain.

WDYT?

Mathieu

David Truby writes:

> ---
>  gnu/packages/llvm.scm | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/gnu/packages/llvm.scm b/gnu/packages/llvm.scm
> index 33fb53d65e..0a7efe980f 100644
> --- a/gnu/packages/llvm.scm
> +++ b/gnu/packages/llvm.scm
> @@ -202,6 +202,7 @@ compiler.  In LLVM this library is called \"compiler-rt\".")
>                     (lambda* (#:key inputs #:allow-other-keys)
>                       (let ((libc (assoc-ref inputs "libc"))
>                             (compiler-rt (assoc-ref inputs "clang-runtime"))
> +                           (gcc (assoc-ref inputs "gcc"))
>                             (version
>                              (string->number
>                               ,(version-major (package-version clang-runtime)))))
> @@ -218,6 +219,10 @@ compiler.  In LLVM this library is called \"compiler-rt\".")
>                              (("(^[[:blank:]]+LibDir = ).*" _ declaration)
>                               (string-append declaration "\"" libc "/lib\";\n"))
>  
> +                            ;; Make clang look for libstdc++ in the right location
> +                            (("LibStdCXXIncludePathCandidates\\[\\] = \\{")
> +                             (string-append "LibStdCXXIncludePathCandidates[] = { \"" gcc "/include/c++\","))
> +
>                              ;; Make sure libc's libdir is on the search path, to
>                              ;; allow crt1.o & co. to be found.
>                              (("@GLIBC_LIBDIR@")

bug#32773: [PATCH] Fix clang libstdc++ header location search

[David Truby]

Hi Mathieu,

> mathieu@elbruz ~/guix [env]$ ./pre-inst-env guix environment --ad-hoc 
> clang --pure
> mathieu@elbruz ~/guix [env]$ clang ~/tmp/t.cpp 
> clang-8: error: unable to execute command: Executable "ld" doesn't
> exist!
> clang-8: error: linker command failed with exit code 1 (use -v to see
> invocation)

Interesting, for some reason this worked for me. Although now that I
think about it I may not have used a pure environment, because I do
remember seeing this before. 

> 
> This is because the linker provided by "binutils" is not accessible.
> I
> guess we need to make something analog to make-gcc-toolchain.

I think adding gcc-toolchain to propagated-inputs should work in
principle, other than the fact that the gcc used for gcc-lib and
libstdc++ need to be the same as the propagated gcc-toolchain. I did
have a fix for this but I'm not sure if it's the best way of writing
it.
I'll send another patch with the fix though so at least others can
comment on it!

David Truby

bug#32773: [PATCH] Fix clang libstdc++ header location search

[Mathieu Othacehe]

[-- Attachment #1: Type: text/plain, Size: 544 bytes --]


> I think adding gcc-toolchain to propagated-inputs should work in
> principle, other than the fact that the gcc used for gcc-lib and
> libstdc++ need to be the same as the propagated gcc-toolchain. I did
> have a fix for this but I'm not sure if it's the best way of writing
> it.
> I'll send another patch with the fix though so at least others can
> comment on it!

Yes propagating gcc-toolchain would work but, would also cause gcc to be
available as a side effect. Maybe the patch attached, on top of yours,
would work?

Thanks,

Mathieu

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-Add-clang-toolchain.patch --]
[-- Type: text/x-diff, Size: 3501 bytes --]

From ea662ff1ed62183ae0036242f53a14bb0889cb47 Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <m.othacehe@gmail.com>
Date: Thu, 14 Nov 2019 13:25:00 +0100
Subject: [PATCH] gnu: Add clang-toolchain.

* gnu/packages/llvm.scm (make-clang-toolchain): New method.
(clang-toolchain): New public variable defined using previous method.
---
 gnu/packages/llvm.scm | 50 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/gnu/packages/llvm.scm b/gnu/packages/llvm.scm
index 0a7efe980f..8c47b884ae 100644
--- a/gnu/packages/llvm.scm
+++ b/gnu/packages/llvm.scm
@@ -37,7 +37,9 @@
   #:use-module (guix build-system cmake)
   #:use-module (guix build-system emacs)
   #:use-module (guix build-system python)
+  #:use-module (guix build-system trivial)
   #:use-module (gnu packages)
+  #:use-module (gnu packages base)
   #:use-module (gnu packages gcc)
   #:use-module (gnu packages bootstrap)           ;glibc-dynamic-linker
   #:use-module (gnu packages compression)
@@ -295,6 +297,51 @@ project includes the Clang front end, the Clang static analyzer, and several
 code analysis tools.")
     (license license:ncsa)))
 
+(define (make-clang-toolchain clang)
+  (package
+    (name (string-append (package-name clang) "-toolchain"))
+    (version (package-version clang))
+    (source #f)
+    (build-system trivial-build-system)
+    (arguments
+     '(#:modules ((guix build union))
+       #:builder (begin
+                   (use-modules (ice-9 match)
+                                (srfi srfi-26)
+                                (guix build union))
+
+                   (let ((out (assoc-ref %outputs "out")))
+
+                     (match %build-inputs
+                       (((names . directories) ...)
+                        (union-build out directories)))
+
+                     (union-build (assoc-ref %outputs "debug")
+                                  (list (assoc-ref %build-inputs
+                                                   "libc-debug")))
+                     (union-build (assoc-ref %outputs "static")
+                                  (list (assoc-ref %build-inputs
+                                                   "libc-static")))
+                     #t))))
+
+    (native-search-paths (package-native-search-paths clang))
+    (search-paths (package-search-paths clang))
+
+    (license (package-license clang))
+    (home-page "https://clang.llvm.org")
+    (synopsis "Complete Clang toolchain for C/C++ development")
+    (description "This package provides a complete Clang toolchain for C/C++
+development to be installed in user profiles.  This includes Clang, as well as
+libc (headers and binaries, plus debugging symbols in the @code{debug}
+output), and Binutils.")
+    (outputs '("out" "debug" "static"))
+    (inputs `(("clang" ,clang)
+              ("ld-wrapper" ,(car (assoc-ref (%final-inputs) "ld-wrapper")))
+              ("binutils" ,binutils)
+              ("libc" ,glibc)
+              ("libc-debug" ,glibc "debug")
+              ("libc-static" ,glibc "static")))))
+
 (define-public libcxx
   (package
     (name "libcxx")
@@ -404,6 +451,9 @@ with that of libgomp, the GNU Offloading and Multi Processing Library.")
                    "0svk1f70hvpwrjp6x5i9kqwrqwxnmcrw5s7f4cxyd100mdd12k08"
                    #:patches '("clang-7.0-libc-search-path.patch")))
 
+(define-public clang-toolchain
+  (make-clang-toolchain clang))
+
 (define-public llvm-7
   (package
     (inherit llvm)
-- 
2.24.0

bug#32773: [PATCH] Fix clang libstdc++ header location search

[David Truby]

Hi Mathieu,

Applying both your patch and my patch works for me, in both pure and
non-pure environments. I would suggest taht we also hide clang in the
same way as gcc is hidden (in favour of clang-toolchain) to avoid
confusion, but otherwise it seems these two patches together at least
get things to a working state!

I'd like to see make-clang-toolchain allow you to pick between a
specific libstdc++ or libc++, but that requires more thinking so I
think ideally it'd be better to apply these patches first to get things
to a fixed state.

Thanks
David Truby

On Thu, 2019-11-14 at 14:40 +0100, Mathieu Othacehe wrote:
> > I think adding gcc-toolchain to propagated-inputs should work in
> > principle, other than the fact that the gcc used for gcc-lib and
> > libstdc++ need to be the same as the propagated gcc-toolchain. I
> > did
> > have a fix for this but I'm not sure if it's the best way of
> > writing
> > it.
> > I'll send another patch with the fix though so at least others can
> > comment on it!
> 
> Yes propagating gcc-toolchain would work but, would also cause gcc to
> be
> available as a side effect. Maybe the patch attached, on top of
> yours,
> would work?
> 
> Thanks,
> 
> Mathieu

bug#32773: [PATCH] Fix clang libstdc++ header location search

[Mathieu Othacehe]

> Applying both your patch and my patch works for me, in both pure and
> non-pure environments. I would suggest taht we also hide clang in the
> same way as gcc is hidden (in favour of clang-toolchain) to avoid
> confusion, but otherwise it seems these two patches together at least
> get things to a working state!

Great! 

> I'd like to see make-clang-toolchain allow you to pick between a
> specific libstdc++ or libc++, but that requires more thinking so I
> think ideally it'd be better to apply these patches first to get things
> to a fixed state.

I agree. Let's wait to see if someone has objections and I'll push both
patches in a few days.

Thanks for your support on this :)

Mathieu

bug#32773: [PATCH] Fix clang libstdc++ header location search

[David Truby]

No problem, happy to help!

I'm also vaguely thinking if there's a way to upstream some of this
stuff in to clang so that we aren't patching it manually here. The same
file that we're patching has stuff specifically for other distros (e.g.
it has some Gentoo and Cray Linux specific fixes) so I don't think
they'd be opposed to accepting patches that are specific to Guix.
I'm not sure what the answer is there though, as we still need to patch
in the full path to the headers... maybe a cmake option and associated
macro to be used in that file? 

Not that this has any bearing on accepting these patches now, it's just
another thought for the future :)

David Truby

On Thu, 2019-11-14 at 17:54 +0100, Mathieu Othacehe wrote:
> > Applying both your patch and my patch works for me, in both pure
> > and
> > non-pure environments. I would suggest taht we also hide clang in
> > the
> > same way as gcc is hidden (in favour of clang-toolchain) to avoid
> > confusion, but otherwise it seems these two patches together at
> > least
> > get things to a working state!
> 
> Great! 
> 
> > I'd like to see make-clang-toolchain allow you to pick between a
> > specific libstdc++ or libc++, but that requires more thinking so I
> > think ideally it'd be better to apply these patches first to get
> > things
> > to a fixed state.
> 
> I agree. Let's wait to see if someone has objections and I'll push
> both
> patches in a few days.
> 
> Thanks for your support on this :)
> 
> Mathieu

bug#32773: [PATCH] Fix clang libstdc++ header location search

[Mathieu Othacehe]

> I'm not sure what the answer is there though, as we still need to patch
> in the full path to the headers... maybe a cmake option and associated
> macro to be used in that file? 

They already have a CMake option to select libc++ or libstdc++, so
adding another option to specify the path of the selected C++ library
would be really nice.

Plus it would benefit Nix and other distros where the path of those
libraries cannot be guessed.

Mathieu