Hello automakers. Currently, automake *force* the disabling of `-Wportability' when in "foreign" mode. What is the rationale for this? I don't see any, since: 1. `-Wportability' is not enabled by default, and 2. even if it were, a user who doesn't want to see portability warnings can always explicitly add `-Wno-portability' to AUTOMAKE_OPTIONS or AM_INIT_AUTOMAKE.
See also the report PR/547 at: <http://sources.redhat.com/cgi-bin/gnatsweb.pl?database=automake> So what about the attached patches (for maint), which cause automake to *enable portability warnings even in "foreign" mode*? The testsuite still passes with the patches applied. I agree that a better fix would probably be making "foreign" just *not* *disable* portability warnings if they are already enabled, but this would require a deeper change in the current (IMVHO real messy) code implementating the setting of warnings and options (from either command line, AM_INIT_AUTOMAKE, or AUTOMAKE_OPTIONS). In the (not so) long run, the best thing to do IMHO would be making *all* the `-W' options take precedence over all "mode" options (foreign, gnu, gnits, cygnus), regardless of command line order, but honouring the hierarchy "AUTOMAKE_OPTIONS -> cmdline -> AM_INIT_AUTOMAKE". That should be quite easy to implement, with the current shape of the code. That said, I still found the attached patches an improvement over the current situation, and I'd like to apply them (the refactoring sketched above can be done later, at which point the even better semantic for "foreign mode" + "portability wanings" described above can be easier to implement). WDYT? Regards, Stefano
From ebd6c7a18666da5a970fee5a8b43d0987f89bd28 Mon Sep 17 00:00:00 2001 From: Stefano Lattarini <stefano.lattar...@gmail.com> Date: Mon, 4 Oct 2010 14:22:18 +0200 Subject: [PATCH 1/2] Add tests for future foreign semantic w.r.t. -Wportability. * tests/foreignwarn.test: New test, xfailing. * tests/foreignwarn2.test: Likewise. * tests/Makefile.am (TESTS, XFAIL_TESTS): Update. --- ChangeLog | 7 +++++ tests/Makefile.am | 4 +++ tests/Makefile.in | 4 +++ tests/foreignwarn.test | 61 +++++++++++++++++++++++++++++++++++++++++++++++ tests/foreignwarn2.test | 49 +++++++++++++++++++++++++++++++++++++ 5 files changed, 125 insertions(+), 0 deletions(-) create mode 100755 tests/foreignwarn.test create mode 100755 tests/foreignwarn2.test diff --git a/ChangeLog b/ChangeLog index 06a6e47..5d02c5c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2010-10-04 Stefano Lattarini <stefano.lattar...@gmail.com> + + Add tests for future "foreign" semantic w.r.t. `-Wportability'. + * tests/foreignwarn.test: New test, xfailing. + * tests/foreignwarn2.test: Likewise. + * tests/Makefile.am (TESTS, XFAIL_TESTS): Updated. + 2010-10-03 Stefano Lattarini <stefano.lattar...@gmail.com> Ralf Wildenhues <ralf.wildenh...@gmx.de> diff --git a/tests/Makefile.am b/tests/Makefile.am index 6c6f24f..16cd73a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -20,6 +20,8 @@ XFAIL_TESTS = \ all.test \ auxdir2.test \ cond17.test \ +foreignwarn.test \ +foreignwarn2.test \ gcj6.test \ txinfo5.test @@ -324,6 +326,8 @@ fnoc.test \ fo.test \ forcemiss.test \ forcemiss2.test \ +foreignwarn.test \ +foreignwarn2.test \ fort1.test \ fort2.test \ fort4.test \ diff --git a/tests/Makefile.in b/tests/Makefile.in index a9b1cbb..e323a7c 100644 --- a/tests/Makefile.in +++ b/tests/Makefile.in @@ -276,6 +276,8 @@ XFAIL_TESTS = \ all.test \ auxdir2.test \ cond17.test \ +foreignwarn.test \ +foreignwarn2.test \ gcj6.test \ txinfo5.test @@ -591,6 +593,8 @@ fnoc.test \ fo.test \ forcemiss.test \ forcemiss2.test \ +foreignwarn.test \ +foreignwarn2.test \ fort1.test \ fort2.test \ fort4.test \ diff --git a/tests/foreignwarn.test b/tests/foreignwarn.test new file mode 100755 index 0000000..fdffc4a --- /dev/null +++ b/tests/foreignwarn.test @@ -0,0 +1,61 @@ +#! /bin/sh +# Copyright (C) 2010 Free Software Foundation, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2, or (at your option) +# any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +# Check that `-Wportability' is not forcibly disabled in foreign mode. + +. ./defs || Exit 1 + +set -e + +cat >> configure.in << 'END' +AC_PROG_CC +AC_OUTPUT +END + +$ACLOCAL + +# We want complete control over warning and foreign flags. +AUTOMAKE=`echo " $AUTOMAKE " | sed -e 's/ -W[^ ]*//g' -e 's/ --foreign //'` + +cat > Makefile.am <<'END' +## this should cause a portability warning (enabled in our case) +foo = $(wildcard *) +## this should cause a gnu warning (disabled in our case) +CFLAGS = foo +## this is required to enable warnings on CFLAGS +bin_PROGRAMS = foo +END + +AUTOMAKE_fails -Werror -Wportability --foreign +grep '^Makefile\.am:2:.*wildcard' stderr +grep 'CFLAGS' stderr && Exit 1 + +cp Makefile.am Makefile.sav +echo 'AUTOMAKE_OPTIONS = foreign' >> Makefile.am +AUTOMAKE_fails -Werror -Wportability +grep '^Makefile\.am:2:.*wildcard' stderr +grep 'CFLAGS' stderr && Exit 1 +mv -f Makefile.sav Makefile.am + +cp configure.in configure.sav +sed 's/^AM_INIT_AUTOMAKE$/&([foreign])/' configure.sav >configure.in +cmp configure.in configure.sav && Exit 99 # sanity check +AUTOMAKE_fails -Werror -Wportability +grep '^Makefile\.am:2:.*wildcard' stderr +grep 'CFLAGS' stderr && Exit 1 +mv -f configure.sav configure.in + +: diff --git a/tests/foreignwarn2.test b/tests/foreignwarn2.test new file mode 100755 index 0000000..f4fde1b --- /dev/null +++ b/tests/foreignwarn2.test @@ -0,0 +1,49 @@ +#! /bin/sh +# Copyright (C) 2010 Free Software Foundation, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2, or (at your option) +# any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +# Check that portability warnings are enabled by default, even when in +# foreign mode. + +. ./defs || Exit 1 + +set -e + +$ACLOCAL + +# We want complete control over warning and foreign flags. +AUTOMAKE=`echo " $AUTOMAKE " | sed -e 's/ -W[^ ]*//g' -e 's/ --foreign //'` + +cat > Makefile.am <<'END' +foo = $(wildcard *) +END + +AUTOMAKE_fails -Werror --foreign +grep '^Makefile\.am:1:.*wildcard' stderr + +cp Makefile.am Makefile.sav +echo 'AUTOMAKE_OPTIONS = foreign' >> Makefile.am +AUTOMAKE_fails -Werror +grep '^Makefile\.am:1:.*wildcard' stderr +mv -f Makefile.sav Makefile.am + +cp configure.in configure.sav +sed 's/^AM_INIT_AUTOMAKE$/&([foreign])/' configure.sav >configure.in +cmp configure.in configure.sav && Exit 99 # sanity check +AUTOMAKE_fails -Werror +grep '^Makefile\.am:1:.*wildcard' stderr +mv -f configure.sav configure.in + +: -- 1.7.1
From 12cab94570fdcb835cc1cad87db1772349d10884 Mon Sep 17 00:00:00 2001 From: Stefano Lattarini <stefano.lattar...@gmail.com> Date: Mon, 4 Oct 2010 14:35:29 +0200 Subject: [PATCH 2/2] Mode "foreign" now implies `-Wno-portability'. * lib/Automake/ChannelDefs.pm (set_strictness): Set the `portability' channel to "verbose" even when strictness is "foreign". * NEWS: Updated. * doc/automake.texi (Creating a Makefile.in): Updated (plus a minor typofix). * tests/Makefile.am (XFAIL_TESTS): Remove `foreignwarn.test' and `foreignwarn2.test'. --- ChangeLog | 10 ++++++++++ NEWS | 3 +++ doc/automake.texi | 9 ++++----- lib/Automake/ChannelDefs.pm | 2 +- tests/Makefile.am | 2 -- tests/Makefile.in | 2 -- 6 files changed, 18 insertions(+), 10 deletions(-) diff --git a/ChangeLog b/ChangeLog index 5d02c5c..aba8b67 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,15 @@ 2010-10-04 Stefano Lattarini <stefano.lattar...@gmail.com> + Mode "foreign" now implies `-Wno-portability'. + * lib/Automake/ChannelDefs.pm (set_strictness): Set the + `portability' channel to "verbose" even when strictness + is "foreign". + * NEWS: Updated. + * doc/automake.texi (Creating a Makefile.in): Updated (plus + a minor typofix). + * tests/Makefile.am (XFAIL_TESTS): Remove `foreignwarn.test' + and `foreignwarn2.test'. + Add tests for future "foreign" semantic w.r.t. `-Wportability'. * tests/foreignwarn.test: New test, xfailing. * tests/foreignwarn2.test: Likewise. diff --git a/NEWS b/NEWS index b3d4131..a206b3c 100644 --- a/NEWS +++ b/NEWS @@ -5,6 +5,9 @@ New in 1.11.0a: - The `lzma' compression scheme and associated automake option `dist-lzma' is obsoleted by `xz' and `dist-xz' due to upstream changes. + - Portability warnings are now enabled by default for "foreign" strictness + too. + Bugs fixed in 1.11.0a: * Bugs introduced by 1.11: diff --git a/doc/automake.texi b/doc/automake.texi index 8848dcd..6e33743 100644 --- a/doc/automake.texi +++ b/doc/automake.texi @@ -2618,11 +2618,10 @@ A category can be turned off by prefixing its name with @samp{no-}. For instance, @option{-Wno-syntax} will hide the warnings about unused variables. -The categories output by default are @samp{syntax} and -...@samp{unsupported}. Additionally, @samp{gnu} and @samp{portability} -are enabled in @option{--gnu} and @option{--gnits} strictness. -On the other hand, the @option{silent-rules} options (@pxref{Options}) -turns off portability warnings about recursive variable expansions. +The categories output by default are @samp{syntax}, @samp{unsupported} +and @samp{portability}. On the other hand, the @option{silent-rules} +option (@pxref{Options}) turns off portability warnings about recursive +variable expansions. @vindex WARNINGS The environment variable @env{WARNINGS} can contain a comma separated diff --git a/lib/Automake/ChannelDefs.pm b/lib/Automake/ChannelDefs.pm index aaca979..a1fcb79 100644 --- a/lib/Automake/ChannelDefs.pm +++ b/lib/Automake/ChannelDefs.pm @@ -360,7 +360,7 @@ sub set_strictness ($) setup_channel 'error-gnu', silent => 1; setup_channel 'error-gnu/warn', silent => 0, type => 'warning'; setup_channel 'error-gnits', silent => 1; - setup_channel 'portability', silent => 1; + setup_channel 'portability', silent => 0; setup_channel 'gnu', silent => 1; } else diff --git a/tests/Makefile.am b/tests/Makefile.am index 16cd73a..50e6f06 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -20,8 +20,6 @@ XFAIL_TESTS = \ all.test \ auxdir2.test \ cond17.test \ -foreignwarn.test \ -foreignwarn2.test \ gcj6.test \ txinfo5.test diff --git a/tests/Makefile.in b/tests/Makefile.in index e323a7c..5f8ce82 100644 --- a/tests/Makefile.in +++ b/tests/Makefile.in @@ -276,8 +276,6 @@ XFAIL_TESTS = \ all.test \ auxdir2.test \ cond17.test \ -foreignwarn.test \ -foreignwarn2.test \ gcj6.test \ txinfo5.test -- 1.7.1