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

deps: relax zip dependency to allow building with v0.6 and v1 #14

Merged
merged 2 commits into from
May 21, 2024
Merged

deps: relax zip dependency to allow building with v0.6 and v1 #14

merged 2 commits into from
May 21, 2024

Conversation

decathorpe
Copy link
Contributor

This crate is compatible with both zip v0.6 and zip v1. Crate ownership changed between these two release streams and maintenance of the v1 branch has been questionable so far. For people who would like to stick with zip v0.6 for the time being, both versions currently work.

Tests pass when pinning "zip" to v0.6.6. Tests also pass with both -Zminimal-versions and -Zdirect-minimal-versions when using latest stable Rust with one minor change - bumping the minimum flate2 dependency from 1.0.20 to 1.0.22.

Note that "-Zminimal-versions" fails on Nightly Rust due to a problem in an unrelated crate: The version of ahash pulled in via rfc2047-decoder v1.0.5 -> chumsky v0.9.0 -> hashbrown v0.12.3 -> ahash v0.7.7 is using an unstable feature that is no longer present in nightly Rust (stdsimd). I see that the latest version of chumsky (v0.9.3) depends on hashbrown v0.14 instead, so bumping the chumsky dependency in rfc2047-decoder might resolve that issue.

Fixes #13

@adamreichold
Copy link
Member

Just wanted to chime in as we have a similar semver-spanning dependency in rust-numpy, c.f. https://github.com/PyO3/rust-numpy?tab=readme-ov-file#dependency-on-ndarray, and it is not the nicest thing to have as it can require manual unification with other dependency specifications in downstream crates.

So just using 0.6 as the requirement until the problems with zip 1.x are resolved might be simpler.

As an aside, was the whole "zip situation" discussed on URLO or somewhere else where one could read up on it?

@decathorpe
Copy link
Contributor Author

Both >=0.6,<2 and reverting to 0.6 temporarily would be fine with me.

This RUSTSEC issue has a good summary of the situation:
rustsec/advisory-db#1956

In particular, the discussion here is ... "interesting":
zip-rs/zip-old#446 (comment)

Copy link
Member

@konstin konstin left a comment

Choose a reason for hiding this comment

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

Thank you for this detailed investigation! I added testing for minimal version based on that in #16

@konstin konstin enabled auto-merge May 21, 2024 08:35
@konstin konstin merged commit bfd695b into PyO3:main May 21, 2024
6 checks passed
@decathorpe
Copy link
Contributor Author

Great, thank you!

@konstin
Copy link
Member

konstin commented May 21, 2024

In particular, the discussion here is ... "interesting":
zip-rs/zip-old#446 (comment)

Did you try verifying his identity or at least contacting him before implying him in some nefarious scheme? Personally, it would make me very uncomfortable if someone implied i was an infiltrator and started removing my open source contributions from other projects, these kinds of rumors can do a lot of damage.

@decathorpe
Copy link
Contributor Author

Sorry, I did not intend to imply any nefarious scheme. I can't verify the new crate maintainer's identity, but that's also not really the point. It's about lost trust, for multiple reasons (the way the transfer from zip-pre-1.0 to zip-post-1.0 was handled, pushing breaking API changes and not fixing them, pushing test data with questionable origins and/or associated licenses, etc.). Even if I could verify their identity, that would not magically restore my trust in the project.

@konstin
Copy link
Member

konstin commented May 21, 2024

@decathorpe Thanks for your reply, please consider also commenting at zip-rs/zip-old#446 (comment).

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.

concerns with the zip v1 update in version 0.6.1
3 participants