MSRV is not actually enforced

[luisschwab]

We are not actually enforcing the declared MSRV of 1.81.0. We need to introduce a dependency pinning mechanism (eg a ci/pin-msrv.sh script) and a msrv recipe that runs on CI.

Currently, CI only tests floresta-common with MSRV.

Floresta % cargo +1.81.0 build
    Updating crates.io index
     Locking 407 packages to latest compatible versions
      Adding allocator-api2 v0.2.21 (latest: v0.4.0)
      Adding anes v0.1.6 (latest: v0.2.1)
      [...]
      Adding wit-parser v0.244.0 (latest: v0.245.1)
      Adding zeromq-src v0.2.6+4.3.4 (latest: v0.3.6+4.3.5)
error: failed to download `time-core v0.1.8`

Caused by:
  unable to get packages from source

Caused by:
  failed to parse manifest at `/Users/luisschwab/.cargo/registry/src/index.crates.io-6f17d22bba15001f/time-core-0.1.8/Cargo.toml`

Caused by:
  feature `edition2024` is required

  The package requires the Cargo feature called `edition2024`, but that feature is not stabilized in this version of Cargo (1.81.0 (2dbb1af80 2024-08-20)).
  Consider trying a newer version of Cargo (this may require the nightly release).
  See https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#edition-2024 for more information about the status of this feature.

[JoseSK999]

Are you using the same Cargo.lock to build? Can you try also with --locked? Floresta locked that time-core crate to 0.1.6.

I printed the versions in CI and it says we are indeed using 1.81.0: https://github.com/JoseSK999/Floresta/actions/runs/22023966207/job/63637513204#step:10:1

[luisschwab]

I meant it fails when building the library crates. When we split off the repo's into a library repo and a binary repo, we won't have a Cargo.lock, so we'll need to pin dependencies that break in the manner I described above. We don't need to enforce MSRV for florestad and floresta-cli, since they're applications.

cargo +1.81.0 build --package floresta-wire
    Updating crates.io index
     Locking 407 packages to latest compatible versions
      Adding allocator-api2 v0.2.21 (latest: v0.4.0)
      [...]
      Adding zeromq-src v0.2.6+4.3.4 (latest: v0.3.6+4.3.5)
error: failed to download `time-core v0.1.8`

Caused by:
  unable to get packages from source

Caused by:
  failed to parse manifest at `/Users/luisschwab/.cargo/registry/src/index.crates.io-6f17d22bba15001f/time-core-0.1.8/Cargo.toml`

Caused by:
  feature `edition2024` is required

  The package requires the Cargo feature called `edition2024`, but that feature is not stabilized in this version of Cargo (1.81.0 (2dbb1af80 2024-08-20)).
  Consider trying a newer version of Cargo (this may require the nightly release).
  See https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#edition-2024 for more information about the status of this feature.

[JoseSK999]

I’ve been reading about this. Apparently it is no longer discouraged to commit a Cargo.lock in git for libraries. See Change in Guidance on Committing Lockfiles.

A big reason for the original recommendation was that libraries should be tested with the newest dependency releases, as many consumers will be using those. Now they say for MSRV and reproducibility it is quite useful to keep a lockfile, as long as we keep testing fresh deps. (We don’t test fresh deps in Floresta, we should add another feature matrix for that, and make explicit our current one is for MSRV).

Without a lockfile:

• A PR CI may fail not because our code changed behavior, but because a transitive dependency did, and we will waste time figuring out the reason (lockfile committed = we know these deps work, so PR failed because of our code).
• We would also have to pin MSRV deps with a manual script or something, as you point out. Again a transitive dep may break MSRV and our CI check would fail sporadically.

Having a MSRV lockfile is nice as we say “we compile and test MSRV with these precise deps”. rust-bitcoin does have two lockfiles for MSRV: Cargo-minimal.lock and Cargo-recent.lock. For us I think it is fine to keep recent dep versions that work in our MSRV (since it is assumed newer is better, but not sure).

Note: even if we commit to a lockfile for the libs, downstream users will be using their own lockfile. They will need to pin deps on their own if needed.

I think we also want contributors to use a recent stable toolchain (locally they test fresh deps, while on CI we should do that too), but they wouldn’t be able to update the MSRV lockfile. So I think it may make sense to have both: Cargo-msrv.lock for MSRV testing and Cargo.lock with recent/fresh transitive dep.

Cc @Davidson-Souza @csgui @jaoleal @moisesPompilio

[csgui]

I like the approach of adding a Cargo-msrv.lock for MSRV stuff. I think it is reasonable especially if MSRV guarantees are important for us. But I can also see some things that we need to take attention if we go this way.

My suggestions are:

• Explicitly document the purpose of each lockfile
• Add CI jobs for both MSRV (locked) and fresh deps
• Define a process for updating the MSRV lockfile to keep it in sync with the current dependency ecosystem