| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
| [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.
Sorry, something went wrong.
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?
Sorry, something went wrong.
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.
Sorry, something went wrong.
Co-authored-by: Victor Stinner <vstinner@python.org>
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
|
I think that I now prefer PR gh-135568: don't add --with-libmpdec option. I don't think that ./configure should fail if mpdecimal library is missing, but a warning should be written at the end of the ./configure script. |
Sorry, something went wrong.
Done. I moved this after _decimal module check at the end. JFR:
$ ./configure --with-pydebug 2>&1|grep -i mpd checking for --with-system-libmpdec... yes checking for libmpdec >= 2.5.0... no checking for rl_compdisp_func_t... yes configure: WARNING: no system libmpdecimal found; falling back to pure-Python version for the decimal module $ make -s Written build/lib.linux-x86_64-3.15/_sysconfigdata_d_linux_x86_64-linux-gnu.py Written build/lib.linux-x86_64-3.15/_sysconfig_vars_d_linux_x86_64-linux-gnu.json The necessary bits to build these optional modules were not found: _decimal To find the necessary bits, look in configure.ac and config.log. Checked 114 modules (35 built-in, 77 shared, 1 n/a on linux-x86_64, 0 disabled, 1 missing, 0 failed on import)
$ ./configure --with-pydebug --without-system-libmpdec 2>&1|grep -i mpd checking for --with-system-libmpdec... no configure: WARNING: the bundled copy of libmpdecimal is scheduled for removal in Python 3.16; consider using a system installed mpdecimal library. checking for decimal libmpdec machine... uint128 checking for rl_compdisp_func_t... yes
$ ./configure --with-pydebug --without-system-libmpdec 2>&1|grep -i mpd checking for --with-system-libmpdec... no configure: WARNING: the bundled copy of libmpdecimal is scheduled for removal in Python 3.16; consider using a system installed mpdecimal library. checking for decimal libmpdec machine... uint128 checking for rl_compdisp_func_t... yes
$ ./configure --with-pydebug 2>&1|grep -i mpd checking for --with-system-libmpdec... yes checking for libmpdec >= 2.5.0... yes checking for rl_compdisp_func_t... yes |
Sorry, something went wrong.
There was a problem hiding this comment.
Thanks for making the changes. I think this solution makes for a better user experience.
Sorry, something went wrong.
|
I am no longer sure if I like this change or not. My main worry is that some users may build Python without _decimal and would silently get a 100x slower decimal module. On the other hand, I don't consider that decimal is part of the most important stdlib modules, so it's ok if decimal is (way) slower. Many users don't use decimal at all. 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 :-) |
Sorry, something went wrong.
There was a problem hiding this comment.
LGTM
Sorry, something went wrong.
|
Merged, thank you. |
Sorry, something went wrong.
|
|
||
| * 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
Sorry, something went wrong.
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.
Sorry, something went wrong.
|
Should we recommend installing libmpdec dependency in https://docs.python.org/dev/using/configure.html#build-requirements ? |
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
Ok, I created #136239 to document the dependency. |
Sorry, something went wrong.
…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>
| Back | FazBrowse Home | New Git URL |
📚 Documentation preview 📚: https://cpython-previews--134078.org.readthedocs.build/