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

Warn, don't error, when adding an already installed target for the current toolchain #1533

Closed
mcandre opened this issue Nov 5, 2018 · 7 comments

Comments

@mcandre
Copy link

mcandre commented Nov 5, 2018

$ rustup show
Default host: x86_64-unknown-freebsd

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

x86_64-unknown-freebsd

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

stable-x86_64-unknown-freebsd (default)
rustc 1.30.0 (da5f414c2 2018-10-24)

$ rustup target add x86_64-unknown-freebsd
error: component 'rust-std' for target 'x86_64-unknown-freebsd' was automatically added because it is required for toolchain 'stable-x86_64-unknown-freebsd'

$ echo "$?"
1

This is not a particularly helpful behavior. When the user instructs rustup to install an already installed target, better to warn and exit with code zero (successful), rather than trigger a scary, unnecessary error.

@mcandre mcandre changed the title Warn, don't error, when adding a target for the current toolchain Warn, don't error, when adding an already installed target for the current toolchain Nov 5, 2018
@ncalexan
Copy link

Yeah, I just saw this, and it was surprising. It also makes it slightly harder to do what I want to do in automation, where idempotence would be handy.

@hbobenicio
Copy link

I think it would be better if it's an info instead of a warning.

For reference, this is what happens when adding an uninstalled new target

$ rustup target add wasm32-unknown-emscripten
info: downloading component 'rust-std' for 'wasm32-unknown-emscripten'
 13.4 MiB /  13.4 MiB (100 %)   2.2 MiB/s ETA:   0 s                
info: installing component 'rust-std' for 'wasm32-unknown-emscripten'

How about this:

$ rustup target add wasm32-unknown-emscripten
info: component 'rust-std' for 'wasm32-unknown-emscripten' already installed
$ echo $?
0

warns generally alerts users and developers that something may not be well set up or may lead to unexpected results or even that some action should be made in order to get rid of it.

Reinstalling something already installed didn't fit this mindset, IMHO.
What do you think?

@hbobenicio
Copy link

BTW, something similar happens with other commands too:

$ rustup component add rust-docs
error: component 'rust-docs' for target 'x86_64-unknown-linux-gnu' was automatically added because it is required for toolchain 'nightly-x86_64-unknown-linux-gnu'
$ echo $?
1

Why is this an error? Maybe a deeper revision/refactoring would be needed in order to keep error handling, error/warn messages and RC consistant.

@clarfonthey
Copy link

I also ran into this:

$ rustup target add --toolchain=nightly x86_64-unknown-linux-gnu
error: component 'rust-std' for target 'x86_64-unknown-linux-gnu' was automatically added because it is required for toolchain 'nightly-x86_64-unknown-linux-gnu'

I have a Makefile which runs this for all targets I want to build for, including the current one. Was very upset to find out that this errors instead of simply printing that it's already installed.

@mati865
Copy link
Contributor

mati865 commented Dec 4, 2018

@clarcharr it's fixed on the master #441

@DeeDeeG
Copy link

DeeDeeG commented Dec 4, 2018

Hi, I wanted to mention: Maybe since this is telling us we already have something we then tried to install again, the message could be friendlier? e.g.:

Already installed!

At first glance I thought I had done something wrong and that I didn't actually add the component I wanted to add.

Edit: (This is basically what @hbobenicio said. Sorry I didn't notice earlier.)

@kinnison
Copy link
Contributor

These days the messages are:

somniloquy(git)(🦀🏠)% rustup component add rustc                             
info: component 'rustc' for target 'x86_64-unknown-linux-gnu' is up to date
somniloquy(git)(🦀🏠)% rustup target add x86_64-unknown-linux-gnu             
info: component 'rust-std' for target 'x86_64-unknown-linux-gnu' is up to date
somniloquy(git)(🦀🏠)%                                                        

As such, I consider this resolved.

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

No branches or pull requests

7 participants