[RFC + SIMPLE PATCHES] {maint} foreign mode and -Wportability

[Stefano Lattarini]

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 <address@hidden>
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  <address@hidden>
+
+       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  <address@hidden>
            Ralf Wildenhues  <address@hidden>
 
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 <address@hidden>
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  <address@hidden>
 
+       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
address@hidden  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