bug#22186: Use {C,CPLUS,OBJC}_INCLUDE_PATH instead of CPATH

bug#22186: Use {C,CPLUS,OBJC}_INCLUDE_PATH instead of CPATH

[Andy Wingo]

Hi,

I am building GDB from git, because I want to hack on GDB.  I have a few
build-related things in my profile, including GCC.  I enter an
environment for GDB like this:

  guix environment gdb --ad-hoc flex autoconf-2.64

Cool.  Very good.  I build:

  mkdir +2.0
  cd +2.0
  ../configure --prefix=/opt/gdb
  make

However, eventually:

  /bin/sh ./libtool --tag=CC   --mode=compile ccache gcc -DHAVE_CONFIG_H -I. -I../../bfd -I. -I../../bfd -I../../bfd/../include  -DHAVE_x86_64_elf64_vec -DHAVE_i386_elf32_vec -DHAVE_iamcu_elf32_vec -DHAVE_x86_64_elf32_vec -DHAVE_i386_aout_linux_vec -DHAVE_i386_pei_vec -DHAVE_x86_64_pei_vec -DHAVE_l1om_elf64_vec -DHAVE_k1om_elf64_vec -DHAVE_elf64_le_vec -DHAVE_elf64_be_vec -DHAVE_elf32_le_vec -DHAVE_elf32_be_vec -DHAVE_plugin_vec  -DBINDIR='"/opt/guile-2.0/bin"'  -W -Wall -Wstrict-prototypes -Wmissing-prototypes -Wshadow -Werror -I../../bfd/../zlib -g -O2 -MT peigen.lo -MD -MP -MF .deps/peigen.Tpo -c -o peigen.lo peigen.c
  libtool: compile:  ccache gcc -DHAVE_CONFIG_H -I. -I../../bfd -I. -I../../bfd -I../../bfd/../include -DHAVE_x86_64_elf64_vec -DHAVE_i386_elf32_vec -DHAVE_iamcu_elf32_vec -DHAVE_x86_64_elf32_vec -DHAVE_i386_aout_linux_vec -DHAVE_i386_pei_vec -DHAVE_x86_64_pei_vec -DHAVE_l1om_elf64_vec -DHAVE_k1om_elf64_vec -DHAVE_elf64_le_vec -DHAVE_elf64_be_vec -DHAVE_elf32_le_vec -DHAVE_elf32_be_vec -DHAVE_plugin_vec -DBINDIR=\"/opt/guile-2.0/bin\" -W -Wall -Wstrict-prototypes -Wmissing-prototypes -Wshadow -Werror -I../../bfd/../zlib -g -O2 -MT peigen.lo -MD -MP -MF .deps/peigen.Tpo -c peigen.c -o peigen.o
  In file included from peigen.c:66:0:
  /home/wingo/.guix-profile/include/wchar.h: In function 'wctob':
  /home/wingo/.guix-profile/include/wchar.h:397:47: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
   { return (__builtin_constant_p (__wc) && __wc >= L'\0' && __wc <= L'\x7f'
                                                 ^
This is an error, and the error is in wchar.h, not in the invocation or
use of wchar.h.  Let's see where this file comes from:

  ls -l /home/wingo/.guix-profile/include/wchar.h
  lrwxrwxrwx 21 root guixbuild 79 Jan  1  1970 /home/wingo/.guix-profile/include/wchar.h -> /gnu/store/q7k2dc3ylpjnjlhb59f01bxxab8fjxr6-gcc-toolchain-5.2.0/include/wchar.h

So!  gcc-toolchain-5.2.  Why is there an error?  Well, of course,
although harmless, the "error" exists in the source, though perhaps GCC
itself doesn't build with -Werror=type-limits, so they didn't see this
error in the release.  The error comes from the combination of the
-Werror and warning flags that GDB is building with and the files from
gcc-toolchain.

Backing up a bit, it's not something that I actually care about right
now.  I'm working on GDB, the error is innocuous, and I can't easily fix
GCC right now, nor do I care.  I'm also surprised this is happening --
wouldn't it be caught by some other distro user?  Well yes, except that
normally this header is in /usr/include, and by default, all warnings
are disabled for system headers.

And there's the rub!  Why am I getting a warning for code in
~/.guix-profile/include ?

The answer is interesting!  I quote the GCC manual, section "Environment
Variables":

    Some additional environment variables affect the behavior of the
    preprocessor.

    'CPATH'
    'C_INCLUDE_PATH'
    'CPLUS_INCLUDE_PATH'
    'OBJC_INCLUDE_PATH'
         Each variable's value is a list of directories separated by a
         special character, much like 'PATH', in which to look for header
         files.  The special character, 'PATH_SEPARATOR', is
         target-dependent and determined at GCC build time.  For Microsoft
         Windows-based targets it is a semicolon, and for almost all other
         targets it is a colon.

         'CPATH' specifies a list of directories to be searched as if
         specified with '-I', but after any paths given with '-I' options on
         the command line.  This environment variable is used regardless of
         which language is being preprocessed.

         The remaining environment variables apply only when preprocessing
         the particular language indicated.  Each specifies a list of
         directories to be searched as if specified with '-isystem', but
         after any paths given with '-isystem' options on the command line.

         In all these variables, an empty element instructs the compiler to
         search its current working directory.  Empty elements can appear at
         the beginning or end of a path.  For instance, if the value of
         'CPATH' is ':/special/include', that has the same effect as
         '-I. -I/special/include'.

So!  CPATH is like -I but C_INCLUDE_PATH et al are like -isystem.
Here's the docs for -isystem ("Preprocessor Options"):

    '-isystem DIR'
         Search DIR for header files, after all directories specified by
         '-I' but before the standard system directories.  Mark it as a
         system directory, so that it gets the same special treatment as is
         applied to the standard system directories.  If DIR begins with
         '=', then the '=' will be replaced by the sysroot prefix; see
         '--sysroot' and '-isysroot'.

What is a system directory?  Well, it's searched for after all -I
includes, but also header files in it are marked as system headers.
Many warnings are not issued for system headers; search the manual for
the phrase "system headers".  For example:

    '-Wsystem-headers'
         Issue warnings for code in system headers.  These are normally
         unhelpful in finding bugs in your own code, therefore suppressed.
         If you are responsible for the system library, you may want to see
         them.

So.  We should be using C_INCLUDE_PATH instead of CPATH, to mark system
headers as system headers.  Except that C_INCLUDE_PATH only works for C,
so we need to also set CPLUS_INCLUDE_PATH and OBJC_INCLUDE_PATH.  And
that's the proposal of this bug :)

Andy

bug#22186: Use {C,CPLUS,OBJC}_INCLUDE_PATH instead of CPATH

[Efraim Flashner]

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

On Wed, 16 Dec 2015 14:41:11 +0000
Andy Wingo <wingo@igalia.com> wrote:

> Hi,
> 
> I am building GDB from git, because I want to hack on GDB.  I have a few
> build-related things in my profile, including GCC.  I enter an
> environment for GDB like this:
> 
>   guix environment gdb --ad-hoc flex autoconf-2.64
> 
> Cool.  Very good.  I build:
> 
>   mkdir +2.0
>   cd +2.0
>   ../configure --prefix=/opt/gdb
>   make
> 
[...]
> The answer is interesting!  I quote the GCC manual, section "Environment
> Variables":
> 
>     Some additional environment variables affect the behavior of the
>     preprocessor.
> 
>     'CPATH'
>     'C_INCLUDE_PATH'
>     'CPLUS_INCLUDE_PATH'
>     'OBJC_INCLUDE_PATH'
[...]
> 
> So!  CPATH is like -I but C_INCLUDE_PATH et al are like -isystem.
> Here's the docs for -isystem ("Preprocessor Options"):
> 
>     '-isystem DIR'
>          Search DIR for header files, after all directories specified by
>          '-I' but before the standard system directories.  Mark it as a
>          system directory, so that it gets the same special treatment as is
>          applied to the standard system directories.  If DIR begins with
>          '=', then the '=' will be replaced by the sysroot prefix; see
>          '--sysroot' and '-isysroot'.
> 
> What is a system directory?  Well, it's searched for after all -I
> includes, but also header files in it are marked as system headers.
> Many warnings are not issued for system headers; search the manual for
> the phrase "system headers".  For example:
> 
>     '-Wsystem-headers'
>          Issue warnings for code in system headers.  These are normally
>          unhelpful in finding bugs in your own code, therefore suppressed.
>          If you are responsible for the system library, you may want to see
>          them.
> 
> So.  We should be using C_INCLUDE_PATH instead of CPATH, to mark system
> headers as system headers.  Except that C_INCLUDE_PATH only works for C,
> so we need to also set CPLUS_INCLUDE_PATH and OBJC_INCLUDE_PATH.  And
> that's the proposal of this bug :)
> 
> Andy
> 

Are there other ones that could be set? Every time I compile it I see options
for java and go.

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

bug#22186: Use {C,CPLUS,OBJC}_INCLUDE_PATH instead of CPATH

[Andy Wingo]

On Wed 16 Dec 2015 20:18, Efraim Flashner <efraim@flashner.co.il> writes:

>> We should be using C_INCLUDE_PATH instead of CPATH, to mark system
>> headers as system headers.  Except that C_INCLUDE_PATH only works for
>> C, so we need to also set CPLUS_INCLUDE_PATH and OBJC_INCLUDE_PATH.
>> And that's the proposal of this bug :)
>
> Are there other ones that could be set? Every time I compile it I see options
> for java and go.

No, the only ones mentioned in the manual are the ones I mention above.
To me this makes sense, as ObjC and C++ can include C, but that is not
the case for Java and Go.

Andy

bug#22186: Use {C,CPLUS,OBJC}_INCLUDE_PATH instead of CPATH

[Ludovic Courtès]

Efraim Flashner <efraim@flashner.co.il> skribis:

> Are there other ones that could be set? Every time I compile it I see options
> for java and go.

GCJ honors ‘CLASSPATH’ (see gcc/java/jcf-path.c in the GCC tree), but
the Go front-end doesn’t seem to have anything similar.

Ludo’.

bug#22186: Use {C,CPLUS,OBJC}_INCLUDE_PATH instead of CPATH

[Ludovic Courtès]

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

Andy Wingo <wingo@igalia.com> skribis:

> So!  CPATH is like -I but C_INCLUDE_PATH et al are like -isystem.
> Here's the docs for -isystem ("Preprocessor Options"):
>
>     '-isystem DIR'
>          Search DIR for header files, after all directories specified by
>          '-I' but before the standard system directories.  Mark it as a
>          system directory, so that it gets the same special treatment as is
>          applied to the standard system directories.  If DIR begins with
>          '=', then the '=' will be replaced by the sysroot prefix; see
>          '--sysroot' and '-isysroot'.
>
> What is a system directory?  Well, it's searched for after all -I
> includes, but also header files in it are marked as system headers.
> Many warnings are not issued for system headers; search the manual for
> the phrase "system headers".  For example:
>
>     '-Wsystem-headers'
>          Issue warnings for code in system headers.  These are normally
>          unhelpful in finding bugs in your own code, therefore suppressed.
>          If you are responsible for the system library, you may want to see
>          them.
>
> So.  We should be using C_INCLUDE_PATH instead of CPATH, to mark system
> headers as system headers.  Except that C_INCLUDE_PATH only works for C,
> so we need to also set CPLUS_INCLUDE_PATH and OBJC_INCLUDE_PATH.  And
> that's the proposal of this bug :)

The intent of this “system header” classification, AIUI, is to not
bother users with issues in libc headers.

The problem is that if we use C_INCLUDE_PATH & co., every header in the
search path, not just libc’s, would now be considered a system header,
and thus exempt from warnings.  This would be undesirable.

Here’s an experiment:

--8<---------------cut here---------------start------------->8---
$ guix environment --ad-hoc gcc ld-wrapper glibc binutils --container
sh-4.3# gcc -Werror=type-limits -O2 -c sysp.c
In file included from sysp.c:1:0:
/gnu/store/n96gwg9fwmxmz1bhy219v3vvmsjsk7gz-glibc-2.22/include/wchar.h: In function 'wctob':
/gnu/store/n96gwg9fwmxmz1bhy219v3vvmsjsk7gz-glibc-2.22/include/wchar.h:397:47: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
 { return (__builtin_constant_p (__wc) && __wc >= L'\0' && __wc <= L'\x7f'
                                               ^
cc1: some warnings being treated as errors
sh-4.3# echo $CPATH
/gnu/store/lq595bjrgav37b05bmmafigwargasy8k-binutils-2.25.1/include:/gnu/store/n96gwg9fwmxmz1bhy219v3vvmsjsk7gz-glibc-2.22/include:/gnu/store/14xgip7wslikzqfkr67vln0kpcclwy37-linux-libre-headers-3.14.37/include:/gnu/store/0wq6hcbfjh5clyz6pjcqxjkl60rmijjf-gcc-5.2.0/include
sh-4.3# export CPATH=/gnu/store/lq595bjrgav37b05bmmafigwargasy8k-binutils-2.25.1/include:/gnu/store/14xgip7wslikzqfkr67vln0kpcclwy37-linux-libre-headers-3.14.37/include:/gnu/store/0wq6hcbfjh5clyz6pjcqxjkl60rmijjf-gcc-5.2.0/include
sh-4.3# gcc -Werror=type-limits -O2 -c sysp.c
--8<---------------cut here---------------end--------------->8---

Where sysp.c is:

--8<---------------cut here---------------start------------->8---
#include <wchar.h>

int foo () { return 42; }
--8<---------------cut here---------------end--------------->8---

Compare with this:

--8<---------------cut here---------------start------------->8---
$ guix environment --ad-hoc gcc ld-wrapper -e '(@@ (gnu packages commencement) glibc-final)' binutils --container
sh-4.3# gcc -Werror=type-limits -O2 -c sysp.c
sh-4.3# echo $CPATH
/gnu/store/lq595bjrgav37b05bmmafigwargasy8k-binutils-2.25.1/include:/gnu/store/qv7bk62c22ms9i11dhfl71hnivyc82k2-glibc-2.22/include:/gnu/store/lyn2331ilik14yy2jqhndshvxmv9r6w5-linux-libre-headers-3.14.37/include:/gnu/store/0wq6hcbfjh5clyz6pjcqxjkl60rmijjf-gcc-5.2.0/include
--8<---------------cut here---------------end--------------->8---

The lesson here is that, if the libc headers that are in CPATH come from
the libc that GCC was built against, then they do not lose their
system-headerness.


Now, when using ‘gcc-toolchain’, CPATH contains an entry that is a
*symlink* to libc.  So from the point of view of libcpp, these are not
system headers.  Thus:

--8<---------------cut here---------------start------------->8---
$ guix environment --ad-hoc gcc-toolchain --container
sh-4.3# gcc -Werror=type-limits -O2 -c sysp.c
In file included from sysp.c:1:0:
/gnu/store/q7k2dc3ylpjnjlhb59f01bxxab8fjxr6-gcc-toolchain-5.2.0/include/wchar.h: In function 'wctob':
/gnu/store/q7k2dc3ylpjnjlhb59f01bxxab8fjxr6-gcc-toolchain-5.2.0/include/wchar.h:397:47: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
 { return (__builtin_constant_p (__wc) && __wc >= L'\0' && __wc <= L'\x7f'
                                               ^
cc1: some warnings being treated as errors
sh-4.3# echo $CPATH
/gnu/store/q7k2dc3ylpjnjlhb59f01bxxab8fjxr6-gcc-toolchain-5.2.0/include
--8<---------------cut here---------------end--------------->8---

This particular problem with gcc-toolchain in ‘guix environment’ is
solved by this patch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 1419 bytes --]

diff --git a/gnu/packages/commencement.scm b/gnu/packages/commencement.scm
index 73b0ce4..fbf4361 100644
--- a/gnu/packages/commencement.scm
+++ b/gnu/packages/commencement.scm
@@ -794,9 +794,13 @@ COREUTILS-FINAL vs. COREUTILS, etc."
 
                    (let ((out (assoc-ref %outputs "out")))
 
-                     (match %build-inputs
-                       (((names . directories) ...)
-                        (union-build out directories)))
+                     (define (select label)
+                       (assoc-ref %build-inputs label))
+
+                     (union-build out
+                                  (list (select "gcc")
+                                        (select "ld-wrapper")
+                                        (select "binutils")))
 
                      (union-build (assoc-ref %outputs "debug")
                                   (list (assoc-ref %build-inputs
@@ -820,8 +824,9 @@ and binaries, plus debugging symbols in the 'debug' output), and Binutils.")
     (inputs `(("gcc" ,gcc)
               ("ld-wrapper" ,(car (assoc-ref %final-inputs "ld-wrapper")))
               ("binutils" ,binutils-final)
-              ("libc" ,glibc-final)
-              ("libc-debug" ,glibc-final "debug")))))
+              ("libc-debug" ,glibc-final "debug")))
+
+    (propagated-inputs `(("libc" ,glibc-final)))))
 
 (define-public gcc-toolchain-4.8
   (gcc-toolchain gcc-final))

[-- Attachment #3: Type: text/plain, Size: 791 bytes --]


However, this doesn’t help with the case where gcc-toolchain is
installed in a profile.


Libcpp does have a file canonicalization method, enabled by default (see
--enable-canonical-system-headers.)  Specifically, in files.c,
‘find_file_in_dir’ does:

    /* We try to canonicalize system headers.  */
    if (CPP_OPTION (pfile, canonical_system_headers) && file->dir->sysp)
      {
        char * canonical_path = maybe_shorter_path (path);

Unfortunately, ‘maybe_shorter_path’ (which calls ‘lrealpath’ from
libiberty) doesn’t seem to be called for libc headers, presumably
because it’s too late: file->dir->sysp is false.


In summary, so far I can only think of half a solution, which is the
‘gcc-toolchain’ fix above.

Thoughts?

Ludo’.

bug#22186: Use {C,CPLUS,OBJC}_INCLUDE_PATH instead of CPATH

[Andy Wingo]

Thank you for thinking about this :)

On Thu 17 Dec 2015 22:43, ludo@gnu.org (Ludovic Courtès) writes:

> Andy Wingo <wingo@igalia.com> skribis:
>
>> We should be using C_INCLUDE_PATH instead of CPATH, to mark system
>> headers as system headers.  Except that C_INCLUDE_PATH only works for
>> C, so we need to also set CPLUS_INCLUDE_PATH and OBJC_INCLUDE_PATH.
>> And that's the proposal of this bug :)
>
> The intent of this “system header” classification, AIUI, is to not
> bother users with issues in libc headers.

I don't think this is true, for what it's worth :)  If we take FHS
systems to be the de-facto default standard on how things should behave,
-isystem covers all of /usr/include, so in practice it covers not just
libc warnings, but many other warnings, which when you pass -Werror
would then become errors.

I think it's reasonable to suppose that there are many packages out
there that have warnings in their header files for some set of -W
arguments that the package author didn't test.  I specifically remember
getting warnings related to ELF headers for a different project, before
I understood this problem.  On an FHS system of course these problems
are swept under the rug and we don't see them -- no warning, no problem
with -Werror.

It seems to me that just as all packages in /usr/include are marked
"system" on FHS systems, a Guix profile should mark its headers as
"system".

> The problem is that if we use C_INCLUDE_PATH & co., every header in the
> search path, not just libc’s, would now be considered a system header,
> and thus exempt from warnings.  This would be undesirable.

Obviously the best thing would be if there were never any warnings in
the headers exported by projects.  However I think this is unlikely:
warnings evolve over time, and the author of libfoo doesn't choose the
warnings that users of her package enable, and probably doesn't test all
warnings.  Most users, especially users that build on FHS systems, will
never see these warnings anyway.

I think on this question Guix should choose the pragmatic way that lets
users get on with hacking and doesn't detour them into warnings in
headers of packages they use :)

Andy

bug#22186: Use {C,CPLUS,OBJC}_INCLUDE_PATH instead of CPATH

[Ludovic Courtès]

Andy Wingo <wingo@igalia.com> skribis:

> On Thu 17 Dec 2015 22:43, ludo@gnu.org (Ludovic Courtès) writes:
>
>> Andy Wingo <wingo@igalia.com> skribis:
>>
>>> We should be using C_INCLUDE_PATH instead of CPATH, to mark system
>>> headers as system headers.  Except that C_INCLUDE_PATH only works for
>>> C, so we need to also set CPLUS_INCLUDE_PATH and OBJC_INCLUDE_PATH.
>>> And that's the proposal of this bug :)
>>
>> The intent of this “system header” classification, AIUI, is to not
>> bother users with issues in libc headers.
>
> I don't think this is true, for what it's worth :)  If we take FHS
> systems to be the de-facto default standard on how things should behave,
> -isystem covers all of /usr/include, so in practice it covers not just
> libc warnings, but many other warnings, which when you pass -Werror
> would then become errors.

Now that you mention it, it makes a lot of sense to me; I must have
lived away from FHS for too long now.  ;-)

We’re right on time to make the change you propose in ‘core-updates’.
I’ll give it a try and we’ll see how it goes.

Thanks for bearing with me!

Ludo’.

bug#22186: Use {C,CPLUS,OBJC}_INCLUDE_PATH instead of CPATH

[Ludovic Courtès]

Done in 009b53f (current ‘core-updates’.)

Ludo’.