verify acts weird in workspace

[cschramm]

I fail to use cargo msrv verify on a cargo workspace. This actually splits up into multiple issues.

Common setup: A workspace with crates a and b. a does not depend on b, it's not important if b depends on a. a has rust-version = "1.56", b has rust-version = "1.58". b does not build with 1.56.

Issue 1: No workspace support. Ideally I'd just run verify in the workspace directory and it would iterate over the crates. That is not supported. verify expects a package section in the virtual manifest.

Issue 2: verify acts weird. When I run verify in directory a it runs rustup run 1.56.1 cargo check --all. As --all is a deprecated alias for --workspace, this tries to build the whole workspace with Rust 1.56.1 which fails for crate b. That is completely unrelated to the verification of crate a, though. I suspect a mistake in the default check command. Could it be that --all should rather be --all-targets (nowadays)?

Issue 3: I cannot set a check command for verify. From how I read the documentation and the outcome of failed tries it seems like msrv accepts either a subcommand like verify or a check command provided after --, so I cannot work around my problem with something like cargo msrv verify -- cargo check --all-targets (Found argument 'cargo' which wasn't expected, or isn't valid in this context).

[foresterre]

I've setup a sample repository, which aims to mimic the crate structure you described above, here: https://github.com/foresterre/cargo-msrv-issue-293

Issue 1: No workspace support. Ideally I'd just run verify in the workspace directory and it would iterate over the crates. That is not supported. verify expects a package section in the virtual manifest.

I've created #295 to address this first issue. Currently, cargo-msrv does indeed not deal with workspaces on its own (I rarely use virtual manifests myself, even with workspaces, which is probably why I missed it in the first place), but I can see why it is desirable to differentiate between the root package and virtual manifest workspace structures.

~~As a side note: while this is different from most cargo commands, it's more alike cargo publish which also requires you to enter each crate separately.~~ No longer true since 1.56.1 -p is now supported.

[foresterre]

Issue 2: verify acts weird. When I run verify in directory a it runs rustup run 1.56.1 cargo check --all. As --all is a deprecated alias for --workspace, this tries to build the whole workspace with Rust 1.56.1 which fails for crate b. That is completely unrelated to the verification of crate a, though. I suspect a mistake in the default check command. Could it be that --all should rather be --all-targets (nowadays)?

In order to determine the MSRV for a workspace with a root package, it should run with --all (or --workspace), as otherwise only the root crate is checked. I chose --all, over --workspace simply because it supported by newer and older toolchains (even though it's indeed deprecated in favour of the newer --workspace flag).

[foresterre]

Issue 3: I cannot set a check command for verify. From how I read the documentation and the outcome of failed tries it seems like msrv accepts either a subcommand like verify or a check command provided after --, so I cannot work around my problem with something like cargo msrv verify -- cargo check --all-targets (Found argument 'cargo' which wasn't expected, or isn't valid in this context).

You can either:

1. navigate to your crate directory and run a command like:

cargo-msrv-issue-293 on  main [!?] via 🦀 v1.58.1
❯ cd a

cargo-msrv-issue-293/a on  main [!?] is 📦 v0.1.0 via 🦀 v1.58.1
❯ cargo msrv -- cargo check --all-targets
Fetching index
Determining the Minimum Supported Rust Version (MSRV) for toolchain x86_64-pc-windows-msvc
Using check command cargo check --all-targets
Check for toolchain '1.57.0-x86_64-pc-windows-msvc' succeeded
Check for toolchain '1.56.1-x86_64-pc-windows-msvc' succeeded
   Finished The MSRV is: 1.56.1   █████████████████████████████████████████████████████████████████████████████ 00:00:01

2. or run from the root crate with --path ./a:

cargo-msrv-issue-293/a on  main [!?] is 📦 v0.1.0 via 🦀 v1.58.1
❯ cd ..

cargo-msrv-issue-293 on  main [!?] via 🦀 v1.58.1
❯ cargo msrv --path ./a -- cargo check --all-targets
Fetching index
Determining the Minimum Supported Rust Version (MSRV) for toolchain x86_64-pc-windows-msvc
Using check command cargo check --all-targets
Check for toolchain '1.57.0-x86_64-pc-windows-msvc' succeeded
Check for toolchain '1.56.1-x86_64-pc-windows-msvc' succeeded
   Finished The MSRV is: 1.56.1   █████████████████████████████████████████████████████████████████████████████ 00:00:01

If that doesn't work for you it's a bug :).

[foresterre]

Thanks for the report by the way 😄!

[cschramm]

You're welcome. :slightly_smiling_face:

Issue 2: verify acts weird. When I run verify in directory a it runs rustup run 1.56.1 cargo check --all. As --all is a deprecated alias for --workspace, this tries to build the whole workspace with Rust 1.56.1 which fails for crate b. That is completely unrelated to the verification of crate a, though. I suspect a mistake in the default check command. Could it be that --all should rather be --all-targets (nowadays)?

In order to determine the MSRV for a workspace with a root package, it should run with --all (or --workspace), as otherwise only the root crate is checked. I chose --all, over --workspace simply because it supported by newer and older toolchains (even though it's indeed deprecated in favour of the newer --workspace flag).

Ok, with the root package setup in mind, I see where you're coming from. I'm not quite sure what the --all is there for, though. If a package in the workspace is a dependency of the root package (or more general: of the checked package), it will affect cargo check on it anyway. If it's not, it should not affect the MSRV of the root / checked package. With --all in place, you will find the "greatest common MSRV" of all packages in the workspace, no matter which package you are running the check for and how they are related. That does make sense for a root package if all other packages are dependencies of it but you can find the same version without --all then and it does not make sense for any other package.

Issue 3: I cannot set a check command for verify. From how I read the documentation and the outcome of failed tries it seems like msrv accepts either a subcommand like verify or a check command provided after --, so I cannot work around my problem with something like cargo msrv verify -- cargo check --all-targets (Found argument 'cargo' which wasn't expected, or isn't valid in this context).

You can either:

1. navigate to your crate directory and run a command like:

❯ cargo msrv -- cargo check --all-targets

2. or run from the root crate with --path ./a:

❯ cargo msrv --path ./a -- cargo check --all-targets

Those commands are fine but providing a check command is only possible for the main msrv command, not its verify subcommand. verify has its benefits, e.g. in a CI where msrv itself does not seem like a good fit.

Wrapping up what I'm trying to achieve:

• For the common setup I described I want my CI to verify that a builds with its specified MSRV 1.56 and b builds with it's specified MSRV 1.58. cargo-msrv seems like it should be a good fit for that job.
• Ideally it would be done by a simple cargo msrv verify on the workspace but virtual manifests are not addressed.
• I'd thus solve it with cargo msrv --path a verify && cargo msrv --path b verify but due to the --all in the default check command it will fail for a as the perfectly fine 1.56 is lower than the "greatest common MSRV" 1.58.
• I would work around that by providing a check command that does not have --all but providing a custom check command to the verify subcommand is not possible.

[foresterre]

Ok, with the root package setup in mind, I see where you're coming from. I'm not quite sure what the --all is there for, though. If a package in the workspace is a dependency of the root package (or more general: of the checked package), it will affect cargo check on it anyway. If it's not, it should not affect the MSRV of the root / checked package. With --all in place, you will find the "greatest common MSRV" of all packages in the workspace, no matter which package you are running the check for and how they are related. That does make sense for a root package if all other packages are dependencies of it but you can find the same version without --all then and it does not make sense for any other package.

Thanks for your explanation! The difference is subtle, but matters, and I must admit I agree with your reasoning.

[foresterre]

Those commands are fine but providing a check command is only possible for the main msrv command, not its verify subcommand. verify has its benefits, e.g. in a CI where msrv itself does not seem like a good fit.

Oof, I thought I did run verify; I think I wasn't awake any more 😜. That's a bug, thanks!

[foresterre]

Issue 3: I cannot set a check command for verify. From how I read the documentation and the outcome of failed tries it seems like msrv accepts either a subcommand like verify or a check command provided after --, so I cannot work around my problem with something like cargo msrv verify -- cargo check --all-targets (Found argument 'cargo' which wasn't expected, or isn't valid in this context).

This part should be fixed as part of #296 (PR #299). Released as cargo-msrv v0.14.2.

[foresterre]

Issue 2: verify acts weird. When I run verify in directory a it runs rustup run 1.56.1 cargo check --all. As --all is a deprecated alias for --workspace, this tries to build the whole workspace with Rust 1.56.1 which fails for crate b. That is completely unrelated to the verification of crate a, though. I suspect a mistake in the default check command. Could it be that --all should rather be --all-targets (nowadays)?

Ok, with the root package setup in mind, I see where you're coming from. I'm not quite sure what the --all is there for, though. If a package in the workspace is a dependency of the root package (or more general: of the checked package), it will affect cargo check on it anyway. If it's not, it should not affect the MSRV of the root / checked package. With --all in place, you will find the "greatest common MSRV" of all packages in the workspace, no matter which package you are running the check for and how they are related. That does make sense for a root package if all other packages are dependencies of it but you can find the same version without --all then and it does not make sense for any other package.

Changed in #297 (PR #300). Will be released as cargo-msrv v0.15.0