automake-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[FYI] {maint} cmdline parsing: move into a dedicated perl module


From: Stefano Lattarini
Subject: [FYI] {maint} cmdline parsing: move into a dedicated perl module
Date: Wed, 18 Jan 2012 18:16:12 +0100

With this change, we delegate most of the automake and aclocal code
for command-line options parsing to a new module "Automake::Getopt".
This allows better code sharing between automake and aclocal, and
also with Autoconf, which will sync the new module from us.  See
also autoconf commit 'v2.68-120-gf4be358' (2012-01-17, "getopt: new
Autom4te::Getopt module"), and this mailing list discussion:
<http://lists.gnu.org/archive/html/autoconf-patches/2012-01/msg00033.html>

This change might interact with the behaviour described in automake
bug#7434; for example, starting from now, "automake -Wfoo --version"
will cause automake to emit diagnostic like "unknown warning
category 'foo'" before actually printing the version number and
exiting.  This is not a big deal in practice, and the code sharing
and simplifications introduced by this patch is certainly worth it.
Still, we should revisited the issue in the future.

* lib/Automake/Getopt.pm: New module, basically a slightly-edited
copy of the 'lib/Autom4te/Getopt.pm' file from the autoconf devel
repository (commit v2.68-120-gf4be358).  It defines and exports ...
(parse_options): ... this new function.
* automake.in (parse_arguments): Use the new function.
* aclocal.in (parse_arguments): Likewise .
* lib/Automake/Makefile.am (dist_perllib_DATA): Add the new file.
---
 aclocal.in               |   54 ++-------------------
 automake.in              |   56 ++---------------------
 lib/Automake/Getopt.pm   |  115 ++++++++++++++++++++++++++++++++++++++++++++++
 lib/Automake/Makefile.am |    1 +
 4 files changed, 125 insertions(+), 101 deletions(-)
 create mode 100644 lib/Automake/Getopt.pm

diff --git a/aclocal.in b/aclocal.in
index 2ae9a89..ed1d8d9 100644
--- a/aclocal.in
+++ b/aclocal.in
@@ -945,6 +945,8 @@ sub parse_arguments ()
 
   my %cli_options =
     (
+     'help'            => sub { usage(0); },
+     'version'         => \&version,
      'acdir=s'         => \&handle_acdir_option,
      'system-acdir=s'  => sub { shift; @system_includes = @_; },
      'automake-acdir=s'        => sub { shift; @automake_includes = @_; },
@@ -958,55 +960,9 @@ sub parse_arguments ()
      'verbose'         => sub { setup_channel 'verb', silent => 0; },
      'W|warnings=s'     => \&parse_warnings,
      );
-  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 guarantee this
-  # if we treat other options first.  (Handling other options first
-  # could produce error diagnostics, and in this condition it is
-  # confusing if aclocal does `exit 0'.)
-  my %cli_options_1st_pass =
-    (
-     'version' => \&version,
-     'help'    => sub { usage(0); },
-     # Recognize all other options (and their arguments) but do nothing.
-     map { $_ => sub {} } (keys %cli_options)
-     );
-  my @ARGV_backup = @ARGV;
-  Getopt::Long::GetOptions %cli_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, but we specify them nonetheless so
-  # that ambiguous abbreviation are diagnosed.
-  Getopt::Long::GetOptions %cli_options, 'version' => sub {}, 'help' => sub {}
-    or exit 1;
-
-  if (@ARGV)
-    {
-      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.");
-       }
-    }
+
+  use Automake::Getopt ();
+  Automake::Getopt::parse_options %cli_options;
 
   if ($print_and_exit)
     {
diff --git a/automake.in b/automake.in
index 24d5872..3c70b38 100644
--- a/automake.in
+++ b/automake.in
@@ -8496,6 +8496,8 @@ sub parse_arguments ()
   my $cli_where = new Automake::Location;
   my %cli_options =
     (
+     'version' => \&version,
+     'help'    => \&usage,
      'libdir=s'        => \$libdir,
      'gnu'             => sub { set_strictness ('gnu'); },
      'gnits'           => sub { set_strictness ('gnits'); },
@@ -8516,32 +8518,9 @@ sub parse_arguments ()
      '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 guarantee this
-  # if we treat other options first.  (Handling other options first
-  # could produce error diagnostics, and in this condition it is
-  # confusing if Automake does `exit 0'.)
-  my %cli_options_1st_pass =
-    (
-     'version' => \&version,
-     'help'    => \&usage,
-     # Recognize all other options (and their arguments) but do nothing.
-     map { $_ => sub {} } (keys %cli_options)
-     );
-  my @ARGV_backup = @ARGV;
-  Getopt::Long::GetOptions %cli_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, but we specify them nonetheless so
-  # that ambiguous abbreviation are diagnosed.
-  Getopt::Long::GetOptions %cli_options, 'version' => sub {}, 'help' => sub {}
-    or exit 1;
+  use Automake::Getopt ();
+  Automake::Getopt::parse_options %cli_options;
 
   if (defined $output_directory)
     {
@@ -8555,33 +8534,6 @@ sub parse_arguments ()
 
   return unless @ARGV;
 
-  if ($ARGV[0] =~ /^-./)
-    {
-      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 `$ARGV[0]'.\n"
-                . "Try `$0 --help' for more information.");
-       }
-    }
-
   my $errspec = 0;
   foreach my $arg (@ARGV)
     {
diff --git a/lib/Automake/Getopt.pm b/lib/Automake/Getopt.pm
new file mode 100644
index 0000000..a925571
--- /dev/null
+++ b/lib/Automake/Getopt.pm
@@ -0,0 +1,115 @@
+# Copyright (C) 2012 Free Software Foundation, Inc.
+
+# This program 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 3 of the License, or
+# (at your option) any later version.
+
+# This program 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 this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package Automake::Getopt;
+
+=head1 NAME
+
+Automake::Getopt - GCS conforming parser for command line options
+
+=head1 SYNOPSIS
+
+  use Automake::Getopt;
+
+=head1 DESCRIPTION
+
+Export a function C<parse_options>, performing parsing of command
+line options in conformance to the GNU Coding standards.
+
+=cut
+
+use 5.006_002;
+use strict;
+use warnings FATAL => 'all';
+use Exporter ();
+use Getopt::Long ();
+use Automake::ChannelDefs qw/fatal/;
+use Carp qw/croak confess/;
+
+use vars qw (@ISA @EXPORT);
address@hidden = qw (Exporter);
address@hidden qw/getopt/;
+
+=item C<parse_options (%option)>
+
+Wrapper around C<Getopt::Long>, trying to conform to the GNU
+Coding Standards for error messages.
+
+=cut
+
+sub parse_options (%)
+{
+  my %option = @_;
+
+  Getopt::Long::Configure ("bundling", "pass_through");
+  # Unrecognized options are passed through, so GetOption can only fail
+  # due to internal errors or misuse of options specification.
+  Getopt::Long::GetOptions (%option)
+    or confess "error in options specification (likely)";
+
+  if (@ARGV && $ARGV[0] =~ /^-./)
+    {
+      my %argopts;
+      for my $k (keys %option)
+       {
+         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 `$ARGV[0]'.\n"
+                . "Try `$0 --help' for more information.");
+       }
+    }
+}
+
+=back
+
+=head1 SEE ALSO
+
+L<Getopt::Long>
+
+=cut
+
+1; # for require
+
+### Setup "GNU" style for perl-mode and cperl-mode.
+## Local Variables:
+## perl-indent-level: 2
+## perl-continued-statement-offset: 2
+## perl-continued-brace-offset: 0
+## perl-brace-offset: 0
+## perl-brace-imaginary-offset: 0
+## perl-label-offset: -2
+## cperl-indent-level: 2
+## cperl-brace-offset: 0
+## cperl-continued-brace-offset: 0
+## cperl-label-offset: -2
+## cperl-extra-newline-before-brace: t
+## cperl-merge-trailing-else: nil
+## cperl-continued-statement-offset: 2
+## End:
diff --git a/lib/Automake/Makefile.am b/lib/Automake/Makefile.am
index 9805024..ac9356a 100644
--- a/lib/Automake/Makefile.am
+++ b/lib/Automake/Makefile.am
@@ -25,6 +25,7 @@ dist_perllib_DATA = \
   DisjConditions.pm \
   FileUtils.pm \
   General.pm \
+  Getopt.pm \
   Item.pm \
   ItemDef.pm \
   Location.pm \
-- 
1.7.7.3




reply via email to

[Prev in Thread] Current Thread [Next in Thread]