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

Panicked on new platforms (like riscv64gc) if MSRV is too old #585

Open
light4 opened this issue Jan 10, 2023 · 5 comments
Open

Panicked on new platforms (like riscv64gc) if MSRV is too old #585

light4 opened this issue Jan 10, 2023 · 5 comments

Comments

@light4
Copy link

light4 commented Jan 10, 2023

The dist riscv64gc became available from version 1.47.0, some tests like find_msrv will fail because rustup cannot download the old dist.

@SpriteOvO
Copy link

Full test log: cargo-msrv-0.15.1-3-riscv64-check.log

@foresterre
Copy link
Owner

These are all integration tests, which depend on Rustup being able to install the older toolchains.

Ideas to resolve the issue:

  • disable the subset of integration tests for riscv64gc
  • port tests to use newer versions, in a manner such that the minimum accessed version would be Rust 1.47.0

What do you think the best course of action would be?

@SpriteOvO
Copy link

@foresterre I haven't read the code carefully, but I think there should be a way to distinguish if the installation failure is caused by the specified target not existing or any other error.

I think skipping all non-existent target errors is a good solution (provided at least one version is installed successfully) so that we can avoid maintaining a minimum support version table for each new architecture.

@Avimitin
Copy link

Avimitin commented Feb 10, 2023

In my opinion, cargo-msrv should try to find the minimum supported version binary for a specific arch. Different MSRV on different architecture is more acceptable. We can't ask a developer who working on RISC-V to follow the MSRV generated from x86_64.

For example, if there is a new architecture implementation coming in Rust 1.999999 or whatever, developer will not able to compile some crate with rustc v1.999998, so the MSRV for that architecture should be 1.999999.

@foresterre
Copy link
Owner

In my opinion, cargo-msrv should try to find the minimum supported version binary for a specific arch. Different MSRV on different architecture is more acceptable. We can't ask a developer who working on RISC-V to follow the MSRV generated from x86_64.

For example, if there is a new architecture implementation coming in Rust 1.999999 or whatever, developer will not able to compile some crate with rustc v1.999998, so the MSRV for that architecture should be 1.999999.

I agree.

Cargo itself explicitly states that the rust-version key in the Cargo manifest affects all targets in the package. That is, only one MSRV can be set for a package (if you want 'official cargo support').

For a tool like cargo-msrv however, it may very well be desirable to search for the MSRV independently for each target, and maybe optionally set the MSRV per target specifically. It would then be up to the crate developer to decide which MSRV to pick from the set of MSRV's per target (assuming no target will be removed, this might always be the highest MSRV in the set).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants