eclipse-zenoh / ci

GitHub Actions and workflows used across eclipse-zenoh
Other
1 stars 3 forks source link

feat: Add sync-toolchains workflow #3

Closed fuzzypixelz closed 4 months ago

fuzzypixelz commented 8 months ago

This pull request adds a workflow that fetches the latest stable Rust release, and updates all eclipse-zenoh Rust toolchains to use it. The workflow is scheduled to run nightly.

Mallets commented 8 months ago

I kind of disagree to have a toolchain like that to run nightly and to automatically update the toolchain to the whole Rust project. We need to have a better control on that and to decide when bumping the toolchain. OSs and Distributions may package and ship a specific version of the rust toolchain and compiler that people rely on. Adding such CI action will break those scenarios too easily.

I'd rather prefer to have a CI that can be triggered manually to sync all the repos to a specific rust toolchain version once we decide it's the right time to bump the toolchain.

fuzzypixelz commented 8 months ago

I kind of disagree to have a toolchain like that to run nightly and to automatically update the toolchain to the whole Rust project. We need to have a better control on that and to decide when bumping the toolchain. OSs and Distributions may package and ship a specific version of the rust toolchain and compiler that people rely on. Adding such CI action will break those scenarios too easily.

I'd rather prefer to have a CI that can be triggered manually to sync all the repos to a specific rust toolchain version once we decide it's the right time to bump the toolchain.

I can change the workflow to take a Rust version as input, it's a trivial change.

However, this raises questions about how the Rust toolchain should be set.

  1. Arguably, it should be set to the MSRV of zenoh. This is a bit ill-defined as each crate in eclipse-zenoh has a different MSRV. For instance zenoh-collections's MSRV is 1.66.1 while zenoh's MSRV is 1.72.1 currently. And I don't see why out-of-tree plugins and backends will always share the same MSRV as zenoh. So the toolchain would be set to the maximum MSRV of all eclipse-zenoh crates, to ensure that the same toolchain is set everywhere.

  2. Calculating the MSRV depends on the lockfile. For instance, currently running cargo-msrv on the zenoh crate yields an MSRV of 1.72.1. However, if we delete the lockfile, then the MSRV becomes 1.74.1 instead! This is because the lockfile is not updated (if it were, we would be forced to update toolchain to 1.74.1).

This is why I sought to simplify all this by using the latest stable toolchain. Third parties can simply override the toolchain if they like (as long as they respect the MSRV). Moreover, crates.io ignores the toolchain altogether.

Here's an example of what can go wrong:

Such an issue would be avoided by either:

  1. Keeping the Rust toolchain higher than any dependency can set it (stable)
  2. Setting the Rust toolchain to the actual MSRV (i.e. assuming the lockfile is removed)
  3. Continuously updating the lockfile
fuzzypixelz commented 8 months ago

@Mallets I updated the workflow to take the Rust toolchain version as an input.