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

Add message in case it's already up-to-date #2152

Merged
merged 2 commits into from
Jan 25, 2020

Conversation

GuillaumeGomez
Copy link
Member

No description provided.

@rbtcollins
Copy link
Contributor

Hmm, can I ask why?
The current output is

$ rustup self update
info: checking for self-updates

And this combined with the exit code of 0 is pretty unambiguous. If an error occurs, its logged, if there is work to be done it is logged. I'm not against this per se, but not really for it either.

@GuillaumeGomez
Copy link
Member Author

Unambiguous but pretty weird. At first I wondered if it crashed somehow. At least this is now explicit, no need to check the exit code.

@rbtcollins
Copy link
Contributor

Well the thing is its also used in the large rustup update flow implicitly, and that looks like this:

info: installing component 'rust-docs'
 12.0 MiB /  12.0 MiB (100 %)   7.3 MiB/s in  1s ETA:  0s
info: installing component 'rust-std' for 'wasm32-unknown-unknown'
info: checking for self-updates

   stable-x86_64-unknown-linux-gnu updated - rustc 1.39.0 (4560ea788 2019-11-04)
  nightly-x86_64-unknown-linux-gnu updated - rustc 1.41.0-nightly (76a252ea9 2019-12-09)

And when there is nothing they show like so:

$ rustup update
info: syncing channel updates for 'stable-x86_64-unknown-linux-gnu'
info: syncing channel updates for 'nightly-x86_64-unknown-linux-gnu'
info: checking for self-updates

   stable-x86_64-unknown-linux-gnu unchanged - rustc 1.39.0 (4560ea788 2019-11-04)
  nightly-x86_64-unknown-linux-gnu unchanged - rustc 1.41.0-nightly (76a252ea9 2019-12-09)

I don't want to get in the way here, but I wonder if what you really want is to have rustup self updates reported (or not) somehow more consistently with the toolchain information.

If you look at what we output for toolchains, just the one message is consistent, but we then do a summary at the bottom, which we don't do for rustup itself. Perhaps showing something like this would be good:

$ rustup update
info: syncing channel updates for 'stable-x86_64-unknown-linux-gnu'
info: syncing channel updates for 'nightly-x86_64-unknown-linux-gnu'
info: checking for self-updates

   stable-x86_64-unknown-linux-gnu unchanged - rustc 1.39.0 (4560ea788 2019-11-04)
  nightly-x86_64-unknown-linux-gnu unchanged - rustc 1.41.0-nightly (76a252ea9 2019-12-09)
  rustup unchanged - rustup 1.20 (2019-10-21)

This would give more information

@GuillaumeGomez
Copy link
Member Author

Hum... I'm shared on this. I really like this output for example:

$ rustup update
info: syncing channel updates for 'stable-x86_64-unknown-linux-gnu'
info: syncing channel updates for 'nightly-x86_64-unknown-linux-gnu'
info: checking for self-updates
info: already up-to-date

Having more "state messages" would be nice. Something like:

$ rustup update
info: syncing channel updates for 'stable-x86_64-unknown-linux-gnu'
info: done
info: syncing channel updates for 'nightly-x86_64-unknown-linux-gnu'
info: done
info: checking for self-updates
info: already up-to-date

@kinnison
Copy link
Contributor

I quite like @rbtcollins's point that it'd be nice if the format mirrored how we report updates to toolchain channels. This would require a little more plumbing but would be consistent which users may appreciate.

@GuillaumeGomez
Copy link
Member Author

I'll do it in the next days then.

@kinnison
Copy link
Contributor

kinnison commented Jan 6, 2020

@GuillaumeGomez Happy new year. I wonder if you might have time in the coming week or so to make progress on this?

@GuillaumeGomez
Copy link
Member Author

I'll try to!

@GuillaumeGomez
Copy link
Member Author

As suggested, I re-used the code from common to "show".

@GuillaumeGomez GuillaumeGomez force-pushed the up-to-date-msg branch 2 times, most recently from 92d08d9 to 88b2c04 Compare January 24, 2020 20:40
@GuillaumeGomez
Copy link
Member Author

Updated!

Copy link
Contributor

@kinnison kinnison left a comment

Choose a reason for hiding this comment

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

One bogon, one query (not important if you choose to decline it), and then the commits need rebasing because we don't like fmt type commits, and also something has changed underneath you so I can't merge now even if I wanted to.

src/errors.rs Outdated Show resolved Hide resolved
src/cli/self_update.rs Outdated Show resolved Hide resolved
@GuillaumeGomez
Copy link
Member Author

Updated!

Copy link
Contributor

@kinnison kinnison left a comment

Choose a reason for hiding this comment

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

LGTM. 👍

@kinnison kinnison merged commit 870f3a5 into rust-lang:master Jan 25, 2020
@GuillaumeGomez GuillaumeGomez deleted the up-to-date-msg branch January 25, 2020 23:33
@GuillaumeGomez
Copy link
Member Author

\o/

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.

3 participants