Skip Menu |
Report information
Id: 132790
Status: open
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: andreas.koenig.7os6VVqR [at] franz.ak.mind.de
Cc:
AdminCc:

Operating System: (no value)
PatchStatus: (no value)
Severity: low
Type: unknown
Perl Version: (no value)
Fixed In: (no value)

Attachments
0001-TODO-Add-explanation-as-to-why-this-patch-works.patch



To: perlbug [...] perl.org
Subject: Test dist/Storable/t/blessed.t fails with threaded perls < 5.14 under not too old Test2
Date: Wed, 31 Jan 2018 05:19:47 +0100
From: Andreas Koenig <andreas.koenig.7os6VVqR [...] franz.ak.mind.de>
Download (untitled) / with headers
text/plain 1.3k
Three little birdies twittered the story to me, I lend them my voice to let you know about it. This is not about a bug in Storable. It's about a bug in old threaded perls and how this bug unfolded over time. The bug in perl itself was fixed in v5.19.2-116-g82b84d0. Test::More 1.001014 did not trigger the bug, it was only exposed within dist/Storable/t/blessed.t through the changes in Test2. But Test2 developed a workaround for the bug. Unfortunately that workaround could only be applied to perls >= 5.14 not older ones. The workaround is in EXODIST/Test2-0.000035-TRIAL.tar.gz. It can be enforced by setting the environment variable T2_CHECK_DEPTH=1. So older perls can run T2_CHECK_DEPTH=1 make test in order to pass all Storable tests. There is a contrived workaround for blessed.t itself. It avoids triggering the bug by changing compilation order. I didn't understand the birdies' explanation on the inner workings of this solution. Here it is: % diff -U1 t/blessed.t{~,} --- t/blessed.t~ 2018-01-30 20:44:59.935027614 +0000 +++ t/blessed.t 2018-01-30 20:51:01.133495863 +0000 @@ -18,4 +18,2 @@ -use Test::More; - use Storable qw(freeze thaw store retrieve); @@ -28,2 +26,4 @@ +use Test::More; + { If the patch would be applied, we could get rid of an inconvenience for everybody who taps into this test failure with old perls in the future. -- andreas
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.8k
On Wed, 31 Jan 2018 04:20:26 GMT, andreas.koenig.7os6VVqR@franz.ak.mind.de wrote: Show quoted text
> Three little birdies twittered the story to me, I lend them my voice > to let you know about it. > > This is not about a bug in Storable. It's about a bug in old threaded > perls and how this bug unfolded over time. > > The bug in perl itself was fixed in v5.19.2-116-g82b84d0. > > Test::More 1.001014 did not trigger the bug, it was only exposed > within dist/Storable/t/blessed.t through the changes in Test2. > > But Test2 developed a workaround for the bug. Unfortunately that > workaround could only be applied to perls >= 5.14 not older ones. The > workaround is in EXODIST/Test2-0.000035-TRIAL.tar.gz. It can be > enforced by setting the environment variable T2_CHECK_DEPTH=1. So > older perls can run > > T2_CHECK_DEPTH=1 make test > > in order to pass all Storable tests. > > There is a contrived workaround for blessed.t itself. It avoids > triggering the bug by changing compilation order. I didn't understand > the birdies' explanation on the inner workings of this solution. Here > it is: > > % diff -U1 t/blessed.t{~,} > --- t/blessed.t~ 2018-01-30 20:44:59.935027614 +0000 > +++ t/blessed.t 2018-01-30 20:51:01.133495863 +0000 > @@ -18,4 +18,2 @@ > > -use Test::More; > - > use Storable qw(freeze thaw store retrieve); > @@ -28,2 +26,4 @@ > > +use Test::More; > + > { > > If the patch would be applied, we could get rid of an inconvenience > for everybody who taps into this test failure with old perls in the > future. >
I'm having trouble following you and, consequently, don't know how to reproduce the bug or test for its elimination once a correction has been made. Are you saying that that if one combines a 7-year-old Perl with a contemporary Test-Simple distribution, you get a failure in a test for Storable in blead? Thank you very much. -- James E Keenan (jkeenan@cpan.org)
Date: Wed, 31 Jan 2018 18:12:17 +0100
From: Andreas Koenig <andreas.koenig.7os6VVqR [...] franz.ak.mind.de>
Subject: Re: [perl #132790] Test dist/Storable/t/blessed.t fails with threaded perls < 5.14 under not too old Test2
To: "James E Keenan via RT" <perlbug-followup [...] perl.org>
Download (untitled) / with headers
text/plain 3.2k
Show quoted text
>>>>> On Wed, 31 Jan 2018 05:45:16 -0800, "James E Keenan via RT" <perlbug-followup@perl.org> said:
Show quoted text
> On Wed, 31 Jan 2018 04:20:26 GMT, andreas.koenig.7os6VVqR@franz.ak.mind.de wrote:
>> Three little birdies twittered the story to me, I lend them my voice >> to let you know about it. >> >> This is not about a bug in Storable. It's about a bug in old threaded >> perls and how this bug unfolded over time. >> >> The bug in perl itself was fixed in v5.19.2-116-g82b84d0. >> >> Test::More 1.001014 did not trigger the bug, it was only exposed >> within dist/Storable/t/blessed.t through the changes in Test2. >> >> But Test2 developed a workaround for the bug. Unfortunately that >> workaround could only be applied to perls >= 5.14 not older ones. The >> workaround is in EXODIST/Test2-0.000035-TRIAL.tar.gz. It can be >> enforced by setting the environment variable T2_CHECK_DEPTH=1. So >> older perls can run >> >> T2_CHECK_DEPTH=1 make test >> >> in order to pass all Storable tests. >> >> There is a contrived workaround for blessed.t itself. It avoids >> triggering the bug by changing compilation order. I didn't understand >> the birdies' explanation on the inner workings of this solution. Here >> it is: >> >> % diff -U1 t/blessed.t{~,} >> --- t/blessed.t~ 2018-01-30 20:44:59.935027614 +0000 >> +++ t/blessed.t 2018-01-30 20:51:01.133495863 +0000 >> @@ -18,4 +18,2 @@ >> >> -use Test::More; >> - >> use Storable qw(freeze thaw store retrieve); >> @@ -28,2 +26,4 @@ >> >> +use Test::More; >> + >> { >> >> If the patch would be applied, we could get rid of an inconvenience >> for everybody who taps into this test failure with old perls in the >> future. >>
Show quoted text
> I'm having trouble following you and, consequently, don't know how to > reproduce the bug or test for its elimination once a correction has > been made.
Sorry, I'm not sure which information I left out. Maybe the fact that Storable is upstream perl? Version 2.51 is on CPAN and should ever somebody consider a new release, they would probably take the tests from some recent perl. This is the reason why the report went to perlbug. I'm pretty sure, no offense would be taken when you would not test the outcome. From the point of view of bleadperl, you would probably prefer just to ensure that the patch does not fail on bleadperl. Show quoted text
> Are you saying that that if one combines a 7-year-old Perl with a > contemporary Test-Simple distribution, you get a failure in a test for > Storable in blead?
Yes. perl 5.12.5 was released 2012-Nov-10, so is now 5 1/4 years old. Many older perls would behave the same. On first sight we only see in the matrix that old perls have more red color than newer perls. See http://matrix.cpantesters.org/?dist=Storable%202.51 Recipe once again: take old perl, e.g. 5.12.5, compile it with threads, combine it with any recent Test::More, e.g. 1.302120 and test Storable 2.51 with it. That will fail on unpatched blessed.t and pass with the patched one. Adding an example: take random pick from the matrix http://www.cpantesters.org/cpan/report/350affc0-6a71-11e6-856c-4bbe58b9f28c this is a perl 5.8.5 and archname=x86_64-linux-thread-multi and Test::More 1.302052 and blessed.t failed. Show quoted text
> Thank you very much.
Thank you, I hope this clarifies what this is about, -- andreas
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 3.7k
On Wed, 31 Jan 2018 17:12:57 GMT, andreas.koenig.7os6VVqR@franz.ak.mind.de wrote: Show quoted text
> >>>>> On Wed, 31 Jan 2018 05:45:16 -0800, "James E Keenan via RT" > >>>>> <perlbug-followup@perl.org> said:
>
> > On Wed, 31 Jan 2018 04:20:26 GMT, > > andreas.koenig.7os6VVqR@franz.ak.mind.de wrote:
> >> Three little birdies twittered the story to me, I lend them my voice > >> to let you know about it. > >> > >> This is not about a bug in Storable. It's about a bug in old > >> threaded > >> perls and how this bug unfolded over time. > >> > >> The bug in perl itself was fixed in v5.19.2-116-g82b84d0. > >> > >> Test::More 1.001014 did not trigger the bug, it was only exposed > >> within dist/Storable/t/blessed.t through the changes in Test2. > >> > >> But Test2 developed a workaround for the bug. Unfortunately that > >> workaround could only be applied to perls >= 5.14 not older ones. > >> The > >> workaround is in EXODIST/Test2-0.000035-TRIAL.tar.gz. It can be > >> enforced by setting the environment variable T2_CHECK_DEPTH=1. So > >> older perls can run > >> > >> T2_CHECK_DEPTH=1 make test > >> > >> in order to pass all Storable tests. > >> > >> There is a contrived workaround for blessed.t itself. It avoids > >> triggering the bug by changing compilation order. I didn't > >> understand > >> the birdies' explanation on the inner workings of this solution. > >> Here > >> it is: > >> > >> % diff -U1 t/blessed.t{~,} > >> --- t/blessed.t~ 2018-01-30 20:44:59.935027614 +0000 > >> +++ t/blessed.t 2018-01-30 20:51:01.133495863 +0000 > >> @@ -18,4 +18,2 @@ > >> > >> -use Test::More; > >> - > >> use Storable qw(freeze thaw store retrieve); > >> @@ -28,2 +26,4 @@ > >> > >> +use Test::More; > >> + > >> { > >> > >> If the patch would be applied, we could get rid of an inconvenience > >> for everybody who taps into this test failure with old perls in the > >> future. > >>
>
> > I'm having trouble following you and, consequently, don't know how to > > reproduce the bug or test for its elimination once a correction has > > been made.
> > Sorry, I'm not sure which information I left out. Maybe the fact that > Storable is upstream perl? Version 2.51 is on CPAN and should ever > somebody consider a new release, they would probably take the tests > from > some recent perl. This is the reason why the report went to perlbug. > > I'm pretty sure, no offense would be taken when you would not test the > outcome. From the point of view of bleadperl, you would probably > prefer > just to ensure that the patch does not fail on bleadperl. >
> > Are you saying that that if one combines a 7-year-old Perl with a > > contemporary Test-Simple distribution, you get a failure in a test > > for > > Storable in blead?
> > Yes. > > perl 5.12.5 was released 2012-Nov-10, so is now 5 1/4 years old. Many > older perls would behave the same. On first sight we only see in the > matrix that old perls have more red color than newer perls. See > http://matrix.cpantesters.org/?dist=Storable%202.51 > > Recipe once again: take old perl, e.g. 5.12.5, compile it with > threads, > combine it with any recent Test::More, e.g. 1.302120 and test Storable > 2.51 with it. That will fail on unpatched blessed.t and pass with the > patched one. > > Adding an example: take random pick from the matrix > http://www.cpantesters.org/cpan/report/350affc0-6a71-11e6-856c- > 4bbe58b9f28c > this is a perl 5.8.5 and archname=x86_64-linux-thread-multi and > Test::More > 1.302052 and blessed.t failed. >
> > Thank you very much.
> > Thank you, I hope this clarifies what this is about,
Please smoke branch smoke-me/jkeenan/132790-storable-blessed-t. Since, as noted in the commit message, I don't know why this fix "works", we'll need further discussion and a better commit message before merging to blead. -- James E Keenan (jkeenan@cpan.org)
Subject: 0001-TODO-Add-explanation-as-to-why-this-patch-works.patch
From 17e66fadd7c4d573e1b4cf6e1df5a4e5a4fc60c7 Mon Sep 17 00:00:00 2001 From: James E Keenan <jkeenan@cpan.org> Date: Wed, 31 Jan 2018 13:25:22 -0500 Subject: [PATCH] TODO: Add explanation as to why this patch "works". In RT #132790, Andreas reported that 3 unit tests in t/blessed.t were failing under these conditions: * pre-perl-5.14 perl * modern Test-Simple distribution * modern Storable from CPAN (which is behind blead) For some reason TK moving the import of Test::More below %::weird_refs clears up the test failures. This appears to have no impact on blead. The benefit of this will, of course, only become evident when a new version of Storable is released to CPAN. --- dist/Storable/t/blessed.t | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dist/Storable/t/blessed.t b/dist/Storable/t/blessed.t index 352eb5f..667746a 100644 --- a/dist/Storable/t/blessed.t +++ b/dist/Storable/t/blessed.t @@ -26,25 +26,25 @@ sub BEGIN { if ($ENV{PERL_CORE} and $Config{'extensions'} !~ /\bStorable\b/) { print "1..0 # Skip: Storable was not built\n"; exit 0; } } -use Test::More; - use Storable qw(freeze thaw store retrieve); { %::weird_refs = ( REF => \(my $aref = []), VSTRING => \(my $vstring = v1.2.3), 'long VSTRING' => \(my $vstring = eval "v" . 0 x 300), LVALUE => \(my $substr = substr((my $str = "foo"), 0, 3)), ); } +use Test::More; + my $test = 12; my $tests = $test + 23 + (2 * 6 * keys %::immortals) + (3 * keys %::weird_refs); plan(tests => $tests); package SHORT_NAME; -- 2.7.4


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org