Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.2k
GH-132983: Support finding libzstd without pkg-config#133550
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
pythonGH-133479 removed the logic to check for libzstd outside of pkg-config.This commit adds that logic back with a check for the version so thatusers can provide their own libzstd. This is to bring parity with lzma,bz2, and zlib detection.
!buildbot RHEL8 |
bedevere-bot commentedMay 7, 2025
🤖 New build scheduled with the buildbot fleet by@emmatyping for commitf3dd7f7 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F133550%2Fmerge The command will test the builders whose names match following regular expression: The builders matched are:
|
Scheduled RHEL 8 but I also want to double check the |
emmatyping commentedMay 7, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Okay this is looking good. macOS Sonoma: The |
I've opened#133565 as an alternative with slightly different (and I think also faster) autoconf syntax, what do you think? A |
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
!buildbot RHEL8 |
bedevere-bot commentedMay 7, 2025
🤖 New build scheduled with the buildbot fleet by@emmatyping for commit235fe69 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F133550%2Fmerge The command will test the builders whose names match following regular expression: The builders matched are:
|
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, this seems correct to me! It would be unfortunate if we had a different set of configure rules for libzstd than for other third-party dependencies.
Closing in favor of#133565 |
Uh oh!
There was an error while loading.Please reload this page.
GH-133479 removed the logic to check for libzstd outside of pkg-config. This commit adds that logic back with a check for the version so that users can provide their own libzstd. This is to bring parity with lzma, bz2, and zlib detection.
The issues previously seen on RHEL 8 were due to how the checks are ordered. After#133479, if a library version was too old, the
PKG_CHECK_MODULESmacro would take the "not found" path. This would then search for and find zstd.h and a libzstd with the symbolZDICT_finalizeDictionary, but then compilation would fail when the header check occured.#133502 removed this path solving the issue on RHEL 8. However, this also make it impossible for a user to set a custom build of libzstd as the dependency (as they can do for liblzma, libbz2, etc.).In this PR we add this path back, but add an additional check for the version of the libraries. If a user/third party packager would like to point to their own libzstd, they can do so. We avoid the issue#133479 was meaning to solve by ensuring that the version check only runs if we already have the zstd.h header and libzstd library found.