search

Dual-Life / Devel-PPPort

Perl/Pollution/Portability
9 stars 28 forks source link

Implement SV_GMAGIC flag for sv_2pv_flags() and sv_pvn_force_flags() for Perl versions < 5.7.2 #159

Closed pali closed 4 years ago

pali commented 4 years ago

If sv_2pv_flags() is called without SV_GMAGIC flag then temporary turn off SVs_GMG flag (via SvGMAGICAL_off) on passed scalar, call standard sv_2pv() and restore SVs_GMG flag (via SvGMAGICAL_on). Same for sv_pvn_force_flags().

pali commented 4 years ago

@atoomic @khwilliamson Can you review this PR?

khwilliamson commented 4 years ago

This PR doesn't pass for below 5.6.0: Here's a sample failure: 'Can\'t load \'blib/arch/auto/Devel/PPPort/PPPort.so\' for module Devel::PPPort: blib/arch/auto/Devel/PPPort/PPPort.so: undefined symbol: Perl_eval_sv at / home/khw/devel/lib/perl5/5.00504/x86_64-linux/DynaLoader.pm line 169.

pali commented 4 years ago

Ah... this is because @haarg still has not merged travis fix: https://github.com/travis-perl/helpers/pull/72

pali commented 4 years ago

Anyway, what in this PR is related to Perl_eval_sv?? This looks very suspicious.

khwilliamson commented 4 years ago

I'm investigating

khwilliamson commented 4 years ago

It turns out it is because your G_RETHROW patch is breaking old perls. Rather than go back and forth on that, I fixed that and will issue a PR for review with that fix.

When I do this, this commit works back to 5.4.0. Before that PL_Xpv is not available. My suggestion is that we stop there, and not define the xs sub for testing unless that is available, and skip the tests for earlier versions. B ut I wanted to check that out with you first.

pali commented 4 years ago

PL_Xpv is available also on 5.3.0. Just it is called Xpv, like PL_Sv and Sv. Following define patch should help:

diff --git a/parts/inc/variables b/parts/inc/variables
index 9a6df02..2ada503 100644
--- a/parts/inc/variables
+++ b/parts/inc/variables
@@ -18,6 +18,7 @@ PL_DBsingle
 PL_DBsub
 PL_DBtrace
 PL_Sv
+PL_Xpv
 PL_bufend
 PL_bufptr
 PL_compiling
@@ -98,6 +99,7 @@ __NEED_VAR__ U32 PL_signals = D_PPP_PERL_SIGNALS_INIT;
 #  define PL_DBsub                  DBsub
 #  define PL_DBtrace                DBtrace
 #  define PL_Sv                     Sv
+#  define PL_Xpv                    Xpv
 #  define PL_bufend                 bufend
 #  define PL_bufptr                 bufptr
 #  define PL_compiling              compiling
khwilliamson commented 4 years ago

Is there some way I can easily get your fix to 'variables' as a patch from you? I copied it and applied it, so could gurn it into a patch, but is there an easier way to get it?

pali commented 4 years ago

I do not think that there is easier way as copy and apply patch.

pali commented 4 years ago

'Can't load 'blib/arch/auto/Devel/PPPort/PPPort.so' for module Devel::PPPort: blib/arch/auto/Devel/PPPort/PPPort.so: undefined symbol: Perl_eval_sv at / home/khw/devel/lib/perl5/5.00504/x86_64-linux/DynaLoader.pm line 169.

I manually compiled Perl 5.5.3 and fixed this problem. PR: https://github.com/Dual-Life/Devel-PPPort/pull/165

pali commented 4 years ago

I put above PL_Xpv patch into PR: https://github.com/Dual-Life/Devel-PPPort/pull/167