FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-115119: Removed bundled copy of the libmpdec by skirpichev · Pull Request #133964 · python/cpython · GitHub

/ cpython Public

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

Merged
zware merged 38 commits into
python:mainfrom
skirpichev:remove-bundled-mpdecimal/115119
May 12, 2026
ConversationCommits (38)ChecksFiles changed
Merged

gh-115119: Removed bundled copy of the libmpdec#133964
zware merged 38 commits into
python:mainfrom
skirpichev:remove-bundled-mpdecimal/115119

Conversation

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

Copy link
Copy Markdown
Member

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

AA-Turner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

quick comments, haven't looked at build system changes

Comment thread Doc/using/configure.rst
Comment thread Doc/whatsnew/3.13.rst Outdated
Comment thread Doc/whatsnew/3.15.rst Outdated
Comment thread Misc/sbom.spdx.json Outdated
Comment thread Modules/Setup.stdlib.in Outdated
skirpichev and others added 2 commits May 13, 2025 19:21
Apply suggestions from code review
5de2aa2 
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
+ configure
dfd9f40
zware previously requested changes May 13, 2025
View reviewed changes
Comment thread PCbuild/_decimal.vcxproj.filters

This comment was marked as resolved.

Sign in to view

This comment was marked as resolved.

Sign in to view

This comment was marked as resolved.

Sign in to view

This comment was marked as resolved.

Sign in to view
bedevere-app Bot requested a review from zware May 14, 2025 03:41

This comment was marked as resolved.

Sign in to view
skirpichev force-pushed the remove-bundled-mpdecimal/115119 branch from 0a33f9f to 2ba6b8b Compare May 14, 2025 05:49
skirpichev requested a review from zware May 9, 2026 05:59
sethmlarson approved these changes May 11, 2026
View reviewed changes

sethmlarson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Reviewed the changes to sbom.spdx.json and generate_sbom.py and those changes LGTM.

zware reviewed May 11, 2026
View reviewed changes

zware left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Quite close, I think, just a few more cleanups.

Comment thread Doc/license.rst
Comment thread Doc/tools/removed-ids.txt
Comment thread Doc/whatsnew/3.16.rst Outdated
Comment thread Modules/_decimal/README.txt
Comment thread configure.ac Outdated
Comment thread Modules/_decimal/README.txt
skirpichev requested a review from zware May 12, 2026 04:07
zware reviewed May 12, 2026
View reviewed changes
Comment thread Doc/whatsnew/3.16.rst
Comment thread Doc/license.rst Outdated
Comment on lines 962 to 964
Previously, the :mod:`!_decimal` C extension underlying the :mod:`decimal` module
was built using an included copy of the libmpdec
library unless the build is configured ``--with-system-libmpdec``::

zware May 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Wouldn't hurt to fix the libffi section as well. Suggested wording here, though:

Suggested change
Previously, the :mod:`!_decimal` C extension underlying the :mod:`decimal` module
was built using an included copy of the libmpdec
library unless the build is configured ``--with-system-libmpdec``::
The :mod:`!_decimal` C extension underlying the :mod:`decimal` module
is linked to the libmpdec C library::

On the other hand, we could just leave this file alone for now and get someone more knowledgeable about such things to tell us what we should do in a separate issue :)

skirpichev May 12, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Then lets keep this intact.

Comment thread configure.ac Outdated
Comment thread configure.ac
# Disable forced inlining in debug builds, see GH-94847
AS_VAR_IF(
[with_pydebug], [yes],
[AS_VAR_APPEND([LIBMPDEC_CFLAGS], [" -DTEST_COVERAGE"])])

zware May 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This is only relevant for building libmpdec itself, isn't it?

There is a usage of TEST_COVERAGE in Modules/_decimal/_decimal.c, but I don't see anything ever defining it for that module.

skirpichev May 12, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

There is a usage of TEST_COVERAGE in Modules/_decimal/_decimal.c, but I don't see anything ever defining it for that module.

It's defined there, no?

zware May 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Ok, sorry, my understanding of how LIBMPDEC_{CFLAGS,LDFLAGS} work was incorrect; this is fine.

skirpichev May 13, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Given that this define used only in one place, we could replace it with Py_DEBUG in the code.

Comment thread configure.ac Outdated
Comment thread Modules/_decimal/README.txt
skirpichev requested a review from zware May 12, 2026 20:18
zware reviewed May 12, 2026
View reviewed changes
Comment thread Doc/whatsnew/3.13.rst Outdated
Comment thread Doc/license.rst Outdated
Apply suggestions from code review
b4cc60d 
Co-authored-by: Zachary Ware <zachary.ware@gmail.com>
zware enabled auto-merge (squash) May 12, 2026 21:08
zware merged commit 9eb3b14 into python:main May 12, 2026
69 checks passed
skirpichev deleted the remove-bundled-mpdecimal/115119 branch May 13, 2026 01:42
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

ned-deily ned-deily left review comments
zware zware approved these changes
sethmlarson sethmlarson approved these changes
AA-Turner Awaiting requested review from AA-Turner AA-Turner is a code owner
ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner
hugovk Awaiting requested review from hugovk hugovk is a code owner
StanFromIreland Awaiting requested review from StanFromIreland
erlend-aasland Awaiting requested review from erlend-aasland erlend-aasland is a code owner
emmatyping Awaiting requested review from emmatyping emmatyping is a code owner
corona10 Awaiting requested review from corona10 corona10 is a code owner
ezio-melotti Awaiting requested review from ezio-melotti ezio-melotti is a code owner
webknjaz Awaiting requested review from webknjaz webknjaz is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL