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

Notify Users of cargo +nightly install when using cargo install +nightly #10362

Closed
DrRuhe opened this issue Feb 5, 2022 · 8 comments · Fixed by #12226
Closed

Notify Users of cargo +nightly install when using cargo install +nightly #10362

DrRuhe opened this issue Feb 5, 2022 · 8 comments · Fixed by #12226
Assignees
Labels
A-diagnostics Area: Error and warning messages generated by Cargo itself. C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-add Command-install E-easy Experience: Easy S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review

Comments

@DrRuhe
Copy link

DrRuhe commented Feb 5, 2022

Problem

When running cargo install +nightly ... there is no indication that it does not actually use the nightly toolchain, until the compilation runs into errors because e.g. features need nightly.

Proposed Solution

Either:
Let the user know that cargo install +nightly ... doesn't actually use nightly and instead they should use cargo +nightly install ....

Or:
just use nightly...

Notes

No response

@DrRuhe DrRuhe added the C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` label Feb 5, 2022
@ehuss ehuss added A-diagnostics Area: Error and warning messages generated by Cargo itself. Command-install labels Feb 27, 2022
@weihanglo weihanglo added the S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. label May 24, 2023
@weihanglo
Copy link
Member

Should be fairly easy to tackle for only cargo install. That being said, I personally want a solution for all cargo commands. People may also type cargo build +nightly and we should catch that. The difficult part is that rustup proxy name can be custom defined.

If we got #11702 or clap-rs/clap#4706 landed, I think it's sufficient that we target only a subset of most common ones like +stable, +beta, +nightly.

@epage
Copy link
Contributor

epage commented May 24, 2023

The problem is +<channel> isn't a flag per se but a positional argument.

cargo install and cargo add are some of the few commands that even accept a positional argument, so I can see doing this in a one-off way.

(some others accept positional arguments but those are just meant to forward to an underlying command)

@weihanglo
Copy link
Member

cargo install and cargo add are some of the few commands that even accept a positional argument, so I can see doing this in a one-off way.

Agree with that. cargo run is a thing that we probably cannot fix at this moment.

Here are some pointers where cargo install and cargo add collect their positional arguments

I think Cargo can send an ad-hoc error message when detecting any crate starting with +. Something like,

error: invalid character `+` in package name: `nightly`
     Use `cargo +nightly add` if you meant to use `nightly` toolchain.

(you could find a better message than mine)

@weihanglo weihanglo added E-easy Experience: Easy S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review Command-add and removed S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. labels Jun 1, 2023
@danilhendrasr
Copy link
Contributor

Hi, I would like to take on this issue. But, just to confirm, from what I understand in order to resolve this issue we only need to make cargo emits the above error message whenever the add and install command is run with a crate name that has + in it. Is it correct?

@weihanglo
Copy link
Member

That's exactly my plan so far!

You may want to tweak the message a bit. Mine is not as good as it looks.

@danilhendrasr
Copy link
Contributor

Got it, I'll see what I can come up with.

@rustbot claim

@danilhendrasr
Copy link
Contributor

danilhendrasr commented Jun 2, 2023

I think I need more clarification.

If the error is emitted any time the command is given a crate name that starts with + regardless if the rest of the crate name is actually the same as one of the three toolchain channels name (stable, beta, nightly), then the following part:

  Use `cargo +<crate name> add` if you meant to use `<crate name>` toolchain.

of the error should only be printed if the rest of crate name after the + sign is actually the same as one of the three toolchain channels name.
Is that correct?

@weihanglo
Copy link
Member

Toolchain name can be anything, such like cargo +stage1 build.

My best guess is, from the array of crates name strings (krates and crates in source code), if there is a string starting with +, give the user an error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Error and warning messages generated by Cargo itself. C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-add Command-install E-easy Experience: Easy S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants