Re: FYI: two $exit_code and a fatal

[Alexandre Duret-Lutz]

>>> "Tom" == Tom Tromey <address@hidden> writes:

[...]

 adl> Akim told me that one way to comply with the GCS this is to make
 adl> a first pass on the command line to handle the --version and
 adl> --help options, and then proceed with the other switches in a
 adl> second pass.  I'll try to do that this evening.

 Tom> As long as the parsing is precise, I'm ok with it.

No problem, I just call Getopt twice, with empty actions
for anything but --version and --help the first time.

Here is what I'm checking in.

2002-07-10  Alexandre Duret-Lutz  <address@hidden>

        * automake.in (parse_arguments): Call Getopt twice: once to catch
        --help or --verbose, and once to process the other arguments.
        (usage, verbose): Always exit 0, ignoring $exit_code (which anyway
        should always be 0 because of the change to parse_arguments).
        * tests/Makefile.am (TESTS): Add getopt.test.
        * tests/getopt.test: New file.

Index: automake.in
===================================================================
RCS file: /cvs/automake/automake/automake.in,v
retrieving revision 1.1324
diff -u -r1.1324 automake.in
--- automake.in 10 Jul 2002 05:36:45 -0000      1.1324
+++ automake.in 10 Jul 2002 20:21:08 -0000
@@ -1410,47 +1410,68 @@
 # Parse command line.
 sub parse_arguments ()
 {
-    # Start off as gnu.
-    &set_strictness ('gnu');
+  # Start off as gnu.
+  &set_strictness ('gnu');
 
-    use Getopt::Long;
-    Getopt::Long::config ("bundling", "pass_through");
-    Getopt::Long::GetOptions
-      (
-       'version'       => \&version,
-       'help'          => \&usage,
-       'libdir:s'      => \$libdir,
-       'gnu'           => sub { &set_strictness ('gnu'); },
-       'gnits'                 => sub { &set_strictness ('gnits'); },
-       'cygnus'        => \$cygnus_mode,
-       'foreign'       => sub { &set_strictness ('foreign'); },
-       'include-deps'  => sub { $cmdline_use_dependencies = 1; },
-       'i|ignore-deps'         => sub { $cmdline_use_dependencies = 0; },
-       'no-force'      => sub { $force_generation = 0; },
-       'f|force-missing'=> \$force_missing,
-       '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,
-       # These long options (--Werror and --Wno-error) for backward
-       # compatibility.  Use -Werror and -Wno-error today.
-       'Werror'         => sub { parse_warnings 'W', 'error'; },
-       'Wno-error'      => sub { parse_warnings 'W', 'no-error'; },
-      )
-       or exit 1;
+  my %options =
+    (
+     'libdir:s'        => \$libdir,
+     'gnu'             => sub { &set_strictness ('gnu'); },
+     'gnits'           => sub { &set_strictness ('gnits'); },
+     'cygnus'          => \$cygnus_mode,
+     'foreign'                 => sub { &set_strictness ('foreign'); },
+     'include-deps'    => sub { $cmdline_use_dependencies = 1; },
+     'i|ignore-deps'   => sub { $cmdline_use_dependencies = 0; },
+     'no-force'        => sub { $force_generation = 0; },
+     'f|force-missing'  => \$force_missing,
+     '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,
+     # These long options (--Werror and --Wno-error) for backward
+     # compatibility.  Use -Werror and -Wno-error today.
+     'Werror'           => sub { parse_warnings 'W', 'error'; },
+     'Wno-error'        => sub { parse_warnings 'W', 'no-error'; },
+     );
+
+  use Getopt::Long;
+  Getopt::Long::config ("bundling", "pass_through");
+
+  # See if --version or --help is used.  We want to process these before
+  # anything else because the GNU Coding Standards require us to
+  # `exit 0' after processing these options, and we can't garanty this
+  # if we treat other options first.  (Handling other options first
+  # could produce error diagnostics, and in this condition it is
+  # confusing if Automake `exit 0'.)
+  my %options_1st_pass =
+    (
+     'version' => \&version,
+     'help'    => \&usage,
+     # Recognize all other options (and their arguments) but do nothing.
+     map { $_ => sub {} } (keys %options)
+     );
+  my @ARGV_backup = @ARGV;
+  Getopt::Long::GetOptions %options_1st_pass
+    or exit 1;
+  @ARGV = @ARGV_backup;
+
+  # Now *really* process the options.  This time we know
+  # that --help and --version are not present.
+  Getopt::Long::GetOptions %options
+    or exit 1;
 
-    if (defined $output_directory)
+  if (defined $output_directory)
     {
       msg 'obsolete', "`--output-dir' is deprecated\n";
     }
-    else
+  else
     {
       # In the next release we'll remove this entirely.
       $output_directory = '.';
     }
 
-    foreach my $arg (@ARGV)
+  foreach my $arg (@ARGV)
     {
       if ($arg =~ /^-./)
        {
@@ -1479,9 +1500,9 @@
       $output_files{$input} = join (':', ($local, @rest));
     }
 
-    # Take global strictness from whatever we currently have set.
-    $default_strictness = $strictness;
-    $default_strictness_name = $strictness_name;
+  # Take global strictness from whatever we currently have set.
+  $default_strictness = $strictness;
+  $default_strictness_name = $strictness_name;
 }
 
 ################################################################
@@ -8578,7 +8599,8 @@
 
     print "\nReport bugs to <address@hidden>.\n";
 
-    exit $exit_code;
+    # --help always returns 0 per GNU standards.
+    exit 0;
 }
 
 
@@ -8595,7 +8617,8 @@
 This is free software; see the source for copying conditions.  There is NO
 warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
 EOF
-  exit $exit_code;
+  # --version always returns 0 per GNU standards.
+  exit 0;
 }
 
 ### Setup "GNU" style for perl-mode and cperl-mode.
Index: tests/Makefile.am
===================================================================
RCS file: /cvs/automake/automake/tests/Makefile.am,v
retrieving revision 1.414
diff -u -r1.414 Makefile.am
--- tests/Makefile.am   8 Jul 2002 19:41:24 -0000       1.414
+++ tests/Makefile.am   10 Jul 2002 20:21:11 -0000
@@ -174,6 +174,7 @@
 gcj3.test \
 gcj4.test \
 gcj5.test \
+getopt.test \
 gnits.test \
 gnits2.test \
 header.test \
Index: tests/getopt.test
===================================================================
RCS file: tests/getopt.test
diff -N tests/getopt.test
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ tests/getopt.test   10 Jul 2002 20:21:11 -0000
@@ -0,0 +1,31 @@
+#! /bin/sh
+
+# Automake --help, and --version should have priority over any other option
+# so that their `exit 0' is coherent.
+
+. $srcdir/defs || exit 1
+
+set -e
+
+# This is expected to fail ...
+$AUTOMAKE -Wnonexistant 2>stderr && exit 1
+cat stderr
+grep ':.*nonexistant' stderr
+
+# ... but this should not.
+$AUTOMAKE -Wnonexistant --help 2>stderr
+cat stderr
+grep ':.*nonexistant' stderr && exit 1
+
+
+# Similarly, this should fail ...
+$AUTOMAKE --nonexistant 2>stderr && exit 1
+cat stderr
+grep ':.*nonexistant' stderr
+
+# ... but this should not.
+$AUTOMAKE --nonexistant --help 2>stderr
+cat stderr
+grep ':.*nonexistant' stderr && exit 1
+
+:

-- 
Alexandre Duret-Lutz