Skip to content

gh-115119: Removed bundled copy of the libmpdec #133964

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 17 commits into from
Closed

gh-115119: Removed bundled copy of the libmpdec #133964

wants to merge 17 commits into from

Conversation

skirpichev
Copy link
Member

@skirpichev skirpichev commented May 13, 2025 •
edited by github-actions bot
Loading

@bedevere-app bedevere-app bot mentioned this pull request May 13, 2025
Open
15 tasks
@skirpichev skirpichev force-pushed the remove-bundled-mpdecimal/115119 branch from 7cba56a to 9d6c9b5 Compare May 13, 2025 15:37
AA-Turner
AA-Turner reviewed May 13, 2025
View reviewed changes
Copy link
Member

@AA-Turner AA-Turner left a comment

Choose a reason for hiding this comment

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

quick comments, haven't looked at build system changes

Doc/using/configure.rst
Comment on lines -838 to -852
.. option:: --with-system-libmpdec

Build the ``_decimal`` extension module using an installed ``mpdecimal``
library, see the :mod:`decimal` module (default is yes).

.. versionadded:: 3.3

.. versionchanged:: 3.13
Default to using the installed ``mpdecimal`` library.

.. deprecated-removed:: 3.13 3.15
A copy of the ``mpdecimal`` library sources will no longer be distributed
with Python 3.15.

.. seealso:: :option:`LIBMPDEC_CFLAGS` and :option:`LIBMPDEC_LIBS`.
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if it's worth keeping a mention of --with-system-libmpdec, just for the purpose of saying it's been removed? Unsure though.

Copy link
Member Author

Choose a reason for hiding this comment

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

Is this something that expected for removed stuff?

I see one removed option, --with-wctype-functions - it's only in whatsnew.

skirpichev and others added 2 commits May 13, 2025 19:21
@skirpichev @AA-Turner
Apply suggestions from code review
5de2aa2 
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
@skirpichev
+ configure
dfd9f40
zware
zware requested changes May 13, 2025
View reviewed changes
@bedevere-app

This comment was marked as resolved.

Sign in to view
@AA-Turner
Copy link
Member

Ubuntu is failing as version 24.04 doesn't have a libmpdec-dev package so the _decimal module isn't built. 25.04 and the upcoming 25.10 do have the mpdecimal sources, though. I don't know enough about Ubuntu packaging to work out how to install those packages on 24.04 (or the version from 22.04).

A

@skirpichev

This comment was marked as resolved.

Sign in to view
@bedevere-app

This comment was marked as resolved.

Sign in to view
@bedevere-app bedevere-app bot requested a review from zware May 14, 2025 03:41
@skirpichev
Copy link
Member Author

skirpichev commented May 14, 2025 •
edited
Loading

Ubuntu is failing as version 24.04 doesn't have a libmpdec-dev package so the _decimal module isn't built.

Yes. One solution is build it for CI by usual "./configure && make && make install". Other - filter out the _decimal module from the Tools/build/check_extension_modules.py output. Not sure how to do this, PY_STDLIB_MOD_SET_NA? Probably it should be disabled (with a warning) if libmpdec is missing.

Edit: I did proposed (module marked as n/a if libmpdec is missing). One Linux job works now on the ubuntu 22.04, which has an appropriate libmpdec version.

PS:
But see the discussion thread: it was proposed to postpone removal for yet another release.

@skirpichev skirpichev force-pushed the remove-bundled-mpdecimal/115119 branch from 0a33f9f to 2ba6b8b Compare May 14, 2025 05:49
@skirpichev skirpichev requested a review from AA-Turner May 14, 2025 06:28
@skirpichev skirpichev closed this May 16, 2025
@skirpichev skirpichev deleted the remove-bundled-mpdecimal/115119 branch May 16, 2025 04:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zware zware Awaiting requested review from zware

@AA-Turner AA-Turner Awaiting requested review from AA-Turner AA-Turner is a code owner

@sethmlarson sethmlarson Awaiting requested review from sethmlarson sethmlarson will be requested when the pull request is marked ready for review sethmlarson is a code owner

@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently will be requested when the pull request is marked ready for review ericsnowcurrently is a code owner

@1st1 1st1 Awaiting requested review from 1st1 1st1 will be requested when the pull request is marked ready for review 1st1 is a code owner

@erlend-aasland erlend-aasland Awaiting requested review from erlend-aasland erlend-aasland will be requested when the pull request is marked ready for review erlend-aasland is a code owner

@corona10 corona10 Awaiting requested review from corona10 corona10 will be requested when the pull request is marked ready for review corona10 is a code owner

@ezio-melotti ezio-melotti Awaiting requested review from ezio-melotti ezio-melotti will be requested when the pull request is marked ready for review ezio-melotti is a code owner

@hugovk hugovk Awaiting requested review from hugovk hugovk will be requested when the pull request is marked ready for review hugovk is a code owner

Assignees
No one assigned
Labels
awaiting change review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@skirpichev @AA-Turner @zware