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.

Sign up for GitHub

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

Jump to bottom

Emit error when users try to use a toolchain via the add or install command #12226

Merged
merged 2 commits into from
Jun 3, 2023
Merged

Emit error when users try to use a toolchain via the add or install command #12226

merged 2 commits into from
Jun 3, 2023

Conversation

danilhendrasr
Copy link
Contributor

Running cargo install +nightly or cargo add +nightly does not actually use the nightly toolchain, but the user won't know until the compilation fails. With this PR, an error is emitted if the install and add command is given a crate name
that starts with a + as we assume the user's intention was to use a certain toolchain instead of installing/adding a crate.

Example:
image

Fixes #10362

@rustbot
Copy link
Collaborator

rustbot commented Jun 3, 2023

r? @weihanglo

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-cli Area: Command-line interface, option parsing, etc. Command-add Command-install S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 3, 2023
weihanglo
weihanglo reviewed Jun 3, 2023
View reviewed changes
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution. Some writing style and wording suggestion as provided.

src/bin/cargo/commands/install.rs Outdated Show resolved Hide resolved
src/bin/cargo/commands/install.rs Outdated Show resolved Hide resolved
src/bin/cargo/commands/add.rs Outdated Show resolved Hide resolved
src/bin/cargo/commands/add.rs Outdated Show resolved Hide resolved
src/bin/cargo/commands/install.rs Outdated Show resolved Hide resolved
@danilhendrasr danilhendrasr force-pushed the feat/emit-error-on-toolchain-add-and-install branch 2 times, most recently from 67346ea to 06a5153 Compare June 3, 2023 02:00
weihanglo
weihanglo reviewed Jun 3, 2023
View reviewed changes
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the rapid update!

src/bin/cargo/commands/add.rs Outdated Show resolved Hide resolved
@danilhendrasr danilhendrasr force-pushed the feat/emit-error-on-toolchain-add-and-install branch from 06a5153 to 5be09b9 Compare June 3, 2023 11:33
@danilhendrasr danilhendrasr force-pushed the feat/emit-error-on-toolchain-add-and-install branch from 5be09b9 to ce99386 Compare June 3, 2023 13:13
weihanglo
weihanglo reviewed Jun 3, 2023
View reviewed changes
src/bin/cargo/commands/add.rs Outdated Show resolved Hide resolved
src/bin/cargo/commands/install.rs Outdated Show resolved Hide resolved
@danilhendrasr danilhendrasr force-pushed the feat/emit-error-on-toolchain-add-and-install branch from ce99386 to 58b7bce Compare June 3, 2023 13:51
weihanglo
weihanglo approved these changes Jun 3, 2023
View reviewed changes
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@weihanglo
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jun 3, 2023

📌 Commit 58b7bce has been approved by weihanglo

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 3, 2023
@bors
Copy link
Contributor

bors commented Jun 3, 2023

⌛ Testing commit 58b7bce with merge b0fa796...

@bors
Copy link
Contributor

bors commented Jun 3, 2023

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing b0fa796 to master...

1 similar comment
@bors
Copy link
Contributor

bors commented Jun 3, 2023

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing b0fa796 to master...

@bors bors merged commit b0fa796 into rust-lang:master Jun 3, 2023
@bors
Copy link
Contributor

bors commented Jun 3, 2023

👀 Test was successful, but fast-forwarding failed: 422 Changes must be made through a pull request.

@weihanglo weihanglo mentioned this pull request Jun 3, 2023
Merged
@danilhendrasr danilhendrasr deleted the feat/emit-error-on-toolchain-add-and-install branch June 3, 2023 23:41
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 4, 2023
@bors
Auto merge of rust-lang#112259 - weihanglo:update-cargo, r=weihanglo
d8095ac 
Update cargo

14 commits in f7b95e31642e09c2b6eabb18ed75007dda6677a0..b0fa79679e717cd077b7fc0fa4166f47107f1ba9
2023-05-30 19:25:02 +0000 to 2023-06-03 14:19:48 +0000
- Emit error when users try to use a toolchain via the `add` or `install` command (rust-lang/cargo#12226)
- Support "default" option for `build.jobs` (rust-lang/cargo#12222)
- Fix typo in changelog (rust-lang/cargo#12227)
- chore: Sort `-Z` flags match statement (rust-lang/cargo#12223)
- Update curl-sys (rust-lang/cargo#12218)
- Bump to 0.73.0; update changelog (rust-lang/cargo#12219)
- refactor: housekeeping for 1.70.0 (rust-lang/cargo#12217)
- nit: fix typo in changelog for 1.70 (rust-lang/cargo#12215)
- Remove a noop `.clone` (rust-lang/cargo#12213)
- refactor: compiler invocations (rust-lang/cargo#12211)
- cargo clean: use `remove_dir_all` (rust-lang/cargo#11442)
- Add a small note about indexes ignoring SemVer build metadata. (rust-lang/cargo#12206)
- Revert "chore: detect the channel a PR wants to merge into" (rust-lang/cargo#12204)
- Don't distinguish `Debuginfo::None` and `Debuginfo::Explicit(None)` (rust-lang/cargo#12205)

r? `@ghost`
@weihanglo
Copy link
Member

weihanglo commented Jun 5, 2023 •
edited
Loading

Congrats @danilhendrasr ! Your change has been made into the latest nightly toolchain (since nightly-2023-06-05 I believe). Thank you for this helpful enhancement!

@danilhendrasr
Copy link
Contributor Author

Wow, thank you for taking the time to tell me this! I didn't expect that :D
Anyway, thank you for your help as well! I hope the changes served the project well :)

RalfJung pushed a commit to RalfJung/miri that referenced this pull request Jun 11, 2023
@bors
Auto merge of #112259 - weihanglo:update-cargo, r=weihanglo
0b44f85 
Update cargo

14 commits in f7b95e31642e09c2b6eabb18ed75007dda6677a0..b0fa79679e717cd077b7fc0fa4166f47107f1ba9
2023-05-30 19:25:02 +0000 to 2023-06-03 14:19:48 +0000
- Emit error when users try to use a toolchain via the `add` or `install` command (rust-lang/cargo#12226)
- Support "default" option for `build.jobs` (rust-lang/cargo#12222)
- Fix typo in changelog (rust-lang/cargo#12227)
- chore: Sort `-Z` flags match statement (rust-lang/cargo#12223)
- Update curl-sys (rust-lang/cargo#12218)
- Bump to 0.73.0; update changelog (rust-lang/cargo#12219)
- refactor: housekeeping for 1.70.0 (rust-lang/cargo#12217)
- nit: fix typo in changelog for 1.70 (rust-lang/cargo#12215)
- Remove a noop `.clone` (rust-lang/cargo#12213)
- refactor: compiler invocations (rust-lang/cargo#12211)
- cargo clean: use `remove_dir_all` (rust-lang/cargo#11442)
- Add a small note about indexes ignoring SemVer build metadata. (rust-lang/cargo#12206)
- Revert "chore: detect the channel a PR wants to merge into" (rust-lang/cargo#12204)
- Don't distinguish `Debuginfo::None` and `Debuginfo::Explicit(None)` (rust-lang/cargo#12205)

r? `@ghost`
@ehuss ehuss added this to the 1.72.0 milestone Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weihanglo weihanglo weihanglo approved these changes

Assignees

@weihanglo weihanglo

Labels
A-cli Area: Command-line interface, option parsing, etc. Command-add Command-install S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.72.0
Development

Successfully merging this pull request may close these issues.

Notify Users of cargo +nightly install when using cargo install +nightly
5 participants
@danilhendrasr @rustbot @weihanglo @bors @ehuss