GUILE_CFLAGS contains warning options

GUILE_CFLAGS contains warning options

[Bruno Haible]

Hi,

After guile-2.0.0 is installed using gcc, the installed file
$LIBDIR/pkgconfig/guile-2.0.pc contains a line such as

  Cflags: -I${pkgincludedir}/2.0 -Wall -I$LIBUNISTRING_PREFIX/include -pthread -I$GC_PREFIX/include  

and "guile-config compile" (which is used to define GUILE_CFLAGS through the
macro GUILE_FLAGS in $PREFIX/share/aclocal/guile.m4) produces output such as:

  -Wall -pthread -I$PREFIX/include/guile/2.0 -I$LIBUNISTRING_PREFIX/include -I$GC_PREFIX/include  

The presence of -Wall is IMO wrong for two reasons:

1) If I were to use a compiler different from gcc for compiling programs
   that use guile (such as the vendor cc on Solaris, HP-UX, OSF/1, or AIX), it
   would lead to an error when invoking that compiler.

2) Even if I use gcc, it's not guile's business to enable warning options on
   _my_ code just because my code uses guile.

Bruno
-- 
In memoriam Friedrich Weißler <http://en.wikipedia.org/wiki/Friedrich_Weißler>

Re: GUILE_CFLAGS contains warning options

[Bruno Haible]

PS: Some details: I had configured guile with
 PKG_CONFIG_PATH=/arch/x86_64-linux/gnu-inst-libffi/3.0.9/lib64/pkgconfig:/arch/x86_64-linux/gnu/lib64/pkgconfig \
./configure --host=x86_64-pc-linux-gnu \
            --prefix=/arch/x86_64-linux/gnu-inst-guile/2.0.0 \
            --libdir=/arch/x86_64-linux/gnu-inst-guile/2.0.0/lib64 \
            CPPFLAGS=-Wall \
            --with-libunistring-prefix=/arch/x86_64-linux/gnu-inst-libunistring/0.9.3

and none of the files
  /arch/x86_64-linux/gnu-inst-libffi/3.0.9/lib64/pkgconfig/libffi.pc
  /arch/x86_64-linux/gnu/lib64/pkgconfig/bdw-gc.pc
contains a mention of "-Wall".

Bruno
-- 
In memoriam Friedrich Weißler <http://en.wikipedia.org/wiki/Friedrich_Weißler>

Re: GUILE_CFLAGS contains warning options

[Andy Wingo]

On Sat 19 Feb 2011 15:07, Bruno Haible <bruno@clisp.org> writes:

> Hi,
>
> After guile-2.0.0 is installed using gcc, the installed file
> $LIBDIR/pkgconfig/guile-2.0.pc contains a line such as
>
>   Cflags: -I${pkgincludedir}/2.0 -Wall -I$LIBUNISTRING_PREFIX/include -pthread -I$GC_PREFIX/include  

The template is:

  Cflags: -I${pkgincludedir}/@GUILE_EFFECTIVE_VERSION@ @GUILE_CFLAGS@ @BDW_GC_CFLAGS@

In my copy I have:

  Cflags: -I${pkgincludedir}/2.0  -pthread  

And in configure.ac I have:

  GUILE_CFLAGS="$CPPFLAGS $PTHREAD_CFLAGS"

You said that you configured like this:

  ./configure --host=x86_64-pc-linux-gnu \
              --prefix=/arch/x86_64-linux/gnu-inst-guile/2.0.0 \
              --libdir=/arch/x86_64-linux/gnu-inst-guile/2.0.0/lib64 \
              CPPFLAGS=-Wall \
              --with-libunistring-prefix=/arch/x86_64-linux/gnu-inst-libunistring/0.9.3

So it is the CPPFLAGS making it there.

I would just remove CPPFLAGS entirely, but it seems to be used to
communicate libunistring compilation flags to other parts of the build.
Ludo do you have any thoughts on this?

Thanks for the report,

Andy
-- 
http://wingolog.org/

Re: GUILE_CFLAGS contains warning options

[Bruno Haible]

Andy Wingo wrote:
> > $LIBDIR/pkgconfig/guile-2.0.pc contains a line such as
> >
> >   Cflags: -I${pkgincludedir}/2.0 -Wall -I$LIBUNISTRING_PREFIX/include -pthread -I$GC_PREFIX/include  
> 
> The template is:
> 
>   Cflags: -I${pkgincludedir}/@GUILE_EFFECTIVE_VERSION@ @GUILE_CFLAGS@ @BDW_GC_CFLAGS@
> ...
> And in configure.ac I have:
> 
>   GUILE_CFLAGS="$CPPFLAGS $PTHREAD_CFLAGS"

Bingo. This is the problem: The CPPFLAGS variable contains options meant for the
compiler used to build guile, not for the compiler that will use the installed
libguile.

You can extract the -I options from $CPPFLAGS; this would be OK since all
compilers support -I.

For the PTHREAD_CFLAGS it is more tricky: On most platforms you can use
"-lpthread" instead of "-pthread", and all compilers support -l options.
But on OSF/1, the options for using threads are compiler dependent:
  * -pthread for cc,
  * -lpthread for gcc.

Bruno
-- 
In memoriam Juliusz Bursche <http://en.wikipedia.org/wiki/Juliusz_Bursche>

Re: GUILE_CFLAGS contains warning options

[Ken Raeburn]

On Feb 20, 2011, at 17:50, Bruno Haible wrote:
> For the PTHREAD_CFLAGS it is more tricky: On most platforms you can use
> "-lpthread" instead of "-pthread", and all compilers support -l options.
> But on OSF/1, the options for using threads are compiler dependent:
>  * -pthread for cc,
>  * -lpthread for gcc.

There are other platforms where compiling for thread support changes more than just adding a library.  For example, in NetBSD 5.0, it looks like stdio.h defines getc() in a non-thread-safe way if _REENTRANT and _PTHREADS (which get defined by "gcc -pthread") are not defined.  So even with gcc, just replacing "-pthread" with "-lpthread" isn't good enough.

Ken

Re: GUILE_CFLAGS contains warning options

[Ludovic Courtès]

Hi Bruno,

Bruno Haible <bruno@clisp.org> writes:

> Andy Wingo wrote:
>> > $LIBDIR/pkgconfig/guile-2.0.pc contains a line such as
>> >
>> >   Cflags: -I${pkgincludedir}/2.0 -Wall -I$LIBUNISTRING_PREFIX/include -pthread -I$GC_PREFIX/include  
>> 
>> The template is:
>> 
>>   Cflags: -I${pkgincludedir}/@GUILE_EFFECTIVE_VERSION@ @GUILE_CFLAGS@ @BDW_GC_CFLAGS@
>> ...
>> And in configure.ac I have:
>> 
>>   GUILE_CFLAGS="$CPPFLAGS $PTHREAD_CFLAGS"

[...]

> You can extract the -I options from $CPPFLAGS; this would be OK since all
> compilers support -I.

That’s probably the right thing to do.

However, why do you have -Wall in CPPFLAGS, instead of CFLAGS, in the
first place?  :-)

> For the PTHREAD_CFLAGS it is more tricky: On most platforms you can use
> "-lpthread" instead of "-pthread", and all compilers support -l options.
> But on OSF/1, the options for using threads are compiler dependent:
>   * -pthread for cc,
>   * -lpthread for gcc.

This and more, as Ken mentioned, so I’d leave it as is for now.

Thanks,
Ludo’.

Re: GUILE_CFLAGS contains warning options

[Bruno Haible]

Ludovic Courtès wrote:
> However, why do you have -Wall in CPPFLAGS, instead of CFLAGS, in the
> first place?  :-)

Because CPPFLAGS=-Wall is shorter to type than CFLAGS="-g -O2 -Wall",
and I know that -Wall has no effect when linking a program or library.

Bruno
-- 
In memoriam Sophie Scholl <http://en.wikipedia.org/wiki/Sophie_Scholl>

Re: GUILE_CFLAGS contains warning options

[Ludovic Courtès]

Hi Bruno,

Bruno Haible <bruno@clisp.org> writes:

> Ludovic Courtès wrote:
>> However, why do you have -Wall in CPPFLAGS, instead of CFLAGS, in the
>> first place?  :-)
>
> Because CPPFLAGS=-Wall is shorter to type than CFLAGS="-g -O2 -Wall",
> and I know that -Wall has no effect when linking a program or library.

Sure, but it’s not a cpp flag, right?

What I mean is that we won’t prevent you from shooting yourself in the
foot.  ;-)

I only ever use CPPFLAGS for -I flags and can’t think of other likely
uses.

Thanks,
Ludo’.

Re: GUILE_CFLAGS contains warning options

[Bruno Haible]

Ludovic Courtès wrote:
> I only ever use CPPFLAGS for -I flags and can’t think of other likely
> uses.

If someone wants to disable assert()s, he uses the option -DNDEBUG, and
according to the autoconf manual [1]
   "If a compiler option affects only the behavior of the preprocessor
    (e.g., -Dname), it should be put into CPPFLAGS"
this option belongs in CPPFLAGS.

But just because guile has been built with assertions turned off, doesn't
mean that every program that uses guile wants to disable assertions. So
this is a second reason to put only -I options from $CPPFLAGS into
guile-2.0.pc.

Bruno

[1] http://www.gnu.org/software/autoconf/manual/html_node/Preset-Output-Variables.html
-- 
In memoriam Henri Salmide <http://en.wikipedia.org/wiki/Henri_Salmide>

Re: GUILE_CFLAGS contains warning options

[Ludovic Courtès]

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

Hello!

I’ll commit the patch below:


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

diff --git a/configure.ac b/configure.ac
index 423ae99..9b8553f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1566,9 +1566,18 @@ AC_SUBST(LIBGUILE_I18N_INTERFACE)
 
 #######################################################################
 
-dnl Tell guile-config what flags guile users should compile and link with.
+dnl Tell guile-config what flags guile users should compile and link
+dnl with, keeping only `-I' flags from $CPPFLAGS.
 GUILE_LIBS="$LDFLAGS $LIBS"
-GUILE_CFLAGS="$CPPFLAGS $PTHREAD_CFLAGS"
+GUILE_CFLAGS=""
+for flag in "$CPPFLAGS"
+do
+  case "$flag" in
+    -I*) GUILE_CFLAGS="$GUILE_CFLAGS $flag";;
+    *)   ;;
+  esac
+done
+GUILE_CFLAGS="$GUILE_CFLAGS $PTHREAD_CFLAGS"
 AC_SUBST(GUILE_LIBS)
 AC_SUBST(GUILE_CFLAGS)
 

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


Thanks,
Ludo’.

Re: GUILE_CFLAGS contains warning options

[Bruno Haible]

Hello Ludo',

> I’ll commit the patch below:

Thanks; this is what I was aiming at.

But the patch contains two small mistakes:

1) In this line
       for flag in "$CPPFLAGS"
   you should not use double-quotes, because the intent is to do word-splitting
   on the contents of $CPPFLAGS.

   See:
   $ CPPFLAGS="-I/opt/local/include -Wall"
   $ for flag in "$CPPFLAGS"; do echo $flag; done
   -I/opt/local/include -Wall
   $ for flag in $CPPFLAGS; do echo $flag; done
   -I/opt/local/include
   -Wall

2) Include options can also written with whitespace after the -I:
   -I/somedir   is equivalent to   -I /somedir

   To take this into account, rewrite the loop like this:

   next_is_includedir=false
   for flag in $CPPFLAGS
   do
     if $next_is_includedir; then
       GUILE_CFLAGS="$GUILE_CFLAGS -I $flag"
       next_is_includedir=false
     else
       case "$flag" in
         -I)  next_is_includedir=true;;
         -I*) GUILE_CFLAGS="$GUILE_CFLAGS $flag";;
         *)   ;;
       esac
     fi
   done

Bruno
-- 
In memoriam Hedwig Burgheim <http://de.wikipedia.org/wiki/Hedwig_Burgheim>

Re: GUILE_CFLAGS contains warning options

[Ludovic Courtès]

Hi Bruno,

Bruno Haible <bruno@clisp.org> writes:

> 1) In this line
>        for flag in "$CPPFLAGS"
>    you should not use double-quotes, because the intent is to do word-splitting
>    on the contents of $CPPFLAGS.

Oops.

>    next_is_includedir=false
>    for flag in $CPPFLAGS
>    do
>      if $next_is_includedir; then
>        GUILE_CFLAGS="$GUILE_CFLAGS -I $flag"
>        next_is_includedir=false
>      else
>        case "$flag" in
>          -I)  next_is_includedir=true;;
>          -I*) GUILE_CFLAGS="$GUILE_CFLAGS $flag";;
>          *)   ;;
>        esac
>      fi
>    done

Thanks, applied!

  http://git.savannah.gnu.org/cgit/guile.git/commit/?h=stable-2.0&id=62cdb4e478d58ade852b04bdcfcf79c1ac815e21

Ludo’.