rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.54k stars 12.74k forks source link

`rustc --version` should not trigger/initiate toolchain updates #127649

Closed Doineann closed 4 months ago

Doineann commented 4 months ago

rustc --version should, in my humble opinion, not perform any unexpected behavior like initiating toolchain updates.

As a user I expect just the current active version of rustc to be returned and nothing else to happen.

Instead running rustc --version within a newly cloned rust-project triggers installation of a toolchain.

I ran into this issue within an application like oh-my-posh which uses rustc --version to display rustc's version number in the command prompt.

The issue can easily be replicated by cloning a project that require a toolchain update and then running rustc --version:

curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh
source .bashrc
rustc --version
git clone https://github.com/zed-industries/zed.git
cd zed
rustc --version

output:

user@machine:~$ curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh
info: downloading installer

Welcome to Rust!

This will download and install the official compiler for the Rust
programming language, and its package manager, Cargo.

Rustup metadata and toolchains will be installed into the Rustup
home directory, located at:

  /home/ray/.rustup

This can be modified with the RUSTUP_HOME environment variable.

The Cargo home directory is located at:

  /home/ray/.cargo

This can be modified with the CARGO_HOME environment variable.

The cargo, rustc, rustup and other commands will be added to
Cargo's bin directory, located at:

  /home/ray/.cargo/bin

This path will then be added to your PATH environment variable by
modifying the profile files located at:

  /home/ray/.profile
  /home/ray/.bashrc

You can uninstall at any time with rustup self uninstall and
these changes will be reverted.

Current installation options:

   default host triple: x86_64-unknown-linux-gnu
     default toolchain: stable (default)
               profile: default
  modify PATH variable: yes

1) Proceed with standard installation (default - just press enter)
2) Customize installation
3) Cancel installation
>1

info: profile set to 'default'
info: default host triple is x86_64-unknown-linux-gnu
info: syncing channel updates for 'stable-x86_64-unknown-linux-gnu'
info: latest update on 2024-06-13, rust version 1.79.0 (129f3b996 2024-06-10)
info: downloading component 'cargo'
info: downloading component 'clippy'
info: downloading component 'rust-docs'
info: downloading component 'rust-std'
info: downloading component 'rustc'
 63.8 MiB /  63.8 MiB (100 %)  63.1 MiB/s in  1s ETA:  0s
info: downloading component 'rustfmt'
info: installing component 'cargo'
info: installing component 'clippy'
info: installing component 'rust-docs'
 15.3 MiB /  15.3 MiB (100 %)   8.0 MiB/s in  1s ETA:  0s
info: installing component 'rust-std'
 24.4 MiB /  24.4 MiB (100 %)  15.5 MiB/s in  1s ETA:  0s
info: installing component 'rustc'
 63.8 MiB /  63.8 MiB (100 %)  17.5 MiB/s in  3s ETA:  0s
info: installing component 'rustfmt'
info: default toolchain set to 'stable-x86_64-unknown-linux-gnu'

  stable-x86_64-unknown-linux-gnu installed - rustc 1.79.0 (129f3b996 2024-06-10)

Rust is installed now. Great!

To get started you may need to restart your current shell.
This would reload your PATH environment variable to include
Cargo's bin directory ($HOME/.cargo/bin).

To configure your current shell, you need to source
the corresponding env file under $HOME/.cargo.

This is usually done by running one of the following (note the leading DOT):
. "$HOME/.cargo/env"            # For sh/bash/zsh/ash/dash/pdksh
source "$HOME/.cargo/env.fish"  # For fish
user@machine:~$ source .bashrc
user@machine:~$ rustc --version
rustc 1.79.0 (129f3b996 2024-06-10)
user@machine:~$ git clone https://github.com/zed-industries/zed.git
Cloning into 'zed'...
remote: Enumerating objects: 247512, done.
remote: Counting objects: 100% (19933/19933), done.
remote: Compressing objects: 100% (617/617), done.
remote: Total 247512 (delta 19597), reused 19443 (delta 19316), pack-reused 227579
Receiving objects: 100% (247512/247512), 101.19 MiB | 25.22 MiB/s, done.
Resolving deltas: 100% (181420/181420), done.
user@machine:~$ rustc --version
rustc 1.79.0 (129f3b996 2024-06-10)
user@machine:~$ cd zed
user@machine:~/zed$ rustc --version
info: syncing channel updates for '1.79-x86_64-unknown-linux-gnu'
info: latest update on 2024-06-13, rust version 1.79.0 (129f3b996 2024-06-10)
info: downloading component 'cargo'
info: downloading component 'clippy'
info: downloading component 'rust-std' for 'aarch64-apple-darwin'
info: downloading component 'rust-std' for 'wasm32-wasi'
info: downloading component 'rust-std' for 'x86_64-apple-darwin'
info: downloading component 'rust-std'
info: downloading component 'rustc'
info: downloading component 'rustfmt'
info: installing component 'cargo'
info: installing component 'clippy'
info: installing component 'rust-std' for 'aarch64-apple-darwin'
info: installing component 'rust-std' for 'wasm32-wasi'
info: installing component 'rust-std' for 'x86_64-apple-darwin'
info: installing component 'rust-std'
info: installing component 'rustc'
info: installing component 'rustfmt'
rustc 1.79.0 (129f3b996 2024-06-10)
user@machine:~/zed$

Meta

rustc --version --verbose:

rustc 1.79.0 (129f3b996 2024-06-10)
binary: rustc
commit-hash: 129f3b9964af4d4a709d1383930ade12dfe7c081
commit-date: 2024-06-10
host: x86_64-unknown-linux-gnu
release: 1.79.0
LLVM version: 18.1.7
ChrisDenton commented 4 months ago

This is a rustup issue (the toolchain manager) and not caused by rust itself. See: https://github.com/rust-lang/rustup/issues/3635

bjorn3 commented 4 months ago

What is the alternative? Have it report an error to the user that they need to manually invoke rustup to update their toolchain? That is a lot less ergonomic and likely breaks tools that invoke rustc or cargo and don't expect this error and may not even have a good way to show the error.

Doineann commented 4 months ago

I expect rustc --version to just output version information, like how --version would behave in any application.

In my humble opinion rustc --version should not be responsible for upgrading toolchains, the user should do that either explicitly or maybe when running rustc with any other flags that -actually- require the updates to be performed.

The --version argument should really just be a 'request' for version information and not trigger any other actions.

To be clear... I think it's ok to trigger updates for calls to rustc with different arguments, but I believe --version should really only output the version.

Kobzol commented 4 months ago

That justification makes sense, but it would also mean that if you run rustc --version in a Cargo project with rust-toolchain.toml, it would return X, but then pretty much any other rustc/cargo command in that directory would download the different toolchain Y anyway. So rustc --version would give you a version without doing anything else, but it would not be the actual version used for a given Cargo project.

ChrisDenton commented 4 months ago

Note that when the toolchain is managed by rustup, the rustc command is a rustup proxy. rustc itself will never download or install toolchains (it wouldn't even know how to).

So discussion should move to the rustup repository. Discussing it here will not affect changes to rustup.

Doineann commented 4 months ago

I can even imagine this behavior to pose a security risk, as users would not expect --version to perform any installation/updates. I don't know what kind of actions a call to rustc --version actually performs, so I cannot tell if this is the case, but in the very least, it is unexpected behavior.

Doineann commented 4 months ago

That justification makes sense, but it would also mean that if you run rustc --version in a Cargo project with rust-toolchain.toml, it would return X, but then pretty much any other rustc/cargo command in that directory would download the different toolchain Y anyway. So rustc --version would give you a version without doing anything else, but it would not be the actual version used for a given Cargo project.

If I run rustc --version I honestly expect to see the version of the 'currently active' or default rustc. I understand your argument that this may not match with the version of the project and maybe needs an update...

So maybe then it should indeed also report that the versions do not match... and how to update. Or maybe an extra flag to also trigger an update if necessary.

Doineann commented 4 months ago

Note that when the toolchain is managed by rustup, the rustc command is a rustup proxy. rustc itself will never download or install toolchains (it wouldn't even know how to).

So discussion should move to the rustup repository. Discussing it here will not affect changes to rustup.

I understand. Should I repeat this Issue on the rustup repo, or did you effectively do that already with your first reply?

(thanks for the quick response, by the way)

ChrisDenton commented 4 months ago

It looks like there is already a related PR https://github.com/rust-lang/rustup/pull/2658.

ChrisDenton commented 4 months ago

You could mention rustc --version explicitly in the issue I linked in my first post.

eggyal commented 4 months ago

If you invoke a rustc directly rather that via rustup's proxy, then rustc --version will behave exactly as you expect.

But since you may well have many versions of rustc installed, the question then becomes which of those many rustc's do you want to invoke? The proxy decided (from the rust-toolchain.toml) that the one you intended to invoke was one that wasn't yet installed, and it went ahead and installed it for you. However if you knew that you wanted to invoke some other (already installed) version then you could have specified that in the command eg rustc +1.79 --version (but I guess that's kinda pointless unless you're using +stable, +beta or +nightly).

rami3l commented 4 months ago

It looks like there is already a related PR rust-lang/rustup#2658.

@ChrisDenton Err not quite; that's not about the shim but about rustup itself.

Anyway https://github.com/rust-lang/rustup/issues/3635 has already covered this even before this issue's appearance, so I suggest we close this issue in favor of that one.

ChrisDenton commented 4 months ago

Ah thanks! Ok then, closing this in favour of the rustup issue.

rami3l commented 4 months ago

@ChrisDenton Thanks! BTW can you transfer the issue to Rustup so that we might keep the conversation history while keeping this sub-task tracked individually? (Don't worry if you don't have the permission, I'll make a new issue there instead if that is the case.)

ChrisDenton commented 4 months ago

Yeah sorry, I don't have any permissions on the rustup repo.

fmease commented 4 months ago

(@ChrisDenton you can use @rustbot transfer rustup for that)