From 52246cc7355cedbc9cb992095870572beb767ca1 Mon Sep 17 00:00:00 2001 From: Stefano Lattarini Date: Wed, 18 Jan 2012 17:55:40 +0100 Subject: [PATCH] cmdline parsing: move into a dedicated perl module 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: 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. * tests/getopt.test: Remove. * tests/list-of-tests.mk: Update. --- aclocal.in | 54 +++------------------- automake.in | 56 ++--------------------- lib/Automake/Getopt.pm | 115 +++++++++++++++++++++++++++++++++++++++++++++++ lib/Automake/Makefile.am | 1 + tests/getopt.test | 41 ----------------- tests/list-of-tests.mk | 1 - 6 files changed, 125 insertions(+), 143 deletions(-) create mode 100644 lib/Automake/Getopt.pm delete mode 100755 tests/getopt.test 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..84cee5e --- /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 . + +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, 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); +@ISA = qw (Exporter); +@EXPORT= qw/getopt/; + +=item C + +Wrapper around C, 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 + +=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 \ diff --git a/tests/getopt.test b/tests/getopt.test deleted file mode 100755 index bc82984..0000000 --- a/tests/getopt.test +++ /dev/null @@ -1,41 +0,0 @@ -#! /bin/sh -# Copyright (C) 2002, 2003, 2008 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 2, 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 . - -# Automake --help, and --version should have priority over any other option -# so that their `Exit 0' is coherent. - -. ./defs || Exit 1 - -set -e - -# This is expected to fail ... -AUTOMAKE_fails -Wnonexistent -grep ':.*nonexistent' stderr - -# ... but this should not. -AUTOMAKE_run 0 -Wnonexistent --help -grep ':.*nonexistent' stderr && Exit 1 - - -# Similarly, this should fail ... -AUTOMAKE_fails --nonexistent -grep ':.*nonexistent' stderr - -# ... but this should not. -AUTOMAKE_run 0 --nonexistent --help -grep ':.*nonexistent' stderr && Exit 1 - -: diff --git a/tests/list-of-tests.mk b/tests/list-of-tests.mk index 7585e2b..556989e 100644 --- a/tests/list-of-tests.mk +++ b/tests/list-of-tests.mk @@ -393,7 +393,6 @@ gcj3.test \ gcj4.test \ gcj5.test \ gcj6.test \ -getopt.test \ gettext.test \ gettext2.test \ gettext3.test \ -- 2.7.4