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. 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
> * 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. 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 see two ways out: warnings are only switched after all options are
processed. Or: process_option_list is documented to require *all*
options. The latter seems fragile.
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.
> + foreach my $cat (@warnings)
> + {
> + msg 'unsupported', $where, "unknown warning category `$cat'"
> + if switch_warning $cat;
> + }
> return 0;
> }
Thanks,
Ralf
