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. |
configure.ac
Outdated
| [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.
Co-authored-by: Victor Stinner <vstinner@python.org>
.github/workflows/posix-deps-apt.sh
Outdated
| @@ -23,4 +29,5 @@ apt-get -yq install \ | |||
| tk-dev \ | |||
| uuid-dev \ | |||
| xvfb \ | |||
| zlib1g-dev | |||
| zlib1g-dev \ | |||
| libmpdec-dev | |||
There was a problem hiding this comment.
Let's keep this list alphabetical.
There was a problem hiding this comment.
I can't:) It's already non-alphabetical. Should I fix that?
Misc/NEWS.d/next/Build/2025-05-16-07-46-06.gh-issue-115119.ALBgS_.rst
Outdated
Show resolved
Hide resolved
Misc/NEWS.d/next/Build/2025-05-16-07-46-06.gh-issue-115119.ALBgS_.rst
Outdated
Show resolved
Hide resolved
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
| @@ -851,6 +851,12 @@ Libraries options | |||
|
|
|||
| .. seealso:: :option:`LIBMPDEC_CFLAGS` and :option:`LIBMPDEC_LIBS`. | |||
|
|
|||
| .. option:: --with-libmpdec | |||
|
|
|||
| Whether to build the ``_decimal`` extension module. | |||
There was a problem hiding this comment.
Can you document the default value?
Whether to build the ``_decimal`` extension module (default: yes).
I suggest to add:
See also the :option:`--with-system-libmpdec` option.
libmpdecsources #115119📚 Documentation preview 📚: https://cpython-previews--134078.org.readthedocs.build/