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

rustup show shouldn't install things #3360

Closed
wants to merge 1 commit into from

Conversation

john-h-k
Copy link

Fixes #1397

Currently rustup show will install missing components or missing toolchains when ran. This has a couple of issues:

  • It's unintuitive - I don't think anyone expects a show command to install things
  • It causes lots of programs to accidentally cause installations
    • Lots of shell prompts use rustup show to show current toolchain and similar. This means cd'ing into a rust project with an uninstalled toolchain can cause the shell to freeze while it installs the entire toolchain

rustup home:  /Users/johnk/.rustup

installed toolchains
--------------------

stable-aarch64-apple-darwin (default)
nightly-2023-04-23-aarch64-apple-darwin
nightly-aarch64-apple-darwin
1.60-aarch64-apple-darwin
1.69-aarch64-apple-darwin
dev
stage1
stage2
1.65.0-aarch64-apple-darwin
1.69.0-aarch64-apple-darwin

installed targets for active toolchain
--------------------------------------

aarch64-apple-darwin
aarch64-pc-windows-msvc
wasm32-unknown-unknown

active toolchain
----------------

stable-aarch64-apple-darwin (default)
rustc 1.69.0 (84c898d65 2023-04-16) shouldn't install things
@rbtcollins
Copy link
Contributor

Please see the history in that bug, from around #1397 (comment)

In particular, this is a breaking change, we rather suspect we need an RFC, though rustup hasn't been doing that process often so I'd expect some friction there.

Regardless of the RFC thing though, we have to have the replacement UX present before we contract the functionality.

We have rough consensus in that bug that:

  • we should add an ensure verb that autoinstall the current toolchain name. E.g. rustup +stable ensure will install stable, rustup ensure will install the default or otherwise overridden toolchain, etc. Possibly that should be rustup toolchain ensure or some other spelling. This is a new command that doesn't exist yet.
  • we then then stop autoinstalling on anything outside of proxy-mode

To make this change will require a decent time investment. Adding the new command is quite easy; then you have to follow that up by migrating the test suite to the new command. And then finally we can remote autoinstalling for the non-proxy-mode commands.

I think that that last step should be staggered over at least 2 release cycles and coordinated with a rust-lang blog post.

@rbtcollins
Copy link
Contributor

I'm going to close this PR as it doesn't seem to be getting updated; please do reopen it if you start work on the suggestions in #3360 (comment)

@rbtcollins rbtcollins closed this Aug 15, 2023
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.

rustup show should not force-install the default toolchain if it is not installed.
2 participants