From dc4c5064832f5539f82d0b30f6816a86990ec315 Mon Sep 17 00:00:00 2001 From: Frank Cizmich Date: Mon, 6 Oct 2014 19:04:49 -0200 Subject: [PATCH] many scripts failed when reading no-version-check from global config file - 1361293 --- bin/pt-align | 8 ++++++ bin/pt-fifo-split | 8 ++++++ bin/pt-fingerprint | 8 ++++++ bin/pt-ioprofile | 5 +++- bin/pt-mext | 52 ++++++++++++++++++++++----------------- bin/pt-mysql-summary | 2 ++ bin/pt-pmp | 5 +++- bin/pt-show-grants | 8 ++++++ bin/pt-sift | 5 +++- bin/pt-slave-find | 8 ++++++ bin/pt-stalk | 5 +++- bin/pt-summary | 2 ++ bin/pt-table-usage | 8 ++++++ bin/pt-visual-explain | 8 ++++++ lib/OptionParser.pm | 10 ++++++++ lib/bash/parse_options.sh | 3 +++ t/lib/OptionParser.t | 23 +++++++++++++++++ 17 files changed, 141 insertions(+), 27 deletions(-) diff --git a/bin/pt-align b/bin/pt-align index de20e4a7..51adef65 100755 --- a/bin/pt-align +++ b/bin/pt-align @@ -901,6 +901,14 @@ sub _read_config_file { $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*)?$/) ) { diff --git a/bin/pt-fifo-split b/bin/pt-fifo-split index 05db8e10..de48ce69 100755 --- a/bin/pt-fifo-split +++ b/bin/pt-fifo-split @@ -902,6 +902,14 @@ sub _read_config_file { $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*)?$/) ) { diff --git a/bin/pt-fingerprint b/bin/pt-fingerprint index ff48b202..c90c1ae1 100755 --- a/bin/pt-fingerprint +++ b/bin/pt-fingerprint @@ -903,6 +903,14 @@ sub _read_config_file { $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*)?$/) ) { diff --git a/bin/pt-ioprofile b/bin/pt-ioprofile index 80b91582..b68662f9 100755 --- a/bin/pt-ioprofile +++ b/bin/pt-ioprofile @@ -86,9 +86,10 @@ usage() { 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 @@ _parse_config_files() { [ "$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 diff --git a/bin/pt-mext b/bin/pt-mext index 86f3b7e8..7dba0f03 100755 --- a/bin/pt-mext +++ b/bin/pt-mext @@ -251,34 +251,38 @@ parse_options() { _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 { - chomp; - my ($attrib, $val) = split(/: /, $_); - print $opt_fh "$attrib:$val\n"; - } split(/; /, $para); - $para = <>; + 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 ($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 @@ _parse_config_files() { [ "$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 diff --git a/bin/pt-mysql-summary b/bin/pt-mysql-summary index a9aaf88a..0e62d74c 100755 --- a/bin/pt-mysql-summary +++ b/bin/pt-mysql-summary @@ -313,6 +313,8 @@ _parse_config_files() { [ "$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 diff --git a/bin/pt-pmp b/bin/pt-pmp index 2cfaebce..8540817a 100755 --- a/bin/pt-pmp +++ b/bin/pt-pmp @@ -129,9 +129,10 @@ usage() { 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 @@ _parse_config_files() { [ "$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 diff --git a/bin/pt-show-grants b/bin/pt-show-grants index 7d7589dc..a6d3cb97 100755 --- a/bin/pt-show-grants +++ b/bin/pt-show-grants @@ -903,6 +903,14 @@ sub _read_config_file { $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*)?$/) ) { diff --git a/bin/pt-sift b/bin/pt-sift index 227ad1fd..f3c5c500 100755 --- a/bin/pt-sift +++ b/bin/pt-sift @@ -127,9 +127,10 @@ usage() { 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 @@ _parse_config_files() { [ "$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 diff --git a/bin/pt-slave-find b/bin/pt-slave-find index 53c0fcd3..d03fce04 100755 --- a/bin/pt-slave-find +++ b/bin/pt-slave-find @@ -911,6 +911,14 @@ sub _read_config_file { $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*)?$/) ) { diff --git a/bin/pt-stalk b/bin/pt-stalk index 7a5f8eca..f75bd5bb 100755 --- a/bin/pt-stalk +++ b/bin/pt-stalk @@ -140,9 +140,10 @@ usage() { 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 @@ _parse_config_files() { [ "$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 diff --git a/bin/pt-summary b/bin/pt-summary index 41c1e42a..d7266b28 100755 --- a/bin/pt-summary +++ b/bin/pt-summary @@ -320,6 +320,8 @@ _parse_config_files() { [ "$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 diff --git a/bin/pt-table-usage b/bin/pt-table-usage index 7352894b..cfbd45d3 100755 --- a/bin/pt-table-usage +++ b/bin/pt-table-usage @@ -1340,6 +1340,14 @@ sub _read_config_file { $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*)?$/) ) { diff --git a/bin/pt-visual-explain b/bin/pt-visual-explain index aef873a8..7464187c 100755 --- a/bin/pt-visual-explain +++ b/bin/pt-visual-explain @@ -1577,6 +1577,14 @@ sub _read_config_file { $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*)?$/) ) { diff --git a/lib/OptionParser.pm b/lib/OptionParser.pm index e42fd74d..99a6fa8c 100644 --- a/lib/OptionParser.pm +++ b/lib/OptionParser.pm @@ -1132,6 +1132,16 @@ sub _read_config_file { $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*)?$/) ) { diff --git a/lib/bash/parse_options.sh b/lib/bash/parse_options.sh index 297b99d4..b07074ec 100644 --- a/lib/bash/parse_options.sh +++ b/lib/bash/parse_options.sh @@ -347,6 +347,9 @@ _parse_config_files() { # 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 diff --git a/t/lib/OptionParser.t b/t/lib/OptionParser.t index a85a13bf..a7718f4d 100644 --- a/t/lib/OptionParser.t +++ b/t/lib/OptionParser.t @@ -2141,6 +2141,29 @@ is ( '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 " +); +$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`); + # #############################################################################