Re: [PATCH 2/2] warnings: new 'extra-portability' category, for AM_PROG_AR

[Peter Rosin]

Stefano Lattarini skrev 2011-10-20 20:15:
> On Thursday 20 October 2011, Peter Rosin wrote:
>> Stefano Lattarini skrev 2011-10-20 17:11:
>>
>>> Micro-nit: this would be clearer IMHO:
>>>
>>>   # Interactions between `portability' and `extra-portability'
>>>   # warning categories:
>>>   #   1. `-Wextra-portability' must imply `-Wportability'.
>>>   #   2. `-Wno-portability' must imply `-Wno-portability'.
>>
>> fine with me (but with -Wno-extra-portability at the end of 2).
>>
> Right, sorry.
>  
>>>> +AUTOMAKE_fails -Wnone -Wextra-portability
>>>> +# The expected diagnostic is
>>>> +#    Makefile.am:2: compiling `foo.c' with per-target flags requires 
>>>> `AM_PROG_CC_C_O' in `configure.in'
>>>> +#    .../lib/am/library.am: `libfoo.a': linking libraries using a 
>>>> non-POSIX
>>>> +#    .../lib/am/library.am: archiver requires `AM_PROG_AR' in 
>>>> `configure.in'
>>>> +#    Makefile.am:1:   while processing library `libfoo.a'
>>>> +grep '^Makefile.am:2:.*requires.*AM_PROG_CC_C_O' stderr
>>>> +grep '/library.am:.*requires.*AM_PROG_AR' stderr
>>>> +
>>> No need to also grep the filenames here.  Similarly for other usages
>>> throughout the file.
>>
>> Do you mean /library.am only, or do you also mean Makefile.am:2:?
>>
> Both.
> 
>> If you do not think Makefile.am:2: is needed, then why did you want me
>> to grep `FILE:LINENO' for the other patch, but not here?
>>
> Because here we are only interested in checking the relationship
> between the `portability' and `extra-portability' warning classes,
> not to ensure the competelness of the error messages; these are
> orthogonal concerns IMHO, and should better pertain to distinct
> tests (unless keeping them separate would become to cumbersome,
> but I think this is not the case here).  Do you agree?

Ok, agreed.  Now doing greps like this:

grep 'requires.*AM_PROG_CC_C_O' stderr
grep 'requires.*AM_PROG_AR' stderr


>>>> diff --git a/tests/extra-portability2.test b/tests/extra-portability2.test
>>>> new file mode 100755
>>>> index 0000000..9f4948b
>>>> --- /dev/null
>>>> +++ b/tests/extra-portability2.test
>>>> @@ -0,0 +1,57 @@
>>>> +#! /bin/sh
>>>> +# Copyright (C) 2011  Free Software Foundation, Inc.
>>>> +#
>>>> +# Make sure that extra-portability is not enabled by --gnits, --gnu
>>>> +
>>>> +cat >>configure.in <<END
>>>> +AC_PROG_CC
>>>> +AC_PROG_RANLIB
>>>> +AC_OUTPUT
>>>> +END
>>>> +
>>>> +cat >Makefile.am <<END
>>>> +EXTRA_LIBRARIES = libfoo.a
>>>> +libfoo_a_SOURCES = foo.c
>>>> +END
>>>> +
>>>> +$ACLOCAL
>>>> +
>>>> +# Make sure the test is useful.
>>>> +AUTOMAKE_fails
>>>>
>>> How is it that automake is failing here, without explicilty
>>> using `-Wextra-portability'?
>>
>> -Wall implies *all* warnings, remember?
>>
> Right, sorry *blush*.  But then ...
> 
>>>> +$AUTOMAKE --foreign
>>>> +$AUTOMAKE --gnu
>>>> +$AUTOMAKE --gnits
>>>> +
> ... why is not automake failing here?  $AUTOMAKE still contains the
> `-Wall' option ...  Oh, got it, you are relying on the fact that a
> strictness option also reset the warnings to its corresponding
> defaults.

right

>            But this is was a long-standing bug of automake, which
> has been fixed in the master branch.  So, once msvc is merged back,
> this test will porbably start to fail ...  oh well, I guess we can
> cross the bridge when we come to it.

Ok, I'll leave that for now if that's ok.  I'm beginning to get
fed up with these patches :-)

Anyone want to to see the final spin?

Cheers,
Peter