GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-115119: removed implicit fallback to the bundled libmpdec by skirpichev · Pull Request #134078 · python/cpython

Conversation

@skirpichev

@skirpichev skirpichev commented May 16, 2025 •
edited by github-actions Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
  • Issue: Unvendor libmpdec sources #115119

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
skirpichev added 2 commits May 16, 2025 09:35
@skirpichev
pythongh-115119: defer removal of bundled libmpdec to 3.16 (configure…
14266c8
… changes)
@skirpichev
pythongh-115119: removed implicit fallback to the bundled libmpdec
39d1e6b
@bedevere-app bedevere-app Bot mentioned this pull request May 16, 2025
Unvendor libmpdec sources #115119
Closed
15 tasks done
@skirpichev

skirpichev commented May 16, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@skirpichev
skirpichev marked this pull request as ready for review May 16, 2025 06:59
@skirpichev
skirpichev requested review from AA-Turner, corona10, erlend-aasland, ezio-melotti and hugovk as code owners May 16, 2025 06:59
@bedevere-app bedevere-app Bot added the awaiting review label May 16, 2025
AA-Turner
AA-Turner reviewed May 16, 2025
View reviewed changes
Comment thread .github/workflows/build.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread configure.ac Outdated
Comment on lines +4141 to +4144
[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])])

@AA-Turner AA-Turner May 16, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AC_MSG_ERROR? I think opting in to the pure Python version should be explicit.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@skirpichev skirpichev May 16, 2025

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@picnixz picnixz Jun 2, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@skirpichev
address review: remove runner version downgrade
b328702
@skirpichev
skirpichev requested review from picnixz and vstinner May 30, 2025 05:41
picnixz

This comment was marked as resolved.

Sign in to view

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@skirpichev skirpichev mentioned this pull request Jun 2, 2025
gh-115119: defer removal of bundled libmpdec to 3.16 #133997
Merged
skirpichev added 3 commits June 3, 2025 06:55
@skirpichev
+ use libmpdec-dev from PPA for PHP
94ce315
@skirpichev
Merge branch 'master' into remove-fallback-to-bundled-mpdec/115119
fed39e1
@skirpichev
address review: add --with-libmpdec option
4c98b8d
vstinner
vstinner reviewed Jun 3, 2025
View reviewed changes
Comment thread .github/workflows/posix-deps-apt.sh Resolved

Uh oh!

There was an error while loading. Please reload this page.

vstinner
vstinner reviewed Jun 3, 2025
View reviewed changes
Comment thread .github/workflows/posix-deps-apt.sh Resolved

Uh oh!

There was an error while loading. Please reload this page.

@skirpichev @vstinner
Update .github/workflows/posix-deps-apt.sh
6cd1730
Co-authored-by: Victor Stinner <[email protected]>
hugovk
hugovk reviewed Jun 9, 2025
View reviewed changes
Comment thread .github/workflows/posix-deps-apt.sh Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Doc/using/configure.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Doc/whatsnew/3.15.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Doc/whatsnew/3.15.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Misc/NEWS.d/next/Build/2025-05-16-07-46-06.gh-issue-115119.ALBgS_.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Misc/NEWS.d/next/Build/2025-05-16-07-46-06.gh-issue-115119.ALBgS_.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

skirpichev and others added 3 commits June 10, 2025 05:03
@skirpichev @hugovk
Apply suggestions from code review
9d29df1
Co-authored-by: Hugo van Kemenade <[email protected]>
@skirpichev
Merge branch 'master' into remove-fallback-to-bundled-mpdec/115119
b733718
@skirpichev
address review: reorder list in posix-deps-apt.sh
d53bdf3
@skirpichev
skirpichev requested a review from hugovk June 10, 2025 02:15
vstinner
vstinner reviewed Jun 10, 2025
View reviewed changes
Comment thread Doc/using/configure.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

@skirpichev
address review: expand --with-libmpdec description
c3a1889
@skirpichev
address review: redo posix-deps-apt.sh workaround (install last)
9690208
@skirpichev
skirpichev force-pushed the remove-fallback-to-bundled-mpdec/115119 branch 2 times, most recently from 2422e7d to 9690208 Compare June 16, 2025 15:03
@vstinner

This comment was marked as resolved.

Sign in to view
@skirpichev
Merge branch 'master' into remove-fallback-to-bundled-mpdec/115119
f0e6757
@vstinner

vstinner commented Jun 17, 2025

Copy link
Copy Markdown
Member

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

skirpichev added 3 commits June 17, 2025 15:53
@skirpichev
Revert "address review: add --with-libmpdec option"
1eca085
This reverts commit 4c98b8d.
@skirpichev
address review: move warning in the configure
73f0729
@skirpichev
+1
cece14c
@skirpichev

skirpichev commented Jun 17, 2025

Copy link
Copy Markdown
Member Author

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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@skirpichev
skirpichev requested review from ned-deily and vstinner June 17, 2025 13:54
ned-deily
ned-deily approved these changes Jun 18, 2025
View reviewed changes

@ned-deily ned-deily left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making the changes. I think this solution makes for a better user experience.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@vstinner

vstinner commented Jun 26, 2025

Copy link
Copy Markdown
Member

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

erlend-aasland reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner
vstinner approved these changes Jun 26, 2025
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@skirpichev
skirpichev requested a review from picnixz June 28, 2025 16:37
@vstinner
vstinner merged commit 17cf0a3 into python:main Jul 1, 2025
@bedevere-app bedevere-app Bot removed the awaiting merge label Jul 1, 2025
@vstinner

vstinner commented Jul 1, 2025

Copy link
Copy Markdown
Member

Merged, thank you.

zware reacted with thumbs up emoji zware reacted with hooray emoji
All reactions
  • 👍 1 reaction
  • 🎉 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@skirpichev
skirpichev deleted the remove-fallback-to-bundled-mpdec/115119 branch July 1, 2025 15:53
hugovk
hugovk reviewed Jul 2, 2025
View reviewed changes
Comment thread Doc/whatsnew/3.15.rst

* 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`.

@hugovk hugovk Jul 2, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@skirpichev skirpichev Jul 2, 2025

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@vstinner

vstinner commented Jul 2, 2025

Copy link
Copy Markdown
Member

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@skirpichev

skirpichev commented Jul 2, 2025

Copy link
Copy Markdown
Member Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vstinner

vstinner commented Jul 3, 2025

Copy link
Copy Markdown
Member

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 #136239 to document the dependency.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

AndPuQing pushed a commit to AndPuQing/cpython that referenced this pull request Jul 11, 2025
@skirpichev @vstinner
@hugovk @AndPuQing
pythongh-115119: Remove implicit fallback to the bundled libmpdec (py…
5038d9a
…thon#134078)

Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025
@skirpichev @vstinner
@hugovk @Pranjal095
pythongh-115119: Remove implicit fallback to the bundled libmpdec (py…
8ee8d1f
…thon#134078)

Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
picnixz pushed a commit to picnixz/cpython that referenced this pull request Jul 13, 2025
@skirpichev @vstinner
@hugovk @picnixz
pythongh-115119: Remove implicit fallback to the bundled libmpdec (py…
91705c9
…thon#134078)

Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
taegyunkim pushed a commit to taegyunkim/cpython that referenced this pull request Aug 4, 2025
@skirpichev @vstinner
@hugovk @taegyunkim
pythongh-115119: Remove implicit fallback to the bundled libmpdec (py…
b18a788
…thon#134078)

Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull request Aug 19, 2025
@skirpichev @vstinner
@hugovk @Agent-Hellboy
pythongh-115119: Remove implicit fallback to the bundled libmpdec (py…
eb83e8c
…thon#134078)

Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
@skirpichev skirpichev mentioned this pull request Oct 12, 2025
Only pure Python decimal module is used in Python 3.15 on Ubuntu #139994
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hugovk hugovk hugovk left review comments
@AA-Turner AA-Turner AA-Turner left review comments
@vstinner vstinner vstinner approved these changes
@ned-deily ned-deily ned-deily approved these changes
@erlend-aasland erlend-aasland Awaiting requested review from erlend-aasland erlend-aasland is a code owner
@corona10 corona10 Awaiting requested review from corona10 corona10 is a code owner
@ezio-melotti ezio-melotti Awaiting requested review from ezio-melotti ezio-melotti is a code owner
@picnixz picnixz Awaiting requested review from picnixz

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@skirpichev @vstinner @picnixz @hugovk @ned-deily @AA-Turner
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.