dbus2 / zbus

Rust D-Bus crate.
Other
383 stars 87 forks source link

Pinning `zvariant_utils` makes it impossible to resolve version conflicts #652

Closed PolyMeilex closed 8 months ago

PolyMeilex commented 8 months ago

https://github.com/dbus2/zbus/blob/a2e3d17358942b893d74bf593ffb95f5484b3b97/zbus_macros/Cargo.toml#L28 https://github.com/dbus2/zbus/blob/d9b367a4c44fb57bdf820eab6cc743e8e6f89570/zbus_macros/Cargo.toml#L28

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: https://github.com/PolyMeilex/rfd/issues/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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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: https://github.com/odilia-app/atspi/pull/155#issuecomment-1949132789

So I'm stuck with my lib being unusable because I updated zbus to early :smile: (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 commented 8 months ago

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.
PolyMeilex commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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

PolyMeilex commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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

Done.

PolyMeilex commented 8 months ago

Done.

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

zeenix commented 8 months ago

Done.

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

I'm very glad to hear it. 👍

lunixbochs commented 8 months ago

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 commented 8 months ago

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 :grimacing:

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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

zeenix commented 8 months ago

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 commented 8 months ago

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 :smile: Thanks anyway! :heart:

zeenix commented 8 months ago

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 😄

:+1: