rustup_mode: add toolchain install --allow-downgrade option #2126
Conversation
kinnison
changed the title
payload
force-pushed
the
allow-downgrade
branch
from
a9b2325
to
db96bf9
Compare
payload
changed the title
|
The failed CI run ended with the message: I consider this flakyness of the underlying system, probably some filesystem hickup. I don't know if it is probably related to code in rustup set default-host which could be more robust against yet uncertain filesystem races. @kinnison what do you think of it? |
|
@payload Yes that's a transient issue I only ever tend to see on Travis. As such I've not managed to track it down to where it actually happens to add a retry in for that case. |
There was a problem hiding this comment.
For the test, I'd be tempted to add the --allow-downgrade variant to the end of the longer test, so that the scenario plays out in full as:
- Install
2 Check for rls -- should not be present - Attempt to add rls -- this should fail
- Attempt to add rls with downgrate permitted -- this should succeed
- Check for rls -- should be present
- Check hash for rustc -- should have downgraded
| "component 'rls' for target '{}' is unavailable for download for channel nightly", | ||
| trip, | ||
| ), | ||
| ); |
There was a problem hiding this comment.
Perhaps here add a check that the rls component is not installed You can do that with:
expect_component_not_executable(config, "rls");There was a problem hiding this comment.
Although I added it I would rather see that in a different testcase which checks if toolchain install does not change anything without allow-downgrade. When I look at the subcommand toolchain install I see 7 arguments and would expect at least as many as 7 tests at the level of cli-v2.rs, but there are not. Or else I don't know yet about all the test levels. Well that's obvious :)
| "nightly", | ||
| "--no-self-update", | ||
| ], | ||
| ); |
There was a problem hiding this comment.
After this, I'd add an expect to check the rustc version is the version we expect (i.e. one without rls)
There was a problem hiding this comment.
Yes, I add that. It is though like a sanity check to find out if our test setup is based on the right assumptions.
| "--allow-downgrade", | ||
| ], | ||
| ); | ||
| expect_stdout_ok(config, &["rustc", "--version"], "hash-nightly-2"); |
There was a problem hiding this comment.
While this is useful to check that it did downgrade, that'd be clearer if you'd done the equivalent above as commented. Also you could usefully assert that the rls component is now available via expect_component_executable()
There was a problem hiding this comment.
I like the expect_component_executable check much more than the version check. Thx
In case you have an already installed nightly toolchain and you want to install a nightly toolchain with a component which is not available in the installed toolchain, rustup is searching for the most recent nightly supporting all requested components and which is not older than the currently installed nightly. With this option set it will also search for toolchains older than the currently installed one.
payload
force-pushed
the
allow-downgrade
branch
from
a11791f
to
cabee8c
Compare
|
@kinnison squashed it together. I have put the code change and its test together. When you split the test from the implementation it is less clear what belongs together. One reason to split test from implementation is of course to put the test before the implementation so you see the test failing first. Do you mean to put a separate test commit before the implementation commit? |
In case you have an already installed nightly toolchain
and you want to install a nightly toolchain with a component
which is not available in the installed toolchain,
rustup is searching for the most recent nightly supporting
all requested components and which is not older than the currently
installed nightly.
With this option set it will also search for toolchains older
than the currently installed one.
Should fix #2067