feat: improve error message for `rustup which` #4429

Bogay · 2025-07-26T18:21:11Z

This PR improves the error message for rustup which by providing a suggested command to install the missing component, provided that we know which one it is.

Limitations

Some edge cases are not coverd, e.g. if a binary is expected to be provided by a component but not found on disk. (which is handled in DistributableToolchain::recursion_error)

djc

Seems like a good start!

Obviously this should pass CI -- would also be great if you could make sure there are tests to cover the new code paths.

djc · 2025-07-26T19:58:32Z

src/cli/rustup_mode.rs

+    let toolchain_name = toolchain.name();
+    let Some(component_name) = component_for_bin(binary) else {
+        return Err(anyhow!(
+            "Unknown binary '{binary}' in toolchain '{toolchain_name}'.",


Nit: should drop the capital at the start and period at the end here for consistency.

djc · 2025-07-26T19:59:06Z

src/cli/rustup_mode.rs

+    return Err(anyhow!(
+        "'{binary}' is not installed for the toolchain '{toolchain_name}'.\nTo install, run `rustup component add {selector}{component_name}`"
+    ));


Nit: idiomatically, no need to use return here, can just make Err(..) the tail expression.

djc · 2025-07-26T20:00:30Z

src/cli/rustup_mode.rs

+    let active = matches!(cfg.active_toolchain(), Ok(Some((t, _))) if &t == toolchain_name);
+    let selector = if active {
+        String::new()
+    } else {
+        format!("--toolchain {} ", toolchain.name())
+    };


This seems little roundabout, what about something more like:

let selector = match cfg.active_toolchain() { Ok(Some((t, _))) if t == toolchain_name => String::new(), _ => format!("--toolchain {} ", toolchain.name()), };

djc · 2025-07-26T20:01:05Z

src/cli/rustup_mode.rs

@@ -47,6 +46,7 @@ use crate::{
    },
    utils::{self, ExitCode},
 };
+use crate::{component_for_bin, dist::AutoInstallMode};


If you're going to move this around, would be nicer to move both of these into the big crate block just above. 👍

djc

LGTM! Would be good to squash your commits into a single one, and mark the PR as ready for review.

rami3l

Thanks @djc for doing the review! I have one question though:

What do you think about the edge case mentioned here?

djc · 2025-07-27T11:55:22Z

What do you think about the edge case mentioned here?

The situation with this PR merged seems strictly better to me than the current state on master. Maybe there are still further improvements to be made but it's not obvious to me they should necessarily be a part of this PR.

rami3l · 2025-07-27T12:18:57Z

What do you think about the edge case mentioned here?

The situation with this PR merged seems strictly better to me than the current state on master. Maybe there are still further improvements to be made but it's not obvious to me they should necessarily be a part of this PR.

@djc I agree. That said, it might still be worth it if @Bogay can put these considerations at the top of this thread (in a separate section, say Limitations) instead of a separate commit comment :)

Bogay · 2025-07-28T16:51:00Z

I have put these considerations to the PR description and squash my commits into one. I think it's ready for review now. Regarding the edge case I mentioned, I am not sure whether I should try to solve it? or wait for someone to encounter it and open an issue.

src/cli/rustup_mode.rs

rami3l

LGTM, and thanks for the patch!

@djc Can you confirm that your concerns have been properly addressed as well?

rami3l · 2025-07-29T15:41:25Z

I have put these considerations to the PR description and squash my commits into one. I think it's ready for review now. Regarding the edge case I mentioned, I am not sure whether I should try to solve it? or wait for someone to encounter it and open an issue.

@Bogay If you think a further PR is possible, I would greatly appreciate it :)

Bogay

I've updated the relevant error message to suggest how users can install the missing component. For the long term, I'm thinking we should find a more consistent way to manage error messages across the project. Do you have any thoughts on how we could approach this, perhaps with custom error types or utility functions?

Bogay · 2025-07-29T15:33:29Z

src/cli/rustup_mode.rs

-            "To install, try `rustup component add --toolchain {} rust-docs`",
+            "run `rustup component add --toolchain {} rust-docs` to install it",


Because info! add info: prefix to output message and I think info: help: looks a little weird. I don't add the help: prefix here. Not sure whether it's better to merge these two lines info the error message below?

Nice catch! Yeah, I think in that case it'd be better to just do a single info!() call instead of two...

Updated. We have only one info!() call now 👍

Also adds more tests for `rustup which`. Includes: - ask binary included by uninstalled component - ask binary included by uninstalled component with non-default toolchain - ask binary not known to be included by what toolchain

to suggest command to install missing components. make them more consistent.

Bogay mentioned this pull request Jul 26, 2025

Give a more helpful error message for rustup which rustfmt if it's not installed #3321

Open

djc reviewed Jul 26, 2025

View reviewed changes

Bogay force-pushed the 3321-improve-rustup-which-error-message branch from fbfb71c to aa79802 Compare July 27, 2025 07:46

djc approved these changes Jul 27, 2025

View reviewed changes

rami3l reviewed Jul 27, 2025

View reviewed changes

Bogay force-pushed the 3321-improve-rustup-which-error-message branch from 54ef67d to 02c0813 Compare July 28, 2025 16:07

Bogay marked this pull request as ready for review July 28, 2025 16:09

Bogay requested review from djc and rami3l July 28, 2025 16:51

rami3l reviewed Jul 29, 2025

View reviewed changes

src/cli/rustup_mode.rs Outdated Show resolved Hide resolved

rami3l approved these changes Jul 29, 2025

View reviewed changes

Bogay commented Jul 29, 2025

View reviewed changes

Bogay added 2 commits July 30, 2025 00:03

feat: improve error message for rustup which

07437e7

Also adds more tests for `rustup which`. Includes: - ask binary included by uninstalled component - ask binary included by uninstalled component with non-default toolchain - ask binary not known to be included by what toolchain

feat(rustup_mode): revise help message

0bcf123

to suggest command to install missing components. make them more consistent.

Bogay force-pushed the 3321-improve-rustup-which-error-message branch from 1248e0e to 0bcf123 Compare July 29, 2025 16:03

		"To install, try `rustup component add --toolchain {} rust-docs`",
		"run `rustup component add --toolchain {} rust-docs` to install it",

feat: improve error message for rustup which #4429

Are you sure you want to change the base?

feat: improve error message for rustup which #4429

Uh oh!

Conversation

Bogay commented Jul 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Limitations

Uh oh!

djc left a comment

Choose a reason for hiding this comment

Uh oh!

djc Jul 26, 2025

Choose a reason for hiding this comment

Uh oh!

djc Jul 26, 2025

Choose a reason for hiding this comment

Uh oh!

djc Jul 26, 2025

Choose a reason for hiding this comment

Uh oh!

djc Jul 26, 2025

Choose a reason for hiding this comment

Uh oh!

djc left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rami3l left a comment

Choose a reason for hiding this comment

Uh oh!

djc commented Jul 27, 2025

Uh oh!

rami3l commented Jul 27, 2025

Uh oh!

Bogay commented Jul 28, 2025

Uh oh!

Uh oh!

rami3l left a comment

Choose a reason for hiding this comment

Uh oh!

rami3l commented Jul 29, 2025

Uh oh!

Bogay left a comment

Choose a reason for hiding this comment

Uh oh!

Bogay Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

rami3l Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Bogay Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

feat: improve error message for `rustup which` #4429

feat: improve error message for `rustup which` #4429

Bogay commented Jul 26, 2025 •

edited

Loading

djc left a comment •

edited

Loading

rami3l Jul 29, 2025 •

edited

Loading