Skip to content

GH-132983: Use a standard autoconf version check for zstd #133479

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 6, 2025

Conversation

AA-Turner
Copy link
Member

@AA-Turner AA-Turner commented May 5, 2025

cc @ned-deily, it seems that this works & is simpler? I'll run buildbots.

A

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @AA-Turner for commit afbc562 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F133479%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 5, 2025
@ned-deily
Copy link
Member

@AA-Turner That looks promising: it seems to handle my use case. Thanks!

cc: @emmatyping

@ned-deily
Copy link
Member

... and the ghcr.io/cirruslabs/macos-runner:sonoma runner also now appears to successfully build _zstd.

Copy link
Member

@emmatyping emmatyping left a comment

Choose a reason for hiding this comment

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

Thanks! I didn't know this was something one could do. TIL

I'd like to wait for a RHEL8 buildbot to go green before merging to ensure that it works when there is a version of libzstd that is too old. Otherwise it looks much better.

@emmatyping
Copy link
Member

And thank you for catching the original issue @ned-deily !

@ned-deily
Copy link
Member

I didn't know this was something one could do.

There is always something in Autotools that no one knows you can do.

@AA-Turner
Copy link
Member Author

https://buildbot.python.org/#/builders/170/builds/1765

checking for libzstd >= 1.4.5... no
checking for zstd.h... no
checking for zdict.h... no
...
checking for stdlib extension module _zstd... missing

Seems to work.

A

@AA-Turner AA-Turner requested a review from emmatyping May 6, 2025 00:29
@AA-Turner AA-Turner merged commit 793402e into python:main May 6, 2025
56 of 63 checks passed
@AA-Turner AA-Turner deleted the zstd-conf branch May 6, 2025 00:35
emmatyping added a commit to emmatyping/cpython that referenced this pull request May 7, 2025
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 that
users can provide their own libzstd. This is to bring parity with lzma,
bz2, and zlib detection.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants