Re: [PATCH 4/9] Warnings win over strictness in AM_INIT_AUTOMAKE.

[Stefano Lattarini]

Hello Ralf.

There's still a wart to be fixed before this patch series can be
committed ...

On Thursday 13 January 2011, Ralf Wildenhues wrote:
 * Stefano Lattarini wrote on Wed, Jan 12, 2011 at 09:11:41PM CET:
  On Wednesday 12 January 2011, Ralf Wildenhues wrote:
   So, now with that said, I'm not sure whether I should approve this
   patch.  What do you think?
  
  I think that you should, provided that I add the sanity check you
  suggested above.
 
 OK.
 
Hmm...  Wait, this sanity check is OK only starting from [PATCH 7/9]
onward, as until then the 'handle_options()' subroutine still calls
'process_option_list()' multiple times (that's why the bug is still
present for AUTOMAKE_OPTIONS at this point).

So, OK to add the sanity check in [PATCH 7/9] (or in a new follow-up
patch to be placed between patches 7 and 8 in this series)?

Regards,
   Stefano

Re: [PATCH 4/9] Warnings win over strictness in AM_INIT_AUTOMAKE.

[Stefano Lattarini]

[Ralf Wildenhues]
If some code later calls it like
 process_option_list (first-set-of-options);
 process_option_list (second-set-of-options);
   
   then things will go wrong again.  I suspect that it will mean that
 AM_INIT_AUTOMAKE([foreign -Wno-portability])
 AUTOMAKE_OPTIONS = gnu
   
   won't do what we want.  Hmm.  What exactly is it that we want to happen
   in this case?  Should gnu override -Wno-portability if specified in a
   (to-be) higher order place?
  
[Stefano Lattarini]
  I assumed without saying that yes, this was to be the intended behaviour.
  And I still think it should be.  Sorry for not having been explicit about
  that before.
[Ralf Wildenhues]
 
 I agree that it should be, but this, too, should be documented (in
 autoconf.texi and maybe also NEWS) and tested, when it works.
 
What about the attached patch?  It also adds a test for another situation
I hadn't thought about previously.

OK to apply the patch in a new commit between [PATCH 2/9] and [PATCH 3/9]?

Regards,
   Stefano
From 02668242613812bce666243dafc1e12edf317b13 Mon Sep 17 00:00:00 2001
From: Stefano Lattarini stefano.lattar...@gmail.com
Date: Thu, 13 Jan 2011 23:57:16 +0100
Subject: [PATCH] More tests on warnings and strictness.

* tests/warnings-strictness-interactions.test: New test.
* tests/warnings-unknown.test: Likewise.
* tests/Makefile.am (TESTS): Update.
---
 ChangeLog   |7 +++
 tests/Makefile.am   |2 +
 tests/Makefile.in   |2 +
 tests/warnings-strictness-interactions.test |   59 +++
 tests/warnings-unknown.test |   44 
 5 files changed, 114 insertions(+), 0 deletions(-)
 create mode 100755 tests/warnings-strictness-interactions.test
 create mode 100755 tests/warnings-unknown.test

diff --git a/ChangeLog b/ChangeLog
index 7956b15..efa43d1 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
 2011-01-02  Stefano Lattarini  stefano.lattar...@gmail.com
 
+	More tests on warnings and strictness.
+	* tests/warnings-strictness-interactions.test: New test.
+	* tests/warnings-unknown.test: Likewise.
+	* tests/Makefile.am (TESTS): Update.
+
+2011-01-02  Stefano Lattarini  stefano.lattar...@gmail.com
+
 	New test on silent-rules mode and portability warnings.
 	* tests/silent-nowarn.test: New test.
 	* tests/Makefile.am (TESTS): Update.
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 53dbe9c..972f8a9 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -947,6 +947,8 @@ vtexi3.test \
 vtexi4.test \
 warnings-override.test \
 warnings-precedence.test \
+warnings-strictness-interactions.test \
+warnings-unknown.test \
 warnopts.test \
 werror.test \
 werror2.test \
diff --git a/tests/Makefile.in b/tests/Makefile.in
index f2d695f..282afe0 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -1210,6 +1210,8 @@ vtexi3.test \
 vtexi4.test \
 warnings-override.test \
 warnings-precedence.test \
+warnings-strictness-interactions.test \
+warnings-unknown.test \
 warnopts.test \
 werror.test \
 werror2.test \
diff --git a/tests/warnings-strictness-interactions.test b/tests/warnings-strictness-interactions.test
new file mode 100755
index 000..e2c7675
--- /dev/null
+++ b/tests/warnings-strictness-interactions.test
@@ -0,0 +1,59 @@
+#! /bin/sh
+# Copyright (C) 2011 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 the default warnings triggered by a strictness specified
+# in AUTOMAKE_OPTIONS take precedence over explicit warnings given in
+# AM_INIT_AUTOMAKE.
+
+. ./defs || Exit 1
+
+# We want (almost) complete control over automake options.
+AUTOMAKE=$original_AUTOMAKE -Werror
+
+cat  Makefile.am END
+AUTOMAKE_OPTIONS =
+FOO := bar
+END
+
+set_am_opts ()
+{
+  set +x
+  sed $2 $2-t -e s|^\\(AUTOMAKE_OPTIONS\\) *=.*|\\1 = $1| \
+-e s|^\\(AM_INIT_AUTOMAKE\\).*|\\1([$1])|
+  mv -f $2-t $2
+  set -x
+  cat $2
+}
+
+set_am_opts '-Wportability' configure.in
+set_am_opts 'foreign' Makefile.am
+
+$ACLOCAL
+$AUTOMAKE
+
+rm -rf autom4te*.cache
+
+# Files required in gnu strictness.
+touch README INSTALL NEWS AUTHORS ChangeLog COPYING
+
+set_am_opts '-Wno-portability' configure.in
+set_am_opts 'gnu' Makefile.am
+
+AUTOMAKE_fails
+$ACLOCAL
+grep '^Makefile\.am:2:.*:=.*not portable' stderr
+
+:
diff --git 

Re: [PATCH 4/9] Warnings win over strictness in AM_INIT_AUTOMAKE.

[Ralf Wildenhues]

* Stefano Lattarini wrote on Thu, Jan 13, 2011 at 10:27:48PM CET:
 On Thursday 13 January 2011, Ralf Wildenhues wrote:
  * Stefano Lattarini wrote on Wed, Jan 12, 2011 at 09:11:41PM CET:
   On Wednesday 12 January 2011, Ralf Wildenhues wrote:
So, now with that said, I'm not sure whether I should approve this
patch.  What do you think?
   
   I think that you should, provided that I add the sanity check you
   suggested above.
  
  OK.
  
 Hmm...  Wait, this sanity check is OK only starting from [PATCH 7/9]
 onward, as until then the 'handle_options()' subroutine still calls
 'process_option_list()' multiple times (that's why the bug is still
 present for AUTOMAKE_OPTIONS at this point).
 
 So, OK to add the sanity check in [PATCH 7/9] (or in a new follow-up
 patch to be placed between patches 7 and 8 in this series)?

Well yes, but why not show the check diff if you have it already?

Thanks,
Ralf

Re: [PATCH 4/9] Warnings win over strictness in AM_INIT_AUTOMAKE.

[Ralf Wildenhues]

* Stefano Lattarini wrote on Fri, Jan 14, 2011 at 12:11:27AM CET:
 [Ralf Wildenhues]
 If some code later calls it like
  process_option_list (first-set-of-options);
  process_option_list (second-set-of-options);

then things will go wrong again.  I suspect that it will mean that
  AM_INIT_AUTOMAKE([foreign -Wno-portability])
  AUTOMAKE_OPTIONS = gnu

won't do what we want.  Hmm.  What exactly is it that we want to happen
in this case?  Should gnu override -Wno-portability if specified in a
(to-be) higher order place?
   
 [Stefano Lattarini]
   I assumed without saying that yes, this was to be the intended behaviour.
   And I still think it should be.  Sorry for not having been explicit about
   that before.
 [Ralf Wildenhues]
  
  I agree that it should be, but this, too, should be documented (in
  autoconf.texi and maybe also NEWS) and tested, when it works.
  
 What about the attached patch?  It also adds a test for another situation
 I hadn't thought about previously.
 
 OK to apply the patch in a new commit between [PATCH 2/9] and [PATCH 3/9]?

Well yes, but why omit the documentation bits that I asked for?
(efficient communication, and all that)

Thanks,
Ralf

 Subject: [PATCH] More tests on warnings and strictness.
 
 * tests/warnings-strictness-interactions.test: New test.
 * tests/warnings-unknown.test: Likewise.
 * tests/Makefile.am (TESTS): Update.

Re: [PATCH 4/9] Warnings win over strictness in AM_INIT_AUTOMAKE.

[Ralf Wildenhues]

Here comes my (belated, as always) re-review; I am not repeating nits
already dealt with before.

* Stefano Lattarini wrote on Wed, Jan 05, 2011 at 08:22:13PM CET:
 On Wednesday 05 January 2011, Ralf Wildenhues wrote:
  * Stefano Lattarini wrote on Tue, Jan 04, 2011 at 06:38:04PM CET:
   This is derived from [PATCH 07/10] of the older series.
   It requires a review.

   Warnings win over strictness in AM_INIT_AUTOMAKE.
   
   This change ensures that, for what concerns the options specified
   in AM_INIT_AUTOMAKE,  explicitly-defined warnings always take
   precedence over implicit strictness-implied warnings.  Related to
   Automake bug#7669 a.k.a. PR/547.

   * lib/Automake/Options.pm (_process_option_list): Parse explicit
   warnings only after the strictness level has been set.
   * tests/warnings-win-over-strictness.test: Extend.
  
   --- a/lib/Automake/Options.pm
   +++ b/lib/Automake/Options.pm

   @@ -313,11 +314,7 @@ sub _process_option_list (\%$@)
 }
  elsif (/^(?:--warnings=|-W)(.*)$/)
 {
   -   foreach my $cat (split (',', $1))
   - {
   -   msg 'unsupported', $where, unknown warning category `$cat'
   - if switch_warning $cat;
   - }
   +   push @warnings, split (',', $1);
 }
  else
 {
   @@ -326,6 +323,14 @@ sub _process_option_list (\%$@)
   return 1;
 }
}
   +  # We process warnings here, so that any explicitly-given warning 
   setting
   +  # will take precedence over warning settings defined implicitly by the
   +  # strictness.
  
  Well, this works in the current code base, but only by accident: namely,
  only because process_option_list is only ever called once, and with all
  options at once.
 
 Hmm... no, it's potentially called many times in `handle_options()'.
 But the later [PATCH 7/9] takes care of this.

But that's exactly what I mean: you need patch 7/9 precisely because
you have changed the requirements that you push onto callers of
process_*options_list.  Let me show you what I mean: with your patch
series, you would also need something like the following:

To this POD text in Options.pm:

| =item Cprocess_option_list ($where, @options)
| 
| =item Cprocess_global_option_list ($where, @options)
| 
| Process Automake's option lists.  C@options should be a list of
| words, as they occur in CAUTOMAKE_OPTIONS or CAM_INIT_AUTOMAKE.

you would need to add the following:

  These functions should be called at most once for each set of options
  having the same precedence; i.e., do not call it twice for two options
  from CAM_INIT_AUTOMAKE.

It is important for maintainability that the POD documentation is
correct.

And actually, now that we see the requirement printed, it is clear that
this is not a good move: the API has suddenly become less easy to use,
because it is easier for the callers of process_*option_list to use it
wrongly.  http://ozlabs.org/~rusty/index.cgi/tech/2008-03-30.html

One better API would be one that either
- checks that options of the same precedence are not split up when
  passed, or
- computes the set of active warnings only strictly after all options
  (of a certain precedence) have been processed.


And upon rereading the code, it is also quite clear where the other
reported precedence bugs come from: the global_options list is abused
for two sets of options: those from AM_INIT_AUTOMAKE, which should have
lower precedence than AUTOMAKE_OPTIONS, and those from the command line,
which should have higher precedence than AUTOMAKE_OPTIONS.  The early
conflation made correct precedence semantics impossible.  The right way
here would be to have three stores of options.

And now that also makes it clear why the automake code printed some of
the options as arguments in the rebuild rule for Makefile.in: because
precedence was botched, that was needed in order to avoid these options
to get lost.  Or so at least I believe.

Hmm, set_strictness has some of the same problems, except one
complication is that some warnings may apply already at global level,
before any Makefile.am file is even opened.  I'm guessing three sets are
needed here too.


So, now with that said, I'm not sure whether I should approve this
patch.  What do you think?

   If some code later calls it like
process_option_list (first-set-of-options);
process_option_list (second-set-of-options);
  
  then things will go wrong again.  I suspect that it will mean that
AM_INIT_AUTOMAKE([foreign -Wno-portability])
AUTOMAKE_OPTIONS = gnu
  
  won't do what we want.  Hmm.  What exactly is it that we want to happen
  in this case?  Should gnu override -Wno-portability if specified in a
  (to-be) higher order place?
 
 I assumed without saying that yes, this was to be the intended behaviour.
 And I still think it should be.  Sorry for not having been explicit about
 that before.

I agree that it should be, but this, too, should be documented (in
autoconf.texi and maybe also NEWS) and tested, when it works.

  I see two ways 

Re: [PATCH 4/9] Warnings win over strictness in AM_INIT_AUTOMAKE.

[Stefano Lattarini]

On Wednesday 12 January 2011, Ralf Wildenhues wrote:
 Here comes my (belated, as always) re-review; I am not repeating nits
 already dealt with before.
 
 * Stefano Lattarini wrote on Wed, Jan 05, 2011 at 08:22:13PM CET:
  On Wednesday 05 January 2011, Ralf Wildenhues wrote:
   * Stefano Lattarini wrote on Tue, Jan 04, 2011 at 06:38:04PM CET:
This is derived from [PATCH 07/10] of the older series.
It requires a review.
 
Warnings win over strictness in AM_INIT_AUTOMAKE.

This change ensures that, for what concerns the options specified
in AM_INIT_AUTOMAKE,  explicitly-defined warnings always take
precedence over implicit strictness-implied warnings.  Related to
Automake bug#7669 a.k.a. PR/547.
 
* lib/Automake/Options.pm (_process_option_list): Parse explicit
warnings only after the strictness level has been set.
* tests/warnings-win-over-strictness.test: Extend.
   
--- a/lib/Automake/Options.pm
+++ b/lib/Automake/Options.pm
 
@@ -313,11 +314,7 @@ sub _process_option_list (\%$@)
}
   elsif (/^(?:--warnings=|-W)(.*)$/)
{
- foreach my $cat (split (',', $1))
-   {
- msg 'unsupported', $where, unknown warning category 
`$cat'
-   if switch_warning $cat;
-   }
+ push @warnings, split (',', $1);
}
   else
{
@@ -326,6 +323,14 @@ sub _process_option_list (\%$@)
  return 1;
}
 }
+  # We process warnings here, so that any explicitly-given warning 
setting
+  # will take precedence over warning settings defined implicitly by 
the
+  # strictness.
   
   Well, this works in the current code base, but only by accident: namely,
   only because process_option_list is only ever called once, and with all
   options at once.
  
  Hmm... no, it's potentially called many times in `handle_options()'.
  But the later [PATCH 7/9] takes care of this.
 
 But that's exactly what I mean: you need patch 7/9 precisely because
 you have changed the requirements that you push onto callers of
 process_*options_list.  Let me show you what I mean: with your patch
 series, you would also need something like the following:
 
 To this POD text in Options.pm:
 
 | =item Cprocess_option_list ($where, @options)
 | 
 | =item Cprocess_global_option_list ($where, @options)
 | 
 | Process Automake's option lists.  C@options should be a list of
 | words, as they occur in CAUTOMAKE_OPTIONS or CAM_INIT_AUTOMAKE.
 
 you would need to add the following:
 
   These functions should be called at most once for each set of options
   having the same precedence; i.e., do not call it twice for two options
   from CAM_INIT_AUTOMAKE.
 
 It is important for maintainability that the POD documentation is
 correct.

Agreed on this.

 And actually, now that we see the requirement printed, it is clear that
 this is not a good move: the API has suddenly become less easy to use,

Well, before it was easier to use, but wrong.  So I think this would
be a good enough move indeed (not great or perfect, but good enough).

 because it is easier for the callers of process_*option_list to use it
 wrongly.  http://ozlabs.org/~rusty/index.cgi/tech/2008-03-30.html
 
 One better API would be one that either

 - checks that options of the same precedence are not split up when
   passed, or

This check would indeed be useful, and I think the patch could be easily
amended to add it.  In fact, this wouldn't create a new API, but just
add some sanity checks to the old one (+1 from me thus).

 - computes the set of active warnings only strictly after all options
   (of a certain precedence) have been processed.

This seems even cleaner in the long run, but it can come later IMVHO,
if and when the necessity arises.

 And upon rereading the code, it is also quite clear where the other
 reported precedence bugs come from: the global_options list is abused
 for two sets of options: those from AM_INIT_AUTOMAKE, which should have
 lower precedence than AUTOMAKE_OPTIONS, and those from the command line,
 which should have higher precedence than AUTOMAKE_OPTIONS.  The early
 conflation made correct precedence semantics impossible.  The right way
 here would be to have three stores of options.

I agree, but I contend that implementing this should be a prerequisite
to the fix of (after all pretty simple) -Wportability --foreign bug.

 And now that also makes it clear why the automake code printed some of
 the options as arguments in the rebuild rule for Makefile.in: because
 precedence was botched, that was needed in order to avoid these options
 to get lost.  Or so at least I believe.

How so?  The precedence of the command-line options is even lower than
that of the AM_INIT_AUTOMAKE options ...

 Hmm, set_strictness has some of the same problems, except one
 complication is that some warnings may apply already at global
 level, before any 

Re: [PATCH 4/9] Warnings win over strictness in AM_INIT_AUTOMAKE.

[Ralf Wildenhues]

* Stefano Lattarini wrote on Wed, Jan 12, 2011 at 09:11:41PM CET:
 On Wednesday 12 January 2011, Ralf Wildenhues wrote:
  So, now with that said, I'm not sure whether I should approve this
  patch.  What do you think?
 
 I think that you should, provided that I add the sanity check you
 suggested above.

OK.

Thanks,
Ralf

Re: [PATCH 4/9] Warnings win over strictness in AM_INIT_AUTOMAKE.

[Ralf Wildenhues]

* Stefano Lattarini wrote on Wed, Jan 05, 2011 at 08:22:13PM CET:
 On Wednesday 05 January 2011, Ralf Wildenhues wrote:
  * Stefano Lattarini wrote on Tue, Jan 04, 2011 at 06:38:04PM CET:
  
   +  # will take precedence over warning settings defined implicitly by the
   +  # strictness.
  
  Well, this works in the current code base, but only by accident: namely,
  only because process_option_list is only ever called once, and with all
  options at once.
 
 Hmm... no, it's potentially called many times in `handle_options()'.
 But the later [PATCH 7/9] takes care of this.

Ah, ok.

   If some code later calls it like
process_option_list (first-set-of-options);
process_option_list (second-set-of-options);
  
  then things will go wrong again.  I suspect that it will mean that
AM_INIT_AUTOMAKE([foreign -Wno-portability])
AUTOMAKE_OPTIONS = gnu
  
  won't do what we want.  Hmm.  What exactly is it that we want to happen
  in this case?  Should gnu override -Wno-portability if specified in a
  (to-be) higher order place?
 
 I assumed without saying that yes, this was to be the intended behaviour.
 And I still think it should be.  Sorry for not having been explicit about
 that before.
 
  I see two ways out: warnings are only switched after all options are
  processed.
 
 This is not good IMO, as it breaks usages like the the one in your
 example above.

Makes sense.

Thanks for explaining patiently, I think I now understand better.  I
hope to finish review (and approval) of this patch series this weekend.

Cheers,
Ralf

Re: [PATCH 4/9] Warnings win over strictness in AM_INIT_AUTOMAKE.

[Stefano Lattarini]

On Wednesday 05 January 2011, Ralf Wildenhues wrote:
 Hi Stefano,
 
 * Stefano Lattarini wrote on Tue, Jan 04, 2011 at 06:38:04PM CET:
  This is derived from [PATCH 07/10] of the older series.
  It requires a review.
 
 Thank you for rewriting the patch.

Really you should thank git rebase -i ;-)

 See below for comments.
 
  Warnings win over strictness in AM_INIT_AUTOMAKE.
  
  This change ensures that, for what concerns the options specified
  in AM_INIT_AUTOMAKE,  explicitly-defined warnings always take
  precedence over implicit strictness-implied warnings.  Related to
  Automake bug#7669 a.k.a. PR/547.
 
 PR automake/547

Will fix.

  * lib/Automake/Options.pm (_process_option_list): Parse explicit
  warnings only after the strictness level has been set.
  * tests/warnings-win-over-strictness.test: Extend.
 
  --- a/lib/Automake/Options.pm
  +++ b/lib/Automake/Options.pm
  @@ -242,6 +242,7 @@ Return 1 on error, 0 otherwise.
   sub _process_option_list (\%$@)
   {
 my ($options, $where, @list) = @_;
  +  my @warnings = ();
   
 foreach (@list)
   {
  @@ -313,11 +314,7 @@ sub _process_option_list (\%$@)
  }
 elsif (/^(?:--warnings=|-W)(.*)$/)
  {
  - foreach my $cat (split (',', $1))
  -   {
  - msg 'unsupported', $where, unknown warning category `$cat'
  -   if switch_warning $cat;
  -   }
  + push @warnings, split (',', $1);
  }
 else
  {
  @@ -326,6 +323,14 @@ sub _process_option_list (\%$@)
return 1;
  }
   }
  +  # We process warnings here, so that any explicitly-given warning setting
 
 s/We p/P/
 
  +  # will take precedence over warning settings defined implicitly by the
  +  # strictness.
 
 Well, this works in the current code base, but only by accident: namely,
 only because process_option_list is only ever called once, and with all
 options at once.

Hmm... no, it's potentially called many times in `handle_options()'.
But the later [PATCH 7/9] takes care of this.

  If some code later calls it like
   process_option_list (first-set-of-options);
   process_option_list (second-set-of-options);
 
 then things will go wrong again.  I suspect that it will mean that
   AM_INIT_AUTOMAKE([foreign -Wno-portability])
   AUTOMAKE_OPTIONS = gnu
 
 won't do what we want.  Hmm.  What exactly is it that we want to happen
 in this case?  Should gnu override -Wno-portability if specified in a
 (to-be) higher order place?

I assumed without saying that yes, this was to be the intended behaviour.
And I still think it should be.  Sorry for not having been explicit about
that before.

 I see two ways out: warnings are only switched after all options are
 processed.

This is not good IMO, as it breaks usages like the the one in your
example above.

 Or: process_option_list is documented to require *all*
 options.  The latter seems fragile.

Still, currently this is the best option IMVHO.  And in fact, the later
[PATCH 7/9] works exactly by ensuring that `handle_options()' calls
`process_option_list()' only once (this is the reason the preliminary
PATCH [6/9] Change signature of Automake::Options::_process_option_list
is required).

 Either way though, we need a consistent definition (and documentation)
 of intended semantics, both internally of process_option_list and user
 side of option handling semantics.

Agreed on the user-side documentation (which can be done in a follow-up
patch though).  But I don't think that internal documentation is really
required; nor can I think about how to formulate it (suggestions?)

  +  foreach my $cat (@warnings)
  +{
  +  msg 'unsupported', $where, unknown warning category `$cat'
  +   if switch_warning $cat;
  +}
 return 0;
   }
 
 Thanks,
 Ralf
 

Regards,
  Stefano

[PATCH 4/9] Warnings win over strictness in AM_INIT_AUTOMAKE.

[Stefano Lattarini]

This is derived from [PATCH 07/10] of the older series.
It requires a review.

Thanks,
   Stefano

-*-*-

Warnings win over strictness in AM_INIT_AUTOMAKE.

This change ensures that, for what concerns the options specified
in AM_INIT_AUTOMAKE,  explicitly-defined warnings always take
precedence over implicit strictness-implied warnings.  Related to
Automake bug#7669 a.k.a. PR/547.

* lib/Automake/Options.pm (_process_option_list): Parse explicit
warnings only after the strictness level has been set.
* tests/warnings-win-over-strictness.test: Extend.
---
 ChangeLog   |   12 
 lib/Automake/Options.pm |   15 ++-
 tests/warnings-win-over-strictness.test |   22 ++
 3 files changed, 44 insertions(+), 5 deletions(-)
From 2a8950bdf9c3e34e308ff6d1bed2646af8ab87fe Mon Sep 17 00:00:00 2001
From: Stefano Lattarini stefano.lattar...@gmail.com
Date: Mon, 20 Dec 2010 14:57:27 +0100
Subject: [PATCH 4/9] Warnings win over strictness in AM_INIT_AUTOMAKE.

This change ensures that, for what concerns the options specified
in AM_INIT_AUTOMAKE,  explicitly-defined warnings always take
precedence over implicit strictness-implied warnings.  Related to
Automake bug#7669 a.k.a. PR/547.

* lib/Automake/Options.pm (_process_option_list): Parse explicit
warnings only after the strictness level has been set.
* tests/warnings-win-over-strictness.test: Extend.
---
 ChangeLog   |   12 
 lib/Automake/Options.pm |   15 ++-
 tests/warnings-win-over-strictness.test |   22 ++
 3 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index d2dd6a2..44ea412 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,6 +1,18 @@
 2011-01-02  Stefano Lattarini  stefano.lattar...@gmail.com
 
 	For PR automake/547:
+	Warnings win over strictness in AM_INIT_AUTOMAKE.
+	This change ensures that, for what concerns the options specified
+	in AM_INIT_AUTOMAKE,  explicitly-defined warnings always take
+	precedence over implicit strictness-implied warnings.  Related to
+	Automake bug#7669 a.k.a. PR/547.
+	* lib/Automake/Options.pm (_process_option_list): Parse explicit
+	warnings only after the strictness level has been set.
+	* tests/warnings-win-over-strictness.test: Extend.
+
+2011-01-02  Stefano Lattarini  stefano.lattar...@gmail.com
+
+	For PR automake/547:
 	Warnings win over strictness on command line.
 	Ensure that, on the command line at least, explicitly defined
 	warnings always take precedence over implicit strictness-implied
diff --git a/lib/Automake/Options.pm b/lib/Automake/Options.pm
index a6d65a8..42ec0fd 100644
--- a/lib/Automake/Options.pm
+++ b/lib/Automake/Options.pm
@@ -242,6 +242,7 @@ Return 1 on error, 0 otherwise.
 sub _process_option_list (\%$@)
 {
   my ($options, $where, @list) = @_;
+  my @warnings = ();
 
   foreach (@list)
 {
@@ -313,11 +314,7 @@ sub _process_option_list (\%$@)
 	}
   elsif (/^(?:--warnings=|-W)(.*)$/)
 	{
-	  foreach my $cat (split (',', $1))
-	{
-	  msg 'unsupported', $where, unknown warning category `$cat'
-		if switch_warning $cat;
-	}
+	  push @warnings, split (',', $1);
 	}
   else
 	{
@@ -326,6 +323,14 @@ sub _process_option_list (\%$@)
 	  return 1;
 	}
 }
+  # We process warnings here, so that any explicitly-given warning setting
+  # will take precedence over warning settings defined implicitly by the
+  # strictness.
+  foreach my $cat (@warnings)
+{
+  msg 'unsupported', $where, unknown warning category `$cat'
+	if switch_warning $cat;
+}
   return 0;
 }
 
diff --git a/tests/warnings-win-over-strictness.test b/tests/warnings-win-over-strictness.test
index ef42c4f..53de473 100755
--- a/tests/warnings-win-over-strictness.test
+++ b/tests/warnings-win-over-strictness.test
@@ -37,6 +37,15 @@ ko ()
   test `wc -l stderr` -eq 1
 }
 
+set_am_opts()
+{
+  set +x
+  sed -e s|^\\(AM_INIT_AUTOMAKE\\).*|\\1([$1])| $2 $2-t
+  mv -f $2-t $2
+  set -x
+  cat $2
+}
+
 # Files required in gnu strictness.
 touch README INSTALL NEWS AUTHORS ChangeLog COPYING
 
@@ -51,4 +60,17 @@ ko -Wportability --foreign
 ok --gnu -Wno-portability
 ok -Wno-portability --gnu
 
+rm -rf autom4te*.cache
+set_am_opts 'foreign -Wportability' configure.in
+ko
+rm -rf autom4te*.cache
+set_am_opts '-Wportability foreign' configure.in
+ko
+rm -rf autom4te*.cache
+set_am_opts 'gnu -Wno-portability' configure.in
+ok
+rm -rf autom4te*.cache
+set_am_opts '-Wno-portability gnu' configure.in
+ok
+
 :
-- 
1.7.2.3