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

gv_fetchpvn_flags() is now supported? #42

Closed
wyoung opened this issue Aug 24, 2016 · 9 comments
Closed

gv_fetchpvn_flags() is now supported? #42

wyoung opened this issue Aug 24, 2016 · 9 comments

Comments

@wyoung
Copy link

wyoung commented Aug 24, 2016

While chasing a different problem I seem to have stumbled across a documentation bug. According to man Devel::PPPort, gv_fetchpvn_flags() is unsupported, but on grepping the source code, I find that there is an implementation, and that the text of that routine is built into the local PPPort.so here on Perl 5.8.8.

Did it get implemented and someone forgot to remove this item from the manual page?

@wyoung
Copy link
Author

wyoung commented Aug 24, 2016

I see that this was added in 3.33.

However, I have a new problem, which is that the addition of this function breaks Devel::PPPort on CentOS 5, which ships with Perl 5.8.8. See the linked bug report for the symptoms and the step-by-step on how I chased it down.

If you just want a simple test case, however:

$ sudo cpanm Mouse

Rolling back to 3.32 allows Mouse, Text::Xslate, and other Devel::PPPort-using modules to install via cpanm on this system.

@mhx
Copy link
Member

mhx commented Aug 27, 2016

I believe you're doing it wrong :)

The ppport.h generated by Devel::PPPort isn't meant to be generated
automatically during the build process — which apparently is what you're
doing — but rather meant to be updated occasionally and shipped with
your code. The reason for that is just what you experienced: ppport.h
can change, and these changes /may/ require modifications to your
code. So you update on demand, because there's something you want
to get out of using (a new version of) ppport.h.

In your case, your code is using gv_fetchpvs. Within ppport.h, this has
a dependency on gv_fetchpvn_flags (which is the function you're having
problems with). However, as providing gv_fetchpvn_flags produces real
code, you have to explicitly enable support for it by defining
NEED_gv_fetchpvn_flags.

If you run ppport.h, it actually tells you exactly that:

$ perl ppport.h xs-src/MouseTypeConstraints.xs
[...]
Uses gv_fetchpvs, which depends on gv_fetchpvn_flags, SVt_PVHV, Safefree, gv_fetchpv, savepvn
[...]
File needs gv_fetchpvn_flags, adding static request
[...]
--- xs-src/MouseTypeConstraints.xs
+++ xs-src/MouseTypeConstraints.xs.patched
[...]
+#define NEED_gv_fetchpvn_flags

So, you've got two options:

  1. Simply use the ppport.h generated by Devel::PPPort 3.32
  2. Use the latest ppport.h and define NEED_gv_fetchpvn_flags

In any case, you should stop generating ppport.h as part of the build
process. This has never been and will likely never be supported.

@mhx mhx closed this as completed Aug 27, 2016
@jjn1056
Copy link

jjn1056 commented Nov 8, 2016

@mhx I have the exact same error with Data::Clone on an older CentoOS with Perl 5.8.8 and not sure I fully understand your explanation. FWIW downgrading also solves it, which is why it feels like a bug to me (given the point of this module I thought was to help with back compat, broken back compat feels icky). Are you saying this is a bug in Data::Clone, and that I should report there and the author is supposed to make a change like you described above?

I'm not actually using Data::Clone, its coming via HTML::FormHandler so this is impacting stuff well downstream dependency wise (and I'm seeing that if I look at modules that depend on Devel::PPPort, there's a growing trend of fails with older versions of Perl in basically everything that depends on this module. If all those module owners are 'holding it wrong' then we need to get the word out since this is having impact well downstream for those of us stuck on older perl.

You do actually have clear docs on this: https://metacpan.org/pod/Devel::PPPort but maybe those docs are more recent. When I look at Data::Clone I can't figure out if the author followed your instructions. I can see here https://metacpan.org/source/GFUJI/Data-Clone-0.004/inc/Module/Install/XSUtil.pm it looks like they are build requiring Devel::PPPort which I guess is not right but I have no idea what sort of patch to offer... suggestions?

@wolfsage
Copy link
Collaborator

wolfsage commented Nov 8, 2016

"Uses gv_fetchpvs, which depends on gv_fetchpvn_flags, SVt_PVHV, Safefree, gv_fetchpv, savepv..."

So ppport.h knows which functions it provides and knows which ones depend on other functions. Can it implicitly set NEED_* in these cases without needing that to be added to consumers...?

@mhx
Copy link
Member

mhx commented Jan 11, 2017

@wolfsage, yes, adding NEED_ dependencies is something worth thinking about, but it's a completely different issue; a large part of the API doesn't even need any NEED_ definitions and having one for each API would bloat the module even further and simply defining all of them would bloat the code that's using D::PPP unnecessarily.

@jjn1056, build-requiring Devel::PPPort is just plain wrong. The generated ppport.h provides compatibilty for the module using it against a large number of Perl versions. However, upgrading ppport.h itself may require some work on the code using it, just as initially starting to use ppport.h does. Devel::PPPort was at no point meant to be compatible between releases, which is exactly why module authors are supposed to ship a particular version of ppport.h with their code.

@atoomic
Copy link
Member

atoomic commented Aug 6, 2020

#32 changed gv_fetchpvn_flags from a macro to a function

I'm going to check what is the status of Mouse 2.4.5 (and the last released version) and Data::Clone with an updated version of ppport.h to confirm the status of this issue

@pali
Copy link
Contributor

pali commented Aug 6, 2020

And see also my (merged) PR "Fix gv_fetchpvn_flags": #85

@atoomic
Copy link
Member

atoomic commented Aug 7, 2020

I can reproduce the described bug on CentOS 5 using Mouse v2.4.5 with the shipped ppport.h or updated ppport.h to 3.58.

This is fixed when using the last version available of Mouse v2.5.10
Updating ppport.h from Devel-PPPort at 3.58 for Mouse v2.5.10 is working fine

@atoomic
Copy link
Member

atoomic commented Aug 7, 2020

Data-Clone-0.004 is using Module::Install::XSUtil helper use_ppport to dynamically generate ppport.h
I checked that Data-Clone is currently working fine with an updated ppport.h from Devel-PPPort-3.58_01

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

6 participants