[cairo] Build system b0rkage and pthread

[Behdad Esfahbod]

On 08/11/10 17:33, M Joonas Pihlaja wrote:
> 
> On Wed, 11 Aug 2010, Behdad Esfahbod wrote:
> 
>>   - In configure.ac.pthread:
>>
>>     * Please add "static" to all the C functions you define.
> 
> Why, does the compiler complain that the functions are unused or 
> something?

I use -Wmissing-prototypes, because it's helpful discovering missed "static"
qualifiers on functions.


>>     * Use of CAIRO_CC_TRY_LINK_WITH_ENV_SILENT() in pthread check or for ANY
>> OTHER REASON is B0RKED.  There is no way that you can write code that does not
>> generate warnings with my compiler.  Just don't make such stupid assumptions.  Ok?
> 
> While I agree that CAIRO_CC_TRY_LINK_WITH_ENV_SILENT() _may_ be 
> overkill for the pthread checks, it is most definitely not for the 
> purpose it was made for: checking that some random flag or feature 
> thrown at the compiler doesn't produce errors or warnings.

Ok.


>> Right now master simply doesn't succeed configuring on my machines.  Have to
>> change that stupid check.

So, there's one more bug to fix here:

- Currently configure doesn't detect pthread, but succeeds.  But then when
building, the recursive-mutex implementation (when was *that* added?!) doesn't
work with our dummy mutex implementation.  So that's another thing to fix.


> Thanks for the heads up.  If you describe the problem with specifics 
> we can probably come up with an acceptable solution.

I don't really like to describe the specifics.  Because if I do, you fix those
special cases.  What I want to see fixed is the logic itself.

Ok, for the specifics:

configure:32816: checking for cairo's pthread feature
configure:32928: gcc -ggdb -O2 -Wall -Wextra  -Wno-missing-field-initializers
-Wmissing-noreturn -Wundef -Wredundant-decls -Wsign-compare
-Wlarger-than-65500 -Wpointer-arith -Ww↪rite-strings -Wformat=2 -Winit-self
-Wswitch-enum -Wpacked -Wunsafe-loop-optimizations -Wmissing-format-attribute
-fno-common -Wp,-D_FORTIFY_SOURCE=2 -Wno-strict-aliasing -Wcas↪t-align
-Wlogical-op -Waggregate-return -Wdisabled-optimization -Wvla
-Wmissing-declarations -Wmissing-prototypes -Wstrict-prototypes
-Wold-style-definition -Wdeclaration-after↪-statement -Wnested-externs -o
conftest -D_REENTRANT -I/home/behdad/.local/include  -Wno-error -Wfatal-errors
-L/home/behdad/.local/lib  conftest.c -lpthread >&5
conftest.c:94: warning: no previous prototype for 'test_mutex'
conftest.c:105: warning: no previous prototype for 'test_mutex_attr'
conftest.c:121: warning: no previous prototype for 'test_once_init'
conftest.c:122: warning: no previous prototype for 'test_once'
conftest.c:128: warning: no previous prototype for 'test_specific'
conftest.c:137: warning: no previous prototype for 'cleaner'
conftest.c:140: warning: no previous prototype for 'test_thread_main'
conftest.c:149: warning: no previous prototype for 'test_threads'
conftest.c: In function 'test_thread_main':
conftest.c:140: warning: function might be possible candidate for attribute
'noreturn'


The -Wmissing-noreturn flag is useful for annotating library code. Go figure
how to *fix* that warning in your test code...!


My point is: warnings are warnings.  At no time should they become fatal.  I
like turning on shitload of warnings (and in ways that can't be disabled in
configure; I set CC="gcc -W...").

I had this conversation back in the days with Owen (over glib, since the
visibility macros there use similar checks), and he suggested that if we can't
write a short warning-free code, we may as well give up writing code
altogether.  I disagree.  There are legit cases that cause warnings given my
settings, and that's fine; I check the code, decide that the warning does not
need fixing.

For the record, here's the current flags I use:

GCCOPTS="-ggdb -O2 \
-Wall -Wextra  -Wno-missing-field-initializers \
-Wmissing-noreturn -Wundef -Wredundant-decls \
-Wsign-compare \
-Wlarger-than-65500 -Wpointer-arith -Wwrite-strings \
-Wformat=2 -Winit-self -Wswitch-enum -Wpacked \
-Wunsafe-loop-optimizations -Wmissing-format-attribute \
-fno-common -Wp,-D_FORTIFY_SOURCE=2 -Wno-strict-aliasing \
-Wcast-align -Wlogical-op -Waggregate-return \
-Wdisabled-optimization -Wvla"
export CC="gcc $GCCOPTS \
-Wmissing-declarations -Wmissing-prototypes -Wstrict-prototypes \
-Wold-style-definition -Wdeclaration-after-statement -Wnested-externs"
export CXX="g++ $GCCOPTS"
export CFLAGS="$CFLAGS -Wno-error -Wno-unused-parameter -Wfatal-errors"
export CPPFLAGS="$CPPFLAGS -Wno-error -Wfatal-errors"
export CXXFLAGS="$CXXFLAGS -Wno-error -Wno-unused-parameter -Wfatal-errors"


Take -Wunsafe-loop-optimizations for example.  It's perfectly right to have
code that causes warnings using that option, but many times 1) I can optimize
the code instead, and 2) avoid possible infinite loops.  Take the following
code for example

  guint16 count = some.count;
  int i;

  for (i = 1; i <= count; i++)
    ...


That code has a bug in it: it can loop infinitely if count = (guint16)-1.  The
-Wunsafe-loop-optimizations flag warns about it.  It's useful.  On the other
hand, there are cases in pango and cairo that cause such warnings, but the
code is just fine.



>> What's wrong with just checking that the linking succeeds again?
> 
> Some compilers produce spurious output or code when given flags or 
> pragmas/attributes which weren't meant for them.  They may be ignored 
> with nothing but a "hey, I didn't really grok this flag/feature but 
> never mind, gonna pretend I didn't see it" warning to stderr, causing 
> a link-only test to yield a false positive when checking for some 
> feature.

I'm talking about the case of detecting pthreads.  Not flags.  Even for flags,
I suggest comparing the stderr output with and without, and see which one is
longer.


> This wouldn't be such a problem if we weren't trying to throw fifteen 
> million random flags and features at the compiler at configure time 
> which are really only useful for developers (especially all the 
> warning flags, __attributes__, and even pthread flags, I'm looking at 
> you lot.)

I'm of the opinion that developers are responsible for setting up the correct
development platform for themselves.  But I'm fine with cairo adding them as
long is it doesn't break my build...

That said, I suggest we file for a feature request upstream.  Something like
GCC_WARN_FLAGS, that gcc will simply pickup from the environment or something...


> *All* compilers manage to compile hello-world.c without moaning, so 
> our tests should be at the level of hello-world.c + the feature under 
> test. 

Well, which hello-world.c? :).


 If it can't compile that without warnings, it may not be safe
> to use the feature on that system.  If our tests don't fit that 
> template for some reason, we need to modify them until they do.

Then at least try without the flag first and make sure it does compile without
moaning...


>> Please, please, please, avoid creative solutions to build problems.  
>> Kick me hard and I'll come up with a good way to do it.
> 
> Welcome back. :)

Heh.

Cheers,
behdad

> Cheers,
> 
> Joonas
>