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

Pinning zvariant_utils makes it impossible to resolve version conflicts #652

Closed
PolyMeilex opened this issue Feb 22, 2024 · 21 comments · Fixed by #653
Closed

Pinning zvariant_utils makes it impossible to resolve version conflicts #652

PolyMeilex opened this issue Feb 22, 2024 · 21 comments · Fixed by #653

Comments

@PolyMeilex
Copy link

PolyMeilex commented Feb 22, 2024

zvariant_utils = { path = "../zvariant_utils", version = "=1.0.1" }

zvariant_utils = { path = "../zvariant_utils", version = "=1.1.0" }

zbus 4 pins 1.1, zbus 3 pins 1.0, and both of those versions are part of the same semver 1.x.x so cargo is not able to resolve the version conflict.

(most likely) causes: PolyMeilex/rfd#180

repro:

[package]
name = "crate_that_uses_4_and_3"
version = "0.1.0"
edition = "2021"

[dependencies]
zbus_macros = "4"
zbus_macros3 = { package = "zbus_macros", version = "3.15"}
@zeenix
Copy link
Contributor

zeenix commented Feb 22, 2024

Not sure what we can do about that. Best option is to port the whole stack to zbus 4 or don't pin zvariant_utils.

I'm happy to reopen it if you have any suggestions on how we can help.

@zeenix zeenix closed this as not planned Won't fix, can't repro, duplicate, stale Feb 22, 2024
@zeenix
Copy link
Contributor

zeenix commented Feb 22, 2024

Oh it's zbus pinning it. Sorry I missed that. Still, I don't know what I can do about this. The pinning is done for good reasons.

@PolyMeilex
Copy link
Author

PolyMeilex commented Feb 22, 2024

Sure I get that there are probably reasons for that, but this breaks for people as I updated my lib to zbus 4, but the accessibility stack is and will remain on zbus 3 for some time because of MSRV reasons: odilia-app/atspi#155 (comment)

So I'm stuck with my lib being unusable because I updated zbus to early 😄 (for people that want to have accessibility in their apps, that is)

Even if we can't address that, it would be cool to somehow prevent that in the future (aka. once zbus 5 is a thing)
Could you explain reasons for the pining rather than just letting it bump to 1.1? So I can better understand what is the problem at hand.

@zeenix
Copy link
Contributor

zeenix commented Feb 22, 2024

accessibility stack is and will remain on zbus 3 for some time because of MSRV reasons: odilia-app/atspi#155 (comment)

They sound a lot less sure about that in their next comment so hopefully they'll be able to bump it in the near future.

So I'm stuck with my lib being unusable because I updated zbus to early 😄 (for people that want to have accessibility in their apps, that is)

You're ahead of your time. Nothing wrong with that. :)

Could you explain reasons for the pining rather than just letting it bump to 1.1?

TBH I don't remember if zvariant_utils itself has a good reason to be pinned. I do know that we pin the macro crates to ensure compatibility w/o introducing a cyclic dep between the main and the macro crate. It's entirely possible that we just followed the trend. Two things to consider here:

  1. Don't we have the same issue with zbus_macros and zbus? If so, we've a good reason there and unpinning zvariant_utils will not get us far.
  2. There is a good chance we'll add macros in zvariant_utils. If we don't have a good reason to pin now, we will likely do then. Since we're thinking of future major releases, this will then again be an issue but unpinning would then not be a solution.

@zeenix zeenix reopened this Feb 22, 2024
@PolyMeilex
Copy link
Author

PolyMeilex commented Feb 23, 2024

Don't we have the same issue with zbus_macros and zbus? If so, we've a good reason there and unpinning zvariant_utils will not get us far.

Macros crate had a breaking version bump, so cargo should not treat it as a single dep, and everything should work as expected.

Utils on the other hand had only a minor bump, therefore cargo treats it as a same dep.

Aka.
This is totally fine to do:

[dependencies]
zbus_macros = "=4.0.0"
zbus_macros2 = { package = "zbus_macros", version = "=3.0.0"}

This is not:

[dependencies]
zvariant_utils = "=1.0.1"
zvariant_utils2 = { package = "zvariant_utils", version = "=1.1.0"}

@zeenix
Copy link
Contributor

zeenix commented Feb 23, 2024

Macros crate had a breaking version bump, so cargo should not treat it as a single dep, and everything should work as expected.

Ah right, we release the macro crates in tandem.

Utils on the other hand had only a minor bump, therefore cargo treats it as a same dep.

Ok then. Let's unpin for now.

zeenix added a commit to zeenix/zbus that referenced this issue Feb 23, 2024
Otherwise, it breaks the build for folks who happen to have both zbus 3
and 4 in their dependency tree.

Fixes dbus2#652.
@zeenix
Copy link
Contributor

zeenix commented Feb 23, 2024

@PolyMeilex I made releases now so cargo update should fix your issue.

@PolyMeilex
Copy link
Author

PolyMeilex commented Feb 23, 2024

Thanks!
zbus 3 has version 1.0.1 pined, zbus 4 is now unpinned and sets the minimal version to 1.1.0, and as 1.1.0 > 1.0.1 this will still fail to resolve, as zbus 3 is pinned to a version lower than the minimal required by zbus 4

@zeenix
Copy link
Contributor

zeenix commented Feb 23, 2024

zbus 3 has version 1.0.1 pined, zbus 4 is now unpinned and sets the minimal version to 1.1.0, and as 1.1.0 > 1.0.1 this will still fail to resolve, as zbus 3 is pinned to a version lower than the minimal required by zbus 4

Ughh. I can't catch a break. :) Let me unpin for zbus 3 as well..

zeenix added a commit that referenced this issue Feb 23, 2024
Otherwise, it breaks the build for folks who happen to have both zbus 3
and 4 in their dependency tree.

Fixes #652.
@zeenix
Copy link
Contributor

zeenix commented Feb 23, 2024

Ughh. I can't catch a break. :) Let me unpin for zbus 3 as well..

Done.

@PolyMeilex
Copy link
Author

Done.

Nice, works like a charm. Thank you for quickly resolving this!

@zeenix
Copy link
Contributor

zeenix commented Feb 24, 2024

Done.

Nice, works like a charm. Thank you for quickly resolving this!

I'm very glad to hear it. 👍

@lunixbochs
Copy link

lunixbochs commented Feb 24, 2024

I think this bumped the msrv on zbus3 to 1.75, zbus_macros and zvariant_derive v3.15.1 are now pulling in zvariant_utils 1.1.0 for me

@PolyMeilex
Copy link
Author

PolyMeilex commented Feb 24, 2024

FYI you can pin it yourself, but yeah that turns out to be a braking change, it's a bummer that cargo is not smart enough to not bump above project's MSRV 😬

EDIT: I was assuming that zbus treats MSRV bump as a braking change, so I did not expect that between 1.0 and 1.1

@zeenix
Copy link
Contributor

zeenix commented Feb 25, 2024

I think this bumped the msrv on zbus3 to 1.75, zbus_macros and zvariant_derive v3.15.1 are now pulling in zvariant_utils 1.1.0 for me

Ughh.. this was totally accidental. :( we could change the MSRV in zvariant_utils though but then it would be out of sync with other crates in the repo and adds to maintenance burden. This also shows why pinning the version was after all a good idea.

So unless someone has a better suggestion, I'm going to revert these changes.

@PolyMeilex
Copy link
Author

PolyMeilex commented Feb 25, 2024

IMO if we are assuming that MSRV change in zbus is a semver braking change, then we should apply the same rule across the whole stack. Aka. If bumping MSRV in zbus is considered braking, so should be bumping MSRV in zvariant.

I know that zvariant is not as widely used directly and that's probably why it has looser semver rules, but still, applying the same rules could help. (Although I have one project that uses it directly without zbus, so it could benefit from that as well)

@zeenix
Copy link
Contributor

zeenix commented Feb 25, 2024

IMO if we are assuming that MSRV change in zbus is a semver braking change,

Not exactly. 3.x series is now in maintenance mode and that's why we want to avoid breaking anything for users of 3.x, not because it's considered a breaking change per say.

If bumping MSRV in zbus is considered braking, so should be bumping MSRV in zvariant.

Sure. What ends up bumping the MSRV is zvariant_utils.

@zeenix
Copy link
Contributor

zeenix commented Feb 25, 2024

Also, at least I consider bumping MSRV in micro/patch release a breaking change, which is what I did. 🤦

@zeenix
Copy link
Contributor

zeenix commented Feb 25, 2024

I think this bumped the msrv on zbus3 to 1.75, zbus_macros and zvariant_derive v3.15.1 are now pulling in zvariant_utils 1.1.0 for me

Ughh.. this was totally accidental. :( we could change the MSRV in zvariant_utils though but then it would be out of sync with other crates in the repo and adds to maintenance burden. This also shows why pinning the version was after all a good idea.

So unless someone has a better suggestion, I'm going to revert these changes.

3.15.2 releases out with unpinning change reverted. I'll now do the same for 4.x since unpinnng in 4.x only doesn't help with the issue in question.

@PolyMeilex I'm terribly sorry but I think we just have to live with the fact that the whole stack needs to move to 4.x at the same time.

@PolyMeilex
Copy link
Author

Sure, I suppose it's not my problem, I'll just redirect people to complain to zbus 3 based libs and call it a day 😄
Thanks anyway! ❤️

@zeenix
Copy link
Contributor

zeenix commented Feb 25, 2024

Sure, I suppose it's not my problem, I'll just redirect people to complain to zbus 3 based libs and call it a day 😄

👍

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 a pull request may close this issue.

3 participants