Skip to content

feat!: zip 3.0: Adjust flate2-related features for 3.0 #352

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 8 commits into from
May 14, 2025

Conversation

joshtriplett
Copy link
Contributor

Move deflate-zlib to deflate-flate2-zlib to make it clear it's a
flate2 backend, and add deflate-flate2-zlib-rs to use the zlib-rs
backend. Make the latter the default.

Bump the required flate2 version to one that works with zlib-rs 0.5.0,
which removes all compatibility considerations by no longer exporting
any C symbols that would conflict with another zlib.

Remove deflate-zlib-ng, largely superseded by
deflate-flate2-zlib-rs.

Remove no-op backend-specific code from an example.

Add documentation for the flate2-related feature flags.

@joshtriplett joshtriplett changed the title zip 3.0: Adjust flate2-related features for 3.0 feat!: zip 3.0: Adjust flate2-related features for 3.0 May 13, 2025
Pr0methean
Pr0methean previously approved these changes May 13, 2025
@Pr0methean
Copy link
Member

Thanks! This looks good. Unfortunately, our meagre allotment of 20 GitHub Actions runners has to cope with an expanding matrix of architectures and feature-flag configurations that now includes 69 jobs for each pull request, and several PRs are already in the queue. So it may take a day or two to merge this.

@Pr0methean
Copy link
Member

I'll check through and close any PRs that this one clearly renders obsolete.

@joshtriplett
Copy link
Contributor Author

@Pr0methean I'll fix the CI failures on this PR.

@Pr0methean
Copy link
Member

@joshtriplett Looks like the Miri tests need their feature configurations updated in https://github.com/zip-rs/zip2/blob/master/.github/workflows/ci.yaml#L50.

@joshtriplett
Copy link
Contributor Author

joshtriplett commented May 13, 2025

Unfortunately, our meagre allotment of 20 GitHub Actions runners has to cope with an expanding matrix of architectures and feature-flag configurations that now includes 69 jobs for each pull request

Potential improvement for the future: I think it might be reasonable to reduce the matrix somewhat, to test features independently of targets. All targets should test --all-features to make sure that every feature compiles on every target, but other parts of the features matrix need only get tested on one target.

Also, caching would speed things up substantially, for the non-nightly jobs. https://github.com/Swatinem/rust-cache works quite well.

@joshtriplett
Copy link
Contributor Author

Rebasing atop 5542e5b to avoid stopping other CI jobs if semver-checks fails (which it will for this change).

Move `deflate-zlib` to `deflate-flate2-zlib` to make it clear it's a
flate2 backend, and add `deflate-flate2-zlib-rs` to use the zlib-rs
backend. Make the latter the default.

Bump the required flate2 version to one that works with zlib-rs 0.5.0,
which removes all compatibility considerations by no longer exporting
any C symbols that would conflict with another zlib.

Remove `deflate-zlib-ng`, largely superseded by
`deflate-flate2-zlib-rs`.

Remove no-op backend-specific code from an example.

Add documentation for the flate2-related feature flags.
@joshtriplett
Copy link
Contributor Author

(Also, I think 5542e5b didn't go far enough, because it only removed the dep for one of the four fuzz jobs.)

@Pr0methean
Copy link
Member

Pr0methean commented May 13, 2025

Fixed in 7cd9f0f. I look forward to the day when our CI workflow is stable (in the sense of not needing any changes) for long enough that we can factor out a job template or a matrix for the 4 fuzz jobs.

@Pr0methean Pr0methean added this to the 3.0.0 milestone May 13, 2025
@Pr0methean
Copy link
Member

Pr0methean commented May 13, 2025

@joshtriplett The doctests for finish_into_readable and merge_archive are failing. I've fixed their feature-flag configurations in master, but could you please fix them if they still fail?

@Pr0methean Pr0methean disabled auto-merge May 14, 2025 05:04
@Pr0methean Pr0methean enabled auto-merge May 14, 2025 05:04
@Pr0methean Pr0methean added this pull request to the merge queue May 14, 2025
Merged via the queue into zip-rs:master with commit 9231930 May 14, 2025
62 of 71 checks passed
@joshtriplett joshtriplett deleted the features branch May 14, 2025 09:47
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.

2 participants