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

Owner: Nobody
Requestors: jkeenan [at] pobox.com
Cc:
AdminCc:

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



To: perlbug [...] perl.org
CC: Carlos Guevara <carlos [...] carlosguevara.com>
Date: Sat, 4 Nov 2017 10:02:53 -0400
From: James E Keenan <jkeenan [...] pobox.com>
Subject: BBC: Commit 7a831b72 breaks UNIVERAL::isa tests
Download (untitled) / with headers
text/plain 3.7k
As first reported to me by Carlos Guevara: ##### It looks like 7a831b721c469aeccfe1110a2d177dd115d5998d broke UNIVERSAL::isa. No report available, since the install just hangs at: t/00-report-prereqs.t .. ok t/basic.t .............. 1/52 when installing Term::ProgressBar::Simple via cpan configured with: recommends_policy [0] suggests_policy [0] ##### Here is the output from attempting to install UNIVERSAL-isa via 'cpanm' at 83a320f: ##### [snip prerequisites, which installed fine] Searching UNIVERSAL::isa (1.20110614) on cpanmetadb ... --> Working on UNIVERSAL::isa Fetching http://www.cpan.org/authors/id/E/ET/ETHER/UNIVERSAL-isa-1.20171012.tar.gz -> OK Unpacking UNIVERSAL-isa-1.20171012.tar.gz Entering UNIVERSAL-isa-1.20171012 Checking configure dependencies from META.json Checking if you have ExtUtils::MakeMaker 6.58 ... Yes (7.30) Configuring UNIVERSAL-isa-1.20171012 Running Makefile.PL Checking if your kit is complete... Looks good Generating a Unix-style Makefile Writing Makefile for UNIVERSAL::isa Writing MYMETA.yml and MYMETA.json -> OK Checking dependencies from MYMETA.json ... Checking if you have warnings::register 0 ... Yes (1.04) Checking if you have strict 0 ... Yes (1.11) Checking if you have ExtUtils::MakeMaker 0 ... Yes (7.30) Checking if you have Test::More 0 ... Yes (1.302103) Checking if you have warnings 0 ... Yes (1.37) Checking if you have Scalar::Util 0 ... Yes (1.49) Checking if you have overload 0 ... Yes (1.28) Checking if you have File::Spec 0 ... Yes (3.68) Building and testing UNIVERSAL-isa-1.20171012 cp lib/UNIVERSAL/isa.pm blib/lib/UNIVERSAL/isa.pm PERL_DL_NONLAZY=1 "/home/jkeenan/testing/blead/bin/perl" "-MExtUtils::Command::MM" "-MTest::Harness" "-e" "undef *Test::Harness::Switches; test_harness(0, 'blib/lib', 'blib/arch')" t/*.t # # Versions for all modules listed in MYMETA.json (including optional ones): [snip: all Requires, Recommends look fine] # === Other Modules === # # Module Have # ------------- ------- # JSON::PP 2.94 # Pod::Coverage missing # Sub::Name missing # YAML missing # autodie 2.29 # t/00-report-prereqs.t .. ok [When you observe an indefinite hang and hit Ctrl-C, you get ...] Makefile:861: recipe for target 'test_dynamic' failed make: *** [test_dynamic] Interrupt ##### t/basic.t hangs going into test 41: ##### $ bleadprove t/basic.t t/basic.t .. 1..52 ok 1 - undef isa nothing ok 2 - not warning by default ok 3 - [] is an array ref [snip] ok 39 - undef isa nothing ok 40 - ... warning in verbose mode [hangs indefinitely going into test 41] ##### Here's what the next tests scheduled look like: ##### ok( isa( {}, 'HASH' ), 'hash reference isa HASH' ); like( $warning, qr/Called.+as a function.+reftyp.+basic.t/s, '... warning in verbose mode' ); ##### However, experimentation suggests that the indefinite hang will occur for any of tests 41-52. Separately, one TODO test in t/warnings.t may need modification: ##### $ bleadprove t/warnings.t t/warnings.t .. 1..12 ok 1 - U::i should warn by default when redirecting to overridden method [snip] ok 11 - ... even when it would return false not ok 12 - No warnings when called properly, as a method # TODO no apparent way of distinguishing between being called as a function and a method # Failed (TODO) test 'No warnings when called properly, as a method' # at t/warnings.t line 85. # got: 'Called UNIVERSAL::isa() as a function, not a method at t/warnings.t line 84. # ' # expected: '' ok All tests successful. Files=1, Tests=12, 0 wallclock secs ( 0.01 usr 0.00 sys + 0.03 cusr 0.00 csys = 0.04 CPU) Result: PASS ##### This may be related to certain bug tickets in UNIVERSAL::isa's queue at https://rt.cpan.org/Dist/Display.html?Name=UNIVERSAL-isa. Thank you very much. Jim Keenan
Download perl_V.83a320f.txt
text/plain 3.2k

Message body is not shown because sender requested not to inline it.

To: perl5-porters [...] perl.org
From: Dave Mitchell <davem [...] iabyn.com>
Subject: Re: [perl #132395] BBC: Commit 7a831b72 breaks UNIVERAL::isa tests
Date: Sat, 4 Nov 2017 16:37:53 +0000
Download (untitled) / with headers
text/plain 674b
On Sat, Nov 04, 2017 at 07:03:16AM -0700, James E Keenan wrote: Show quoted text
> It looks like 7a831b721c469aeccfe1110a2d177dd115d5998d broke UNIVERSAL::isa.
which is: commit 7a831b721c469aeccfe1110a2d177dd115d5998d Author: Nicolas R <atoomic@cpan.org> AuthorDate: Tue Aug 22 13:26:15 2017 -0500 Commit: Nicolas R <atoomic@cpan.org> CommitDate: Fri Nov 3 11:26:19 2017 -0500 Speed up Carp.pm when backtrace arguments are references Avoid downgrading the string when not required. Author: J. Nick Koston <nick@cpanel.net> References: CPANEL-15140 -- But Pity stayed his hand. "It's a pity I've run out of bullets", he thought. -- "Bored of the Rings"
To: perl5-porters [...] perl.org
Date: Sat, 4 Nov 2017 18:54:27 +0000
From: Zefram <zefram [...] fysh.org>
Subject: Re: [perl #132395] BBC: Commit 7a831b72 breaks UNIVERAL::isa tests
Download (untitled) / with headers
text/plain 402b
Dave Mitchell wrote: Show quoted text
>commit 7a831b721c469aeccfe1110a2d177dd115d5998d
... Show quoted text
> Speed up Carp.pm when backtrace arguments are references
That commit should never have been applied, and should be reverted forthwith. We already established that it's buggy and mostly pointless. The only part of it that has any value is the UNIVERSAL::isa() check, which can be applied independent of the rest. -zefram
RT-Send-CC: perl5-porters [...] perl.org, nick [...] cpanel.net, atoomic [...] cpan.org
Download (untitled) / with headers
text/plain 208b
Show quoted text
> Author: J. Nick Koston <nick@cpanel.net> > References: CPANEL-15140
What is this bit referring to? Is this patch intended to fix an issue that cPanel is having? Can someone report on what that issue is?
To: perl5-porters [...] perl.org
CC: nick [...] cpanel.net, atoomic [...] cpan.org
Date: Sat, 4 Nov 2017 20:48:54 +0000
Subject: Re: [perl #132395] BBC: Commit 7a831b72 breaks UNIVERAL::isa tests
From: Zefram <zefram [...] fysh.org>
Download (untitled) / with headers
text/plain 274b
Karen Etheridge via RT wrote: Show quoted text
>Is this patch intended to fix an issue that cPanel is having?
The original version of the patch came with a Changes entry, which commented In testing this decreased the Carp backtrace time by about 35% in production code. -zefram
To: perl5-porters [...] perl.org
Date: Sat, 4 Nov 2017 22:11:47 +0000
From: Zefram <zefram [...] fysh.org>
Subject: Re: [perl #132395] BBC: Commit 7a831b72 breaks UNIVERAL::isa tests
Download (untitled) / with headers
text/plain 332b
I wrote: Show quoted text
>That commit should never have been applied, and should be reverted >forthwith.
Now reverted as commit 0ebeacdeceb9d5e4b7dbd3ef4eb6834aac4a0435. However, the part of the commit that seems potentially useful would presumably still break UNIVERSAL::isa in the same way. So this BBC still needs to be looked into. -zefram
Date: Sat, 4 Nov 2017 15:35:34 -1000
To: Zefram <zefram [...] fysh.org>
CC: perl5-porters [...] perl.org, atoomic [...] cpan.org
From: "J. Nick Koston" <nick [...] cpanel.net>
Subject: Re: [perl #132395] BBC: Commit 7a831b72 breaks UNIVERAL::isa tests
Download (untitled) / with headers
text/plain 956b
It looks like  lib/UNIVERSAL/isa.pm:_report_warning is handling the same problem for Test::Builder and Test::Stream with an explicit check:

 96         # check calling sub
 97         return if (( caller(3) )[3] || '') =~ /::isa$/;
 98         # check calling package - exempt Test::Builder??
 99         return if (( caller(3) )[0] || '') =~ /^Test::Builder/;
100         return if (( caller(2) )[0] || '') =~ /^Test::Stream/;

Adding this for Carp allows it to pass:

101         return if (( caller(2) )[0] || '') =~ /^Carp/;

However there is probably a better way to handle this.

-Nick

Show quoted text
On Nov 4, 2017, at 10:48 AM, Zefram <zefram@fysh.org> wrote:

Karen Etheridge via RT wrote:
Is this patch intended to fix an issue that cPanel is having?

The original version of the patch came with a Changes entry, which
commented

    In testing this decreased the Carp backtrace time by about
    35% in production code.

-zefram


Download smime.p7s
application/pkcs7-signature 3.4k

Message body not shown because it is not plain text.

Date: Sun, 5 Nov 2017 10:04:49 +0100
Subject: Re: [perl #132395] BBC: Commit 7a831b72 breaks UNIVERAL::isa tests
To: "J. Nick Koston" <nick [...] cpanel.net>, Zefram <zefram [...] fysh.org>
From: Sawyer X <xsawyerx [...] gmail.com>
CC: perl5-porters [...] perl.org, atoomic [...] cpan.org
Download (untitled) / with headers
text/plain 1.3k
I think some of it can be applied since they are less pervasive, but likely to have very minimal gains. The biggest part is the optional downgrade. Now that we have a patch that makes it work, is a better way to accomplish this? Meanwhile, I think considering the breakage, it makes sense to revert. On 11/05/2017 02:35 AM, J. Nick Koston wrote: Show quoted text
> It looks like  lib/UNIVERSAL/isa.pm:_report_warning is handling the > same problem for Test::Builder and Test::Stream with an explicit check: > >  96         # check calling sub >  97         returnif(( caller(3) )[3] || '') =~ /::isa$/; >  98         # check calling package - exempt Test::Builder?? >  99         returnif(( caller(3) )[0] || '') =~ /^Test::Builder/; > 100         returnif(( caller(2) )[0] || '') =~ /^Test::Stream/; > > Adding this for Carp allows it to pass: > > 101         returnif(( caller(2) )[0] || '') =~ /^Carp/; > > However there is probably a better way to handle this. > > -Nick >
>> On Nov 4, 2017, at 10:48 AM, Zefram <zefram@fysh.org >> <mailto:zefram@fysh.org>> wrote: >> >> Karen Etheridge via RT wrote:
>>> Is this patch intended to fix an issue that cPanel is having?
>> >> The original version of the patch came with a Changes entry, which >> commented >> >>     In testing this decreased the Carp backtrace time by about >>     35% in production code. >> >> -zefram >>
>


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