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

Better error message when no toolchain is installed #2657

Merged
merged 1 commit into from
Feb 20, 2021

Conversation

kellda
Copy link
Contributor

@kellda kellda commented Feb 11, 2021

Fix #395

src/errors.rs Outdated
@@ -297,7 +297,7 @@ error_chain! {
}
ToolchainNotSelected {
description("toolchain is not selected")
display("no override and no default toolchain set")
display("no override and no default toolchain set; run 'rustup default [toolchain]' to select one")
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like the word 'select' here, I think it implies showing a menu - which it won't (but we do on windows during first install kindof).

I can suggest two alternatives to move this forward.

  1. s/select/set/
  2. extend the places this is used to determine the default toolchain for the platform, and then pass that into the error, so that it can become a copy-and-pastable valid command.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Sure
  2. If there is no default toolchain set, can we really get a default toolchain ? Maybe we should just suggest rustup default stable.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, though stable is just shorthand for stable-{host-triple}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed it. I don't think it is worth to suggest typing the whole host triple.

src/errors.rs Outdated
@@ -297,7 +297,7 @@ error_chain! {
}
ToolchainNotSelected {
description("toolchain is not selected")
display("no override and no default toolchain set")
display("no override and no default toolchain set; run 'rustup default stable' to download and set the stable toolchain as default")
Copy link
Contributor

Choose a reason for hiding this comment

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

lets not mention downloading here: it may already be present, or not, and its not the essence of the thing.

@kinnison
Copy link
Contributor

I believe the test failure is a spurious one, and I've filed an issue to look into it. In the meantime, are you now happy with the wording Robert?

@kinnison kinnison merged commit 968a8e8 into rust-lang:master Feb 20, 2021
@daveloyall
Copy link

I'm attempting to contribute to this conversation even though I don't know what I'm doing. (Maybe that is helpful. Maybe people who know what they are doing never see this error message anyway.)

I thought it would be useful to build rust programs on windows under the GNU tools provided by Msys2... So, my toolchain is nightly-x86_64-pc-windows-gnu. I followed a guide..

rustup toolchain list returns that toolchain, the full name. And then rustup default nightly-x86_64-pc-windows-gnu seems to have stopped cargo from printing out the error message described here in #2657.

If I understand correctly (and that's a big if!), if I had done rustup default stable instead.. then I would have ended up using the Microsoft compiler, instead of the GNU one...

I hope this helps.

@kellda kellda deleted the no-toolchain-msg branch April 6, 2021 20:12
@rbtcollins
Copy link
Contributor

@daveloyall we haven't released this change yet.

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

Successfully merging this pull request may close these issues.

'no updatable toolchains installed'
4 participants