[rt.cpan.org #56225] Innocent-looking Cwd change breaks EPIC + debugger on win32

<URL: https://rt.cpan.org/Ticket/Display.html?id=56225 >

On Thu Jul 07 09:55:49 2011, nick@ccl4.org wrote:
> On Sat, Sep 11, 2010 at 07:13:55PM +0100, Nicholas Clark wrote:
> > Try it. I predict that it's not going to work.
> >
> > (In that it will re-create the bug that this fixed, unless something
> > else has
> > changed).
> > The point of the *string* eval was to avoid early (well normal)
> > binding the CV
> > to the optree, given that later in execution the CV gets deleted.
> > (by MakeMaker::Test::NoXS to simulate miniperl)
> >
> > Thread related to all of this
> > http://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/2009-
> > 09/msg00326.html
> > (which you reference below)
> 
> On Sat, Jun 18, 2011 at 05:08:07AM -0500, Reini Urban wrote:
> 
> > The purpose of the string eval was to avoid autovivication of the CV.
> >
> > If the context stack is broken on win32 somehow,
> > a different, less intrusive fix would be:
> >
> > if ( *DynaLoader::boot_DynaLoader{CODE}  ) {
> >
> 
> However that construction doesn't actually address the issue I
> described
> above, because:
> 
> ./perl -le 'delete $DynaLoader::{boot_DynaLoader}; print defined
> *DynaLoader::boot_DynaLoader{CODE}'
> 1
> 
> whereas we want a construction that returns 0 after
> C<delete $DynaLoader::{boot_DynaLoader}> has been run. Inspired by
> Reini's
> approach, I committed this:
> 
> commit 5ec06e76dc20e3154110c2955f25db63f00c527d
> Author: Nicholas Clark <nick@ccl4.org>
> Date:   Sat Jun 18 14:42:07 2011 +0200
> 
> In Cwd::_win32_cwd() avoid a string eval when checking if we're
> miniperl.
> 
> To allow ExtUtils::MakeMaker to run tests as if it's "miniperl" we
> need to avoid
> taking any sort of reference to the typeglob or the code in the
> optree, as its
> test modules are loaded later than Cwd. Previously this was done with
> a string
> eval, but that was causing problems (for unclear reasons - rt.cpan.org
> #56225).
> Using a symbol table lookup and *foo{THING} syntax avoids the string
> eval.
> 
> Evolved from a suggestion by Reini Urban.
> 
> diff --git a/dist/Cwd/Cwd.pm b/dist/Cwd/Cwd.pm
> index 4683e10..ecd14ae 100644
> --- a/dist/Cwd/Cwd.pm
> +++ b/dist/Cwd/Cwd.pm
> @@ -171,7 +171,7 @@ use strict;
>  use Exporter;
>  use vars qw(@ISA @EXPORT @EXPORT_OK $VERSION);
> 
> -$VERSION = '3.36';
> +$VERSION = '3.37';
>  my $xs_version = $VERSION;
>  $VERSION = eval $VERSION;
> 
> @@ -755,7 +755,14 @@ sub _win32_cwd_simple {
>  }
> 
> sub _win32_cwd {
> -    if (eval 'defined &DynaLoader::boot_DynaLoader') {
> +    # Need to avoid taking any sort of reference to the typeglob or
> the code in
> +    # the optree, so that this tests the runtime state of things, as
> the
> +    # ExtUtils::MakeMaker tests for "miniperl" need to be able to
> fake things at
> +    # runtime by deleting the subroutine. *foo{THING} syntax on a
> symbol table
> +    # lookup avoids needing a string eval, which has been reported to
> cause
> +    # problems (for reasons that we haven't been able to get to the
> bottom of -
> +    # rt.cpan.org #56225)
> +    if (*{$DynaLoader::{boot_DynaLoader}}{CODE}) {
>        $ENV{'PWD'} = Win32::GetCwd();
>     }
>     else { # miniperl
> 
> 
> On Mon, Jun 20, 2011 at 10:46:22AM -0400, Victor Miasnikov via RT
> wrote:
> > <URL: https://rt.cpan.org/Ticket/Display.html?id=56225 >
> >
> > >> 2011/6/17 Victor Miasnikov via RT <bug-PathTools@rt.cpan.org>:
> > > On Sat Jun 18 06:08:16 2011, rurban@x-ray.at wrote:
> > >  a different, less intrusive fix would be:
> > >
> > > if ( *DynaLoader::boot_DynaLoader{CODE}  ) {
> > >
> >
> > Yes -- work Ok
> 
> As the string eval is the problem, I'm assuming that the change I made
> will
> solve the symptoms too.
> 
> Nicholas Clark


There has been no discussion in this ticket for over two years.

Can we therefore conclude that Nicholas's patch has resolved the problem and that this ticket is closable?

Thank you very much.
Jim Keenan

• [rt.cpan.org #56225] Innocent-looking Cwd change breaks EPIC + debugger on win32 by James E Keenan via RT