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

[Feature Request] Warn users of component availability changes #1277

Closed
Frederick888 opened this issue Oct 29, 2017 · 11 comments
Closed

[Feature Request] Warn users of component availability changes #1277

Frederick888 opened this issue Oct 29, 2017 · 11 comments

Comments

@Frederick888
Copy link

Rustup should warn users of component availability changes before going on with an update.

For instance, the rls component was once renamed to rls-preview and I was quite confused then about the missing of rls after the update. And just yesterday, rls-preview was temporarily removed and guys are now struggling downgrading to the last working one (rust-lang/rls#535).

So whenever there is a new component available, it's better to inform users of it, e.g.

A new component `shiny-new-component` has been added to `nightly-toolchain`.
Run `rustup component add shiny-new-component` to install it.

And if there is an installed component that is removed, I reckon we should prompt users for a confirmation, e.g.

The component `dev-essential-component` has been removed from `nightly-toolchain 2017-12-12`.
Are you sure you want to continue? [Y/n]
@toothbrush7777777
Copy link

toothbrush7777777 commented Nov 5, 2017

Yes, this is particularly important for RLS users. It is incredibly frustrating to find out that the latest nightly toolchain doesn't contain rls-preview after you update. 😭

The least we need is a flag which causes rustup to only update the toolchain if all installed components exist in the latest version.

@nrc
Copy link
Member

nrc commented Nov 6, 2017

This should be designed to work well for both users and for automated upgrades (e.g., an editor doing rustup update should be able to not to do that if the update will cause problems.

@davidbarsky
Copy link

Hi! I was thinking about how to approach this problem and wanted to get feedback before writing code. Think of this as a mini-RFC/design doc:

  1. Before running rustup update, check if static.rust-lang.org/dist/channel-rust-nightly.toml has all the user's installed components (Is there an authoritative source that describes all the user's installed components? I think it is defined in toolchains/<toolchain>/lib/rustlib/multirust-config.toml).

  2. If a component availability regression is detected,rustup update should quit and communicate the issue to the user (Should there be a --force flag to update regardless of component availability?)

  3. Proceed with the update normally.

I'm not entirely sure how to approach automated upgrades. What's the best way to communicate that to editors?

@imp
Copy link

imp commented Dec 5, 2017

I'd imagine having a special flag for IDE to use (like --auto-update) that will automatically set safe defaults. (No upgrades that break thing). It will be up to IDE plugin maintainer to use this flag for automatic update.

@nrc
Copy link
Member

nrc commented Dec 5, 2017

@davidbarsky this sounds basically right to me, and thanks for offering to do the work, it is much appreciated!

Before running rustup update, check if static.rust-lang.org/dist/channel-rust-nightly.toml has all the user's installed components

I believe rustup update downloads this manifest as the first step in doing an update, so you shouldn't have to check it separately. Be aware that the manifest can include renames and these should be taken into account (e.g., if foo is renamed to bar then foo should not be counted as missing).

Note also, that the component must be present for all targets and all channels that the user has installed, and when warning the user, we should list the tagets/channels where the component is missing.

If a component availability regression is detected,rustup update should quit and communicate the issue to the user

yes

Should there be a --force flag to update regardless of component availability?

Yes. And the message to the user should suggest this as a possible solution.

I'm not entirely sure how to approach automated upgrades. What's the best way to communicate that to editors?

I think this setup should work well for editors - the editor can attempt the update and if rustup quits, the editor can try to parse stdout to check for this issue and communicate with the user.

I'm very happy to help out with this - please feel free to ping me any time here or on irc.

@davidbarsky
Copy link

this sounds basically right to me, and thanks for offering to do the work, it is much appreciated!

@nrc Of course! I'll be able to take a look on later this week and get a draft out for review (or at least, something that other cans build upon).

I believe rustup update downloads this manifest as the first step in doing an update, so you shouldn't have to check it separately.

Gotcha. Several assorted questions:

  • My understanding of rustup is that it tries to update a toolchain (which will include a compiler, etc.) and then install components, which are separate. Is my mental model correct? (The alternate model that I built up is that components are part of a toolchain, so it's one transaction).

  • Do you have any package and tests that you recommend looking at that are relevant to this issue?

Be aware that the manifest can include renames and these should be taken into account (e.g., if foo is renamed to bar then foo should not be counted as missing).

Is the rename documented/handled in some existing code, or is rustup unaware of this renames?

I'm very happy to help out with this - please feel free to ping me any time here or on irc.

No, thank you for your help!

@nrc
Copy link
Member

nrc commented Dec 6, 2017

My understanding of rustup is that it tries to update a toolchain (which will include a compiler, etc.) and then install components, which are separate. Is my mental model correct? (The alternate model that I built up is that components are part of a toolchain, so it's one transaction

The compiler and libs etc., are just components like the tools. They are all part of a toolchain (rustup is aware of the target vs host difference). There are some differences because some components are optional (like tools) and some are not (like the compiler). Also, tools are installed differently to the compiler and libs.

I'm not sure how 'transactional' rustup actually is, but tools and the compiler are treated pretty much similarly.

Do you have any package and tests that you recommend looking at that are relevant to this issue?

I don't know, sorry

s the rename documented/handled in some existing code, or is rustup unaware of this renames?

Yes. The renames are an item in the manifest and rustup is aware of them.

@Frederick888
Copy link
Author

Frederick888 commented Dec 6, 2017

Is it also possible to provide such a way so that new users can specify "essential components" when installing the Rust stack?

@davidbarsky
Copy link

Thanks very much, Nick! That’s a useful starting point!

(Sorry my questions weren’t better edited.)

@Xanewok
Copy link
Member

Xanewok commented Dec 28, 2017

Hey @davidbarsky! How is the work coming along?

@davidbarsky
Copy link

davidbarsky commented Jan 2, 2018 via email

bors added a commit that referenced this issue Jan 15, 2018
Warn when tools are missing and allow to override

Closes #1277

Idea here is to not update if the RLS is missing (for example). There was actually functionality to do this already, but it was not catching the RLS and Rustfmt since they were missing in an unexpected way.

The second commit adds `rustup update --force` to update even if some components are missing

r? @alexcrichton
bors added a commit that referenced this issue Jan 16, 2018
Warn when tools are missing and allow to override

Closes #1277

Idea here is to not update if the RLS is missing (for example). There was actually functionality to do this already, but it was not catching the RLS and Rustfmt since they were missing in an unexpected way.

The second commit adds `rustup update --force` to update even if some components are missing

r? @alexcrichton
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

6 participants