Re: bug in option parsing

>>> "Eric" == Eric Blake <ebb9@xxxxxxx> writes:

 Eric> First, it does not gracefully handle the null string:
 Eric> $ automake ''
 Eric> Use of uninitialized value in concatenation (.) or string at
 Eric> /usr/autotool/devel/bin/automake line 7436.
 Eric> automake: no Automake input file found for `'
 Eric> automake: no input file found among supplied arguments

 Eric> Obviously, the null string can't name a makefile, so a special-case
 Eric> error message would be better than the uninit value warning.

 Eric> Second, it does not handle the POSIX-recommended -- argument as the
 Eric> ignored argument meaning end of options.

Thanks, I'm checking this in on HEAD.

2004-11-21  Alexandre Duret-Lutz  <adl@xxxxxxx>

        * automake.in (parse_arguments): Diagnose empty arguments, options
        with missing argument, and support `--'.
        * aclocal.in (parse_arguments): Diagnose options with missing
        argument.
        * tests/aclocal.test: More checks.
        * tests/automake.test: New file.
        * tests/postprog.test: Use `--' for fun.
        * tests/Makefile.am (TESTS): Add automake.test.
        Report from Eric Blake.

Index: aclocal.in
===================================================================
RCS file: /cvs/automake/automake/aclocal.in,v
retrieving revision 1.118
diff -u -r1.118 aclocal.in
--- aclocal.in  21 Nov 2004 19:53:05 -0000      1.118
+++ aclocal.in  21 Nov 2004 23:28:22 -0000
@@ -636,7 +636,7 @@
      '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'     => \&parse_warnings,
      );
   use Getopt::Long;
   Getopt::Long::config ("bundling", "pass_through");
@@ -666,8 +666,25 @@
 
   if (@ARGV)
     {
-      fatal ("unrecognized option `$ARGV[0]'\n"
-            . "Try `$0 --help' for more information.");
+      my %argopts;
+      for my $k (keys %cli_options)
+       {
+         if ($k =~ /(.*)=s$/)
+           {
+             map { $argopts{(length ($_) == 1)
+                            ? "-$_" : "--$_" } = 1; } (split (/\|/, $1));
+           }
+       }
+      if (exists $argopts{$ARGV[0]})
+       {
+         fatal ("option `$ARGV[0]' requires an argument\n"
+                . "Try `$0 --help' for more information.");
+       }
+      else
+       {
+         fatal ("unrecognized option `$ARGV[0]'\n"
+                . "Try `$0 --help' for more information.");
+       }
     }
 
   if ($print_and_exit)
Index: automake.in
===================================================================
RCS file: /cvs/automake/automake/automake.in,v
retrieving revision 1.1587
diff -u -r1.1587 automake.in
--- automake.in 21 Oct 2004 21:37:39 -0000      1.1587
+++ automake.in 21 Nov 2004 23:28:22 -0000
@@ -7433,7 +7433,7 @@
   my $cli_where = new Automake::Location;
   my %cli_options =
     (
-     'libdir:s'        => \$libdir,
+     'libdir=s'        => \$libdir,
      'gnu'             => sub { set_strictness ('gnu'); },
      'gnits'           => sub { set_strictness ('gnits'); },
      'cygnus'          => sub { set_global_option ('cygnus', $cli_where); },
@@ -7443,11 +7443,11 @@
                                                    $cli_where); },
      'no-force'        => sub { $force_generation = 0; },
      'f|force-missing'  => \$force_missing,
-     'o|output-dir:s'  => \$output_directory,
+     'o|output-dir=s'  => \$output_directory,
      'a|add-missing'   => \$add_missing,
      'c|copy'          => \$copy_missing,
      'v|verbose'       => sub { setup_channel 'verb', silent => 0; },
-     'W|warnings:s'     => \&parse_warnings,
+     'W|warnings=s'     => \&parse_warnings,
      # These long options (--Werror and --Wno-error) for backward
      # compatibility.  Use -Werror and -Wno-error today.
      'Werror'           => sub { parse_warnings 'W', 'error'; },
@@ -7489,14 +7489,40 @@
       $output_directory = '.';
     }
 
-  my $errspec = 0;
-  foreach my $arg (@ARGV)
+  return unless @ARGV;
+
+  if ($ARGV[0] =~ /^-./)
     {
-      if ($arg =~ /^-./)
+      my %argopts;
+      for my $k (keys %cli_options)
+       {
+         if ($k =~ /(.*)=s$/)
+           {
+             map { $argopts{(length ($_) == 1)
+                            ? "-$_" : "--$_" } = 1; } (split (/\|/, $1));
+           }
+       }
+      if ($ARGV[0] eq '--')
+       {
+         shift @ARGV;
+       }
+      elsif (exists $argopts{$ARGV[0]})
+       {
+         fatal ("option `$ARGV[0]' requires an argument\n"
+                . "Try `$0 --help' for more information.");
+       }
+      else
        {
-         fatal ("unrecognized option `$arg'\n"
+         fatal ("unrecognized option `$ARGV[0]'.\n"
                 . "Try `$0 --help' for more information.");
        }
+    }
+
+  my $errspec = 0;
+  foreach my $arg (@ARGV)
+    {
+      fatal ("empty argument\nTry `$0 --help' for more information.")
+       if ($arg eq '');
 
       # Handle $local:$input syntax.
       my ($local, @rest) = split (/:/, $arg);
Index: tests/Makefile.am
===================================================================
RCS file: /cvs/automake/automake/tests/Makefile.am,v
retrieving revision 1.572
diff -u -r1.572 Makefile.am
--- tests/Makefile.am   10 Nov 2004 20:12:30 -0000      1.572
+++ tests/Makefile.am   21 Nov 2004 23:28:23 -0000
@@ -53,6 +53,7 @@
 autohdr2.test \
 autohdr3.test \
 autohdr4.test \
+automake.test \
 auxdir.test \
 auxdir2.test \
 auxdir3.test \
Index: tests/aclocal.test
===================================================================
RCS file: /cvs/automake/automake/tests/aclocal.test,v
retrieving revision 1.7
diff -u -r1.7 aclocal.test
--- tests/aclocal.test  17 Nov 2004 22:13:33 -0000      1.7
+++ tests/aclocal.test  21 Nov 2004 23:28:23 -0000
@@ -27,7 +27,12 @@
 $ACLOCAL --output=fred
 test -f fred
 
+$ACLOCAL --output 2>stderr && exit 1
+grep 'option.*--output.*an argument' stderr
+grep help stderr
+
 $ACLOCAL --unknown-option 2>stderr && exit 1
+grep 'unrecognized.*--unknown-option' stderr
 grep help stderr
 
 test "`$ACLOCAL --print-ac-dir`" = "$testaclocaldir"
Index: tests/automake.test
===================================================================
RCS file: tests/automake.test
diff -N tests/automake.test
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ tests/automake.test 21 Nov 2004 23:28:23 -0000
@@ -0,0 +1,39 @@
+#! /bin/sh
+# Copyright (C) 2004  Free Software Foundation, Inc.
+#
+# This file is part of GNU Automake.
+#
+# GNU Automake 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.
+#
+# GNU Automake 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 Automake; see the file COPYING.  If not, write to
+# the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+# Boston, MA 02111-1307, USA.
+
+# Test Automake's command-line options.
+. ./defs || exit 1
+
+set -e
+
+$AUTOMAKE --help
+$AUTOMAKE --version
+AUTOMAKE_fails --voo
+grep 'unrecognized option.*--voo' stderr
+AUTOMAKE_fails -- --voo
+grep 'input file.*--voo' stderr
+AUTOMAKE_fails ''
+grep 'empty argument' stderr
+AUTOMAKE_fails -W
+grep 'option.*-W.*requires an argument' stderr
+AUTOMAKE_fails --warnings
+grep 'option.*--warning.*requires an argument' stderr
+AUTOMAKE_fails --warnings --help
+grep 'unknown warning.*--help' stderr
Index: tests/postproc.test
===================================================================
RCS file: /cvs/automake/automake/tests/postproc.test,v
retrieving revision 1.3
diff -u -r1.3 postproc.test
--- tests/postproc.test 14 Nov 2003 21:25:59 -0000      1.3
+++ tests/postproc.test 21 Nov 2004 23:28:23 -0000
@@ -1,5 +1,5 @@
 #! /bin/sh
-# Copyright (C) 2002  Free Software Foundation, Inc.
+# Copyright (C) 2002, 2004  Free Software Foundation, Inc.
 #
 # This file is part of GNU Automake.
 #
@@ -38,7 +38,7 @@
 
 $ACLOCAL || exit 1
 $AUTOCONF || exit 1
-$AUTOMAKE myMakefile || exit 1
+$AUTOMAKE -- myMakefile || exit 1
 
 mv myMakefile.in myMakefile.old
 echo '# Post-processed by post-processor 3.14.' > myMakefile.in

-- 
Alexandre Duret-Lutz