cpython icon indicating copy to clipboard operation
cpython copied to clipboard

gh-115119: removed implicit fallback to the bundled libmpdec

Open skirpichev opened this issue 7 months ago • 7 comments

  • Issue: gh-115119

📚 Documentation preview 📚: https://cpython-previews--134078.org.readthedocs.build/

skirpichev avatar May 16 '25 06:05 skirpichev

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.

skirpichev avatar May 16 '25 06:05 skirpichev

@AA-Turner @picnixz @hugovk: Do you think that this change is ready to be merged?

cc @erlend-aasland @corona10 who modify configure tools sometimes.

vstinner avatar Jun 12 '25 16:06 vstinner

I see a double review but I'm on mobile. Sorry if it got double posted. I'm ok with this change, modulo some wording nits.

picnixz avatar Jun 13 '25 13:06 picnixz

Test without mpdecimal installed:

$ ./configure --with-pydebug
(...)
checking for --with-libmpdec... 
checking for --with-system-libmpdec... yes
checking for libmpdec >= 2.5.0... no
(...)

The fallback to the Python implementation is silent. Would it be possible to have a warning?

The line checking for --with-libmpdec... is surprising, there is no result written. I expect "yes" or "no".

vstinner avatar Jun 17 '25 12:06 vstinner

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.

vstinner avatar Jun 17 '25 12:06 vstinner

warning should be written at the end of the ./configure script.

Done. I moved this after _decimal module check at the end.

JFR:

  1. No system libmpdec, default:
$ ./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)
  1. No system libmpdec, with bundled:
$ ./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
  1. System libmpdec, with bundled:
$ ./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
  1. System libmpdec, default:
$ ./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

skirpichev avatar Jun 17 '25 13:06 skirpichev

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 :-)

vstinner avatar Jun 26 '25 13:06 vstinner

Merged, thank you.

vstinner avatar Jul 01 '25 15:07 vstinner

Should we recommend installing libmpdec dependency in https://docs.python.org/dev/using/configure.html#build-requirements ?

vstinner avatar Jul 02 '25 12:07 vstinner

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 would prefer to keep mpdecimal as an optional dependency of Python.

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.

skirpichev avatar Jul 02 '25 12:07 skirpichev

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).

Ok, I created https://github.com/python/cpython/pull/136239 to document the dependency.

vstinner avatar Jul 03 '25 10:07 vstinner