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

Automatically notify users of available toolchain updates #3688

Open
pitaj opened this issue Feb 28, 2024 · 16 comments
Open

Automatically notify users of available toolchain updates #3688

pitaj opened this issue Feb 28, 2024 · 16 comments

Comments

@pitaj
Copy link

pitaj commented Feb 28, 2024

Problem you are trying to solve

It is not uncommon for Rust users to have old version of Rust installed without realizing it. This is problematic for users:

  • best-case they miss out on new features
  • or miss out on improvements to the type system or borrow checker
  • they may be using a toolchain with known bugs
  • or, worst-case, one with security vulnerabilities

It's especially important for users to be notified of "patch" releases which often roll out only when there is a severe issue with a "minor" release toolchain.

Solution you'd like

Essentially, regularly check for toolchain updates and notify users when they run a rustup command or when they run a cargo command through the rustup proxy.

Prompt on Rust install

On install, users should be asked if they want Rustup to periodically check for updates when invoking cargo and rustup commands:

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 installation (default)
2) Customize installation
3) Cancel installation
>

Rustup can periodically check for and download `stable` toolchain 
updates. These actions will only be started when `cargo` or 
`rustup` commands are invoked.

For more information, see https://rust-lang.github.io

1) Never check or download
2) Check monthly, but don't download
3) Check weekly, but don't download
4) Check daily, but don't download
5) Check monthly and download in the background
6) Check weekly and download in the background (default)
7) Check daily and download in the background
>

Notice when running command

When a rustup command is executed or any cargo command is executed via the Rustup proxy on the stable toolchain, begin checking for updates in a background process. Wait with a short timeout; if the check has not completed by then, display a message to the user on the next invocation. If the update check did complete in time, directly display a message to the user.

The message to display:

$ cargo check
Rustup notice: Update available for the `stable` toolchain.
    
    stable-x86_64-unknown-linux-gnu    1.74.0 -> 1.76.0

Update packages will be downloaded in the background.
Run `rustup update -N` to complete the update.

    Checking project v0.1.0 (/home/user/Dev/project)
    Finished dev [unoptimized + debuginfo] target(s) in 7.95s

Update packages will be downloaded in the background. would obviously not be shown if the user hadn't enabled downloading.

rustup update --notice

The -N flag (short for --notice) tells Rustup to only run the update that the user was informed of in the most recent notice. If the download was not started, then rustup update -N will run essentially the same as a rustup update stable does today: starting the downloads, wait for them to complete, then install them. If the download started but not completed, then rustup update -N will complete the download and install the update. If the download has already completed, then it will just install the downloaded update.

If a background-downloaded update was never installed and a newer version is detected, the older download will be deleted.

Notes

Timeout duration

We want something long enough to allow the update check to complete for most users, but imperceptible if the update check takes a long time. Something around 100ms-200ms.

Display notice after cargo command

We may want the Rustup notice to show after the cargo command runs so the notice doesn't get buried in the cargo output. However, we can't directly print the notice from rustup without changing the way the cargo proxies work. Instead, cargo could add a special command-line option or environment variable for an info item if prints out at the end. It could work something like this:

$ ~/.cargo/bin/cargo check
    CARGO_JSON_PASSTHROUGH = '{
        "reason": "info",
        "placement": "after_compile",
        "message": "Rustup notice: Update available for the `stable` toolchain ..."
    }'
    ~/.rustup/toolchains/stable-.../bin/cargo check

Of course, this would not be compatible with cargo run, so we'd need to do some amount of argument parsing. It could be very basic though, even just checking for run.

How often to display the notice

We don't want to spam the user with notices, otherwise they'll just learn to ignore them. A reasonable behavior might be:

  • Never display a notice while actively downloading in the background
  • Only display a notice at most once an hour
  • Take care to only display notices where users will actually see them
    • when executed directly by the user in a terminal
    • NOT when executed programmatically
    • (maybe) when cargo output is being passed to rust-analyzer or another external tool
@pitaj pitaj changed the title Automatically notify users of avaailable toolchain updates Automatically notify users of available toolchain updates Feb 28, 2024
@djc
Copy link
Contributor

djc commented Feb 28, 2024

I'd like to see some benchmarks for the impact this would have on proxy overhead. Like, even in the most optimistic scenario of spawning a different process that handles most of the work asynchronously, that might be prohibitive (at least from being opt-out)? I also feel like this should not be a separate question from the initial prompt, it should be listed as an installation option so that the defaults path doesn't get longer/more elaborate.

@pitaj
Copy link
Author

pitaj commented Feb 28, 2024

I'd like to see some benchmarks for the impact this would have on proxy overhead. Like, even in the most optimistic scenario of spawning a different process that handles most of the work asynchronously, that might be prohibitive (at least from being opt-out)?

Are you looking for proxy-to-cargo latency, or are you talking about performance overhead?

I also feel like this should not be a separate question from the initial prompt, it should be listed as an installation option so that the defaults path doesn't get longer/more elaborate.

My thought process was this:

  • really want this to be the default to keep users evergreen
  • while not telemetry, phoning home by default can be very controversial
  • we do mitigate that somewhat by only initiating the check when a command is run
  • but requiring an extra step to opt out could worsen the controversy
  • so always asking the question, with our recommended default of weekly, helps avoid controversy

Essentially, make it as close to opt-in as possible, while still defaulting to what we recommend.

That said, it might not be a problem at all, but I've seen backlash in the past.

@djc
Copy link
Contributor

djc commented Feb 29, 2024

Are you looking for proxy-to-cargo latency, or are you talking about performance overhead?

If someone runs cargo --version, how much slower does that get?

My thought process was this:

I don't think the amount of controversy will change substantially based on whether this is an extra question (with an opt-in default) or part of the initial installation options (with the same opt-in default). We shouldn't make the UX worse.

@pitaj
Copy link
Author

pitaj commented Feb 29, 2024

If someone runs cargo --version, how much slower does that get?

Okay, I'll start by measuring the overhead that's already added by the proxy.

That will give a baseline on which to judge what impact 100ms would have.

Then I can make some quick changes to see what overhead just spawning a process would add.

@pitaj
Copy link
Author

pitaj commented Mar 2, 2024

I've measured the proxy latency by replacing the cargo proxy with a stub Rust program that just prints out the system time. I compare the system time from immediately before executing the cargo proxy to the time printed out by the stub.

Results from 1000 samples:

Average [ms] Min [ms] Max [ms]
Proxy -> stub 28.345899 26.674342 31.096926
Stub only 0.542705 0.508401 1.119083

So if we implement a 100ms timeout it would roughly 5x the latency. To be clear, that price would only be paid once a day at the most. Rustup overhead is already multiple orders of magnitude greater than a process spawn.

I will now fork Rustup and see if I can implement something quick and dirty to test this more realistically.

@rami3l
Copy link
Member

rami3l commented Mar 3, 2024

Note: Some say we're not doing well enough in terms of speed, see #2626.

@rbtcollins
Copy link
Contributor

A different approach which fits into some other vague thoughts we've had would be to have a rustup daemon that is running persistently; such a daemon could start implicitly when run, and take care of things like this update, but also permit other features like excluding concurrent use during updates, and avoid manifest parsing in each proxy invocation, which would drop the proxy latency substantially.

@pitaj
Copy link
Author

pitaj commented Mar 4, 2024

Adding a process spawn for an update check in the background appears to be insignificant, I wasn't able to measure a difference between that and the normal proxy latency.

But adding a blocking download-and-parse of the manifest (without even a process spawn) extended the proxy latency to almost 400ms. So the 200ms timeout won't work, unless we add a special endpoint just for update checking (and even that might not be enough).

For me, even a second of wait time would be acceptable if it only happened once a week. But I wouldn't be opposed to always running the check/download in the background - whether that be by a daemon or not.

(Side note: where does the Rustup team communicate? I don't see a zulip channel and it looks like the channel on the Discord either doesn't exist or is private)

@rami3l
Copy link
Member

rami3l commented Mar 4, 2024

(Side note: where does the Rustup team communicate? I don't see a zulip channel and it looks like the channel on the Discord either doesn't exist or is private)

@pitaj We have wg-rustup on Discord and we do welcome everyone to ask their questions... Maybe it's just an oversight?

@estebank
Copy link

estebank commented Mar 6, 2024

But adding a blocking download-and-parse of the manifest (without even a process spawn) extended the proxy latency to almost 400ms. So the 200ms timeout won't work, unless we add a special endpoint just for update checking (and even that might not be enough).

The check could run concurrently, taking its time and writing to disk so that the next cargo invocation can see that and display it to the user. I don't think this is something that should run every day. At most once a week, at least once every 3/6 months. Once a month sounds reasonable: faster than the release schedule to catch dot releases. I think I would be ok with a purely time based check every, lets say ~12 weeks (2 releases).

@pitaj
Copy link
Author

pitaj commented Mar 6, 2024

The check could run concurrently, taking its time and writing to disk so that the next cargo invocation can see that and display it to the user.

Yes, there are plenty of alternatives ranging from daemons to what you describe. Many were already suggested in the IRLO thread. I probably should have included more than one option in the original issue.

What you describe is actually the same as what I describe, under the condition that the blocking timeout = 0.

I don't think this is something that should run every day. At most once a week, at least once every 3/6 months. Once a month sounds reasonable: faster than the release schedule to catch dot releases. I think I would be ok with a purely time based check every, lets say ~12 weeks (2 releases).

Weekly is the best default, IMO. I think monthly is too slow, but ultimately it should be the user's choice. Down the line, it would be nice to support daily checks for the nightly toolchain anyways. Even on a daily basis, the check is practically zero cost.

@zackw
Copy link

zackw commented Mar 7, 2024

I think we should not do this. I think it will backfire and cause people to become more reluctant to update the toolchain, and/or make them not want to use Rust at all.

There are any number of perfectly good reasons why someone might want to stick to an old version of the toolchain, even a version that has fallen out of support. I estimate that a solid majority of all the Rust users I know personally would hate this notification and would immediately look for a way to turn it off. Some of them would file bugs asking for it to be ripped back out again. A larger group of people would mutter dark thoughts about the software industry's general habit of not respecting user consent and forcing unwanted "feature" upgrades on end users, and some of them would count this as a reason to consider Rust untrustworthy for their purposes.

Furthermore, users have strong expectations about the predictability of whether a command line tool starts a background process (especially a long-lived one) or talks to the network. For an existing tool like rustup or cargo, it's OK to add a new subcommand that does one of those things, but it is not OK to make an existing subcommand start doing one of those things as a side effect. Even if I didn't know a lot of people who want to stick to old versions of Rust, I would be opposed to this proposal for that reason alone.

[EDIT: To be crystal clear, I recognize that the proposal is not to automatically upgrade, only to notify of available upgrades. That is already a step too far in my opinion, for the reasons above.]

@pitaj
Copy link
Author

pitaj commented Mar 7, 2024

Your argument seems to only apply if the option is opt-out. If it was opt-in, would that be acceptable?

What are your thoughts on having an extra question on install?

Edit:

There are any number of perfectly good reasons why someone might want to stick to an old version of the toolchain, even a version that has fallen out of support.

I will just note that the solution to this is a rust-toolchain file or setting an explicit version as the default in rustup. Not upgrading the stable/beta/nightly channel is a very fragile way of accomplishing that goal.

@zackw
Copy link

zackw commented Mar 10, 2024

If it was opt-in, would that be acceptable?

Sure, but anyone who wants this can set up a cron job to run rustup update --check-only at whatever interval they feel is desirable, so I don't really see the point of building it into the tool.

Not upgrading the stable/beta/nightly channel is a very fragile way of [sticking to an old version of the toolchain]

Good point! Now I am tempted to send a PR to change the default behavior of rustup so that when you install stable the first time it pins that version of stable, and you have to explicitly ask for feature updates.

@pitaj
Copy link
Author

pitaj commented Mar 10, 2024

Sure, but anyone who wants this can set up a cron job to run rustup update --check-only at whatever interval they feel is desirable, so I don't really see the point of building it into the tool.

  • cron is not applicable on every platform
  • cron jobs can only be setup from a privileged user in some cases
  • you would still need some way of notifying the user when an update is available

@jeanas
Copy link

jeanas commented Mar 13, 2024

For "prior art", see:

$ pip install pycowsay
Defaulting to user installation because normal site-packages is not writeable
Collecting pycowsay
  Using cached pycowsay-0.0.0.2-py3-none-any.whl (4.0 kB)
Installing collected packages: pycowsay
Successfully installed pycowsay-0.0.0.2

[notice] A new release of pip is available: 23.1 -> 24.0
[notice] To update, run: python3 -m pip install --upgrade pip

Ok, pip is far from being the tool with the best user experience in general, but I don't recall hearing any particular pushback on that particular feature. And personally I don't find a nudge to upgrade intrusive at all, as long as it remains short and doesn't affect responsiveness noticeably. (It seems like a nice way to keep the ecosystem up-to-date; in other languages cough C++, people keep using compilers from decades ago because, you know, people tend to just not upgrade unless they're forced to, and this slows down the rate at which language features can be adopted by libraries dramatically.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants