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

[Maxim Cournoyer]

Hello Ludovic,

Ludovic Courtès <address@hidden> writes:

> 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’.

Thank you for working on a fix, and properly understanding the root
cause of the problem.  I've reviewed it and it seems great!  I see that
it is already in core-updates.  I will go ahead and proceed with
rebuilding the world and see what ensues! :-)

Cheers,

Maxim