Fix min-versions; add min-version CI check

[Finomnis]

In an ongoing quest of mine to run a min-versions test for imxrt-uart-panic, I stumbled across incorrect minimal versions in this crate.

I also added a CI check that makes sure versions are updated in the future.

[mciantyre]

I stumbled across incorrect minimal versions in this crate.

I don't see why today's cortex-m and usb-device versions are incorrect. If I pin these two dependencies to something old, like

[dependencies]
cortex-m = "=0.7.0"
usb-device = "=0.3.0"

run cargo update and build the package with cargo build, then everything succeeds. If imxrt-usbd truly required the minimum versions suggested here, then I would have expected my test build to fail.

What am I missing?

[Finomnis]

Reverting the direct dependencies sadly isn't enough, for a full min-version check the dependencies of dependencies also need to be reverted to their minimal semver compatible version.

---

For cortex-m:

It seems that bare-metal had an error with newer Rust compilers that got fixed in bare-metal 0.2.3.
The min-version of bare-metal is currently 0.2.1, though. That's why cortex-m bumped it in cortex-m 0.7.4.

The exact error with =0.7.0 looks to me like this:

> cargo minimal-versions check
info: running `cargo hack check`
info: running `cargo check` on imxrt-usbd (1/1)
    Checking bare-metal v0.2.1
error[E0557]: feature has been removed
 --> C:\Users\Martin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\bare-metal-0.2.1\src\lib.rs:7:13
  |
7 |     feature(const_fn, const_unsafe_cell_new)
  |             ^^^^^^^^ feature has been removed
  |
  = note: split into finer-grained feature gates

error[E0554]: `#![feature]` may not be used on the stable release channel
 --> C:\Users\Martin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\bare-metal-0.2.1\src\lib.rs:7:23
  |
7 |     feature(const_fn, const_unsafe_cell_new)
  |                       ^^^^^^^^^^^^^^^^^^^^^

Some errors have detailed explanations: E0554, E0557.
For more information about an error, try `rustc --explain E0554`.
error: could not compile `bare-metal` (lib) due to 2 previous errors
error: process didn't exit successfully: `\\?\C:\Users\Martin\.rustup\toolchains\stable-x86_64-pc-windows-msvc\bin\cargo.exe check --manifest-path Cargo.toml` (exit code: 101)
error: process didn't exit successfully: `\\?\C:\Users\Martin\.rustup\toolchains\stable-x86_64-pc-windows-msvc\bin\cargo.exe hack check` (exit code: 1)

---

For usb-device:

The error only occurs when running cargo test, although it's debatable if tests should influence min-versions.
It's actually a failing doctest:

running 6 tests
test src\lib.rs - Peripherals (line 73) - compile ... ok
test src\bus.rs - bus::BusAdapter (line 46) - compile ... FAILED
test src\gpt.rs - gpt (line 13) - compile ... FAILED
test src\buffer.rs - buffer::EndpointMemory (line 20) ... ok
test src\state.rs - state::EndpointState (line 74) ... ok
test src\state.rs - state::EndpointState (line 90) ... ok

failures:

---- src\bus.rs - bus::BusAdapter (line 46) stdout ----
error[E0433]: failed to resolve: use of undeclared type `StringDescriptors`
  --> src\bus.rs:69:16
   |
26 |     .strings(&[StringDescriptors::default().product("imxrt-usbd")]).unwrap()
   |                ^^^^^^^^^^^^^^^^^ use of undeclared type `StringDescriptors`
   |
help: consider importing this struct
   |
2  + use usb_device::device::StringDescriptors;
   |

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0433`.
Couldn't compile the test.
---- src\gpt.rs - gpt (line 13) stdout ----
error[E0433]: failed to resolve: use of undeclared type `StringDescriptors`
  --> src\gpt.rs:47:16
   |
37 |     .strings(&[StringDescriptors::default().product("imxrt-usbd")]).unwrap()
   |                ^^^^^^^^^^^^^^^^^ use of undeclared type `StringDescriptors`
   |
help: consider importing this struct
   |
2  + use usb_device::device::StringDescriptors;
   |

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0433`.
Couldn't compile the test.

failures:
    src\bus.rs - bus::BusAdapter (line 46)
    src\gpt.rs - gpt (line 13)

test result: FAILED. 4 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in 1.18s

It seems that StringDescriptors got added to the prelude in usb_device 0.3.1

[Finomnis]

I yet have to find out what's up with the proc_macro_span_shrink, this doesn't happen on my Windows machine. Gotta debug on a linux machine, I'm on it.

[Finomnis]

@mciantyre Think I fixed it. It was accidentally using the nightly toolchain to do the main MSRV check, and it seems like there is a bug in the current nightly.

[mciantyre]

The error only occurs when running cargo test, although it's debatable if tests should influence min-versions.

We're free to introduce a development dependency that requires a minimum usb-device of 0.3.1. That solves the cargo test issue. Since it's a development dependency, this kind of minimum version doesn't impact all imxrt-usbd users.

Let's focus on cortex-m for simplicity. Consider a user who wants to specifically use cortex-m 0.7.0 throughout their entire build. They pin the dependency in their program's Cargo.toml. If we adopt this change to require at least version 0.7.4, I'm concerned we'll break this user's build.

I pushed a CI job to demonstrate the concern. Why should we prevent this user from building their firmware, especially when we could support their build?

[Finomnis]

The error only occurs when running cargo test, although it's debatable if tests should influence min-versions.

We're free to introduce a development dependency that requires a minimum usb-device of 0.3.1. That solves the cargo test issue. Since it's a development dependency, this kind of minimum version doesn't impact all imxrt-usbd users.

Let's focus on cortex-m for simplicity. Consider a user who wants to specifically use cortex-m 0.7.0 throughout their entire build. They pin the dependency in their program's Cargo.toml. If we adopt this change to require at least version 0.7.4, I'm concerned we'll break this user's build.

I pushed a CI job to demonstrate the concern. Why should we prevent this user from building their firmware, especially when we could support their build?

OK, I get your point now.

I guess in the end this is a convention. If you don't really use the features that caused cortex-m to go from0.6 to 0.7, you could also argue that you should instead define cortex-m = ">= 0.6".

crates.io gives the recommendation that you should always define the full version.

On the one side you might force a user to update a package by specifying a newer version. On the other hand, if you don't specify it, you risk that when the user updates only your package, its dependencies don't get properly updated. It's an opinion thing in the end.

For me the big plus is that if a project has a min version check and at some point dependencies break because of their msrv or similar, I can always revert to the min versions and it is guaranteed to work again.

Otherwise you need to find the right intermediate versions that work, and for a project with 30+ (indirect) this can be a pain (been there).

[Finomnis]

@mciantyre I actually have a good idea that kind of fixes the issues that you have with both of my PRs. What if we combine the two checks? Like, a min-version check on the MSRV.

This guarantees that dependencies cannot break the MSRV through updates, and gives the user a known Rust compiler + dependencies version that will work now and always.

What do you think?

It makes a lot of sense to me that the minimal version of Rust required to build this crate is achieved with the oldest dependency versions.

[Finomnis]

Replaced by #28.