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

to_bytes/from_bytes() requires a newer version of the Math::BigInt::GMP library #4

Closed
eserte opened this issue Sep 26, 2021 · 5 comments

Comments

@eserte
Copy link

eserte commented Sep 26, 2021

The test suite fails on a few of my smokers:

...
from_bytes() requires a newer version of the Math::BigInt::GMP library. at t/04-Helpers.t line 20.
# Tests were run but no plan was declared and done_testing() was not seen.
# Looks like your test exited with 255 just after 1.
t/04-Helpers.t ......... 
Dubious, test returned 255 (wstat 65280, 0xff00)
All 1 subtests passed 
...
to_bytes() requires a newer version of the Math::BigInt::GMP library. at /home/cpansand/.cpan/build/2021092212/Bitcoin-Crypto-1.002-1/blib/lib/Bitcoin/Crypto/Helpers.pm line 144.
# Tests were run but no plan was declared and done_testing() was not seen.
# Looks like your test exited with 255 just after 1.
t/14-ExtPublicKey.t .... 
Dubious, test returned 255 (wstat 65280, 0xff00)
All 1 subtests passed 
...
@bbrtj
Copy link
Member

bbrtj commented Sep 27, 2021

Thanks for the report, but I'm not sure if I can do anything about it

  • Bitcoin::Crypto does not depend on GMP, it is optional
  • it tries to load Math::BigInt GMP backend before LTM to avoid a nasty bug with mixing Math::BigInt backends (plus GMP seems to be a tiny bit faster)
  • users may or may not have GMP, but since it is not in dependencies, I don't think I can have any control over their version
  • I already load Math::BigInt with a explicitly stated version, but that does not affect the backend

I'm open to suggestions on how to fix this

@eserte
Copy link
Author

eserte commented Sep 27, 2021

You may specify the dependency if the user have already an older version of Math::BigInt::GMP installed, for example by using code like this in Makefile.PL (untested):

my $minimum_Math_BigInt_GMP_VERSION = ...;
if (eval { require Math::BigInt::GMP; 1 } && $Math::BigInt::GMP::VERSION < $minimum_Math_BigInt_GMP_VERSION ) {
    ... extend PREREQ_PM ...
}

Or use skip_all on the two failing tests if you detect that Math::BigInt::GMP is too old here.

@bbrtj
Copy link
Member

bbrtj commented Oct 3, 2021

Hello @eserte,
I'm having trouble figuring out which Math::BigInt::GMP version I should require. Math::BigInt::GMP never got the required methods (_to_bytes, _from_bytes), instead it relies on inheritance from Math::BigInt::Lib to get it. I already require Math::BigInt (which contains Math::BigInt::Lib) in the sufficient version, which has the required methods.

Is it possible that these smokers have different version of Math::BigInt::Lib than Math::BigInt? Could you check the version of Math::BigInt, Math::BigInt::Lib and Math::BigInt::GMP on one of the smokers?

@bbrtj
Copy link
Member

bbrtj commented Oct 3, 2021

Seems like Math::BigInt::GMP 1.6003 was the first one to get an author test for existence of _to_bytes method, so it is the most probable version to require. I'm still interested why is it failing in the first place though

bbrtj added a commit that referenced this issue Oct 9, 2021
@bbrtj
Copy link
Member

bbrtj commented Oct 9, 2021

None the tests of my development release reported the problem with Math::BigInt::GMP version, so I'll upload a new version and close this ticket. Thanks!

@bbrtj bbrtj closed this as completed Oct 9, 2021
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

2 participants