Skip to content
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

Do not use cloudflare-zlib-sys 0.3.4 #451

Merged
merged 1 commit into from
Feb 10, 2025

Conversation

jongiddy
Copy link
Contributor

@jongiddy jongiddy commented Feb 9, 2025

cloudflare-zlib-sys released a new version at the same time that tests started failing. Rather than stop testing cloudflare_zlib, can we pin to the previous version that succeeds in CI? With this restriction encoded in the Cargo.toml we avoid the possibility that this problem might be present outside GitHub CI runners and will cause real code to fail.

@joshtriplett
Copy link
Member

Pinning like this tends to be a really big hammer that can cause other problems.

If we have to do so, we can, but first, can we report this upstream and see if they can yank the broken version? (And then publish a new one once the issue is addressed.)

@Byron
Copy link
Member

Byron commented Feb 10, 2025

As a compromise, could 'patching the cloudflare-zlib-sys version on CI' be an avenue? That way tests could run while it's made clear what the problem is.
Alternatively, cloudflare could be allowed to fail so we see when it's fixed more easily?

@joshtriplett
Copy link
Member

joshtriplett commented Feb 10, 2025

It looks like cloudflare's zlib repo has issues turned off.

@kornelski, since you work on that repo, do you know the right place to report this? (Sorry for the ping, but the right place to report was not obvious.)

@kornelski
Copy link
Contributor

kornelski commented Feb 10, 2025

I'm looking into the problem. I've also opened issues on cloudflare-zlib repo.

@joshtriplett
Copy link
Member

@kornelski Thank you! Would you mind yanking the most recent version, until the issue is resolved?

@kornelski
Copy link
Contributor

Yanked

@kornelski
Copy link
Contributor

The problem was caused by CPU feature detection that made MSVC use AVX in non-AVX builds. I've released a fix.

Cargo.toml Outdated Show resolved Hide resolved
Avoid 0.3.4 which caused failures due to bad CPU feature detection.
@joshtriplett joshtriplett merged commit 0cb2749 into rust-lang:main Feb 10, 2025
10 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants