Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

GH-132983: Remove zstd version check in the header file#133502

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

Merged
hugovk merged 2 commits intopython:mainfromAA-Turner:zstd-rm-version-check
May 6, 2025

Conversation

AA-Turner
Copy link
Member

@AA-TurnerAA-Turner commentedMay 6, 2025•
edited by bedevere-appbot
Loading

We now have better version detection in autoconf, including detecting the most recent symbol we used. A hard version check in the header file can lead to spurious failures when using zstd versions between 1.1.3 (symbol added) and 1.4.5 (symbol stabilised).

xref:

A

gpshead reacted with heart emoji
@bedevere-bot

This comment was marked as resolved.

@bedevere-botbedevere-bot removed the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelMay 6, 2025
@AA-TurnerAA-Turner added the 🔨 test-with-refleak-buildbotsTest PR w/ refleak buildbots; report in status section labelMay 6, 2025
@bedevere-bot

This comment was marked as resolved.

@bedevere-botbedevere-bot removed the 🔨 test-with-refleak-buildbotsTest PR w/ refleak buildbots; report in status section labelMay 6, 2025
@AA-Turner

This comment was marked as resolved.

@encukou
Copy link
Member

Argh!
The header file puts it in an#ifdef ZDICT_STATIC_LINKING_ONLY block, saying it's experimental. But,configure looks for the exported function symbol.

@AA-Turner
Copy link
MemberAuthor

cc@encukou@vstinner I'm confused by this error as the struct has existed with the members that we need sincev0.7 (2016) of libzstd. Do you have any suggestions?

A

@AA-Turner

This comment was marked as duplicate.

@AA-Turner
Copy link
MemberAuthor

Is it worth setting theZDICT_STATIC_LINKING_ONLY define? The other option is to remove the fallback symbol detection and have autoconf just check for >=1.4.5.

@encukou
Copy link
Member

IMO, the>=1.4.5 check is safer.

@AA-TurnerAA-Turner added the 🔨 test-with-refleak-buildbotsTest PR w/ refleak buildbots; report in status section labelMay 6, 2025
@bedevere-bot

This comment was marked as resolved.

@bedevere-botbedevere-bot removed the 🔨 test-with-refleak-buildbotsTest PR w/ refleak buildbots; report in status section labelMay 6, 2025
@AA-Turner
Copy link
MemberAuthor

For reference,v1.4.4 and earlier guarded the symbol behind a define. It was stabilised (facebook/zstd#2111) in v1.4.5.

@AA-Turner

This comment was marked as outdated.

@bedevere-bot

This comment was marked as outdated.

@AA-Turner
Copy link
MemberAuthor

free-threaded tsan failure is spurious:1 test failed: test_asyncio.test_free_threading

@encukou
Copy link
Member

encukou commentedMay 6, 2025•
edited
Loading

test_datetime's[24, 24, 24] leaks are also spurious: this PR doesn't include#133498. I checked:

  • buildbot/AMD64 CentOS9 NoGIL Refleaks PR
  • buildbot/ARM64 MacOS M1 Refleaks NoGIL PR
  • buildbot/PPC64LE RHEL8 Refleaks PR

@AA-Turner
Copy link
MemberAuthor

The buildbots don't seem to have scheduled

@AA-Turner

This comment was marked as resolved.

@bedevere-bot

This comment was marked as resolved.

@hugovk
Copy link
Member

The buildbots don't seem to have scheduled

Same thing happened at#133497 (comment). Trying again worked.

@AA-Turner
Copy link
MemberAuthor

I'm still not seeing e.g. 'AMD64 RHEL8 LTO PR' in the pending checks list?

@AA-Turner
Copy link
MemberAuthor

buildbot/PPC64LE RHEL8 Refleaks PR failures are the leaks:

test_datetime leaked [24, 24, 24] memory blocks, sum=72FAIL: test_interrupt (test.test_multiprocessing_spawn.test_processes.WithProcessesTestProcess.test_interrupt)Re-running test.test_multiprocessing_spawn.test_processes in verbose mode (matching: test_interrupt)Re-running test_datetime in verbose modetest_datetime leaked [24, 24, 24] memory blocks, sum=72

@encukou
Copy link
Member

AMD64 RHEL8 FIPS No Builtin Hashes isunstable; failed in the usual way here.

@hugovk
Copy link
Member

Looking good so far, let's merge. Thanks both!

@hugovkhugovk merged commitf869190 intopython:mainMay 6, 2025
56 of 67 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@encukouencukouAwaiting requested review from encukou

@hugovkhugovkAwaiting requested review from hugovk

@erlend-aaslanderlend-aaslandAwaiting requested review from erlend-aaslanderlend-aasland is a code owner

@corona10corona10Awaiting requested review from corona10corona10 is a code owner

Assignees

@hugovkhugovk

Labels
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@AA-Turner@bedevere-bot@encukou@hugovk
[8]ページ先頭
©2009-2025 Movatter.jp