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

Locale::CLDR::Locales::* modules load bignum, breaking every project which sets $Math::BigFloat::downgrade or $Math::BigInt::upgrade #39

Closed
ehuelsmann opened this issue Oct 6, 2023 · 9 comments

Comments

@ehuelsmann
Copy link
Contributor

As discussed in pjacklam/p5-Math-BigInt#11 and also reported in #36, with recent improvements in down- and upgrading in Math::BigInt/Math::BigFloat (released as part of Perl 5.36+), there are now more operations which correctly return the upgraded/downgraded result. However, this also makes it more visible that this project is overriding the up- and downgrade selections of the projects it's embedded in.

What's more: it overrides the selection in the project it's part of, not right from the start, but only once the first Locale::CLDR::Locales::* module is loaded (which happens dynamically). This behaviour makes the type failures, e.g. in LedgerSMB seem random.

Why does this module use bignum itself? why doesn't it advise users to do that or failing that, take other measures?

@pjacklam
Copy link

If the Locale::CLDR::Locales::* modules need support for big numbers, it is better to use either bigint or bigfloat, depending on which type of numbers that are involved. These two pragmas do not upgrade or downgrade, which also makes them faster than bignum since the upgrading and downgrading introduces some overhead.

@ThePilgrim
Copy link
Owner

ThePilgrim commented Oct 10, 2023

I think bigfloat will work I'm going to make a trial version for 0.34.2 and see what happens
This will be on it's own branch called v0.34

@ehuelsmann
Copy link
Contributor Author

Thanks! That would be hugely appreciated. Since I have some code which is reliably failing, I'll reverse the mitigating measures and see if the code remains stable with the new release and without the measures.

@ThePilgrim
Copy link
Owner

You may have noticed a lack of development over the last year, this is a massive project and most of it is written by me. I needed a brake. 😀

@ehuelsmann
Copy link
Contributor Author

Yes. I have. Don't feel pressured. I know how it is; we're using the project in LedgerSMB (https://ledgersmb.org) which is no small effort either (we've been modernizing the code base for more than 15 years now and still not done).

@ThePilgrim
Copy link
Owner

ThePilgrim commented Oct 11, 2023

I'm still uploading to CPAN, have to throttle uploads or I get a slap on the wrist but there's sufficient up there to test with US and British English Look for Locale-CLDR-v0.34.2-TRIAL1 and then *-v0.34.2-TRIAL1for the language packs.
Can you let me know if it works 😀
Upload is fully complete

@ehuelsmann
Copy link
Contributor Author

Can you let me know if it works 😀
Upload is fully complete

In a docker image where this is currently broken:

docker exec -ti --user root <container name> bash
# cpanm --reinstall --notest https://cpan.metacpan.org/authors/id/J/JG/JGNI/Locale-CLDR-v0.34.2-TRIAL1.tar.gz
# cpanm --reinstall --notest https://cpan.metacpan.org/authors/id/S/ST/STRANGE/Locale-CLDR-Locales-En-v0.34.1.tar.gz

And restarting the docker container.

🎉 yes! It works again now! No workarounds necessary anymore to restore the value of $Math::BigFloat::downgrade.

@ehuelsmann
Copy link
Contributor Author

Thank you for the quick response!

@ThePilgrim
Copy link
Owner

I'm getting a none development version generated, once that's done I'll upload it to CPAN. With any luck it will be uploaded by the end of today

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

3 participants