Consistently process -W(no-)error after all other warning options.

automake and aclocal were processing ‘-W(no-)error’ whenever it
appeared on the command line, which means that
‘-Werror,something-strange’ would issue a hard error, but
‘-Wsomething-strange,error’ would only issue a warning.

It is not desirable for warnings about unknown warning categories ever to be
treated as a hard error; that leads to problems for driver scripts like
autoreconf, which would like to pass whatever -W options it got on its own
command line down to all the tools and not worry about which tools understand
which warning categories.  Also, this sort of order dependence is confusing
for humans.

Change parse_warnings to take just one option, the _complete_ list of warning
categories seen on the command line, and to process -Werror / -Wno-error after
processing all other warnings options.  Thus, unknown warnings categories will
always just be a plain warning.  This does mean aclocal has to stop using
parse_warnings as a Getopt::Long callback, but that’s not a big deal.

Similarly, change parse_WARNINGS to record whether ‘error’ appeared in the
environment variable, but not activate warnings-are-errors mode itself.
parse_warnings picks up the record and honors it, unless it’s overridden by
the command line.

* lib/Automake/ChannelDefs.pm ($werror): New package global (not exported).
  (parse_WARNINGS): Do not call switch_warning for ‘error’ / ‘no-error’;
  just toggle the value of $werror.
  (parse_warnings): Do not call switch_warning immediately for
  ‘error’ / ‘no-error’; toggle $werror instead.  Call switch_warning ‘error’
  at the very end if $werror is true.  Remove unused $OPTION argument.
* bin/automake.in: parse_warnings now takes only one argument.
* bin/aclocal.in: Call parse_warnings after parse_options instead of
  using it as a parse_options callback.

diff --git a/bin/aclocal.in b/bin/aclocal.in
index 1e734ea48dffcd83bcdba82fdd9e309886eae685..42e8d839ab11359a944f2f2d545b74eaf1b55b0e 100644 (file)
--- a/bin/aclocal.in
+++ b/bin/aclocal.in
@@ -1081,6 +1081,7 @@ sub parse_arguments ()
 {
   my $print_and_exit = 0;
   my $diff_command;
+  my @warnings = ();
 
   my %cli_options =
     (
@@ -1096,11 +1097,12 @@ sub parse_arguments ()
      'output=s'                => \$output_file,
      'print-ac-dir'     => \$print_and_exit,
      'verbose'         => sub { setup_channel 'verb', silent => 0; },
-     'W|warnings=s'     => \&parse_warnings,
+     'W|warnings=s'     => \@warnings,
      );
 
   use Automake::Getopt ();
   Automake::Getopt::parse_options %cli_options;
+  parse_warnings @warnings;
 
   if (@ARGV > 0)
     {

diff --git a/bin/automake.in b/bin/automake.in
index ad91d875a5db3c48eb4d3a5b07492ad648ed56d0..67b7290452d29386dc90df034e487479186d1c4e 100644 (file)
--- a/bin/automake.in
+++ b/bin/automake.in
@@ -8184,7 +8184,7 @@ sub parse_arguments ()
   set_strictness ($strict);
   my $cli_where = new Automake::Location;
   set_global_option ('no-dependencies', $cli_where) if $ignore_deps;
-  parse_warnings ('-W', @warnings);
+  parse_warnings @warnings;
 
   return unless @ARGV;
 

diff --git a/lib/Automake/ChannelDefs.pm b/lib/Automake/ChannelDefs.pm
index 59728bdbef380e868205f9d164607f0f1f05b2f6..1693c05d5205af3482726e3676880098c2ca9c02 100644 (file)
--- a/lib/Automake/ChannelDefs.pm
+++ b/lib/Automake/ChannelDefs.pm
@@ -352,36 +352,62 @@ Parse the WARNINGS environment variable.
 
 =cut
 
+# Used to communicate from parse_WARNINGS to parse_warnings.
+our $_werror = 0;
+
 sub parse_WARNINGS ()
 {
   if (exists $ENV{'WARNINGS'})
     {
       # Ignore unknown categories.  This is required because WARNINGS
       # should be honored by many tools.
-      switch_warning $_ foreach (split (',', $ENV{'WARNINGS'}));
+      # For the same reason, do not turn on -Werror at this point, just
+      # record that we saw it; parse_warnings will turn on -Werror after
+      # the command line has been processed.
+      foreach (split (',', $ENV{'WARNINGS'}))
+        {
+          if (/^(no-)?error$/)
+            {
+              $_werror = !defined $1;
+            }
+          else
+            {
+              switch_warning $_;
+            }
+        }
     }
 }
 
-=item C<parse_warnings ($OPTION, @ARGUMENT)>
+=item C<parse_warnings (@CATEGORIES)>
 
 Parse the argument of C<--warning=CATEGORY> or C<-WCATEGORY>.
+C<@CATEGORIES> is the accumulated set of warnings categories.
+Use like this:
 
-C<$OPTIONS> is C<"--warning"> or C<"-W">, C<@ARGUMENT> is a list of
-C<CATEGORY>.
-
-This can be used as an argument to C<Getopt>.
+    Automake::GetOpt::parse_options (
+        # ...
+        'W|warnings=s' => \@warnings,
+    )
+    # possibly call set_strictness here
+    parse_warnings @warnings;
 
 =cut
 
-sub parse_warnings ($@)
+sub parse_warnings (@)
 {
-  my ($opt, @categories) = @_;
-
-  foreach my $cat (map { split ',' } @categories)
+  foreach my $cat (map { split ',' } @_)
     {
-      msg 'unsupported', "unknown warning category '$cat'"
-       if switch_warning $cat;
+      if ($cat =~ /^(no-)?error$/)
+        {
+          $_werror = !defined $1;
+        }
+      elsif (switch_warning $cat)
+        {
+          msg 'unsupported', "unknown warning category '$cat'";
+        }
     }
+
+  switch_warning ($_werror ? 'error' : 'no-error');
 }
 
 =item C<set_strictness ($STRICTNESS_NAME)>