Uh oh!
There was an error while loading. Please reload this page.
Refine suggestions about missing components - #3453
Conversation
Uh oh!
There was an error while loading. Please reload this page.
b9d70eb to
e152dc7CompareUh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
2d83297 to
d4ae176Compare@rbtcollins Looking at the blackbox test suite, it seems to me that this fix is more than what it seems to be. In toolchain updates, Line 173 in a4dd7d0 does get called, but the generated error then gets caught and re-thrown Lines 938 to 946 in a4dd7d0 to Line 45 in a4dd7d0 , which instead suggests using a To move this fix forward, I could simply remove the suggestion in For example, since this help message is very well-written, I wish to be able to see it in toolchain updates as well: Lines 22 to 28 in a4dd7d0 |
…_unavailable_msg` After digging into the codebase, I realized that the message generated by `component_unavailable_msg` is used only when: - Some components are missing from the toolchain so that the installation can no longer proceed; - We are not installing these components as a part of a toolchain-wide operation (e.g. updating the existing `nightly`), which is covered by `components_missing_msg` by catching the `RustupError::RequestedComponentsUnavailable` and re-throwing it as a `DistError::ToolchainComponentsMissing` (see <rust-lang#3453 (comment)> for more info). Thus, I decided to remove the `rustup component remove` suggestion altogether.
d4ae176 to
a3df139CompareNote: ... and they are not accurate anyway since the string form of those errors (before the catch-rethrow dance) is NOT ALWAYS visible to end users, which might cause further confusions for maintainers later on. |
I've yet to see any use case or test case that disobeys the observation stated in 6b5cb9f 's commit message. Therefore, I think by now it is reasonable to do the following:
|
Uh oh!
There was an error while loading. Please reload this page.
…_unavailable_msg` After digging into the codebase, I realized that the message generated by `component_unavailable_msg` is used only when: - Some components are missing from the toolchain so that the installation can no longer proceed; - We are not installing these components as a part of a toolchain-wide operation (e.g. updating the existing `nightly`), which is covered by `components_missing_msg` by catching the `RustupError::RequestedComponentsUnavailable` and re-throwing it as a `DistError::ToolchainComponentsMissing` (see <rust-lang#3453 (comment)> for more info). Thus, I decided to remove the `rustup component remove` suggestion altogether.
03456ff to
3cc2f55Compare…_unavailable_msg` After digging into the codebase, I realized that the message generated by `component_unavailable_msg` is used only when: - Some components are missing from the toolchain so that the installation can no longer proceed; - We are not installing these components as a part of a toolchain-wide operation (e.g. updating the existing `nightly`), which is covered by `components_missing_msg` by catching the `RustupError::RequestedComponentsUnavailable` and re-throwing it as a `DistError::ToolchainComponentsMissing` (see <rust-lang#3453 (comment)> for more info). Thus, I decided to remove the `rustup component remove` suggestion altogether.
3cc2f55 to
6f61089Comparerbtcollins
commented
Aug 30, 2023
seems reasonable to me |
@rbtcollins ... and those two tasks have been done, together with a new black box test case for the latter. |
6f61089 to
bcab5b8Compare…_unavailable_msg` After digging into the codebase, I realized that the message generated by `component_unavailable_msg` is used only when: - Some components are missing from the toolchain so that the installation can no longer proceed; - We are not installing these components as a part of a toolchain-wide operation (e.g. updating the existing `nightly`), which is covered by `components_missing_msg` by catching the `RustupError::RequestedComponentsUnavailable` and re-throwing it as a `DistError::ToolchainComponentsMissing` (see <rust-lang#3453 (comment)> for more info). Thus, I decided to remove the `rustup component remove` suggestion altogether.
7798b4a to
3a0950bCompare
djc
left a comment
There was a problem hiding this comment.
Please keep unrelated changes in separate commits, makes it much easier to review.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…_unavailable_msg` After digging into the codebase, I realized that the message generated by `component_unavailable_msg` is used only when: - Some components are missing from the toolchain so that the installation can no longer proceed; - We are not installing these components as a part of a toolchain-wide operation (e.g. updating the existing `nightly`), which is covered by `components_missing_msg` by catching the `RustupError::RequestedComponentsUnavailable` and re-throwing it as a `DistError::ToolchainComponentsMissing` (see <rust-lang#3453 (comment)> for more info). Thus, I decided to remove the `rustup component remove` suggestion altogether.
3a0950b to
512a2cdCompare512a2cd to
aee34d9Compare…_unavailable_msg` After digging into the codebase, I realized that the message generated by `component_unavailable_msg` is used only when: - Some components are missing from the toolchain so that the installation can no longer proceed; - We are not installing these components as a part of a toolchain-wide operation (e.g. updating the existing `nightly`), which is covered by `components_missing_msg` by catching the `RustupError::RequestedComponentsUnavailable` and re-throwing it as a `DistError::ToolchainComponentsMissing` (see <#3453 (comment)> for more info). Thus, I decided to remove the `rustup component remove` suggestion altogether.
Closes#3418. I also cleaned up the current code a bit along the way.
cc #2355