search

atsams-rs / atsamx7x-rust

Rust HAL, PACs, and examples for the Microship SAM E70/S70/V70/V71
Apache License 2.0
24 stars 11 forks source link

Repository Is Locked To rustc 1.63 #57

Closed martinmortsell closed 9 months ago

martinmortsell commented 10 months ago

As the title says.

The compiler version seems to be set in pac/templates/Cargo.toml.template effectively causing the entire project to be stuck with a compiler version that is just over a year old at this point.

martinmortsell commented 10 months ago

Lol, jk. It seems to be rust-toolchain.toml that locks the compiler version.

michalfita commented 10 months ago

I think @tmplt added this for CI. I don't have problems with using newer compiler.

Regarding PACs - these are reworked in another repository. They build and my changes to this repo build with them, but I haven't time to polish out the PR yet.

martinmortsell commented 10 months ago

This came up in the Rust Embedded Matrix room where the toolchain version was causing issues with cargo-embed making them unable to run the examples.

I guess the crate should be built and tested with whatever the MSRV is (1.63.0 in this case).

michalfita commented 10 months ago

This file is rustup's pinning: https://rust-lang.github.io/rustup/overrides.html

I don't mind if it's removed. Maybe PR this removal and ask @tmplt for review. Cargo.toml holds the minimum version.

blietaer commented 10 months ago

Ah, you were faster than me @martinmortsell, thanks for reporting this.

michalfita commented 10 months ago

More precisely we can remove channel = from this file, as the remaining part is still sound.

BTW. by the looks of it, rustup override should overcome the pinning in there, but I haven't tried.

martinmortsell commented 10 months ago

From what I can tell after looking at other crates for 5 minutes, the preferred way is to have the MSRV specified in the CI process and not in a root level toml file like this.

e.g. in .github/workflows/ci.yml for cortex-m or ci.sh for smoltcp

tmplt commented 10 months ago

The pin is only there for the CI, so we might as well only specify it there.