Merge pull request #3366 from hi-rustin/rustin-patch-target-error

Improve error message for adding/removing invalid target

Author: rbtcollins

src/errors.rs

@@ -10,7 +10,10 @@ use std::path::PathBuf;
 use thiserror::Error as ThisError;
 use url::Url;
 
-use crate::{currentprocess::process, dist::dist::ToolchainDesc};
+use crate::{
+    currentprocess::process,
+    dist::dist::{TargetTriple, ToolchainDesc},
+};
 use crate::{
     dist::manifest::{Component, Manifest},
     toolchain::names::{PathBasedToolchainName, ToolchainName},
@@ -114,6 +117,30 @@ pub(crate) enum RustupError {
         component: String,
         suggestion: Option<String>,
     },
+    #[error("toolchain '{}' does not support target '{}'{}\n\
+    note: you can see a list of supported targets with `rustc --print=target-list`\n\
+    note: if you are adding support for a new target to rustc itself, see https://rustc-dev-guide.rust-lang.org/building/new-target.html", .desc, .target,
+    if let Some(suggestion) = .suggestion {
+        format!("; did you mean '{suggestion}'?")
+    } else {
+        "".to_string()
+    })]
+    UnknownTarget {
+        desc: ToolchainDesc,
+        target: TargetTriple,
+        suggestion: Option<String>,
+    },
+    #[error("toolchain '{}' does not have target '{}' installed{}\n", .desc, .target,
+    if let Some(suggestion) = .suggestion {
+        format!("; did you mean '{suggestion}'?")
+    } else {
+        "".to_string()
+    })]
+    TargetNotInstalled {
+        desc: ToolchainDesc,
+        target: TargetTriple,
+        suggestion: Option<String>,
+    },
     #[error("unknown metadata version: '{0}'")]
     UnknownMetadataVersion(String),
     #[error("manifest version '{0}' is not supported")]

src/toolchain/distributable.rs

@@ -70,11 +70,25 @@ impl<'a> DistributableToolchain<'a> {
                 component = wildcard_component;
             } else {
                 let config = manifestation.read_config()?.unwrap_or_default();
+                let suggestion =
+                    self.get_component_suggestion(&component, &config, &manifest, false);
+                // Check if the target is supported.
+                if !targ_pkg
+                    .components
+                    .iter()
+                    .any(|c| c.target() == component.target())
+                {
+                    return Err(RustupError::UnknownTarget {
+                        desc: self.desc.clone(),
+                        target: component.target.expect("component target should be known"),
+                        suggestion,
+                    }
+                    .into());
+                }
                 return Err(RustupError::UnknownComponent {
                     desc: self.desc.clone(),
                     component: component.description(&manifest),
-                    suggestion: self
-                        .get_component_suggestion(&component, &config, &manifest, false),
+                    suggestion,
                 }
                 .into());
             }
@@ -448,10 +462,25 @@ impl<'a> DistributableToolchain<'a> {
             if config.components.contains(&wildcard_component) {
                 component = wildcard_component;
             } else {
+                let suggestion =
+                    self.get_component_suggestion(&component, &config, &manifest, true);
+                // Check if the target is installed.
+                if !config
+                    .components
+                    .iter()
+                    .any(|c| c.target() == component.target())
+                {
+                    return Err(RustupError::TargetNotInstalled {
+                        desc: self.desc.clone(),
+                        target: component.target.expect("component target should be known"),
+                        suggestion,
+                    }
+                    .into());
+                }
                 return Err(RustupError::UnknownComponent {
                     desc: self.desc.clone(),
                     component: component.description(&manifest),
-                    suggestion: self.get_component_suggestion(&component, &config, &manifest, true),
+                    suggestion,
                 }
                 .into());
             }

tests/suite/cli_v2.rs

@@ -752,9 +752,9 @@ fn add_target_bogus() {
         config.expect_ok(&["rustup", "default", "nightly"]);
         config.expect_err(
             &["rustup", "target", "add", "bogus"],
-            "does not contain component 'rust-std' for target 'bogus'\n\
-                note: not all platforms have the standard library pre-compiled: https://doc.rust-lang.org/nightly/rustc/platform-support.html\n\
-                help: consider using `cargo build -Z build-std` instead",
+            "does not support target 'bogus'\n\
+            note: you can see a list of supported targets with `rustc --print=target-list`\n\
+            note: if you are adding support for a new target to rustc itself, see https://rustc-dev-guide.rust-lang.org/building/new-target.html",
         );
     });
 }
@@ -866,7 +866,7 @@ fn remove_target_not_installed() {
         config.expect_err(
             &["rustup", "target", "remove", clitools::CROSS_ARCH1],
             &format!(
-                "toolchain 'nightly-{}' does not contain component 'rust-std' for target '{}'",
+                "toolchain 'nightly-{}' does not have target '{}' installed",
                 this_host_triple(),
                 clitools::CROSS_ARCH1
             ),
@@ -896,7 +896,7 @@ fn remove_target_bogus() {
         config.expect_ok(&["rustup", "default", "nightly"]);
         config.expect_err(
             &["rustup", "target", "remove", "bogus"],
-            "does not contain component 'rust-std' for target 'bogus'",
+            "does not have target 'bogus' installed",
         );
     });
 }
@@ -941,7 +941,7 @@ fn remove_target_again() {
         config.expect_err(
             &["rustup", "target", "remove", clitools::CROSS_ARCH1],
             &format!(
-                "toolchain 'nightly-{}' does not contain component 'rust-std' for target '{}'",
+                "toolchain 'nightly-{}' does not have target '{}' installed",
                 this_host_triple(),
                 clitools::CROSS_ARCH1
             ),