Update format of show and show active-toolchain

This changes the format of `rustup show` to be in a more logical order,
and changes the format of `rustup show active-toolchain` to match.
Also, as suggested in a comment, these commands will no longer install
the active toolchain if it is not already installed, as they now call
`cfg.find_active_toolchain()` instead of
`cfg.find_or_install_active_toolchain()`. This fixes
#1397

Author: majaha

doc/user-guide/src/overrides.md

@@ -19,10 +19,7 @@ the directory tree toward the filesystem root, and a `rust-toolchain.toml` file
 that is closer to the current directory will be preferred over a directory
 override that is further away.
 
-To verify which toolchain is active, you can use `rustup show`, 
-which will also try to install the corresponding
-toolchain if the current one has not been installed according to the above rules.
-(Please note that this behavior is subject to change, as detailed in issue [#1397].)
+To verify which toolchain is active, you can use `rustup show`.
 
 [toolchain]: concepts/toolchains.md
 [toolchain override shorthand]: #toolchain-override-shorthand

src/cli/rustup_mode.rs

@@ -20,7 +20,7 @@ use crate::{
         topical_doc,
     },
     command,
-    config::ActiveReason,
+    config::{new_toolchain_with_reason, ActiveReason},
     currentprocess::{
         argsource::ArgSource,
         filesource::{StderrSource, StdoutSource},
@@ -38,8 +38,9 @@ use crate::{
         names::{
             custom_toolchain_name_parser, maybe_resolvable_toolchainame_parser,
             partial_toolchain_desc_parser, resolvable_local_toolchainame_parser,
-            resolvable_toolchainame_parser, CustomToolchainName, MaybeResolvableToolchainName,
-            ResolvableLocalToolchainName, ResolvableToolchainName, ToolchainName,
+            resolvable_toolchainame_parser, CustomToolchainName, LocalToolchainName,
+            MaybeResolvableToolchainName, ResolvableLocalToolchainName, ResolvableToolchainName,
+            ToolchainName,
         },
         toolchain::Toolchain,
     },
@@ -1081,120 +1082,99 @@ fn show(cfg: &Cfg, m: &ArgMatches) -> Result<utils::ExitCode> {
 
     let cwd = utils::current_dir()?;
     let installed_toolchains = cfg.list_toolchains()?;
-    // XXX: we may want a find_without_install capability for show.
-    let active_toolchain = cfg.find_or_install_active_toolchain(&cwd);
-
-    // active_toolchain will carry the reason we don't have one in its detail.
-    let active_targets = if let Ok(ref at) = active_toolchain {
-        if let Ok(distributable) = DistributableToolchain::try_from(&at.0) {
-            match distributable.components() {
-                Ok(cs_vec) => cs_vec
-                    .into_iter()
-                    .filter(|c| c.component.short_name_in_manifest() == "rust-std")
-                    .filter(|c| c.installed)
-                    .collect(),
-                Err(_) => vec![],
-            }
+    let active_toolchain_and_reason: Option<(ToolchainName, ActiveReason)> =
+        if let Ok(Some((LocalToolchainName::Named(toolchain_name), reason))) =
+            cfg.find_active_toolchain(&cwd)
+        {
+            Some((toolchain_name, reason))
         } else {
-            // These three vec![] could perhaps be reduced with and_then on active_toolchain.
-            vec![]
-        }
-    } else {
-        vec![]
-    };
+            None
+        };
+
+    let (active_toolchain_name, _active_reason) = active_toolchain_and_reason
+        .as_ref()
+        .map(|atar| (&atar.0, &atar.1))
+        .unzip();
 
-    let show_installed_toolchains = installed_toolchains.len() > 1;
-    let show_active_targets = active_targets.len() > 1;
-    let show_active_toolchain = true;
-
-    // Only need to display headers if we have multiple sections
-    let show_headers = [
-        show_installed_toolchains,
-        show_active_targets,
-        show_active_toolchain,
-    ]
-    .iter()
-    .filter(|x| **x)
-    .count()
-        > 1;
-
-    if show_installed_toolchains {
+    let active_toolchain_targets: Vec<TargetTriple> = active_toolchain_name
+        .and_then(|atn| match atn {
+            ToolchainName::Official(desc) => DistributableToolchain::new(cfg, desc.clone()).ok(),
+            // So far, it is not possible to list targets for a custom toolchain.
+            ToolchainName::Custom(_) => None,
+        })
+        .and_then(|distributable| distributable.components().ok())
+        .map(|cs_vec| {
+            cs_vec
+                .into_iter()
+                .filter(|c| c.installed && c.component.short_name_in_manifest() == "rust-std")
+                .map(|c| c.component.target.expect("rust-std should have a target"))
+                .collect()
+        })
+        .unwrap_or_default();
+
+    // show installed toolchains
+    {
         let mut t = process().stdout().terminal();
 
-        if show_headers {
-            print_header::<Error>(&mut t, "installed toolchains")?;
-        }
-        let default_name = cfg
-            .get_default()?
-            .ok_or_else(|| anyhow!("no default toolchain configured"))?;
-        for it in installed_toolchains {
-            if default_name == it {
-                writeln!(t.lock(), "{it} (default)")?;
-            } else {
-                writeln!(t.lock(), "{it}")?;
-            }
+        print_header::<Error>(&mut t, "installed toolchains")?;
+
+        let default_toolchain_name = cfg.get_default()?;
+
+        let last_index = installed_toolchains.len().wrapping_sub(1);
+        for (n, toolchain_name) in installed_toolchains.into_iter().enumerate() {
+            let is_default_toolchain = default_toolchain_name.as_ref() == Some(&toolchain_name);
+            let is_active_toolchain = active_toolchain_name == Some(&toolchain_name);
+
+            let status_str = match (is_active_toolchain, is_default_toolchain) {
+                (true, true) => " (active, default)",
+                (true, false) => " (active)",
+                (false, true) => " (default)",
+                (false, false) => "",
+            };
+
+            writeln!(t.lock(), "{toolchain_name}{status_str}")?;
+
             if verbose {
-                let toolchain = Toolchain::new(cfg, it.into())?;
-                writeln!(process().stdout().lock(), "{}", toolchain.rustc_version())?;
-                // To make it easy to see what rustc that belongs to what
-                // toolchain we separate each pair with an extra newline
-                writeln!(process().stdout().lock())?;
+                let toolchain = Toolchain::new(cfg, toolchain_name.into())?;
+                writeln!(process().stdout().lock(), "  {}", toolchain.rustc_version())?;
+                // To make it easy to see which rustc belongs to which
+                // toolchain, we separate each pair with an extra newline.
+                if n != last_index {
+                    writeln!(process().stdout().lock())?;
+                }
             }
         }
-        if show_headers {
-            writeln!(t.lock())?
-        };
     }
 
-    if show_active_targets {
+    // show active toolchain
+    {
         let mut t = process().stdout().terminal();
 
-        if show_headers {
-            print_header::<Error>(&mut t, "installed targets for active toolchain")?;
-        }
-        for at in active_targets {
-            writeln!(
-                t.lock(),
-                "{}",
-                at.component
-                    .target
-                    .as_ref()
-                    .expect("rust-std should have a target")
-            )?;
-        }
-        if show_headers {
-            writeln!(t.lock())?;
-        };
-    }
+        writeln!(t.lock())?;
 
-    if show_active_toolchain {
-        let mut t = process().stdout().terminal();
+        print_header::<Error>(&mut t, "active toolchain")?;
 
-        if show_headers {
-            print_header::<Error>(&mut t, "active toolchain")?;
-        }
+        match active_toolchain_and_reason {
+            Some((active_toolchain_name, active_reason)) => {
+                let active_toolchain = new_toolchain_with_reason(
+                    cfg,
+                    active_toolchain_name.clone().into(),
+                    &active_reason,
+                )?;
+                writeln!(t.lock(), "name: {}", active_toolchain.name())?;
+                writeln!(t.lock(), "compiler: {}", active_toolchain.rustc_version())?;
+                writeln!(t.lock(), "active because: {}", active_reason)?;
 
-        match active_toolchain {
-            Ok((ref toolchain, ref reason)) => {
-                writeln!(t.lock(), "{} ({})", toolchain.name(), reason)?;
-                writeln!(t.lock(), "{}", toolchain.rustc_version())?;
-            }
-            Err(err) => {
-                let root_cause = err.root_cause();
-                if let Some(RustupError::ToolchainNotSelected) =
-                    root_cause.downcast_ref::<RustupError>()
-                {
-                    writeln!(t.lock(), "no active toolchain")?;
-                } else if let Some(cause) = err.source() {
-                    writeln!(t.lock(), "(error: {err}, {cause})")?;
-                } else {
-                    writeln!(t.lock(), "(error: {err})")?;
+                // show installed targets for the active toolchain
+                writeln!(t.lock(), "installed targets:")?;
+
+                for target in active_toolchain_targets {
+                    writeln!(t.lock(), "  {}", target)?;
                 }
             }
-        }
-
-        if show_headers {
-            writeln!(t.lock())?
+            None => {
+                writeln!(t.lock(), "no active toolchain")?;
+            }
         }
     }
 
@@ -1203,9 +1183,11 @@ fn show(cfg: &Cfg, m: &ArgMatches) -> Result<utils::ExitCode> {
         E: From<std::io::Error>,
     {
         t.attr(terminalsource::Attr::Bold)?;
-        writeln!(t.lock(), "{s}")?;
-        writeln!(t.lock(), "{}", "-".repeat(s.len()))?;
-        writeln!(t.lock())?;
+        {
+            let mut term_lock = t.lock();
+            writeln!(term_lock, "{s}")?;
+            writeln!(term_lock, "{}", "-".repeat(s.len()))?;
+        } // drop the term_lock
         t.reset()?;
         Ok(())
     }
@@ -1217,27 +1199,24 @@ fn show(cfg: &Cfg, m: &ArgMatches) -> Result<utils::ExitCode> {
 fn show_active_toolchain(cfg: &Cfg, m: &ArgMatches) -> Result<utils::ExitCode> {
     let verbose = m.get_flag("verbose");
     let cwd = utils::current_dir()?;
-    match cfg.find_or_install_active_toolchain(&cwd) {
-        Err(e) => {
-            let root_cause = e.root_cause();
-            if let Some(RustupError::ToolchainNotSelected) =
-                root_cause.downcast_ref::<RustupError>()
-            {
-            } else {
-                return Err(e);
-            }
-        }
-        Ok((toolchain, reason)) => {
+    match cfg.find_active_toolchain(&cwd)? {
+        Some((toolchain_name, reason)) => {
+            let toolchain = new_toolchain_with_reason(cfg, toolchain_name.clone(), &reason)?;
             writeln!(
                 process().stdout().lock(),
-                "{} ({})",
+                "{}\nactive because: {}",
                 toolchain.name(),
                 reason
             )?;
             if verbose {
-                writeln!(process().stdout().lock(), "{}", toolchain.rustc_version())?;
+                writeln!(
+                    process().stdout().lock(),
+                    "compiler: {}",
+                    toolchain.rustc_version()
+                )?;
             }
         }
+        None => writeln!(process().stdout().lock(), "There isn't an active toolchain")?,
     }
     Ok(utils::ExitCode(0))
 }

src/config.rs

@@ -108,8 +108,8 @@ pub(crate) enum ActiveReason {
 impl Display for ActiveReason {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> std::result::Result<(), fmt::Error> {
         match self {
-            Self::Default => write!(f, "default"),
-            Self::Environment => write!(f, "environment override by RUSTUP_TOOLCHAIN"),
+            Self::Default => write!(f, "it's the default toolchain"),
+            Self::Environment => write!(f, "overriden by environment variable RUSTUP_TOOLCHAIN"),
             Self::CommandLine => write!(f, "overridden by +toolchain on the command line"),
             Self::OverrideDB(path) => write!(f, "directory override for '{}'", path.display()),
             Self::ToolchainFile(path) => write!(f, "overridden by '{}'", path.display()),

tests/suite/cli_misc.rs

@@ -1002,7 +1002,7 @@ fn override_by_toolchain_on_the_command_line() {
         config.expect_stdout_ok(&["rustup", "+nightly", "which", "rustc"], "/bin/rustc");
         config.expect_stdout_ok(
             &["rustup", "+nightly", "show"],
-            "(overridden by +toolchain on the command line)",
+            "active because: overridden by +toolchain on the command line",
         );
         config.expect_err(
             &["rustup", "+foo", "which", "rustc"],