Bisect.pl broken for 3 commits in blead #16521
Comments
From @dur-randirCreated by @dur-randirWhile trying to bisect a module failure for #133135 I've found that cc -c -DPERL_CORE -fwrapv -fno-strict-aliasing -pipe
I remember from it's docs that it should skip at least some kinds of Perl Info |
From @jkeenanOn Fri, 20 Apr 2018 21:36:22 GMT, randir wrote:
Can you provide the specific way in which you invoked bisect.pl? Thank you very much. -- |
|
The RT System itself - Status changed from 'new' to 'open' |
From @dur-randirOn Fri, 20 Apr 2018 15:51:28 -0700, jkeenan wrote:
perl Porting/bisect.pl --start v5.26.0 -j 20 --module Lexical::SealRequireHints |
From @khwilliamsonOn 04/20/2018 04:51 PM, James E Keenan via RT wrote:
I must be responsible for these, though I don't understand how it |
From @jkeenanOn Fri, 20 Apr 2018 21:36:22 GMT, randir wrote:
See attachment, which I also attached to 133135. I agree that the make-non-completion in the first two of the three commits poses a problem for bisection going forward. I'm not sure if we currently have any code in bisect.pl or bisect-runner.pl that can permit us to skip over these two commits. Thank you very much. |
From @jkeenan$ install_branch_for_testing 2d74dcf $ install_branch_for_testing 2cdbf8d $ install_branch_for_testing 394d2d3 |
From @khwilliamsonThe attached commit fixes the problem |
From @khwilliamson0001-PATCH-perl-133136-bisect-runner.patchFrom 99074c8d4a858f62bed3619b2bbf615b70a304e5 Mon Sep 17 00:00:00 2001
From: Karl Williamson <khw@cpan.org>
Date: Wed, 25 Apr 2018 16:28:37 -0600
Subject: [PATCH] PATCH: [perl #133136] bisect runner
bisect runner is supposed to keep going if the particular revision being
tested fails to compile. But it wasn't.
Nicholas graciously diagnosed the problem. When the enhancement for
testing when a module got broken, the code to do so was placed before
the check to see if the build for this revision crashed. It's simply a
matter of moving that module code to after that check.
---
Porting/bisect-runner.pl | 59 ++++++++++++++++++++++++------------------------
1 file changed, 29 insertions(+), 30 deletions(-)
diff --git a/Porting/bisect-runner.pl b/Porting/bisect-runner.pl
index 6f9fd9d653..00319211c9 100755
--- a/Porting/bisect-runner.pl
+++ b/Porting/bisect-runner.pl
@@ -1514,10 +1514,36 @@ if ($target ne 'miniperl') {
system "$options{make} $j $real_target </dev/null";
}
-# Testing a cpan module? See if it will install
-my $just_testing;
-if (my $mod_opt = $options{module} || $options{'with-module'}
+my $expected_file_found = $expected_file =~ /perl$/
+ ? -x $expected_file : -r $expected_file;
+
+if ($expected_file_found && $expected_file eq 't/perl') {
+ # Check that it isn't actually pointing to ../miniperl, which will happen
+ # if the sanity check ./miniperl -Ilib -MExporter -e '<?>' fails, and
+ # Makefile tries to run minitest.
+
+ # Of course, helpfully sometimes it's called ../perl, other times .././perl
+ # and who knows if that list is exhaustive...
+ my ($dev0, $ino0) = stat 't/perl';
+ my ($dev1, $ino1) = stat 'perl';
+ unless (defined $dev0 && defined $dev1 && $dev0 == $dev1 && $ino0 == $ino1) {
+ undef $expected_file_found;
+ my $link = readlink $expected_file;
+ warn "'t/perl' => '$link', not 'perl'";
+ die_255("Could not realink t/perl: $!") unless defined $link;
+ }
+}
+
+my $just_testing = 0;
+
+if ($options{'test-build'}) {
+ report_and_exit($expected_file_found, 'could build', 'could not build',
+ $real_target);
+} elsif (!$expected_file_found) {
+ skip("could not build $real_target");
+} elsif (my $mod_opt = $options{module} || $options{'with-module'}
|| ($just_testing++, $options{'test-module'})) {
+ # Testing a cpan module? See if it will install
# First we need to install this perl somewhere
system_or_die('./installperl');
@@ -1580,33 +1606,6 @@ if (my $mod_opt = $options{module} || $options{'with-module'}
}
}
-my $expected_file_found = $expected_file =~ /perl$/
- ? -x $expected_file : -r $expected_file;
-
-if ($expected_file_found && $expected_file eq 't/perl') {
- # Check that it isn't actually pointing to ../miniperl, which will happen
- # if the sanity check ./miniperl -Ilib -MExporter -e '<?>' fails, and
- # Makefile tries to run minitest.
-
- # Of course, helpfully sometimes it's called ../perl, other times .././perl
- # and who knows if that list is exhaustive...
- my ($dev0, $ino0) = stat 't/perl';
- my ($dev1, $ino1) = stat 'perl';
- unless (defined $dev0 && defined $dev1 && $dev0 == $dev1 && $ino0 == $ino1) {
- undef $expected_file_found;
- my $link = readlink $expected_file;
- warn "'t/perl' => '$link', not 'perl'";
- die_255("Could not realink t/perl: $!") unless defined $link;
- }
-}
-
-if ($options{'test-build'}) {
- report_and_exit($expected_file_found, 'could build', 'could not build',
- $real_target);
-} elsif (!$expected_file_found) {
- skip("could not build $real_target");
-}
-
match_and_exit($real_target, @ARGV) if $match;
if (defined $options{'one-liner'}) {
--
2.11.0
|
From @khwilliamsonOn 04/25/2018 04:41 PM, Karl Williamson via RT wrote:
I believe this patch should go into 5.28.0, and am putting this ticket 1) This changes only a Porting/ script so does not affect the perl 2) It fixes a bug that could otherwise create hassles in finding bugs |
From @xsawyerxOn 04/26/2018 03:09 AM, Karl Williamson wrote:
I agree to this.
This is the main reason I'm okay with it.
And this is why it's important. |
From @khwilliamsonFixed with commit |
|
@khwilliamson - Status changed from 'open' to 'pending release' |
From @khwilliamsonThe commit message title in It should have said 133135 -- |
From [Unknown Contact. See original ticket]The commit message title in It should have said 133135 -- |
From @khwilliamsonThank you for filing this report. You have helped make Perl better. With the release yesterday of Perl 5.28.0, this and 185 other issues have been Perl 5.28.0 may be downloaded via: If you find that the problem persists, feel free to reopen this ticket. |
|
@khwilliamson - Status changed from 'pending release' to 'resolved' |
Migrated from rt.perl.org#133136 (status was 'resolved')
Searchable as RT133136$
The text was updated successfully, but these errors were encountered: