Merge opt-parsing-exit-status-bug-1039074.

This commit is contained in:
Daniel Nichter
2012-10-22 12:34:32 -06:00
31 changed files with 124 additions and 70 deletions

View File

@@ -524,7 +524,7 @@ sub get_opts {
else {
print "Error parsing version. See the VERSION section of the tool's documentation.\n";
}
exit 0;
exit 1;
}
if ( @ARGV && $self->{strict} ) {
@@ -779,7 +779,7 @@ sub usage_or_errors {
}
elsif ( scalar @{$self->{errors}} ) {
print $self->print_errors() or die "Cannot print errors: $OS_ERROR";
exit 0 unless $return;
exit 1 unless $return;
}
return;

View File

@@ -523,7 +523,7 @@ sub get_opts {
else {
print "Error parsing version. See the VERSION section of the tool's documentation.\n";
}
exit 0;
exit 1;
}
if ( @ARGV && $self->{strict} ) {
@@ -778,7 +778,7 @@ sub usage_or_errors {
}
elsif ( scalar @{$self->{errors}} ) {
print $self->print_errors() or die "Cannot print errors: $OS_ERROR";
exit 0 unless $return;
exit 1 unless $return;
}
return;

View File

@@ -521,7 +521,7 @@ sub get_opts {
else {
print "Error parsing version. See the VERSION section of the tool's documentation.\n";
}
exit 0;
exit 1;
}
if ( @ARGV && $self->{strict} ) {
@@ -776,7 +776,7 @@ sub usage_or_errors {
}
elsif ( scalar @{$self->{errors}} ) {
print $self->print_errors() or die "Cannot print errors: $OS_ERROR";
exit 0 unless $return;
exit 1 unless $return;
}
return;

View File

@@ -523,7 +523,7 @@ sub get_opts {
else {
print "Error parsing version. See the VERSION section of the tool's documentation.\n";
}
exit 0;
exit 1;
}
if ( @ARGV && $self->{strict} ) {
@@ -778,7 +778,7 @@ sub usage_or_errors {
}
elsif ( scalar @{$self->{errors}} ) {
print $self->print_errors() or die "Cannot print errors: $OS_ERROR";
exit 0 unless $return;
exit 1 unless $return;
}
return;

View File

@@ -1425,7 +1425,7 @@ sub get_opts {
else {
print "Error parsing version. See the VERSION section of the tool's documentation.\n";
}
exit 0;
exit 1;
}
if ( @ARGV && $self->{strict} ) {
@@ -1680,7 +1680,7 @@ sub usage_or_errors {
}
elsif ( scalar @{$self->{errors}} ) {
print $self->print_errors() or die "Cannot print errors: $OS_ERROR";
exit 0 unless $return;
exit 1 unless $return;
}
return;

View File

@@ -495,7 +495,7 @@ sub get_opts {
else {
print "Error parsing version. See the VERSION section of the tool's documentation.\n";
}
exit 0;
exit 1;
}
if ( @ARGV && $self->{strict} ) {
@@ -750,7 +750,7 @@ sub usage_or_errors {
}
elsif ( scalar @{$self->{errors}} ) {
print $self->print_errors() or die "Cannot print errors: $OS_ERROR";
exit 0 unless $return;
exit 1 unless $return;
}
return;

View File

@@ -893,7 +893,7 @@ sub get_opts {
else {
print "Error parsing version. See the VERSION section of the tool's documentation.\n";
}
exit 0;
exit 1;
}
if ( @ARGV && $self->{strict} ) {
@@ -1148,7 +1148,7 @@ sub usage_or_errors {
}
elsif ( scalar @{$self->{errors}} ) {
print $self->print_errors() or die "Cannot print errors: $OS_ERROR";
exit 0 unless $return;
exit 1 unless $return;
}
return;

View File

@@ -496,7 +496,7 @@ sub get_opts {
else {
print "Error parsing version. See the VERSION section of the tool's documentation.\n";
}
exit 0;
exit 1;
}
if ( @ARGV && $self->{strict} ) {
@@ -751,7 +751,7 @@ sub usage_or_errors {
}
elsif ( scalar @{$self->{errors}} ) {
print $self->print_errors() or die "Cannot print errors: $OS_ERROR";
exit 0 unless $return;
exit 1 unless $return;
}
return;

View File

@@ -520,7 +520,7 @@ sub get_opts {
else {
print "Error parsing version. See the VERSION section of the tool's documentation.\n";
}
exit 0;
exit 1;
}
if ( @ARGV && $self->{strict} ) {
@@ -775,7 +775,7 @@ sub usage_or_errors {
}
elsif ( scalar @{$self->{errors}} ) {
print $self->print_errors() or die "Cannot print errors: $OS_ERROR";
exit 0 unless $return;
exit 1 unless $return;
}
return;

View File

@@ -1256,7 +1256,7 @@ sub get_opts {
else {
print "Error parsing version. See the VERSION section of the tool's documentation.\n";
}
exit 0;
exit 1;
}
if ( @ARGV && $self->{strict} ) {
@@ -1511,7 +1511,7 @@ sub usage_or_errors {
}
elsif ( scalar @{$self->{errors}} ) {
print $self->print_errors() or die "Cannot print errors: $OS_ERROR";
exit 0 unless $return;
exit 1 unless $return;
}
return;

View File

@@ -1023,7 +1023,7 @@ sub get_opts {
else {
print "Error parsing version. See the VERSION section of the tool's documentation.\n";
}
exit 0;
exit 1;
}
if ( @ARGV && $self->{strict} ) {
@@ -1278,7 +1278,7 @@ sub usage_or_errors {
}
elsif ( scalar @{$self->{errors}} ) {
print $self->print_errors() or die "Cannot print errors: $OS_ERROR";
exit 0 unless $return;
exit 1 unless $return;
}
return;

View File

@@ -528,7 +528,7 @@ sub get_opts {
else {
print "Error parsing version. See the VERSION section of the tool's documentation.\n";
}
exit 0;
exit 1;
}
if ( @ARGV && $self->{strict} ) {
@@ -783,7 +783,7 @@ sub usage_or_errors {
}
elsif ( scalar @{$self->{errors}} ) {
print $self->print_errors() or die "Cannot print errors: $OS_ERROR";
exit 0 unless $return;
exit 1 unless $return;
}
return;

View File

@@ -500,7 +500,7 @@ sub get_opts {
else {
print "Error parsing version. See the VERSION section of the tool's documentation.\n";
}
exit 0;
exit 1;
}
if ( @ARGV && $self->{strict} ) {
@@ -755,7 +755,7 @@ sub usage_or_errors {
}
elsif ( scalar @{$self->{errors}} ) {
print $self->print_errors() or die "Cannot print errors: $OS_ERROR";
exit 0 unless $return;
exit 1 unless $return;
}
return;

View File

@@ -534,7 +534,7 @@ sub get_opts {
else {
print "Error parsing version. See the VERSION section of the tool's documentation.\n";
}
exit 0;
exit 1;
}
if ( @ARGV && $self->{strict} ) {
@@ -789,7 +789,7 @@ sub usage_or_errors {
}
elsif ( scalar @{$self->{errors}} ) {
print $self->print_errors() or die "Cannot print errors: $OS_ERROR";
exit 0 unless $return;
exit 1 unless $return;
}
return;

View File

@@ -904,7 +904,7 @@ sub get_opts {
else {
print "Error parsing version. See the VERSION section of the tool's documentation.\n";
}
exit 0;
exit 1;
}
if ( @ARGV && $self->{strict} ) {
@@ -1159,7 +1159,7 @@ sub usage_or_errors {
}
elsif ( scalar @{$self->{errors}} ) {
print $self->print_errors() or die "Cannot print errors: $OS_ERROR";
exit 0 unless $return;
exit 1 unless $return;
}
return;

View File

@@ -1042,7 +1042,7 @@ sub get_opts {
else {
print "Error parsing version. See the VERSION section of the tool's documentation.\n";
}
exit 0;
exit 1;
}
if ( @ARGV && $self->{strict} ) {
@@ -1297,7 +1297,7 @@ sub usage_or_errors {
}
elsif ( scalar @{$self->{errors}} ) {
print $self->print_errors() or die "Cannot print errors: $OS_ERROR";
exit 0 unless $return;
exit 1 unless $return;
}
return;

View File

@@ -496,7 +496,7 @@ sub get_opts {
else {
print "Error parsing version. See the VERSION section of the tool's documentation.\n";
}
exit 0;
exit 1;
}
if ( @ARGV && $self->{strict} ) {
@@ -751,7 +751,7 @@ sub usage_or_errors {
}
elsif ( scalar @{$self->{errors}} ) {
print $self->print_errors() or die "Cannot print errors: $OS_ERROR";
exit 0 unless $return;
exit 1 unless $return;
}
return;

View File

@@ -521,7 +521,7 @@ sub get_opts {
else {
print "Error parsing version. See the VERSION section of the tool's documentation.\n";
}
exit 0;
exit 1;
}
if ( @ARGV && $self->{strict} ) {
@@ -776,7 +776,7 @@ sub usage_or_errors {
}
elsif ( scalar @{$self->{errors}} ) {
print $self->print_errors() or die "Cannot print errors: $OS_ERROR";
exit 0 unless $return;
exit 1 unless $return;
}
return;

View File

@@ -500,7 +500,7 @@ sub get_opts {
else {
print "Error parsing version. See the VERSION section of the tool's documentation.\n";
}
exit 0;
exit 1;
}
if ( @ARGV && $self->{strict} ) {
@@ -755,7 +755,7 @@ sub usage_or_errors {
}
elsif ( scalar @{$self->{errors}} ) {
print $self->print_errors() or die "Cannot print errors: $OS_ERROR";
exit 0 unless $return;
exit 1 unless $return;
}
return;

View File

@@ -642,7 +642,7 @@ sub get_opts {
else {
print "Error parsing version. See the VERSION section of the tool's documentation.\n";
}
exit 0;
exit 1;
}
if ( @ARGV && $self->{strict} ) {
@@ -897,7 +897,7 @@ sub usage_or_errors {
}
elsif ( scalar @{$self->{errors}} ) {
print $self->print_errors() or die "Cannot print errors: $OS_ERROR";
exit 0 unless $return;
exit 1 unless $return;
}
return;

View File

@@ -2190,7 +2190,7 @@ sub get_opts {
else {
print "Error parsing version. See the VERSION section of the tool's documentation.\n";
}
exit 0;
exit 1;
}
if ( @ARGV && $self->{strict} ) {
@@ -2445,7 +2445,7 @@ sub usage_or_errors {
}
elsif ( scalar @{$self->{errors}} ) {
print $self->print_errors() or die "Cannot print errors: $OS_ERROR";
exit 0 unless $return;
exit 1 unless $return;
}
return;

View File

@@ -537,7 +537,7 @@ sub get_opts {
else {
print "Error parsing version. See the VERSION section of the tool's documentation.\n";
}
exit 0;
exit 1;
}
if ( @ARGV && $self->{strict} ) {
@@ -792,7 +792,7 @@ sub usage_or_errors {
}
elsif ( scalar @{$self->{errors}} ) {
print $self->print_errors() or die "Cannot print errors: $OS_ERROR";
exit 0 unless $return;
exit 1 unless $return;
}
return;

View File

@@ -445,6 +445,7 @@ sub new {
'default' => 1,
'cumulative' => 1,
'negatable' => 1,
'value_is_optional' => 1,
);
my $self = {
@@ -686,9 +687,10 @@ sub _parse_specs {
$opt->{short} = undef;
}
$opt->{is_negatable} = $opt->{spec} =~ m/!/ ? 1 : 0;
$opt->{is_cumulative} = $opt->{spec} =~ m/\+/ ? 1 : 0;
$opt->{is_required} = $opt->{desc} =~ m/required/ ? 1 : 0;
$opt->{is_negatable} = $opt->{spec} =~ m/!/ ? 1 : 0;
$opt->{is_cumulative} = $opt->{spec} =~ m/\+/ ? 1 : 0;
$opt->{optional_value} = $opt->{spec} =~ m/:/ ? 1 : 0;
$opt->{is_required} = $opt->{desc} =~ m/required/ ? 1 : 0;
$opt->{group} ||= 'default';
$self->{groups}->{ $opt->{group} }->{$long} = 1;
@@ -824,7 +826,7 @@ sub _set_option {
if ( $opt->{is_cumulative} ) {
$opt->{value}++;
}
else {
elsif ( !($opt->{optional_value} && !$val) ) {
$opt->{value} = $val;
}
$opt->{got} = 1;
@@ -881,7 +883,7 @@ sub get_opts {
else {
print "Error parsing version. See the VERSION section of the tool's documentation.\n";
}
exit 0;
exit 1;
}
if ( @ARGV && $self->{strict} ) {
@@ -1136,7 +1138,7 @@ sub usage_or_errors {
}
elsif ( scalar @{$self->{errors}} ) {
print $self->print_errors() or die "Cannot print errors: $OS_ERROR";
exit 0 unless $return;
exit 1 unless $return;
}
return;
@@ -1365,11 +1367,12 @@ sub _parse_size {
sub _parse_attribs {
my ( $self, $option, $attribs ) = @_;
my $types = $self->{types};
my $eq = $attribs->{'value_is_optional'} ? ':' : '=';
return $option
. ($attribs->{'short form'} ? '|' . $attribs->{'short form'} : '' )
. ($attribs->{'negatable'} ? '!' : '' )
. ($attribs->{'cumulative'} ? '+' : '' )
. ($attribs->{'type'} ? '=' . $types->{$attribs->{type}} : '' );
. ($attribs->{'type'} ? $eq . $types->{$attribs->{type}} : '' );
}
sub _parse_synopsis {

View File

@@ -499,7 +499,7 @@ sub get_opts {
else {
print "Error parsing version. See the VERSION section of the tool's documentation.\n";
}
exit 0;
exit 1;
}
if ( @ARGV && $self->{strict} ) {
@@ -754,7 +754,7 @@ sub usage_or_errors {
}
elsif ( scalar @{$self->{errors}} ) {
print $self->print_errors() or die "Cannot print errors: $OS_ERROR";
exit 0 unless $return;
exit 1 unless $return;
}
return;

View File

@@ -499,7 +499,7 @@ sub get_opts {
else {
print "Error parsing version. See the VERSION section of the tool's documentation.\n";
}
exit 0;
exit 1;
}
if ( @ARGV && $self->{strict} ) {
@@ -754,7 +754,7 @@ sub usage_or_errors {
}
elsif ( scalar @{$self->{errors}} ) {
print $self->print_errors() or die "Cannot print errors: $OS_ERROR";
exit 0 unless $return;
exit 1 unless $return;
}
return;

View File

@@ -1444,7 +1444,7 @@ sub get_opts {
else {
print "Error parsing version. See the VERSION section of the tool's documentation.\n";
}
exit 0;
exit 1;
}
if ( @ARGV && $self->{strict} ) {
@@ -1699,7 +1699,7 @@ sub usage_or_errors {
}
elsif ( scalar @{$self->{errors}} ) {
print $self->print_errors() or die "Cannot print errors: $OS_ERROR";
exit 0 unless $return;
exit 1 unless $return;
}
return;

View File

@@ -525,7 +525,7 @@ sub get_opts {
else {
print "Error parsing version. See the VERSION section of the tool's documentation.\n";
}
exit 0;
exit 1;
}
if ( @ARGV && $self->{strict} ) {
@@ -780,7 +780,7 @@ sub usage_or_errors {
}
elsif ( scalar @{$self->{errors}} ) {
print $self->print_errors() or die "Cannot print errors: $OS_ERROR";
exit 0 unless $return;
exit 1 unless $return;
}
return;

View File

@@ -6,7 +6,20 @@
use strict;
use warnings FATAL => 'all';
use constant PTDEBUG => $ENV{PTDEBUG} || 0;
# This tool is "fat-packed": most of its dependent modules are embedded
# in this file. Setting %INC to this file for each module makes Perl aware
# of this so it will not try to load the module from @INC. See the tool's
# documentation for a full list of dependencies.
BEGIN {
$INC{$_} = __FILE__ for map { (my $pkg = "$_.pm") =~ s!::!/!g; $pkg } (qw(
ExplainParser
ExplainTree
OptionParser
DSNParser
Daemon
));
}
# ###########################################################################
# Converts text (e.g. saved output) to a "recordset" -- an array of hashrefs
@@ -719,6 +732,7 @@ sub new {
'default' => 1,
'cumulative' => 1,
'negatable' => 1,
'value_is_optional' => 1,
);
my $self = {
@@ -960,9 +974,10 @@ sub _parse_specs {
$opt->{short} = undef;
}
$opt->{is_negatable} = $opt->{spec} =~ m/!/ ? 1 : 0;
$opt->{is_cumulative} = $opt->{spec} =~ m/\+/ ? 1 : 0;
$opt->{is_required} = $opt->{desc} =~ m/required/ ? 1 : 0;
$opt->{is_negatable} = $opt->{spec} =~ m/!/ ? 1 : 0;
$opt->{is_cumulative} = $opt->{spec} =~ m/\+/ ? 1 : 0;
$opt->{optional_value} = $opt->{spec} =~ m/:/ ? 1 : 0;
$opt->{is_required} = $opt->{desc} =~ m/required/ ? 1 : 0;
$opt->{group} ||= 'default';
$self->{groups}->{ $opt->{group} }->{$long} = 1;
@@ -1098,7 +1113,7 @@ sub _set_option {
if ( $opt->{is_cumulative} ) {
$opt->{value}++;
}
else {
elsif ( !($opt->{optional_value} && !$val) ) {
$opt->{value} = $val;
}
$opt->{got} = 1;
@@ -1155,7 +1170,7 @@ sub get_opts {
else {
print "Error parsing version. See the VERSION section of the tool's documentation.\n";
}
exit 0;
exit 1;
}
if ( @ARGV && $self->{strict} ) {
@@ -1410,7 +1425,7 @@ sub usage_or_errors {
}
elsif ( scalar @{$self->{errors}} ) {
print $self->print_errors() or die "Cannot print errors: $OS_ERROR";
exit 0 unless $return;
exit 1 unless $return;
}
return;
@@ -1639,11 +1654,12 @@ sub _parse_size {
sub _parse_attribs {
my ( $self, $option, $attribs ) = @_;
my $types = $self->{types};
my $eq = $attribs->{'value_is_optional'} ? ':' : '=';
return $option
. ($attribs->{'short form'} ? '|' . $attribs->{'short form'} : '' )
. ($attribs->{'negatable'} ? '!' : '' )
. ($attribs->{'cumulative'} ? '+' : '' )
. ($attribs->{'type'} ? '=' . $types->{$attribs->{type}} : '' );
. ($attribs->{'type'} ? $eq . $types->{$attribs->{type}} : '' );
}
sub _parse_synopsis {

View File

@@ -669,7 +669,7 @@ sub get_opts {
else {
print "Error parsing version. See the VERSION section of the tool's documentation.\n";
}
exit 0;
exit 1;
}
if ( @ARGV && $self->{strict} ) {
@@ -988,7 +988,7 @@ sub usage_or_errors {
}
elsif ( scalar @{$self->{errors}} ) {
print $self->print_errors() or die "Cannot print errors: $OS_ERROR";
exit 0 unless $return;
exit 1 unless $return;
}
return;

View File

@@ -2057,6 +2057,28 @@ is(
"..and has the specified value",
);
# #############################################################################
# Bug 1039074: Tools exit 0 on error parsing options, should exit non-zero
# #############################################################################
# pt-archiver requires at least one of --dest, --file or --purge, as well as
# --where and --source. So specifying no options should cause errors.
@ARGV = qw();
$o = new OptionParser(file => "$trunk/bin/pt-archiver");
$o->get_specs();
$o->get_opts();
my $exit_status = 0;
($output, $exit_status) = full_output(
sub { $o->usage_or_errors("$trunk/bin/pt-archiver"); },
);
is(
$exit_status,
1,
"Non-zero exit status on error parsing options (bug 1039074)"
);
# #############################################################################
# Done.
# #############################################################################

View File

@@ -9,7 +9,7 @@ BEGIN {
use strict;
use warnings FATAL => 'all';
use English qw(-no_match_vars);
use Test::More tests => 3;
use Test::More tests => 4;
use PerconaTest;
@@ -39,6 +39,19 @@ like(
'Dest DSN requires t'
);
# #############################################################################
# Bug 1039074: Tools exit 0 on error parsing options, should exit non-zero
# #############################################################################
system("$trunk/bin/pt-deadlock-logger --i-am-the-error >/dev/null 2>&1");
my $exit_status = $CHILD_ERROR >> 8;
is(
$exit_status,
1,
"Non-zero exit on option error (bug 1039074)"
);
# #############################################################################
# Done.
# #############################################################################