[automake-commit] 02/04: Consistently process -W(no-)error after all other warning options.

[Zack Weinberg]

zackw pushed a commit to branch master
in repository automake.

View the commit online:
https://git.savannah.gnu.org/gitweb/?p=automake.git;a=commitdiff;h=9ae8a896cb83e6693413d80b5c85e527f3bb11f6

commit 9ae8a896cb83e6693413d80b5c85e527f3bb11f6
Author: Zack Weinberg <zackw@panix.com>
AuthorDate: Fri Sep 11 16:17:41 2020 -0400

    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.
---
 bin/aclocal.in              |  4 +++-
 bin/automake.in             |  2 +-
 lib/Automake/ChannelDefs.pm | 50 ++++++++++++++++++++++++++++++++++-----------
 3 files changed, 42 insertions(+), 14 deletions(-)

diff --git a/bin/aclocal.in b/bin/aclocal.in
index 1e734ea..42e8d83 100644
--- 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 ad91d87..67b7290 100644
--- 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 59728bd..1693c05 100644
--- 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)>