zenoh icon indicating copy to clipboard operation
zenoh copied to clipboard

Clap MSRV incompatible

Open samcarey opened this issue 1 year ago • 8 comments

Describe the bug

Zenoh 0.10.1-rc (and main) require Rust version 1.72.0 and specify a dependency on Clap of "4.4.11", which can now resolve to Clap version "4.5" (as of February 7th), which now has a minimum supported rust version of 1.74.

To reproduce

Run cargo update and then try to build zenoh:

error: package `clap v4.5.0` cannot be built because it requires rustc 1.74 or newer, while the currently active rustc version is 1.72.0Either upgrade to rustc 1.74 or newer, or use
cargo update -p [email protected] --precise ver
where `ver` is the latest version of `clap` supporting rustc 1.72.0

System info

N/A

samcarey avatar Feb 15 '24 19:02 samcarey

Hi @samcarey, I encountered the same problem as well. I think this is due to the limit of cargo, and a RFC has been proposed. https://github.com/clap-rs/clap/issues/4811#issuecomment-1490196509

Bumping up the rust toolchain and upgrading the dependencies require more checks. The modification of async runtime going to be added will change the dependence as well. I suggest let's fix the version number of clap at this moment. And add this issue in the roadmap to remind us to upgrade the toolchain. @Mallets , any comments are welcome.

YuanYuYuan avatar Feb 16 '24 03:02 YuanYuYuan

The problem is more rooted in Rust and cargo as it seems and it's not only specific to Zenoh. Bumping the Rust MSRV would work for Zenoh this time, but this doesn't prevent the fact that we could break downstream dependencies as well. Nevertheless, one could argue that those dependencies are already broken since they are not able to build Zenoh...

An alternative approach would be to pin every dependency to an exact version with =, but I'm not sure that would work out with the dependencies of a dependency.

Mallets avatar Feb 16 '24 08:02 Mallets

The problem is more rooted in Rust and cargo as it seems and it's not only specific to Zenoh. Bumping the Rust MSRV would work for Zenoh this time, but this doesn't prevent the fact that we could break downstream dependencies as well. Nevertheless, one could argue that those dependencies are already broken since they are not able to build Zenoh...

An alternative approach would be to pin every dependency to an exact version with =, but I'm not sure that would work out with the dependencies of a dependency.

I found that a new crate with clap >= 4.5.1 still can't compile even if we freeze the clap version inside zenoh to be less than 4.4.18.

YuanYuYuan avatar Feb 20 '24 11:02 YuanYuYuan

I found that a new crate with clap >= 4.5.1 still can't compile even if we freeze the clap version inside zenoh to be less than 4.4.18.

I was looking at the dependencies and clap is a dependency of zenohd and examples, so a create that depends on zenoh should not be impacted by this.

So the problem is when building zenohd right? Not a crate that depends on zenoh

If this is the case using = to fix the version should solve the issue, tbh I do not understand why a crate should depend on zenohd

gabrik avatar Feb 20 '24 11:02 gabrik

I found that a new crate with clap >= 4.5.1 still can't compile even if we freeze the clap version inside zenoh to be less than 4.4.18.

I was looking at the dependencies and clap is a dependency of zenohd and examples, so a create that depends on zenoh should not be impacted by this.

So the problem is when building zenohd right? Not a crate that depends on zenoh

If this is the case using = to fix the version should solve the issue, tbh I do not understand why a crate should depend on zenohd

To be clear, building either zenohd or any example depending on clap is fine. The problem is that users can't compile zenoh once they make a cargo update (inside zenoh repository) since it would upgrade the clap to 4.5.* and break the current MSRC used in zenoh.

I've also encountered this problem while developing on tokio-porting since I was working "inside" the zenoh repository. However, normal users or developers are expected to call the zenoh library outside the zenoh repository, which is just fine after a quick check.

Saying, a plain crate with dependencies of current zenoh and clap >= 4.5.1 ( 4.4.* is also fine) can compile without any errors. Running a cargo update outside is also fine. IMO, this is still an issue, but it's "internal" that we need to keep in mind when upgrading our dependencies. (But don't worry, our CI can check this failure.)

YuanYuYuan avatar Feb 20 '24 11:02 YuanYuYuan

I found that a new crate with clap >= 4.5.1 still can't compile even if we freeze the clap version inside zenoh to be less than 4.4.18.

I was looking at the dependencies and clap is a dependency of zenohd and examples, so a create that depends on zenoh should not be impacted by this.

So the problem is when building zenohd right? Not a crate that depends on zenoh

If this is the case using = to fix the version should solve the issue, tbh I do not understand why a crate should depend on zenohd

The Cargo Binary Dependencies RFC is already in nightly Rust under -Z bindeps. It's supposed to allow crates to use Cargo binary packages in tests and build scripts.

I reproduced this kind of scenario here.

Even if we decide that this is an edge case not worth supporting at the moment and decide to go ahead with pinning the minor version, we should document it in the README.

In any case, using clap = "~4.4" is strictly better than clap = "=4.4.11" because the former leaves the patch flexible (so it accepts bugfixes and decreases the chances of conflicts). Increases in the MSRV of a crate are not considered SemVer-incompatible changes by Cargo (and the Rust community), the official policy is to treat this as a minor change. So for ensuring that dependency MSRVs don't cause issues like this anymore, a "Tilde" requirement should be enough.

As far as crate authors are concerned, only the latest Rust compiler is supported, so we will routinely run into these issues as our Rust toolchain (currently 1.72) gets older. Perhaps we should also put the choice of the toolchain into question.

fuzzypixelz avatar Feb 20 '24 11:02 fuzzypixelz

I found that a new crate with clap >= 4.5.1 still can't compile even if we freeze the clap version inside zenoh to be less than 4.4.18.

I was looking at the dependencies and clap is a dependency of zenohd and examples, so a create that depends on zenoh should not be impacted by this. So the problem is when building zenohd right? Not a crate that depends on zenoh If this is the case using = to fix the version should solve the issue, tbh I do not understand why a crate should depend on zenohd

The Cargo Binary Dependencies RFC is already in nightly Rust under -Z bindeps. It's supposed to allow crates to use Cargo binary packages in tests and build scripts.

I reproduced this kind of scenario here.

Even if we decide that this is an edge case not worth supporting at the moment and decide to go ahead with pinning the minor version, we should document it in the README.

In any case, using clap = "~4.4" is strictly better than clap = "=4.4.11" because the former leaves the patch flexible (so it accepts bugfixes and decreases the chances of conflicts). Increases in the MSRV of a crate are not considered SemVer-incompatible changes by Cargo (and the Rust community), the official policy is to treat this as a minor change. So for ensuring that dependency MSRVs don't cause issues like this anymore, a "Tilde" requirement should be enough.

As far as crate authors are concerned, only the latest Rust compiler is supported, so we will routinely run into these issues as our Rust toolchain (currently 1.72) gets older. Perhaps we should also put the choice of the toolchain into question.

It's interesting that users can introducing zenohd into the test. And I'd like to point out that the native zenoh library right now can pass @fuzzypixelz 's PoC script since the reason to the failure is basically the same as the case I mentioned above.

In summary, we should NOT pin the version of clap at this moment, otherwise the PoC above or any other crates depending on zenoh and clap >= 4.5 at the same time would fail to compile. I think we can keep the Cargo.toml at this moment and upgrade the rust toolchain in the near future.

YuanYuYuan avatar Feb 20 '24 13:02 YuanYuYuan

@samcarey May I ask why do you need to update the lockfile?

fuzzypixelz avatar Feb 26 '24 11:02 fuzzypixelz

Closing as outdated. Feel free to re-open it in case it is still relevant.

Mallets avatar Jul 04 '24 07:07 Mallets