Add --check-shebang to bisect-runner.pl, to avoid mistakes with #! lines
authorNicholas Clark <nick@ccl4.org>
Thu, 17 Nov 2011 16:59:22 +0000 (17:59 +0100)
committerNicholas Clark <nick@ccl4.org>
Thu, 17 Nov 2011 16:59:22 +0000 (17:59 +0100)
This allows bisect.pl to instruct bisect-runner.pl to check that the user's
testcase is not a script with a #! line which will run an installed perl.
This rapidly catches the potential error of specifying a test case that will
not actually be tested with the bisect-build perl. Without this, the user
may end up with an unexpected failure to bisect, and little clue as to what
the actual problem is.

Porting/bisect-runner.pl
Porting/bisect.pl

index 3b82ec1..5896d3a 100755 (executable)
@@ -62,8 +62,8 @@ unless(GetOptions(\%options,
                   'target=s', 'jobs|j=i', 'expect-pass=i',
                   'expect-fail' => sub { $options{'expect-pass'} = 0; },
                   'clean!', 'one-liner|e=s', 'match=s', 'force-manifest',
-                  'force-regen', 'test-build', 'check-args', 'A=s@', 'l', 'w',
-                  'usage|help|?',
+                  'force-regen', 'test-build', 'A=s@', 'l', 'w',
+                  'check-args', 'check-shebang!', 'usage|help|?',
                   'D=s@' => sub {
                       my (undef, $val) = @_;
                       if ($val =~ /\A([^=]+)=(.*)/s) {
@@ -87,6 +87,8 @@ pod2usage(exitval => 255, verbose => 1)
 pod2usage(exitval => 255, verbose => 1)
     if !$options{'one-liner'} && ($options{l} || $options{w});
 
+check_shebang($ARGV[0]) if $options{'check-shebang'} && @ARGV;
+
 exit 0 if $options{'check-args'};
 
 =head1 NAME
@@ -429,6 +431,20 @@ Validate the options and arguments, and exit silently if they are valid.
 
 =item *
 
+--check-shebang
+
+Validate that the test case isn't an executable file with a
+C<#!/usr/bin/perl> line (or similar). As F<bisect-runner.pl> does B<not>
+prepend C<./perl> to the test case, a I<#!> line specifying an external
+F<perl> binary will cause the test case to always run with I<that> F<perl>,
+not the F<perl> built by the bisect runner. Likely this is not what you
+wanted. If your test case is actually a wrapper script to run other
+commands, you should run it with an explicit interpreter, to be clear. For
+example, instead of C<../perl/Porting/bisect.pl ~/test/testcase.pl> you'd
+run C<../perl/Porting/bisect.pl /usr/bin/perl ~/test/testcase.pl>
+
+=item *
+
 --usage
 
 =item *
@@ -526,6 +542,36 @@ sub checkout_file {
         and die "Could not extract $file at revision $commit";
 }
 
+sub check_shebang {
+    my $file = shift;
+    return unless -e $file;
+    if (!-x $file) {
+        die "$file is not executable.
+system($file, ...) is always going to fail.
+
+Bailing out";
+    }
+    my $fh = open_or_die($file);
+    my $line = <$fh>;
+    return unless $line =~ m{\A#!(/\S+/perl\S*)\s};
+    die "$file will always be run by $1
+It won't be tested by the ./perl we build.
+If you intended to run it with that perl binary, please change your
+test case to
+
+    $1 @ARGV
+
+If you intended to test it with the ./perl we build, please change your
+test case to
+
+    ./perl -Ilib @ARGV
+
+[You may also need to add -- before ./perl to prevent that -Ilib as being
+parsed as an argument to bisect.pl]
+
+Bailing out";
+}
+
 sub clean {
     if ($options{clean}) {
         # Needed, because files that are build products in this checked out
index d7def6e..d28e1c5 100755 (executable)
@@ -34,7 +34,7 @@ die "Can't find bisect runner $runner" unless -f $runner;
       if defined $dev1 && $dev0 == $dev1 && $ino0 == $ino1;
 }
 
-system $^X, $runner, '--check-args', @ARGV and exit 255;
+system $^X, $runner, '--check-args', '--check-shebang', @ARGV and exit 255;
 
 # 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