Uh oh!
There was an error while loading. Please reload this page.
gh-115119: removed implicit fallback to the bundled libmpdec - #134078
Conversation
On top of configure changes in #133997. N/B: all linux jobs, except for changed (not sure if it worth) - have no system libmpdec. For MacOS we run tests with system libmpdec. |
Uh oh!
There was an error while loading. Please reload this page.
| [AC_MSG_WARN([m4_normalize([ | ||
| no system libmpdecimal found; falling back to bundled libmpdecimal | ||
| (deprecated and scheduled for removal in Python 3.15)])]) | ||
| USE_BUNDLED_LIBMPDEC()]) | ||
| no system libmpdecimal found; falling back to pure-Python version | ||
| for the decimal module])]) | ||
| AS_VAR_SET([py_cv_module_]_decimal, [n/a])]) |
There was a problem hiding this comment.
AC_MSG_ERROR? I think opting in to the pure Python version should be explicit.
There was a problem hiding this comment.
AC_MSG_ERROR ?
Well, that could be an option.
Though, more complex wrt implementation: all linux jobs will fail, unless we either provide system libmpdec or change ./configure invocations to use a new option.
I think opting in to the pure Python version should be explicit.
I'm not sure it's useful. After all, the pure-Python version is always available as the _pydecimal.py.
Did you suggest a new option like --with-purepython-decimal?
There was a problem hiding this comment.
A warning is actually ok, but users may not see it and then, they would have a slow version of decimal. OTOH, an error might be too abrupt but at least the tansition would be more explicit and would force people to upgrade if they want an efficient way to do it.
I'm +0.25 for AC_MSG_ERROR just to force people to update. We can add an option --allow-fallback-to-pydecimal to handle CI possible failures, though this means that devs should be aware that their C code might not be tested in the CI due to that.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Victor Stinner <vstinner@python.org>
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
2422e7d to
9690208Compare
This comment was marked as resolved.
This comment was marked as resolved.
vstinner
commented
Jun 17, 2025
I think that I now prefer PR gh-135568: don't add |
skirpichev
commented
Jun 17, 2025
Done. I moved this after JFR:
|
ned-deily
left a comment
There was a problem hiding this comment.
Thanks for making the changes. I think this solution makes for a better user experience.
vstinner
commented
Jun 26, 2025
I am no longer sure if I like this change or not. My main worry is that some users may build Python without I would prefer to keep mpdecimal as an optional dependency of Python. And so it's good that the configure script doesn't fail if the dependency is missing. So at the end, I think that I like this change :-) |
vstinner
commented
Jul 1, 2025
Merged, thank you. |
| * Removed implicit fallback to the bundled copy of the ``libmpdec`` library. | ||
| Now this should be explicitly enabled with :option:`--with-system-libmpdec` | ||
| set to ``no`` or with :option:`!--without-system-libmpdec`. |
There was a problem hiding this comment.
Does the devguide need updating? For example the macOS build instructions say to use --with-system-libmpdec, do we want people to stop using that for 3.15+?
https://devguide.python.org/getting-started/setup-building/#install-dependencies
There was a problem hiding this comment.
I think we can just drop this option. It's specified in 3.13+ instructions (where it's default to "yes"). I'll prepare a patch.
vstinner
commented
Jul 2, 2025
Should we recommend installing libmpdec dependency in https://docs.python.org/dev/using/configure.html#build-requirements ? |
skirpichev
commented
Jul 2, 2025
Hmm, a good question. Here we list deps, required to build some optional modules. Though, for the decimal we always have a fallback to the pure-Python module. I think that for now we can document libmpdec dependency just like for SQLite/Tk/Tcl and not mention details (that we have a fallback).
I'm second to this. We have a lot of issues, related to differences for C-coded vs pure-Python version of the decimal module. The libmpdec now is a mature project and I don't see big reasons to keep the old pure-Python version. Nobody will prefer that in the real world - it's just adds a maintenance burden. |
vstinner
commented
Jul 3, 2025
Ok, I created #136239 to document the dependency. |
…thon#134078) Co-authored-by: Victor Stinner <vstinner@python.org> Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
…thon#134078) Co-authored-by: Victor Stinner <vstinner@python.org> Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
…thon#134078) Co-authored-by: Victor Stinner <vstinner@python.org> Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
…thon#134078) Co-authored-by: Victor Stinner <vstinner@python.org> Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
…thon#134078) Co-authored-by: Victor Stinner <vstinner@python.org> Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
libmpdecsources #115119📚 Documentation preview 📚: https://cpython-previews--134078.org.readthedocs.build/