From: Peter Rabbitson <ribasushi@cpan.org>
Date: Mon, 28 Sep 2015 05:59:16 +0000 (+0200)
Subject: Reign in XS portions needed for certain perls under debugging, several things:
X-Git-Tag: 0.26~7
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=df4cbc4ec541e33ee360b5bd1dc0b3d9325f0f6b;p=p5sagit%2Fnamespace-clean.git

Reign in XS portions needed for certain perls under debugging, several things:

- Do not rely on Sub::Identify at all (it is too finicky) - either use
  Sub::Util::subname if already loaded, or just use B (RT#96945)

- Allow either Sub::Name or Sub::Util to be used for namings

- Only add Sub::Name to the install list if neither Sub::* is available

- Clarify constant names, move them to _Util, make sure we use them everywhere

- Make sure that t/07-debugger.t runs on all perls, including the ones not
  needing the XS dance
---

diff --git a/Changes b/Changes
index 4abca90..d6dff06 100644
--- a/Changes
+++ b/Changes
@@ -1,6 +1,8 @@
         - Exclusively use Package::Stash::PP on perls < 5.8.7 until a fixed
           Package::Stash::XS ships - breakage keeps getting reintroduced
           ( RT#107343, RT#74151 )
+        - No longer rely on Sub::Identify - either use Sub::Util or B
+          ( should solve RT#96945 )
 
     [0.25]
         - Fix incorrect ExtUtils::CBuilder detection routine leading to
diff --git a/Makefile.PL b/Makefile.PL
index 225cdf6..266f171 100644
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -16,11 +16,20 @@ my %RUN_DEPS = (
   'B::Hooks::EndOfScope' => '0.12',
 );
 
-# these pieces are needed if using the debugger on the perl range
-my %OPT_RUN_DEPS = ( $] > 5.008_008_9 and $] < 5.013_005_1 and can_xs() )
-  # when changing versions, also change $sn_ver and $si_ver in namespace/clean.pm
-  ? ( 'Sub::Name' => '0.04', 'Sub::Identify' => '0.04' ) : ()
-;
+# a sub-namer is needed if using the debugger on some perls
+my %OPT_RUN_DEPS = ( (
+  require 'lib/namespace/clean/_Util.pm'
+    and
+  namespace::clean::_Util::DEBUGGER_NEEDS_CV_RENAME()
+    and
+  namespace::clean::_Util::_namer_load_error()
+    and
+  can_xs()
+)
+  # when changing version, also change $sn_ver in namespace/clean/_Util.pm
+  ? ( 'Sub::Name' => '0.04' )
+  : ()
+);
 
 my %META_BITS = (
   resources => {
diff --git a/lib/namespace/clean.pm b/lib/namespace/clean.pm
index 9a2a0d6..5d41c9d 100644
--- a/lib/namespace/clean.pm
+++ b/lib/namespace/clean.pm
@@ -33,14 +33,7 @@ sub stash_for (\$) {
 EOS
 }
 
-# Constant to optimise away the unused code branches
-use constant FIXUP_NEEDED => $] < 5.015_005_1;
-use constant FIXUP_RENAME_SUB => $] > 5.008_008_9 && $] < 5.013_005_1;
-{
-  no strict;
-  delete ${__PACKAGE__."::"}{FIXUP_NEEDED};
-  delete ${__PACKAGE__."::"}{FIXUP_RENAME_SUB};
-}
+use namespace::clean::_Util qw( DEBUGGER_NEEDS_CV_RENAME DEBUGGER_NEEDS_CV_PIVOT );
 
 # Debugger fixup necessary before perl 5.15.5
 #
@@ -60,29 +53,20 @@ my $sub_utils_loaded;
 my $DebuggerFixup = sub {
   my ($f, $sub, $cleanee_stash, $deleted_stash) = @_;
 
-  if (FIXUP_RENAME_SUB) {
-    if (! defined $sub_utils_loaded ) {
-      $sub_utils_loaded = do {
-
-        # when changing version also change in Makefile.PL
-        my $sn_ver = 0.04;
-        eval { require Sub::Name; Sub::Name->VERSION($sn_ver) }
-          or die "Sub::Name $sn_ver required when running under -d or equivalent: $@";
-
-        # when changing version also change in Makefile.PL
-        my $si_ver = 0.04;
-        eval { require Sub::Identify; Sub::Identify->VERSION($si_ver) }
-          or die "Sub::Identify $si_ver required when running under -d or equivalent: $@";
-
-        1;
-      } ? 1 : 0;
-    }
-
-    if ( Sub::Identify::sub_fullname($sub) eq ($cleanee_stash->name . "::$f") ) {
-      my $new_fq = $deleted_stash->name . "::$f";
-      Sub::Name::subname($new_fq, $sub);
-      $deleted_stash->add_symbol("&$f", $sub);
-    }
+  if (DEBUGGER_NEEDS_CV_RENAME) {
+    #
+    # Note - both get_subname and set_subname are only compiled when CV_RENAME
+    # is true ( the 5.8.9 ~ 5.12 range ). On other perls this entire block is
+    # constant folded away, and so are the definitions in ::_Util
+    #
+    # Do not be surprised that they are missing without DEBUGGER_NEEDS_CV_RENAME
+    #
+    namespace::clean::_Util::get_subname( $sub ) eq  ( $cleanee_stash->name . "::$f" )
+      and
+    $deleted_stash->add_symbol(
+      "&$f",
+      namespace::clean::_Util::set_subname( $deleted_stash->name . "::$f", $sub ),
+    );
   }
   else {
     $deleted_stash->add_symbol("&$f", $sub);
@@ -105,14 +89,14 @@ my $RemoveSubs = sub {
           or next SYMBOL;
 
         my $need_debugger_fixup =
-          FIXUP_NEEDED
+          ( DEBUGGER_NEEDS_CV_RENAME or DEBUGGER_NEEDS_CV_PIVOT )
             &&
           $^P
             &&
           ref(my $globref = \$cleanee_stash->namespace->{$f}) eq 'GLOB'
         ;
 
-        if (FIXUP_NEEDED && $need_debugger_fixup) {
+        if ($need_debugger_fixup) {
           # convince the Perl debugger to work
           # see the comment on top of $DebuggerFixup
           $DebuggerFixup->(
@@ -134,7 +118,7 @@ my $RemoveSubs = sub {
         # if this perl needs no renaming trick we need to
         # rename the original glob after the fact
         # (see commend of $DebuggerFixup
-        if (FIXUP_NEEDED && !FIXUP_RENAME_SUB && $need_debugger_fixup) {
+        if (DEBUGGER_NEEDS_CV_PIVOT && $need_debugger_fixup) {
           *$globref = $deleted_stash->namespace->{$f};
         }
 
diff --git a/lib/namespace/clean/_Util.pm b/lib/namespace/clean/_Util.pm
new file mode 100644
index 0000000..5c0e3f4
--- /dev/null
+++ b/lib/namespace/clean/_Util.pm
@@ -0,0 +1,99 @@
+###       !!!ACHTUNG!!!
+#
+# This module is to be loaded at configure time straight from the Makefile.PL
+# in order to get access to some of the constants / utils
+# None of the dependencies will be available yet at this point, so make
+# sure to never use anything beyond what the minimum supported perl came with
+# (no, relying on configure_requires is not ok)
+
+package # hide from the pauses
+  namespace::clean::_Util;
+
+use warnings;
+use strict;
+
+use base 'Exporter';
+our @EXPORT_OK = qw( DEBUGGER_NEEDS_CV_RENAME DEBUGGER_NEEDS_CV_PIVOT );
+
+use constant DEBUGGER_NEEDS_CV_RENAME => ( ( $] > 5.008_008 ) and ( $] < 5.013_006 ) );
+use constant DEBUGGER_NEEDS_CV_PIVOT => ( ( ! DEBUGGER_NEEDS_CV_RENAME ) and ( $] < 5.015_005 ) );
+
+# FIXME - ideally this needs to be provided by some abstraction lib
+# but we don't have that yet
+BEGIN {
+  #
+  # Note - both get_subname and set_subname are only called by one block
+  # which is compiled away unless CV_RENAME is true ( the 5.8.9 ~ 5.12 range ).
+  # Hence we compile/provide the definitions here only when needed
+  #
+  DEBUGGER_NEEDS_CV_RENAME and ( eval <<'EOS' or die $@ );
+{
+  my( $sub_name_loaded, $sub_util_loaded );
+
+  sub _namer_load_error {
+    my $err = '';
+
+    return $err if $sub_util_loaded or $sub_name_loaded;
+
+    local $@;
+
+    # prefer Sub::Name to Sub::Util
+    # this is rather arbitrary but remember this code exists only
+    # on perls 5.8.9 ~ 5.13.5
+
+    # when changing version also change in Makefile.PL
+    my $sn_ver = 0.04;
+
+    eval {
+      require Sub::Name;
+      Sub::Name->VERSION($sn_ver);
+      $sub_name_loaded = 1;
+    }
+      or
+    eval {
+      require Sub::Util;
+      $sub_util_loaded = 1;
+    }
+      or
+    $err = "When running under -d on this perl $], namespace::clean requires either Sub::Name $sn_ver or Sub::Util to be installed"
+    ;
+
+    $err;
+  }
+
+  sub set_subname {
+    if( my $err = _namer_load_error() ) {
+      die $err;
+    }
+    elsif( $sub_name_loaded ) {
+      &Sub::Name::subname;
+    }
+    elsif( $sub_util_loaded ) {
+      &Sub::Util::set_subname;
+    }
+    else {
+      die "How the fuck did we get here? Read source and debug please!";
+    }
+  }
+
+  sub get_subname {
+    if(
+      _namer_load_error()
+        or
+      ! $sub_util_loaded
+    ) {
+      require B;
+      my $gv = B::svref_2object( $_[0] )->GV;
+      join '::', $gv->STASH->NAME, $gv->NAME;
+    }
+    else {
+      &Sub::Util::subname;
+    }
+  }
+}
+1;
+EOS
+
+}
+
+1;
diff --git a/t/07-debugger.t b/t/07-debugger.t
index 598dac0..9e72122 100644
--- a/t/07-debugger.t
+++ b/t/07-debugger.t
@@ -1,14 +1,16 @@
 use Test::More;
 
 BEGIN {
-  eval { require Sub::Name }
-    or plan skip_all => "Test requires Sub::Name";
-
-  eval { require Sub::Identify }
-    or plan skip_all => "Test requires Sub::Identify";
+  require namespace::clean;
+  if (
+    namespace::clean::_Util::DEBUGGER_NEEDS_CV_RENAME()
+      and
+    my $missing_xs = namespace::clean::_Util::_namer_load_error()
+  ) {
+    plan skip_all => $missing_xs;
+  }
 }
 
-
 BEGIN {
   # shut up the debugger
   $ENV{PERLDB_OPTS} = 'NonStop';