Auto merge of #13217 - dtolnay-contrib:toolsutil, r=flip1995

Check exit status of subcommands spawned by rustc_tools_util

The git commands `git rev-parse --short HEAD` and `git log -1 --date=short --pretty=format:%cd` that clippy runs from its build script might fail with **"fatal: not a git repository (or any of the parent directories): .git"** if clippy is being built from a source tarball rather than a git repository. That message is written by git to stderr, and nothing is written to stdout.

For `clippy-driver --version` this PR wouldn't make a difference because it treats empty stdout and failed spawns (`git` is not installed) identically:

https://github.com/rust-lang/rust-clippy/blob/7ac242c3d0d3ee867a6c9cdcbdec986c408ac36f/rustc_tools_util/src/lib.rs#L35-L42

But other users of `rustc_tools_util` should be able to expect that the distinction between Some and None is meaningful. They shouldn't need extra code to handle None vs Some-and-empty vs Some-and-nonempty.

---

changelog: none

1 files changed, 27 insertions, 29 deletions

diff --git a/rustc_tools_util/src/lib.rs b/rustc_tools_util/src/lib.rs
index 4c1d8c3733d..f8b8084ca1a 100644
--- a/rustc_tools_util/src/lib.rs
+++ b/rustc_tools_util/src/lib.rs
@@ -1,5 +1,7 @@
 #![cfg_attr(feature = "deny-warnings", deny(warnings))]
 
+use std::str;
+
 /// This macro creates the version string during compilation from the
 /// current environment
 #[macro_export]
@@ -101,49 +103,45 @@ impl std::fmt::Debug for VersionInfo {
 
 #[must_use]
 pub fn get_commit_hash() -> Option<String> {
-    std::process::Command::new("git")
+    let output = std::process::Command::new("git")
         .args(["rev-parse", "--short", "HEAD"])
         .output()
-        .ok()
-        .and_then(|r| String::from_utf8(r.stdout).ok())
+        .ok()?;
+    let stdout = output.status.success().then_some(output.stdout)?;
+    String::from_utf8(stdout).ok()
 }
 
 #[must_use]
 pub fn get_commit_date() -> Option<String> {
-    std::process::Command::new("git")
+    let output = std::process::Command::new("git")
         .args(["log", "-1", "--date=short", "--pretty=format:%cd"])
         .output()
-        .ok()
-        .and_then(|r| String::from_utf8(r.stdout).ok())
+        .ok()?;
+    let stdout = output.status.success().then_some(output.stdout)?;
+    String::from_utf8(stdout).ok()
 }
 
 #[must_use]
 pub fn get_channel() -> String {
-    match std::env::var("CFG_RELEASE_CHANNEL") {
-        Ok(channel) => channel,
-        Err(_) => {
-            // if that failed, try to ask rustc -V, do some parsing and find out
-            match std::process::Command::new("rustc")
-                .arg("-V")
-                .output()
-                .ok()
-                .and_then(|r| String::from_utf8(r.stdout).ok())
-            {
-                Some(rustc_output) => {
-                    if rustc_output.contains("beta") {
-                        String::from("beta")
-                    } else if rustc_output.contains("stable") {
-                        String::from("stable")
-                    } else {
-                        // default to nightly if we fail to parse
-                        String::from("nightly")
-                    }
-                },
-                // default to nightly
-                None => String::from("nightly"),
+    if let Ok(channel) = std::env::var("CFG_RELEASE_CHANNEL") {
+        return channel;
+    }
+
+    // if that failed, try to ask rustc -V, do some parsing and find out
+    if let Ok(output) = std::process::Command::new("rustc").arg("-V").output() {
+        if output.status.success() {
+            if let Ok(rustc_output) = str::from_utf8(&output.stdout) {
+                if rustc_output.contains("beta") {
+                    return String::from("beta");
+                } else if rustc_output.contains("stable") {
+                    return String::from("stable");
+                }
             }
-        },
+        }
     }
+
+    // default to nightly
+    String::from("nightly")
 }
 
 #[cfg(test)]