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

[Ralf Wildenhues]

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