-
-
Notifications
You must be signed in to change notification settings - Fork 31.4k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
gh-115119: Default to --with-system-libmpdec=yes #118539
gh-115119: Default to --with-system-libmpdec=yes #118539
Conversation
Mark _decimal as missing if libmpdecimal is not found.
$ ./configure --with-system-libmpdec=no | grep -E "(decimal|mpdec)"
checking for --with-system-libmpdec... no
checking for --with-decimal-contextvar... yes
checking for decimal libmpdec machine... universal
checking for stdlib extension module _decimal... yes $ ./configure | grep -E "(decimal|mpdec)"
checking for --with-system-libmpdec... yes
checking for libmpdec... yes
checking for --with-decimal-contextvar... yes
checking for stdlib extension module _decimal... yes
$ grep -E "_DECIMAL_(CFLAGS|LDFLAGS)=" Makefile
MODULE__DECIMAL_CFLAGS=-I/opt/homebrew/Cellar/mpdecimal/4.0.0/include
MODULE__DECIMAL_LDFLAGS=-L/opt/homebrew/Cellar/mpdecimal/4.0.0/lib -lmpdec -lm |
@zware: I'm not sure if we should fall back to the internal copy if libmpdec is missing. For now, it is marked as missing if it is missing :) We could change it so |
Things to consider:
For now, we can consider to use the bundled version for the Ubuntu CI. |
Right, an explicit Footnotes
|
Yep
Yes, we need to warn for that case.
Hm, I think this case should fail, similar to how we handle |
I don't want a bare |
Ok, how about this:
|
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 have a couple of questions and qualms, but overall I'm okay with this as is. We can tweak it through the beta phase if needed.
…l of the mpdecimal sources Co-authored-by: Zachary Ware <zachary.ware@gmail.com>
…rsion check in configure
@@ -15,6 +15,7 @@ apt-get -yq install \ | |||
libgdbm-dev \ | |||
libgdbm-compat-dev \ | |||
liblzma-dev \ | |||
libmpdec-dev \ |
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.
@hugovk: FYI, this will fail for Ubuntu 24.04, as libmpdec-dev
is unavailable there.
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've been trying to figure out why that is, and have come up short (I get lost quickly in the Debian/Ubuntu development process). All I've found is a note from @doko42 in the Ubuntu python3.11 3.11.2-4
changelog, saying "Build with internal mpdecimal library, so that mpdecimal can be removed for bookworm." (and indeed, Debian bookworm does not have a libmpdec package either).
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.
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.
@@ -15,6 +15,7 @@ apt-get -yq install \ | |||
libgdbm-dev \ | |||
libgdbm-compat-dev \ | |||
liblzma-dev \ | |||
libmpdec-dev \ |
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've been trying to figure out why that is, and have come up short (I get lost quickly in the Debian/Ubuntu development process). All I've found is a note from @doko42 in the Ubuntu python3.11 3.11.2-4
changelog, saying "Build with internal mpdecimal library, so that mpdecimal can be removed for bookworm." (and indeed, Debian bookworm does not have a libmpdec package either).
Thanks for helping out, Zach! |
Co-authored-by: Zachary Ware <zachary.ware@gmail.com>
fwiw this is going to be painful for ubuntu and debian which has removed the |
That seems to have been mentioned at #118539 (comment) already, but I'm not sure what the problem is. We didn't have Why can't Debian/Ubuntu just set --with-system-libmpdec=no or --without-system-libmpdec (whichever it is) if they can't package libmpdec quickly? |
@thesamesam, @asottile: we're adding a fallback to the bundled version if an external libmpdec cannot be found. FTR, I find the recent removal of libmpdec from Ubuntu/Debian very strange. |
Mark _decimal as missing if libmpdecimal is not found.
libmpdec
sources #115119📚 Documentation preview 📚: https://cpython-previews--118539.org.readthedocs.build/