bug#30756: GCC >= 6 '-isystem' and C_INCLUDE_PATH behavior changed, breaking

[Ludovic Courtès]

Hello comrades!

(+Cc: Marius.)

Ludovic Courtès <address@hidden> skribis:

> ‘%final-inputs’ order actually looks good:
>
> scheme@(gnu packages commencement)> (map car %final-inputs)
> $2 = ("tar" "gzip" "bzip2" "xz" "file" "diffutils" "patch" "findutils" "gawk" 
> "sed" "grep" "coreutils" "make" "bash" "ld-wrapper" "binutils" "gcc" "libc" 
> "libc:static" "locales")
>
>
> But then it breaks when we add everything:
>
> scheme@(guile-user)> (map car (bag-transitive-inputs (package->bag 
> coreutils)))
> $5 = ("source" "perl" "tar" "gzip" "bzip2" "xz" "file" "diffutils" "patch" 
> "findutils" "gawk" "sed" "grep" "coreutils" "make" "bash" "ld-wrapper" 
> "binutils" "gcc" "libc" "libc:static" "locales" "acl" "gmp" "libcap" 
> "kernel-headers")
>
> Here acl, gmp, and libcap should be before libc and all
> (‘bag-transitive-inputs’ is used by ‘bag->derivation’.)
>
> So I think we should arrange to have the right order in
> ‘bag->derivation’.

The attached patch does three things:

  1. Fix the order of inputs computed by (@@ (guix build-system gnu)
     lower) so that implicit inputs come last.  In particular, this
     ensures that libc headers and kernel headers come last.  All user
     libraries passed as ‘inputs’ appear before libc, so they can
     #include_next a libc header.

  2. Add “include/c++” to the list of directories of
     ‘CPLUS_INCLUDE_PATH’.  This is a not-so-elegant hack; the main
     purpose here is to make sure the gcc/libstdc++ include directory
     appears twice in the search path, so that this chain of include
     works as expected:

       <cstdlib> (GCC): #include_next <stdlib.h>
       → <stdlib.h> (GCC): #include_next <stdlib.h>
       → <stdlib.h> (libc)

  3. Switch back to ‘C_INCLUDE_PATH’ & co. instead of ‘CPATH’ (yay!).

I’ve tested it with “guix build coreutils”, which involved building GMP
with its C++ bindings, making it a rather good test.  However, more
testing is needed.

There’s potential for breakage in all the places where we’ve manually
fiddled with C{,PLUS}_INCLUDE_PATH.  I guess we’ll have to review all of
them.

Since it fixes a rather serious issue for C/C++ developers (they’d
rather not see warnings about system headers) but also for packaging
(‘-Werror’ breaks for warnings that shouldn’t be there in the first
place), I’d like to propose merging it in this ‘core-updates’ cycle.
But let’s face it: it’ll keep us busy for a bit.  :-)

Thoughts?

Ludo’.

diff --git a/gnu/packages/commencement.scm b/gnu/packages/commencement.scm
index 851bb02163..473d6b8345 100644
--- a/gnu/packages/commencement.scm
+++ b/gnu/packages/commencement.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019 Ludovic Courtès 
<address@hidden>
+;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2020 Ludovic 
Courtès <address@hidden>
 ;;; Copyright © 2014 Andreas Enge <address@hidden>
 ;;; Copyright © 2012 Nikita Karetnikov <address@hidden>
 ;;; Copyright © 2014, 2015, 2017 Mark H Weaver <address@hidden>
@@ -2303,15 +2303,6 @@ exec ~a/bin/~a-~a -B~a/lib -Wl,-dynamic-linker -Wl,~a/~a 
\"$@\"~%"
                                            char-set:letter)
                                          ,(package-name lib)))
                              (list gmp-6.0 mpfr mpc))
-                      #t)))
-                (add-before 'configure 'treat-glibc-as-system-header
-                  (lambda* (#:key inputs #:allow-other-keys)
-                    (let ((libc (assoc-ref inputs "libc")))
-                      ;; Make sure Glibc is treated as a "system header" so
-                      ;; #include_next does the right thing.
-                      (for-each (lambda (var)
-                                  (setenv var (string-append libc "/include")))
-                                '("C_INCLUDE_PATH" "CPLUS_INCLUDE_PATH"))
                       #t))))))))
 
     ;; This time we want Texinfo, so we get the manual.  Add
diff --git a/gnu/packages/gcc.scm b/gnu/packages/gcc.scm
index 40cc9ed631..4c4b63dbd1 100644
--- a/gnu/packages/gcc.scm
+++ b/gnu/packages/gcc.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019 Ludovic Courtès 
<address@hidden>
+;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2020 Ludovic 
Courtès <address@hidden>
 ;;; Copyright © 2014, 2015, 2018 Mark H Weaver <address@hidden>
 ;;; Copyright © 2014, 2015, 2016, 2017, 2019 Ricardo Wurmus <address@hidden>
 ;;; Copyright © 2015 Andreas Enge <address@hidden>
@@ -340,7 +340,9 @@ where the OS part is overloaded to denote a specific 
ABI---into GCC
                (files '("include")))
               (search-path-specification
                (variable "CPLUS_INCLUDE_PATH")
-               (files '("include")))
+               ;; Add 'include/c++' here so that <cstdlib>'s "#include_next
+               ;; <stdlib.h>" finds GCC's <stdlib.h>, not libc's.
+               (files '("include/c++" "include")))
               (search-path-specification
                (variable "LIBRARY_PATH")
                (files '("lib" "lib64")))))
@@ -476,17 +478,7 @@ Go.  It also includes runtime support libraries for these 
languages.")
                                        "gcc-5.0-libvtv-runpath.patch"))))
     (inputs
      `(("isl" ,isl)
-       ,@(package-inputs gcc-4.7)))
-
-    (native-search-paths
-     ;; We have to use 'CPATH' for GCC > 5, not 'C_INCLUDE_PATH' & co., due to
-     ;; <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70129>.
-     (list (search-path-specification
-            (variable "CPATH")
-            (files '("include")))
-           (search-path-specification
-            (variable "LIBRARY_PATH")
-            (files '("lib" "lib64")))))))
+       ,@(package-inputs gcc-4.7)))))
 
 (define-public gcc-7
   (package
diff --git a/guix/build-system/gnu.scm b/guix/build-system/gnu.scm
index 3cc89f8852..6e66f5ffce 100644
--- a/guix/build-system/gnu.scm
+++ b/guix/build-system/gnu.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017, 2019 Ludovic Courtès 
<address@hidden>
+;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017, 2019, 2020 Ludovic Courtès 
<address@hidden>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -296,13 +296,19 @@ standard packages used as implicit inputs of the GNU 
build system."
                           `(("source" ,source))
                           '())
                     ,@native-inputs
+
+                    ;; When not cross-compiling, ensure implicit inputs come
+                    ;; last.  That way, libc headers come last, which allows
+                    ;; #include_next to work correctly; see
+                    ;; <https://bugs.gnu.org/30756>.
+                    ,@(if target '() inputs)
                     ,@(if (and target implicit-cross-inputs?)
                           (standard-cross-packages target 'host)
                           '())
                     ,@(if implicit-inputs?
                           (standard-packages)
                           '())))
-    (host-inputs inputs)
+    (host-inputs (if target inputs '()))
 
     ;; The cross-libc is really a target package, but for bootstrapping
     ;; reasons, we can't put it in 'host-inputs'.  Namely, 'cross-gcc' is a