Merge lp:~percona-toolkit-dev/percona-toolkit/not-all-scripts-recognize-no-version-check-1361293 into lp:~percona-toolkit-dev/percona-toolkit/release-2.2.12

[Frank Cizmich]

Problem:
When option no-version-check was put in a global configuration file (to avoid unwanted internet connection) , many tools failed as they did not recognize the option.

Fix:
Bash tools were instructed to silently ignore it if it was read from a config file (none does version-check right now)
Perl tools were instructed to ignore it if it was read from a config file AND it was unsupported.

Modification was done on OptionParser.pm and parse_options.sh , and updated to the rest of the tools.

Note:
pt-mext bash script had differences with it's embedded parse_options module ( the _parse_pod function seemed to pipe the source file through a perl code, rather than having the perl code open and read the file ) ... but nevertheless after updating tests ran ok.

[Daniel Nichter (daniel-nichter)]

In the comments there's a question and a minor style change for a line in parse_options.sh.

review: Needs Fixing

[Frank Cizmich (frank-cizmich)]

Answered in the comments.

=== modified file 'bin/pt-align'
--- bin/pt-align	2014-09-25 13:48:22 +0000
+++ bin/pt-align	2014-10-09 17:53:04 +0000
@@ -901,6 +901,14 @@
          $parse  = 0;
          next LINE;
       }
+
+      if (  $parse 
+            && !$self->has('version-check') 
+            && $line =~ /version-check/ 
+      ) {
+         next LINE;
+      }
+
       if ( $parse
          && (my($opt, $arg) = $line =~ m/^\s*([^=\s]+?)(?:\s*=\s*(.*?)\s*)?$/)
       ) {

=== modified file 'bin/pt-fifo-split'
--- bin/pt-fifo-split	2014-09-25 13:48:22 +0000
+++ bin/pt-fifo-split	2014-10-09 17:53:04 +0000
@@ -902,6 +902,14 @@
          $parse  = 0;
          next LINE;
       }
+
+      if (  $parse 
+            && !$self->has('version-check') 
+            && $line =~ /version-check/ 
+      ) {
+         next LINE;
+      }
+
       if ( $parse
          && (my($opt, $arg) = $line =~ m/^\s*([^=\s]+?)(?:\s*=\s*(.*?)\s*)?$/)
       ) {

=== modified file 'bin/pt-fingerprint'
--- bin/pt-fingerprint	2014-09-25 13:48:22 +0000
+++ bin/pt-fingerprint	2014-10-09 17:53:04 +0000
@@ -903,6 +903,14 @@
          $parse  = 0;
          next LINE;
       }
+
+      if (  $parse 
+            && !$self->has('version-check') 
+            && $line =~ /version-check/ 
+      ) {
+         next LINE;
+      }
+
       if ( $parse
          && (my($opt, $arg) = $line =~ m/^\s*([^=\s]+?)(?:\s*=\s*(.*?)\s*)?$/)
       ) {

=== modified file 'bin/pt-ioprofile'
--- bin/pt-ioprofile	2014-09-25 13:48:22 +0000
+++ bin/pt-ioprofile	2014-10-09 17:53:04 +0000
@@ -86,9 +86,10 @@
 
 usage_or_errors() {
    local file="$1"
+   local version=""
 
    if [ "$OPT_VERSION" ]; then
-      local version=$(grep '^pt-[^ ]\+ [0-9]' "$file")
+      version=$(grep '^pt-[^ ]\+ [0-9]' "$file")
       echo "$version"
       return 1
    fi
@@ -310,6 +311,8 @@
 
          [ "$config_opt" = "" ] && continue
 
+         echo "$config_opt" | grep -v 'version-check' >/dev/null 2>&1 || continue
+
          if ! [ "$HAVE_EXT_ARGV" ]; then
             config_opt="--$config_opt"
          fi

=== modified file 'bin/pt-mext'
--- bin/pt-mext	2014-09-25 13:48:22 +0000
+++ bin/pt-mext	2014-10-09 17:53:04 +0000
@@ -251,34 +251,38 @@
 _parse_pod() {
    local file="$1"
 
-   cat "$file" | PO_DIR="$PO_DIR" perl -ne '
-      BEGIN { $/ = ""; }
-      next unless $_ =~ m/^=head1 OPTIONS/;
-      while ( defined(my $para = <>) ) {
-         last if $para =~ m/^=head1/;
-         chomp;
-         if ( $para =~ m/^=item --(\S+)/ ) {
-            my $opt  = $1;
-            my $file = "$ENV{PO_DIR}/$opt";
-            open my $opt_fh, ">", $file or die "Cannot open $file: $!";
-            print $opt_fh "long:$opt\n";
-            $para = <>;
+   PO_FILE="$file" PO_DIR="$PO_DIR" perl -e '
+      $/ = "";
+      my $file = $ENV{PO_FILE};
+      open my $fh, "<", $file or die "Cannot open $file: $!";
+      while ( defined(my $para = <$fh>) ) {
+         next unless $para =~ m/^=head1 OPTIONS/;
+         while ( defined(my $para = <$fh>) ) {
+            last if $para =~ m/^=head1/;
             chomp;
-            if ( $para =~ m/^[a-z ]+:/ ) {
-               map {
+            if ( $para =~ m/^=item --(\S+)/ ) {
+               my $opt  = $1;
+               my $file = "$ENV{PO_DIR}/$opt";
+               open my $opt_fh, ">", $file or die "Cannot open $file: $!";
+               print $opt_fh "long:$opt\n";
+               $para = <$fh>;
+               chomp;
+               if ( $para =~ m/^[a-z ]+:/ ) {
+                  map {
+                     chomp;
+                     my ($attrib, $val) = split(/: /, $_);
+                     print $opt_fh "$attrib:$val\n";
+                  } split(/; /, $para);
+                  $para = <$fh>;
                   chomp;
-                  my ($attrib, $val) = split(/: /, $_);
-                  print $opt_fh "$attrib:$val\n";
-               } split(/; /, $para);
-               $para = <>;
-               chomp;
+               }
+               my ($desc) = $para =~ m/^([^?.]+)/;
+               print $opt_fh "desc:$desc.\n";
+               close $opt_fh;
             }
-            my ($desc) = $para =~ m/^([^?.]+)/;
-            print $opt_fh "desc:$desc.\n";
-            close $opt_fh;
          }
+         last;
       }
-      last;
    '
 }
 
@@ -348,6 +352,8 @@
 
          [ "$config_opt" = "" ] && continue
 
+         echo "$config_opt" | grep -v 'version-check' >/dev/null 2>&1 || continue
+
          if ! [ "$HAVE_EXT_ARGV" ]; then
             config_opt="--$config_opt"
          fi

=== modified file 'bin/pt-mysql-summary'
--- bin/pt-mysql-summary	2014-09-25 13:48:22 +0000
+++ bin/pt-mysql-summary	2014-10-09 17:53:04 +0000
@@ -313,6 +313,8 @@
 
          [ "$config_opt" = "" ] && continue
 
+         echo "$config_opt" | grep -v 'version-check' >/dev/null 2>&1 || continue
+
          if ! [ "$HAVE_EXT_ARGV" ]; then
             config_opt="--$config_opt"
          fi

=== modified file 'bin/pt-pmp'
--- bin/pt-pmp	2014-09-25 13:48:22 +0000
+++ bin/pt-pmp	2014-10-09 17:53:04 +0000
@@ -129,9 +129,10 @@
 
 usage_or_errors() {
    local file="$1"
+   local version=""
 
    if [ "$OPT_VERSION" ]; then
-      local version=$(grep '^pt-[^ ]\+ [0-9]' "$file")
+      version=$(grep '^pt-[^ ]\+ [0-9]' "$file")
       echo "$version"
       return 1
    fi
@@ -353,6 +354,8 @@
 
          [ "$config_opt" = "" ] && continue
 
+         echo "$config_opt" | grep -v 'version-check' >/dev/null 2>&1 || continue
+
          if ! [ "$HAVE_EXT_ARGV" ]; then
             config_opt="--$config_opt"
          fi

=== modified file 'bin/pt-show-grants'
--- bin/pt-show-grants	2014-09-25 13:48:22 +0000
+++ bin/pt-show-grants	2014-10-09 17:53:04 +0000
@@ -903,6 +903,14 @@
          $parse  = 0;
          next LINE;
       }
+
+      if (  $parse 
+            && !$self->has('version-check') 
+            && $line =~ /version-check/ 
+      ) {
+         next LINE;
+      }
+
       if ( $parse
          && (my($opt, $arg) = $line =~ m/^\s*([^=\s]+?)(?:\s*=\s*(.*?)\s*)?$/)
       ) {

=== modified file 'bin/pt-sift'
--- bin/pt-sift	2014-09-25 13:48:22 +0000
+++ bin/pt-sift	2014-10-09 17:53:04 +0000
@@ -127,9 +127,10 @@
 
 usage_or_errors() {
    local file="$1"
+   local version=""
 
    if [ "$OPT_VERSION" ]; then
-      local version=$(grep '^pt-[^ ]\+ [0-9]' "$file")
+      version=$(grep '^pt-[^ ]\+ [0-9]' "$file")
       echo "$version"
       return 1
    fi
@@ -351,6 +352,8 @@
 
          [ "$config_opt" = "" ] && continue
 
+         echo "$config_opt" | grep -v 'version-check' >/dev/null 2>&1 || continue
+
          if ! [ "$HAVE_EXT_ARGV" ]; then
             config_opt="--$config_opt"
          fi

=== modified file 'bin/pt-slave-find'
--- bin/pt-slave-find	2014-09-25 13:48:22 +0000
+++ bin/pt-slave-find	2014-10-09 17:53:04 +0000
@@ -911,6 +911,14 @@
          $parse  = 0;
          next LINE;
       }
+
+      if (  $parse 
+            && !$self->has('version-check') 
+            && $line =~ /version-check/ 
+      ) {
+         next LINE;
+      }
+
       if ( $parse
          && (my($opt, $arg) = $line =~ m/^\s*([^=\s]+?)(?:\s*=\s*(.*?)\s*)?$/)
       ) {

=== modified file 'bin/pt-stalk'
--- bin/pt-stalk	2014-09-25 13:48:22 +0000
+++ bin/pt-stalk	2014-10-09 17:53:04 +0000
@@ -140,9 +140,10 @@
 
 usage_or_errors() {
    local file="$1"
+   local version=""
 
    if [ "$OPT_VERSION" ]; then
-      local version=$(grep '^pt-[^ ]\+ [0-9]' "$file")
+      version=$(grep '^pt-[^ ]\+ [0-9]' "$file")
       echo "$version"
       return 1
    fi
@@ -364,6 +365,8 @@
 
          [ "$config_opt" = "" ] && continue
 
+         echo "$config_opt" | grep -v 'version-check' >/dev/null 2>&1 || continue
+
          if ! [ "$HAVE_EXT_ARGV" ]; then
             config_opt="--$config_opt"
          fi

=== modified file 'bin/pt-summary'
--- bin/pt-summary	2014-09-25 13:48:22 +0000
+++ bin/pt-summary	2014-10-09 17:53:04 +0000
@@ -320,6 +320,8 @@
 
          [ "$config_opt" = "" ] && continue
 
+         echo "$config_opt" | grep -v 'version-check' >/dev/null 2>&1 || continue
+
          if ! [ "$HAVE_EXT_ARGV" ]; then
             config_opt="--$config_opt"
          fi

=== modified file 'bin/pt-table-usage'
--- bin/pt-table-usage	2014-09-25 13:48:22 +0000
+++ bin/pt-table-usage	2014-10-09 17:53:04 +0000
@@ -1340,6 +1340,14 @@
          $parse  = 0;
          next LINE;
       }
+
+      if (  $parse 
+            && !$self->has('version-check') 
+            && $line =~ /version-check/ 
+      ) {
+         next LINE;
+      }
+
       if ( $parse
          && (my($opt, $arg) = $line =~ m/^\s*([^=\s]+?)(?:\s*=\s*(.*?)\s*)?$/)
       ) {

=== modified file 'bin/pt-visual-explain'
--- bin/pt-visual-explain	2014-09-25 13:48:22 +0000
+++ bin/pt-visual-explain	2014-10-09 17:53:04 +0000
@@ -1577,6 +1577,14 @@
          $parse  = 0;
          next LINE;
       }
+
+      if (  $parse 
+            && !$self->has('version-check') 
+            && $line =~ /version-check/ 
+      ) {
+         next LINE;
+      }
+
       if ( $parse
          && (my($opt, $arg) = $line =~ m/^\s*([^=\s]+?)(?:\s*=\s*(.*?)\s*)?$/)
       ) {

=== modified file 'lib/OptionParser.pm'
--- lib/OptionParser.pm	2014-05-24 21:36:33 +0000
+++ lib/OptionParser.pm	2014-10-09 17:53:04 +0000
@@ -1132,6 +1132,16 @@
          $parse  = 0;
          next LINE;
       }
+
+      # Silently ignore option [no]-version-check if it is unsupported and it comes from a config file
+      # TODO: Ideally , this should be generalized for all unsupported options that come from global files
+      if (  $parse 
+            && !$self->has('version-check') 
+            && $line =~ /version-check/ 
+      ) {
+         next LINE;
+      }
+
       if ( $parse
          && (my($opt, $arg) = $line =~ m/^\s*([^=\s]+?)(?:\s*=\s*(.*?)\s*)?$/)
       ) {

=== modified file 'lib/bash/parse_options.sh'
--- lib/bash/parse_options.sh	2014-07-10 20:15:13 +0000
+++ lib/bash/parse_options.sh	2014-10-09 17:53:04 +0000
@@ -347,6 +347,9 @@
          # Skip blank lines.
          [ "$config_opt" = "" ] && continue
 
+         # Skip global option [no]version-check which don't apply
+         echo "$config_opt" | grep -v 'version-check' >/dev/null 2>&1 || continue
+
          # Options in a config file are not prefixed with --,
          # but command line options are, so one or the other has
          # to add or remove the -- prefix.  We add it for config

=== modified file 't/lib/OptionParser.t'
--- t/lib/OptionParser.t	2014-05-24 21:36:33 +0000
+++ t/lib/OptionParser.t	2014-10-09 17:53:04 +0000
@@ -2141,6 +2141,29 @@
    'prompt_no_echo outputs prompt to STDERR'
 );
 
+# #############################################################################
+#  Issue 1361293: Global config file with no-version-check option makes tools
+#  that don't recognize the option fail.
+# #############################################################################
+diag(`echo "no-version-check" > ~/.OptionParser.t.conf`);
+$o = new OptionParser(
+   description  => 'OptionParser.t parses command line options.',
+   usage        => "$PROGRAM_NAME <options>"
+);
+$o->get_specs("$trunk/bin/pt-slave-find"),   # doesn't have version-check option
+$output = output(  
+         sub {
+            $o->get_opts();
+         },
+         stderr=>1
+      );
+unlike(
+   $output,
+   qr/Unknown option: no-version-check/,
+   'no-version-check ignored if unsupported and in config file. issue 1361293'
+);
+diag(`rm -rf ~/.OptionParser.t.conf`);
+
 
 
 # #############################################################################