From ef4a56a8fe1e09431178b236b36cae8ca71b1fdd Mon Sep 17 00:00:00 2001
From: Daniel Nichter <daniel@percona.com>
Date: Wed, 7 Aug 2013 19:02:11 -0700
Subject: [PATCH] Test and fix OptionParser so string opts don't consume the
 next opt if not string value is given.

---
 lib/OptionParser.pm  | 13 ++++++++++-
 t/lib/OptionParser.t | 54 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/lib/OptionParser.pm b/lib/OptionParser.pm
index 0b600fb5..e49d44b9 100644
--- a/lib/OptionParser.pm
+++ b/lib/OptionParser.pm
@@ -554,12 +554,23 @@ sub _set_option {
    my $long = exists $self->{opts}->{$opt}       ? $opt
             : exists $self->{short_opts}->{$opt} ? $self->{short_opts}->{$opt}
             : die "Getopt::Long gave a nonexistent option: $opt";
-
    # Reassign $opt.
    $opt = $self->{opts}->{$long};
    if ( $opt->{is_cumulative} ) {
       $opt->{value}++;
    }
+   elsif ( ($opt->{type} || '') eq 's' && $val =~ m/^--?(.+)/ ) {
+      # https://bugs.launchpad.net/percona-toolkit/+bug/1199589
+      my $next_opt = $1;
+      if (    exists $self->{opts}->{$next_opt}
+           || exists $self->{short_opts}->{$next_opt} ) {
+         $self->save_error("--$long requires a string value");
+         return;
+      }
+      else {
+         $opt->{value} = $val;
+      }
+   }
    else {
       $opt->{value} = $val;
    }
diff --git a/t/lib/OptionParser.t b/t/lib/OptionParser.t
index 23242870..f56b792f 100644
--- a/t/lib/OptionParser.t
+++ b/t/lib/OptionParser.t
@@ -2063,6 +2063,60 @@ is_deeply(
    "set_vars(): var=0 (bug 1182856)"
 ) or diag(Dumper($vars));
 
+
+# #############################################################################
+# https://bugs.launchpad.net/percona-toolkit/+bug/1199589
+# pt-archiver deletes data despite --dry-run
+# #############################################################################
+
+# From the issue: "One problem is that --optimize is not being used correctly:
+# the option takes an argument: d, s, or ds (see --analyze). The real problem
+# is that --optimize is consuming the next option, which is --dry-run in this
+# case. This shouldn't happen; it means the option parser is failing to notice
+# that --dry-run is not the string val to --optimize but rather an option;
+# it should catch this and the tool should fail to start with an error like
+# "--optimize requires a value".
+
+@ARGV = qw(--optimize --dry-run --ascend-first --where 1=1 --purge --source localhost);
+$o = new OptionParser(file => "$trunk/bin/pt-archiver");
+$o->get_specs();
+$o->get_opts();
+
+$output = output(
+   sub { $o->usage_or_errors(undef, 1); },
+);
+
+like(
+   $output,
+   qr/--optimize requires a string value/,
+   "String opts don't consume the next opt (bug 1199589)"
+);
+
+is(
+   $o->get('optimize'),
+   undef,
+   "--optimize didn't consume --dry-run (bug 1199589)"
+);
+
+@ARGV = qw(--optimize ds --dry-run --ascend-first --where 1=1 --purge --source localhost);
+$o->get_opts();
+
+$output = output(
+   sub { $o->usage_or_errors(undef, 1); },
+);
+
+is(
+   $output,
+   '',
+   "String opts still work (bug 1199589)"
+);
+
+is(
+   $o->get('optimize'),
+   'ds',
+   "--optimize got its value (bug 1199589)"
+);
+
 # #############################################################################
 # Done.
 # #############################################################################