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 when tools are missing and allow to override #1337

Merged
merged 3 commits into from Jan 16, 2018

Conversation

Projects
None yet
3 participants
@nrc
Copy link
Member

nrc commented Jan 11, 2018

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

nrc added some commits Jan 5, 2018

Catch some missing components that weren't warned on
There is a lot of refactoring here too.

There was a check to ensure that if there are components that we plan to install (or update) that are missing from the manifest, then we error out. However, this only worked for components that had an entry in the manifest under the Rust package, but were missing components of their own. For tools like the RLS, when they are missing, they are completely missing from the manifest. They therefore didn't get picked up by the check. This patch adds an extra Vec to track such components and then warns about them.

@nrc nrc force-pushed the nrc:ui-missing-tool branch from fba0f3e to 514984b Jan 11, 2018

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Jan 11, 2018

Looks good to me! I think though that the added test may be failing?

tests
This adds a new test and removes test `update_removes_components_that_dont_exist`

Historically, this test was added before the check that aborts update on missing
components (which is tested by the new test in the previous commit, as well as
elsewhere), so I think this test was only testing a bug.

Note that the new behaviour is the same as the ancient behviour when the user
uses `--force`

@nrc nrc force-pushed the nrc:ui-missing-tool branch from 28dd69d to dcf0f9d Jan 14, 2018

@nrc

This comment has been minimized.

Copy link
Member Author

nrc commented Jan 15, 2018

OK, looks like we're passing tests now (I guess Travis is stuck or got bored or something).

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Jan 15, 2018

@bors: r+

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 15, 2018

📌 Commit dcf0f9d has been approved by alexcrichton

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 15, 2018

⌛️ Testing commit dcf0f9d with merge 22f1569...

bors added a commit that referenced this pull request Jan 15, 2018

Auto merge of #1337 - nrc:ui-missing-tool, r=alexcrichton
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

This comment has been minimized.

Copy link
Contributor

bors commented Jan 16, 2018

💥 Test timed out

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Jan 16, 2018

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 16, 2018

⌛️ Testing commit dcf0f9d with merge afbeeaf...

bors added a commit that referenced this pull request Jan 16, 2018

Auto merge of #1337 - nrc:ui-missing-tool, r=alexcrichton
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

This comment has been minimized.

Copy link
Contributor

bors commented Jan 16, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing afbeeaf to master...

@bors bors merged commit dcf0f9d into rust-lang:master Jan 16, 2018

2 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.