From e295b7be5477f6206702c6c24d7f93e5abc7069c Mon Sep 17 00:00:00 2001
From: Nicholas Clark <nick@ccl4.org>
Date: Sun, 2 Oct 2011 11:33:44 +0200
Subject: [PATCH] Refactor bisect.pl to use Getopt::Long's pass_through option.

Add a --check-args option to bisect-runner.pl to validate the arguments that
bisect.pl doesn't know how to check (else the automatic start detection
will get very confused by invalid arguments). Switch bisect.pl to bundling,
which permits the "traditional" make argument form of -j4 to be used instead
of -j=4. As this actually makes -j=4 invalid syntax now (an error about the
unknown option "="), change the syntax messages.
---
 Porting/bisect-runner.pl |  8 ++++++--
 Porting/bisect.pl        | 49 ++++++++++++++----------------------------------
 2 files changed, 20 insertions(+), 37 deletions(-)

diff --git a/Porting/bisect-runner.pl b/Porting/bisect-runner.pl
index 1fa847a..38138a0 100755
--- a/Porting/bisect-runner.pl
+++ b/Porting/bisect-runner.pl
@@ -1,7 +1,7 @@
 #!/usr/bin/perl -w
 use strict;
 
-use Getopt::Long;
+use Getopt::Long qw(:config bundling);
 
 my @targets = qw(miniperl lib/Config.pm perl test_prep);
 
@@ -13,9 +13,10 @@ my $one_liner;
 my $match;
 my $force_manifest;
 my $test_build;
+my $check_args;
 
 sub usage {
-    die "$0: [--target=...] [-j=4] [--expect-pass=0|1] thing to test";
+    die "$0: [--target=...] [-j4] [--expect-pass=0|1] thing to test";
 }
 
 unless(GetOptions('target=s' => \$target,
@@ -27,10 +28,13 @@ unless(GetOptions('target=s' => \$target,
                   'match=s' => \$match,
                   'force-manifest' => \$force_manifest,
                   'test-build' => \$test_build,
+                  'check-args' => \$check_args,
 		 )) {
     usage();
 }
 
+exit 0 if $check_args;
+
 my $exe = $target eq 'perl' || $target eq 'test_prep' ? 'perl' : 'miniperl';
 my $expected = $target eq 'test_prep' ? 'perl' : $target;
 
diff --git a/Porting/bisect.pl b/Porting/bisect.pl
index b401cdc..611e34a 100755
--- a/Porting/bisect.pl
+++ b/Porting/bisect.pl
@@ -3,46 +3,29 @@ use strict;
 
 my $start_time = time;
 
-use Getopt::Long;
+# The default, auto_abbrev will treat -e as an abbreviation of --end
+# Which isn't what we want.
+use Getopt::Long qw(:config pass_through no_auto_abbrev);
 
 sub usage {
-    die "$0: [--start revlike] [--end revlike] [--target=...] [-j=4] [--expect-pass=0|1] thing to test";
+    die "$0: [--start revlike] [--end revlike] [--target=...] [-j4] [--expect-pass=0|1] thing to test";
 }
 
-my %options;
-unless(GetOptions(\%options,
-                  'start=s',
-                  'end=s',
-                  'target=s',
-                  'jobs|j=i',
-                  'expect-pass=i',
-                  'expect-fail',
-                  'one-liner|e=s',
-                  'match=s',
-                  'force-manifest',
-                  'test-build',
+my ($start, $end);
+unless(GetOptions('start=s' => \$start,
+                  'end=s' => \$end,
                  )) {
     usage();
 }
 
-foreach (qw(force-manifest test-build)) {
-    # This is a bodge. I can't see a clean way to pass through suitably exact
-    # strings the various arguments to bisect-runner.pl that are argument-less
-    # flags. It might be easier for this program not to use Getopt::Long, and
-    # instead just grep out --start and --end
-    undef $options{$_} if exists $options{$_};
-}
-
 # We try these in this order for the start revision if none is specified.
 my @stable = qw(perl-5.002 perl-5.003 perl-5.004 perl-5.005 perl-5.6.0
                 perl-5.8.0 v5.10.0 v5.12.0 v5.14.0);
-my $start = delete $options{start};
+
 if ($start) {
     system "git rev-parse $start >/dev/null" and die;
 }
-my $end = delete $options{end};
 $end = 'blead' unless defined $end;
-
 system "git rev-parse $end >/dev/null" and die;
 
 my $modified = () = `git ls-files --modified --deleted --others`;
@@ -52,27 +35,23 @@ die "This checkout is not clean - $modified modified or untracked file(s)"
 
 system "git bisect reset" and die;
 
-my @ARGS;
-foreach (sort keys %options) {
-    push @ARGS, defined $options{$_} ? "--$_=$options{$_}" : "--$_";
-}
-push @ARGS, @ARGV;
-
 my $runner = $0;
 $runner =~ s/bisect\.pl/bisect-runner.pl/;
 
 die "Can't find bisect runner $runner" unless -f $runner;
 
+system $^X, $runner, '--check-args', @ARGV and exit 255;
+
 # Sanity check the first and last revisions:
 if (defined $start) {
     system "git checkout $start" and die;
-    my $ret = system $^X, $runner, @ARGS;
+    my $ret = system $^X, $runner, @ARGV;
     die "Runner returned $ret, not 0 for start revision" if $ret;
 } else {
     # Try to find the earliest version for which the test works
     foreach my $try (@stable) {
         system "git checkout $try" and die;
-        my $ret = system $^X, $runner, @ARGS;
+        my $ret = system $^X, $runner, @ARGV;
         if (!$ret) {
             $start = $try;
             last;
@@ -82,7 +61,7 @@ if (defined $start) {
         unless defined $start;
 }
 system "git checkout $end" and die;
-my $ret = system $^X, $runner, @ARGS;
+my $ret = system $^X, $runner, @ARGV;
 die "Runner returned $ret for end revision" unless $ret;
 
 system "git bisect start" and die;
@@ -90,7 +69,7 @@ system "git bisect good $start" and die;
 system "git bisect bad $end" and die;
 
 # And now get git bisect to do the hard work:
-system 'git', 'bisect', 'run', $^X, $runner, @ARGS and die;
+system 'git', 'bisect', 'run', $^X, $runner, @ARGV and die;
 
 END {
     my $end_time = time;
-- 
2.7.4