Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Blead Breaks CPAN: LEMBARK/Object-Trampoline-1.42.tar.gz #16394

Closed
p5pRT opened this issue Jan 30, 2018 · 28 comments
Closed

Blead Breaks CPAN: LEMBARK/Object-Trampoline-1.42.tar.gz #16394

p5pRT opened this issue Jan 30, 2018 · 28 comments

Comments

@p5pRT
Copy link

@p5pRT p5pRT commented Jan 30, 2018

Migrated from rt.perl.org#132788 (status was 'resolved')

Searchable as RT132788$

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jan 30, 2018

From @eserte

This is a bug report for perl from slaven@​rezic.de,
generated with the help of perlbug 1.41 running under perl 5.27.8.


The test t/03-Universal-Override.t in Object-Trampoline-1.42
fails with 5.27.7 and 5.27.8, and was OK until 5.27.6​:

# Failed test 'Object is 'Object​::Trampoline​::Bounce' (Foo​::Bar)'
# at t/03-Universal-Override.t line 21.
# Looks like you failed 1 test of 5.
t/03-Universal-Override.t ..
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/5 subtests



Flags​:
  category=core
  severity=low


Site configuration information for perl 5.27.8​:

Configured by eserte at Sat Jan 20 09​:22​:10 CET 2018.

Summary of my perl5 (revision 5 version 27 subversion 8) configuration​:
 
  Platform​:
  osname=linux
  osvers=3.16.0-4-amd64
  archname=x86_64-linux
  uname='linux cabulja 3.16.0-4-amd64 #1 smp debian 3.16.51-3 (2017-12-13) x86_64 gnulinux '
  config_args='-ds -e -Dprefix=/opt/perl-5.27.8 -Dusedevel -Dusemallocwrap=no -Dcf_email=srezic@​cpan.org'
  hint=recommended
  useposix=true
  d_sigaction=define
  useithreads=undef
  usemultiplicity=undef
  use64bitint=define
  use64bitall=define
  uselongdouble=undef
  usemymalloc=n
  default_inc_excludes_dot=define
  bincompat5005=undef
  Compiler​:
  cc='cc'
  ccflags ='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2'
  optimize='-O2'
  cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
  ccversion=''
  gccversion='4.9.2'
  gccosandvers=''
  intsize=4
  longsize=8
  ptrsize=8
  doublesize=8
  byteorder=12345678
  doublekind=3
  d_longlong=define
  longlongsize=8
  d_longdbl=define
  longdblsize=16
  longdblkind=3
  ivtype='long'
  ivsize=8
  nvtype='double'
  nvsize=8
  Off_t='off_t'
  lseeksize=8
  alignbytes=8
  prototype=define
  Linker and Libraries​:
  ld='cc'
  ldflags =' -fstack-protector-strong -L/usr/local/lib'
  libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/4.9/include-fixed /usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib
  libs=-lpthread -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat
  perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
  libc=libc-2.19.so
  so=so
  useshrplib=false
  libperl=libperl.a
  gnulibc_version='2.19'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs
  dlext=so
  d_dlsymun=undef
  ccdlflags='-Wl,-E'
  cccdlflags='-fPIC'
  lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector-strong'


@​INC for perl 5.27.8​:
  /opt/perl-5.27.8/lib/site_perl/5.27.8/x86_64-linux
  /opt/perl-5.27.8/lib/site_perl/5.27.8
  /opt/perl-5.27.8/lib/5.27.8/x86_64-linux
  /opt/perl-5.27.8/lib/5.27.8


Environment for perl 5.27.8​:
  HOME=/home/eserte
  LANG=en_US.UTF-8
  LANGUAGE (unset)
  LD_LIBRARY_PATH (unset)
  LOGDIR (unset)
  PATH=/usr/local/bin​:/usr/bin​:/bin​:/usr/local/sbin​:/usr/sbin​:/sbin​:/home/eserte/bin/linux-gnu​:/home/eserte/bin/sh​:/home/eserte/bin​:/home/eserte/bin/pistachio-perl/bin​:/usr/games​:/home/eserte/devel
  PERLDOC=-MPod​::Perldoc​::ToTextOverstrike
  PERL_BADLANG (unset)
  SHELL=/bin/zsh

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jan 30, 2018

From zefram@fysh.org

slaven@​rezic.de wrote​:

The test t/03-Universal-Override.t in Object-Trampoline-1.42
fails with 5.27.7 and 5.27.8, and was OK until 5.27.6​:

Bisects to commit 915a681 "Carp​: optimize
format_arg when arguments contain many references". The issue is that
the new Carp code in that commit refers to $UNIVERSAL​::isa​::VERSION, and
so vivifies $UNIVERSAL​::{"isa​::"}. The test iterates over %UNIVERSAL​::,
using each key found as a method name, and isn't prepared for there to
be a key that doesn't syntactically behave as a method name.

The test is erroneous in using all keys it finds as method names.
It should skip ones that aren't syntactically OK.

The edit to Carp is also arguably faulty. In other areas Carp goes
to significant effort to avoid vivifying stash entries that it doesn't
intend to. It should probably apply similar logic in this search for
&UNIVERSAL​::isa.

-zefram

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jan 30, 2018

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jan 31, 2018

From @jkeenan

On Tue, 30 Jan 2018 07​:13​:47 GMT, zefram@​fysh.org wrote​:

slaven@​rezic.de wrote​:

The test t/03-Universal-Override.t in Object-Trampoline-1.42
fails with 5.27.7 and 5.27.8, and was OK until 5.27.6​:

Bisects to commit 915a681 "Carp​: optimize
format_arg when arguments contain many references". The issue is that
the new Carp code in that commit refers to $UNIVERSAL​::isa​::VERSION, and
so vivifies $UNIVERSAL​::{"isa​::"}. The test iterates over %UNIVERSAL​::,
using each key found as a method name, and isn't prepared for there to
be a key that doesn't syntactically behave as a method name.

The test is erroneous in using all keys it finds as method names.
It should skip ones that aren't syntactically OK.

The edit to Carp is also arguably faulty. In other areas Carp goes
to significant effort to avoid vivifying stash entries that it doesn't
intend to. It should probably apply similar logic in this search for
&UNIVERSAL​::isa.

-zefram

Are you saying we should apply something like the patch attached?

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jan 31, 2018

From @jkeenan

0001-Ensure-we-don-t-unnecessarily-vivify-UNIVERSAL-isa.patch
From 7d30fb46959b73e8ed43fc7ee0eec5775bf8c816 Mon Sep 17 00:00:00 2001
From: James E Keenan <jkeenan@cpan.org>
Date: Wed, 31 Jan 2018 09:52:57 -0500
Subject: [PATCH] Ensure we don't unnecessarily vivify $UNIVERSAL::{"isa::"}.

In response to issue raised in RT #132788
---
 dist/Carp/lib/Carp.pm | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/dist/Carp/lib/Carp.pm b/dist/Carp/lib/Carp.pm
index 65d22b1..f31b5be 100644
--- a/dist/Carp/lib/Carp.pm
+++ b/dist/Carp/lib/Carp.pm
@@ -116,7 +116,7 @@ BEGIN {
 	;
 }
 
-our $VERSION = '1.45';
+our $VERSION = '1.46';
 $VERSION =~ tr/_//d;
 
 our $MaxEvalLen = 0;
@@ -286,7 +286,9 @@ sub format_arg {
 
         # lazy check if the CPAN module UNIVERSAL::isa is used or not
         #   if we use a rogue version of UNIVERSAL this would lead to infinite loop
-        my $isa = $UNIVERSAL::isa::VERSION ? sub { 1 } : \&UNIVERSAL::isa;
+        my $isa = (exists $UNIVERSAL::{"isa::"} and $UNIVERSAL::isa::VERSION)
+            ? sub { 1 }
+            : \&UNIVERSAL::isa;
 
          # legitimate, let's not leak it.
         if (!$in_recurse && $isa->( $arg, 'UNIVERSAL' ) &&
-- 
2.7.4

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jan 31, 2018

From @jkeenan

On Wed, 31 Jan 2018 14​:57​:32 GMT, jkeenan wrote​:

On Tue, 30 Jan 2018 07​:13​:47 GMT, zefram@​fysh.org wrote​:

slaven@​rezic.de wrote​:

The test t/03-Universal-Override.t in Object-Trampoline-1.42
fails with 5.27.7 and 5.27.8, and was OK until 5.27.6​:

Bisects to commit 915a681 "Carp​: optimize
format_arg when arguments contain many references". The issue is that
the new Carp code in that commit refers to $UNIVERSAL​::isa​::VERSION, and
so vivifies $UNIVERSAL​::{"isa​::"}. The test iterates over %UNIVERSAL​::,
using each key found as a method name, and isn't prepared for there to
be a key that doesn't syntactically behave as a method name.

The test is erroneous in using all keys it finds as method names.
It should skip ones that aren't syntactically OK.

The edit to Carp is also arguably faulty. In other areas Carp goes
to significant effort to avoid vivifying stash entries that it doesn't
intend to. It should probably apply similar logic in this search for
&UNIVERSAL​::isa.

-zefram

Are you saying we should apply something like the patch attached?

Thank you very much.

In the branch, to avert a test failure I have also incremented $Carp​::Heavy​::VERSION.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Feb 1, 2018

From zefram@fysh.org

James E Keenan via RT wrote​:

Are you saying we should apply something like the patch attached?
...
+ my $isa = (exists $UNIVERSAL​::{"isa​::"} and $UNIVERSAL​::isa​::VERSION)

That doesn't help​: the reference to $UNIVERSAL​::isa​::VERSION still
vivifies $UNIVERSAL​::{"isa​::"} at compile time. It takes more to
avoid that. Look at the other vivification-avoiding code in Carp,
and the tests thereof.

-zefram

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Feb 15, 2018

From zefram@fysh.org

I wrote​:

The test is erroneous in using all keys it finds as method names.
It should skip ones that aren't syntactically OK.

Reported as [rt.cpan.org #124441].

The edit to Carp is also arguably faulty.

Fixed in commit 682f3ac.

-zefram

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Feb 17, 2018

From @demerphq

I accidentally sent this response only to Zefram.... Resend
---------- Forwarded message ----------
From​: "demerphq" <demerphq@​gmail.com>
Date​: 16 Feb 2018 15​:48
Subject​: Re​: [perl #132788] Blead Breaks CPAN​:
LEMBARK/Object-Trampoline-1.42.tar.gz
To​: "Zefram" <zefram@​fysh.org>
Cc​:

On 30 Jan 2018 15​:13, "Zefram" <zefram@​fysh.org> wrote​:

slaven@​rezic.de wrote​:

The test t/03-Universal-Override.t in Object-Trampoline-1.42
fails with 5.27.7 and 5.27.8, and was OK until 5.27.6​:

Bisects to commit 915a681 "Carp​: optimize
format_arg when arguments contain many references". The issue is that
the new Carp code in that commit refers to $UNIVERSAL​::isa​::VERSION, and
so vivifies $UNIVERSAL​::{"isa​::"}. The test iterates over %UNIVERSAL​::,
using each key found as a method name, and isn't prepared for there to
be a key that doesn't syntactically behave as a method name.

The test is erroneous in using all keys it finds as method names.
It should skip ones that aren't syntactically OK.

The edit to Carp is also arguably faulty. In other areas Carp goes
to significant effort to avoid vivifying stash entries that it doesn't
intend to. It should probably apply similar logic in this search for
&UNIVERSAL​::isa.

I don't think such restrictions are tenable. They mean that in some cases
you can use Carp to trigger c stack overflow and thus a segv. I will soon
be pushing patches that fix this and they will almost certainly result in
loading modules and modifying stash entries.

It is unfortunate that so many critical functions and checks in Perl
require namespace mutations, but that's how things work, and imo not
touching other namespaces has a *much* lower priority than Carp working
correctly and not segfaulting.

Yves

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Feb 17, 2018

From @cpansprout

On Fri, 16 Feb 2018 18​:53​:47 -0800, demerphq wrote​:

It is unfortunate that so many critical functions and checks in Perl
require namespace mutations, but that's how things work, and imo not
touching other namespaces has a *much* lower priority than Carp working
correctly and not segfaulting.

Why cannot we have both? You are being vague.

--

Father Chrysostomos

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Feb 17, 2018

From @demerphq

On 17 Feb 2018 21​:36, "Father Chrysostomos via RT" <
perlbug-followup@​perl.org> wrote​:

On Fri, 16 Feb 2018 18​:53​:47 -0800, demerphq wrote​:

It is unfortunate that so many critical functions and checks in Perl
require namespace mutations, but that's how things work, and imo not
touching other namespaces has a *much* lower priority than Carp working
correctly and not segfaulting.

Why cannot we have both? You are being vague

Carp incorrectly assumes that the only way you can have an overloaded
object is if you have loaded overload.pm.

Which means that this segfaults due to stack overflow​:

perl -MCarp -E 'my $p = "OverloadedInXS"; *{$p."​::(("} = sub{};
*{$p.q!​::(""!} = sub { Carp​::cluck "<My Stringify>" }; sub {
Carp​::cluck("") }->(bless {}, $p);'

See #132828.

To fix that without loading overload.pm I would have to more or less
extract a nice chunk of the internals of overload.pm into Carp.

But I am curious why you feel I need to defend making Carp.pm use a module.
I feel the onus should be the other way around, it should be on you, or any
others who feel Carp should not load modules to explain why Carp is such a
special flower.

We have no less than 4 different subs/mechanisms that are loaded like this​:

$ grep _fetch_sub dist/Carp/lib/Carp.pm
sub _fetch_sub { # fetch sub without autovivifying
  if(defined(my $sub = _fetch_sub utf8 => 'is_utf8')) {
  if(defined(my $sub = _fetch_sub utf8 => 'downgrade')) {
  (_fetch_sub B => 'svref_2object' or return '')
  my $sub = _fetch_sub(overload => 'StrVal');

Some of them appear to be related to back-compat, but some of them are
just egregious reimplementation of another modules functionality for the
purpose of avoiding that module​:

# The downgrade() function defined here is to be used for attempts to
# downgrade where it is acceptable to fail. It must be called with a
# second argument that is a true value.
BEGIN {
  if(defined(my $sub = _fetch_sub utf8 => 'downgrade')) {
  *downgrade = \&{"utf8​::downgrade"};
  } else {
  *downgrade = sub {
  my $r = "";
  my $l = length($_[0]);
  for(my $i = 0; $i != $l; $i++) {
  my $o = ord(substr($_[0], $i, 1));
  return if $o > 255;
  $r .= chr($o);
  }
  $_[0] = $r;
  };
  }
}

I dont see any explanation for these shenanigans in the module, and instead
of "playing along" with some unstated justification for this and unrolling
a large part of overload.pm innards into Carp, I plan to just use
overload.pm and break this expectation.

Around the same time I was thinking about this Zefram then bumped
into/mentioned this issue in this ticket, and I figured I would raise my
objections to the premise and get a dialog going about why this is even
necessary.

If there is a good reason for these measures then I would expect it to be
documented in Carp.pm, since it isnt documented I assume there isn't a good
reason, and if there isn't a good reason then we should stop caring about
it at all, and maybe rip out the complicated and inefficient and
duplicative logic.

So if there is a good reason please explain before I waste my time. On the
other hand, if there isn't a good reason lets stop wasting our time with
trying to support this objective.

Why should it matter if Carp.pm unilaterally loaded overload.pm when first
used?

For that matter, why should Carp be lugging around code to support 5.6 when
we are preparing to release 5.28?

cheers,
Yves

.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Feb 18, 2018

From @demerphq

On 17 February 2018 at 15​:55, demerphq <demerphq@​gmail.com> wrote​:

On 17 Feb 2018 21​:36, "Father Chrysostomos via RT"
<perlbug-followup@​perl.org> wrote​:

On Fri, 16 Feb 2018 18​:53​:47 -0800, demerphq wrote​:

It is unfortunate that so many critical functions and checks in Perl
require namespace mutations, but that's how things work, and imo not
touching other namespaces has a *much* lower priority than Carp working
correctly and not segfaulting.

Why cannot we have both? You are being vague

Carp incorrectly assumes that the only way you can have an overloaded object
is if you have loaded overload.pm.

Which means that this segfaults due to stack overflow​:

perl -MCarp -E 'my $p = "OverloadedInXS"; *{$p."​::(("} = sub{};
*{$p.q!​::(""!} = sub { Carp​::cluck "<My Stringify>" }; sub {
Carp​::cluck("") }->(bless {}, $p);'

See #132828.

To fix that without loading overload.pm I would have to more or less extract
a nice chunk of the internals of overload.pm into Carp.

After writing I revisited solving it without loading overload.pm,
which I was able to do more or less, with only two test result
changes​:

../cpan/Encode/t/truncated_utf8.t (Wstat​: 0 Tests​: 9 Failed​: 0)
  TODO passed​: 9
../cpan/Math-BigInt/t/mbimbf.t (Wstat​: 256 Tests​: 738 Failed​: 1)
  Failed test​: 718
  Non-zero exit status​: 1

It is interesting that a change to Carp fixes cpan/Encode/t/truncated_utf8.t

On the other hand, I havent figured out what to do about Math-BigInt
and I am still digging into why it fails witth the "tricky" logic I am
doing, but not when using overload​::StrVal.

Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Feb 18, 2018

From @khwilliamson

On 02/18/2018 03​:44 AM, demerphq wrote​:

It is interesting that a change to Carp fixes cpan/Encode/t/truncated_utf8.t

Carp did not fix this; commit c31ca20 did.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Feb 18, 2018

From zefram@fysh.org

demerphq wrote​:

To fix that without loading overload.pm I would have to more or less
extract a nice chunk of the internals of overload.pm into Carp.

It would be reasonable to load overload.pm when a reference arg is seen,
in order to deal with that situation. However, it doesn't need to load
overload.pm until that situation arises, and should not. It shouldn't
impose the loading of overload.pm on programs that don't make stack
traces. There's a general principle in Carp of being minimal in what
it loads, because of its unique role and its ubiquity.

Some of them appear to be related to back-compat, but some of them are
just egregious reimplementation of another modules functionality for the
purpose of avoiding that module​:

No, they're not. The downgrade example isn't about whether the utf8.pm
module is loaded, because that's not how utf8​::downgrade() gets defined.
utf8​::downgrade() is defined by the core, regardless of the utf8.pm
module, on any Perl from 5.7.1 onwards. The alternate implementation
exists for compatibility to Perl 5.6, which is still a consideration
for CPAN releases (though we're now on the tail end of that era).
The circumspectness about looking for utf8​::downgrade() is to avoid
vivifying the unwanted utf8​:: stash on 5.6.

The optionality of utf8​::is_utf8() is likewise for 5.6 compat.
The optionality of B​::svref_2object() *is* to avoid a dependency, but
it's not reimplemented if not available​: the extra information that
it would have supplied is just omitted from a rare diagnostic message.
The optionality of overload​::StrVal() is, as you noted, predicated on
the incorrect assumption that overloading only happens with overload.pm
loaded​: if that had been correct then it would mean that overload.pm
wasn't required unless it was already loaded, so a dependency was
avoidable.

                                          I plan to just use

overload.pm and break this expectation.

Don't just do that. Be cautious; follow the precedent of minimising
module loading and avoiding stash vivification. Carp is quite a subtle
module, in response to a unique set of constraints arising from its role
as the standard error-handling module. Heavy-handed editing is likely
to do damage.

-zefram

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Feb 27, 2018

From @cpansprout

On Thu, 15 Feb 2018 10​:32​:28 -0800, zefram@​fysh.org wrote​:

I wrote​:

The test is erroneous in using all keys it finds as method names.
It should skip ones that aren't syntactically OK.

Reported as [rt.cpan.org #124441].

The edit to Carp is also arguably faulty.

Fixed in commit 682f3ac.

In that commit you avoid vivifying the UNIVERSAL stash, but then assume that &UNIVERSAL​::isa is defined anyway. The code carefully uses _fetch_sub, which is designed for fetching a sub that may not even exist, but then assumes that it will return something callable.

Why do we need to avoid vivifying the UNIVERSAL stash (and its isa glob)?

I understand not vivifying utf8, because that causes problems on 5.6, and I understand not vivifying stashes we don’t need, but the UNIVERSAL stash is always there.

I ask because it would take less code (and Carp would load faster), if we could skip checking whether $​::{"UNIVERSAL​::"} exists and just use it.

(Note​: None of this applies to $UNIVERSAL​::{"isa​::"}, which I understand we should not vivify.)

--

Father Chrysostomos

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Feb 27, 2018

From @cpansprout

On Sun, 18 Feb 2018 14​:40​:37 -0800, zefram@​fysh.org wrote​:

demerphq wrote​:

To fix that without loading overload.pm I would have to more or less
extract a nice chunk of the internals of overload.pm into Carp.

It would be reasonable to load overload.pm when a reference arg is seen,
in order to deal with that situation. However, it doesn't need to load
overload.pm until that situation arises, and should not. It shouldn't
impose the loading of overload.pm on programs that don't make stack
traces. There's a general principle in Carp of being minimal in what
it loads, because of its unique role and its ubiquity.

I just want to point out that, as I mentioned in another thread, loading modules at run time is not an option for Carp. It can be (and has been) invoked after a syntax error, which prevents BEGIN blocks from running.

(I had forgotten about it when I first read this message. I had to go digging through the logs to refresh my memory as to why we can’t do that.)

--

Father Chrysostomos

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Mar 2, 2018

From @iabyn

On Tue, Feb 27, 2018 at 12​:56​:40PM -0800, Father Chrysostomos via RT wrote​:

I just want to point out that, as I mentioned in another thread, loading modules at run time is not an option for Carp. It can be (and has been) invoked after a syntax error, which prevents BEGIN blocks from running.

(I had forgotten about it when I first read this message. I had to go digging through the logs to refresh my memory as to why we can’t do that.)

Also, we got rid of Carp/Heavy.pm because of the possibility of an error
triggering Carp which has an error loading Carp/Heavy.pm for whatever
reason and the reporting of either the original error or the loading error
gets lost in the resulting mess.

--
"There's something wrong with our bloody ships today, Chatfield."
  -- Admiral Beatty at the Battle of Jutland, 31st May 1916.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Mar 9, 2018

From zefram@fysh.org

I wrote​:

Reported as [rt.cpan.org #124441].

That ticket has been closed with a patch that doesn't really fix the
problem of syntactically invalid method names, and which introduces a
new bug based on assuming that every value found in the stash is a glob.

-zefram

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Apr 19, 2018

From @iabyn

On Sun, Feb 18, 2018 at 12​:58​:00PM -0700, Karl Williamson wrote​:

On 02/18/2018 03​:44 AM, demerphq wrote​:

It is interesting that a change to Carp fixes cpan/Encode/t/truncated_utf8.t

Carp did not fix this; commit c31ca20 did.

Or more strictly, two commits did. I've just opened this ticket with
Enocde​:

  https://rt.cpan.org/Ticket/Display.html?id=125131

and pushed the following to blead​:

commit 836bb1e
Author​: David Mitchell <davem@​iabyn.com>
AuthorDate​: Thu Apr 19 14​:10​:35 2018 +0100
Commit​: David Mitchell <davem@​iabyn.com>
CommitDate​: Thu Apr 19 14​:37​:45 2018 +0100

  Encode​: truncated_utf8.t TODO passes
 
  This TODO test has been passing since the combination of​:
 
  v5.27.8-40-g37657a5b6c
 
  which added utf8n_to_uvchr_msgs to the perl API
 
  v5.27.8-252-gc31ca2013f
 
  which upgraded blead to Encode 2.96, which makes use of this new
  function if available.
 
  So stop marking it as TODO in blead.
 
  This is patching a cpan/ distribution, but its only a test, and its a
  bit late in code freeze to install a newer Encode released, while we
  don't want a production perl with spurious passing TODO tests.

--
That he said that that that that is is is debatable, is debatable.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Apr 19, 2018

From @xsawyerx

What else is left on this ticket?

On 04/19/2018 03​:42 PM, Dave Mitchell wrote​:

On Sun, Feb 18, 2018 at 12​:58​:00PM -0700, Karl Williamson wrote​:

On 02/18/2018 03​:44 AM, demerphq wrote​:

It is interesting that a change to Carp fixes cpan/Encode/t/truncated_utf8.t
Carp did not fix this; commit c31ca20 did.
Or more strictly, two commits did. I've just opened this ticket with
Enocde​:

https://rt.cpan.org/Ticket/Display.html?id=125131

and pushed the following to blead​:

commit 836bb1e
Author​: David Mitchell <davem@​iabyn.com>
AuthorDate​: Thu Apr 19 14​:10​:35 2018 +0100
Commit​: David Mitchell <davem@​iabyn.com>
CommitDate​: Thu Apr 19 14​:37​:45 2018 +0100

Encode&#8203;: truncated\_utf8\.t TODO passes

This TODO test has been passing since the combination of&#8203;:

v5\.27\.8\-40\-g37657a5b6c

    which added utf8n\_to\_uvchr\_msgs to the perl API

v5\.27\.8\-252\-gc31ca2013f

    which upgraded blead to Encode  2\.96\, which makes use of this new
    function if available\.

So stop marking it as TODO in blead\.

This is patching a cpan/ distribution\, but its only a test\, and its a
bit late in code freeze to install a newer Encode released\, while we
don't want a production perl with spurious passing TODO tests\.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Apr 20, 2018

From @iabyn

On Thu, Apr 19, 2018 at 06​:57​:41PM +0200, Sawyer X wrote​:

What else is left on this ticket?

I don't know.
I particular, Yves, Zefram, is there anything about Carp currently
which would justify keeping this ticket as a 5.28 blocker?

--
Nothing ventured, nothing lost.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Apr 20, 2018

From @demerphq

On 20 April 2018 at 10​:32, Dave Mitchell <davem@​iabyn.com> wrote​:

On Thu, Apr 19, 2018 at 06​:57​:41PM +0200, Sawyer X wrote​:

What else is left on this ticket?

I don't know.
I particular, Yves, Zefram, is there anything about Carp currently
which would justify keeping this ticket as a 5.28 blocker?

I believe that in another patch sequence we (Zefram, FC and I) arrived
a solution that resolved all the Carp issues.

cheers,
Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Apr 20, 2018

From @xsawyerx

On 04/20/2018 12​:19 PM, demerphq wrote​:

On 20 April 2018 at 10​:32, Dave Mitchell <davem@​iabyn.com> wrote​:

On Thu, Apr 19, 2018 at 06​:57​:41PM +0200, Sawyer X wrote​:

What else is left on this ticket?
I don't know.
I particular, Yves, Zefram, is there anything about Carp currently
which would justify keeping this ticket as a 5.28 blocker?
I believe that in another patch sequence we (Zefram, FC and I) arrived
a solution that resolved all the Carp issues.

Is this a merged patch already?

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Apr 20, 2018

From @demerphq

On 20 April 2018 at 13​:06, Sawyer X <xsawyerx@​gmail.com> wrote​:

On 04/20/2018 12​:19 PM, demerphq wrote​:

On 20 April 2018 at 10​:32, Dave Mitchell <davem@​iabyn.com> wrote​:

On Thu, Apr 19, 2018 at 06​:57​:41PM +0200, Sawyer X wrote​:

What else is left on this ticket?
I don't know.
I particular, Yves, Zefram, is there anything about Carp currently
which would justify keeping this ticket as a 5.28 blocker?
I believe that in another patch sequence we (Zefram, FC and I) arrived
a solution that resolved all the Carp issues.

Is this a merged patch already?

Its not clear if you mean merged to blead or something else.

The issues I was involved in were basically wrapped up with FC's

5c8d107

which is merged to blead.

cheers,
Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Apr 20, 2018

From @xsawyerx

On 04/20/2018 02​:29 PM, demerphq wrote​:

On 20 April 2018 at 13​:06, Sawyer X <xsawyerx@​gmail.com> wrote​:

On 04/20/2018 12​:19 PM, demerphq wrote​:

On 20 April 2018 at 10​:32, Dave Mitchell <davem@​iabyn.com> wrote​:

On Thu, Apr 19, 2018 at 06​:57​:41PM +0200, Sawyer X wrote​:

What else is left on this ticket?
I don't know.
I particular, Yves, Zefram, is there anything about Carp currently
which would justify keeping this ticket as a 5.28 blocker?
I believe that in another patch sequence we (Zefram, FC and I) arrived
a solution that resolved all the Carp issues.
Is this a merged patch already?

Its not clear if you mean merged to blead or something else.

The issues I was involved in were basically wrapped up with FC's

5c8d107

which is merged to blead.

Yes. This is what I meant. Thank you.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Apr 20, 2018

From @iabyn

On Fri, Apr 20, 2018 at 02​:29​:19PM +0200, demerphq wrote​:

On 20 April 2018 at 13​:06, Sawyer X <xsawyerx@​gmail.com> wrote​:

On 04/20/2018 12​:19 PM, demerphq wrote​:

On 20 April 2018 at 10​:32, Dave Mitchell <davem@​iabyn.com> wrote​:

On Thu, Apr 19, 2018 at 06​:57​:41PM +0200, Sawyer X wrote​:

What else is left on this ticket?
I don't know.
I particular, Yves, Zefram, is there anything about Carp currently
which would justify keeping this ticket as a 5.28 blocker?
I believe that in another patch sequence we (Zefram, FC and I) arrived
a solution that resolved all the Carp issues.

Is this a merged patch already?

Its not clear if you mean merged to blead or something else.

The issues I was involved in were basically wrapped up with FC's

5c8d107

which is merged to blead.

So it's ok to close this tickety then?

--
Art is anything that has a label (especially if the label is "untitled 1")

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Apr 20, 2018

From @demerphq

I think so, yes.

Yves

On 20 April 2018 at 16​:17, Dave Mitchell <davem@​iabyn.com> wrote​:

On Fri, Apr 20, 2018 at 02​:29​:19PM +0200, demerphq wrote​:

On 20 April 2018 at 13​:06, Sawyer X <xsawyerx@​gmail.com> wrote​:

On 04/20/2018 12​:19 PM, demerphq wrote​:

On 20 April 2018 at 10​:32, Dave Mitchell <davem@​iabyn.com> wrote​:

On Thu, Apr 19, 2018 at 06​:57​:41PM +0200, Sawyer X wrote​:

What else is left on this ticket?
I don't know.
I particular, Yves, Zefram, is there anything about Carp currently
which would justify keeping this ticket as a 5.28 blocker?
I believe that in another patch sequence we (Zefram, FC and I) arrived
a solution that resolved all the Carp issues.

Is this a merged patch already?

Its not clear if you mean merged to blead or something else.

The issues I was involved in were basically wrapped up with FC's

5c8d107

which is merged to blead.

So it's ok to close this tickety then?

--
Art is anything that has a label (especially if the label is "untitled 1")

--
perl -Mre=debug -e "/just|another|perl|hacker/"

@p5pRT p5pRT closed this as completed Apr 21, 2018
@p5pRT
Copy link
Author

@p5pRT p5pRT commented Apr 21, 2018

@iabyn - Status changed from 'open' to 'resolved'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant