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

Redirect stderr output of rustc --version #445

Closed
wants to merge 1 commit into from

Conversation

arpandaze
Copy link

@arpandaze arpandaze commented Sep 25, 2023

Description

Solves the problem described in #444

Motivation and Context

Closes #444

Screenshots (if appropriate)

How Has This Been Tested

  • I have tested using Linux.
  • I have tested using MacOS.

Checklist

  • I am ready to update the wiki accordingly.
  • I have updated the tests accordingly.

@ilya-bobyr
Copy link
Contributor

This would hide the installation process, but the toolchain will still be installed.
What the UX would be for the case when a toolchain for the current path is missing?
Installing the toolchain without any visual indication seems like a degradation, not an improvement.
And, I think, the prompt will also just hang until the installation is finished.

@IlanCosman
Copy link
Owner

IlanCosman commented Sep 30, 2023

Yah, the prompt won't update until the download is finished, and even worse, the download will be restarted every time you press enter, because Tide kills the background shell that is doing the downloading.

Here's what Starship does with this same problem: https://github.com/starship/starship/blob/master/src/modules/rust.rs#L51-L89

// $CARGO_HOME/bin/rustc(.exe) --version may attempt installing a rustup toolchain.
// starship/starship#417
//
// To display appropriate versions preventing rustc from downloading toolchains, we have to
// check
// 1. $RUSTUP_TOOLCHAIN
// 2. The override list from ~/.rustup/settings.toml (like rustup override list)
// 3. rust-toolchain or rust-toolchain.toml in . or parent directories
// 4. The default_toolchain from ~/.rustup/settings.toml (like rustup default)
// 5. rustup default (in addition to the above, this also looks at global fallback config files)
// as rustup does.
// https://github.com/rust-lang/rustup.rs/tree/eb694fcada7becc5d9d160bf7c623abe84f8971d#override-precedence
//
// Probably we have no other way to know whether any toolchain override is specified for the
// current directory. The following commands also cause toolchain installations.
// - rustup show
// - rustup show active-toolchain
// - rustup which

We should probably copy their logic. I'd be happy to accept PRs, but I probably won't be working on this myself in the near term.

For now, it's better to expose the bad behavior to the user than to cover it up.

@IlanCosman IlanCosman closed this Sep 30, 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 causes rustc invocation to automatically download toolchain
3 participants