Skip to content

Conversation

@lukapeschke
Copy link
Contributor

@lukapeschke lukapeschke commented Dec 10, 2025

This avoids having several versions of the zip crate in big projects that have several transitive dependencies on it.

Also, it would allow us to address ToucanToco/fastexcel#413 in fastexcel once zip-rs/zip2#483 is merged

This avois having several versions of the `zip` crate in big projects
that have several transitive dependencies on it.

Also, it would allow us to address ToucanToco/fastexcel#413 in fastexcel
once zip-rs/zip2#483 is merged
@jmcnamara
Copy link
Collaborator

Yes. Will do. The next release will bump all possible dependency versions. See #586 (comment)

The next release should be before the end of December.

@lukapeschke
Copy link
Contributor Author

Thanks! Added a comment there :) Closing as this is no longer required

@sftse
Copy link
Contributor

sftse commented Dec 10, 2025

This would bias cargo to merge dependencies which according to their semver are not compatible. This could lead to compile errors in transitive dependencies of the sort that are quite unobvious how to fix. Was this the intent of this PR?

@lukapeschke
Copy link
Contributor Author

@sftse I'm not sure I really understand your remark, do you have an example in mind where this could happen ?

How I see things is: calamine would declare a dependency on zip>=4, which allows us to use zip 4x -> 6.x, as we know that we are compatible with these versions. If at some point zip 7 is released and breaks calamine on master, it will be caught by CI, and we can decide if we want to add an upper constraint on zip or support the newer version and raise the lower constraint.

Now if by upgrading calamine in a project, zip gets upgraded, and this breaks dependency-x, then it's a bug in dependency-x, which should put stricter constraints on the version of zip it depends on.

Am I missing something ?

@sftse
Copy link
Contributor

sftse commented Dec 11, 2025

The default version contraint of just a number puts the onus on the dependency, here zip, not to break its consumers, here calamine. It wasn't obvious from the PR whether the downsides were considered since only the benefits were mentioned.
Relaxing these constraints shifts more responsibilities to the consumer, here we would need to test at least versions 4, 5 and 6, of which only 4 is tested on master and 6 which is what the CI tested in this PR. I don't know whether 5 works since I'm on mobile.

Another thing is the missing version upper bound, if zip releases a breaking v7 there is a time window within which, if we don't have a PR that runs in CI, users using both calamine and zip v7 would experience a build failure that can be very frustrating depending on how deep in the dependency graph this conflict occurs. For that reason alone, I would be very against the missing upper bound and we should only update the bound to include zip v7 once we have made sure it works.

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.

3 participants