Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"make test" fails #394

Open
dcz-self opened this issue Mar 10, 2022 · 4 comments
Open

"make test" fails #394

dcz-self opened this issue Mar 10, 2022 · 4 comments

Comments

@dcz-self
Copy link
Contributor

"make test-stable" fails here after running "make setup":

[root@5b7ac846a2fc libtock-rs]# make test
[...]
CARGO_TARGET_DIR="target/stable-toolchain" LIBTOCK_PLATFORM=nrf52 cargo \
        +stable check --exclude libtock_unittest --exclude print_sizes --exclude runner --exclude syscalls_tests --target=thumbv7em-none-eabi --workspace
    Checking libtock_runtime v0.1.0 (/mnt/source/libtock-rs/runtime)
error[E0658]: use of unstable library feature 'global_asm': `global_asm!` is not stable enough for use and is subject to change
 --> runtime/src/startup/mod.rs:6:1
  |
6 | core::arch::global_asm!(include_str!("asm_arm.s"));
  | ^^^^^^^^^^^^^^^^^^^^^^

The issue is in the "+stable" in

+stable check $(EXCLUDE_STD) --target=thumbv7em-none-eabi --workspace

I'm not sure how to get the rustc version that is in use through this command. Perhspa make setup should update it to some pre-approved version.

@jrvanwhy
Copy link
Collaborator

You should be able to determine your toolchain version by running cargo +stable --version. The version you currently need is 1.59.0 (the latest stable version).

There are two changes we could make that may improve the situation:

  1. Add rustup toolchain update stable to make setup, as you suggested.
  2. Replace +stable with +1.59.0, requiring the user to have a specific toolchain installed.

Option 1 has the drawbacks of making CI slower and making make setup a bit more intrusive on the host system. Also, make setup is a command you run once, while the required Rust version will go up from time to time, so we'll still encounter errors like the one you encountered. Of course, there is some benefit in making sure it works the first time a new contributor runs make test, even if it still breaks for those of us who have been working on libtock-rs for a while.

Option 2 would effectively declare a "minimum supported Rust version" (MSRV for short). That has some benefits of its own. The drawback is that by specifying a particular Rust version, libtock-rs developers will need to manually install a new toolchain every time the MSRV is increased, which is higher-effort.

Between the above two options as well as the implicit third option of "leave things as-is" (possibly with better documentation?), I'm not sure what to do.

@dcz-self
Copy link
Contributor Author

Thanks, I had 1.58 installed.

Given that the container I use for Tock already has several Rusts and just crossed 6GiB, marking itself for extermination, I think pinning to a version like in 2. makes more sense.

I would also add a target "update" for when the version changes and the user (like me) doesn't know how to use rustup.

Does that sound good?

@jrvanwhy
Copy link
Collaborator

Given that the container I use for Tock already has several Rusts and just crossed 6GiB, marking itself for extermination, I think pinning to a version like in 2. makes more sense.

Minimizing install size is an argument for option 1, not option 2. In option 1, you keep one toolchain (stable) and update it from time to time, whereas in option 2 the developer would end up with an increasing number of toolchains, unless they manually clean them up.

I would also add a target "update" for when the version changes and the user (like me) doesn't know how to use rustup.

That seems reasonable to me.

@dcz-self
Copy link
Contributor Author

Here I thought that updating one version of stable to another would keep the older versions. As another argument against 2., I think I somehow ended up with 1.59.0 unstable where I had some other version before. I'm going to have to get a little more comfortable with rustup before attempting to fix this.

bradjc added a commit that referenced this issue Aug 10, 2023
This seems to be pretty standard with Rust crates.

I don't actually know that we don't need a newer version, but #394
indicates we need 1.59.

Fixes #394.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants