unix: Relax escaping in Debug impl on Command

library/std/src/lib.rs

@@ -285,6 +285,7 @@
 #![feature(cfg_target_thread_local)]
 #![feature(cfi_encoding)]
 #![feature(concat_idents)]
+#![feature(debug_closure_helpers)]
 #![feature(decl_macro)]
 #![feature(deprecated_suggestion)]
 #![feature(doc_cfg)]

library/std/src/process/tests.rs

@@ -552,7 +552,7 @@ fn debug_print() {
 
     let mut command = Command::new("some-boring-name");
 
-    assert_eq!(format!("{command:?}"), format!(r#""some-boring-name""#));
+    assert_eq!(format!("{command:?}"), format!("some-boring-name"));
 
     assert_eq!(
         format!("{command:#?}"),
@@ -568,7 +568,7 @@ fn debug_print() {
 
     command.args(&["1", "2", "3"]);
 
-    assert_eq!(format!("{command:?}"), format!(r#""some-boring-name" "1" "2" "3""#));
+    assert_eq!(format!("{command:?}"), format!("some-boring-name 1 2 3"));
 
     assert_eq!(
         format!("{command:#?}"),
@@ -587,10 +587,7 @@ fn debug_print() {
 
     crate::os::unix::process::CommandExt::arg0(&mut command, "exciting-name");
 
-    assert_eq!(
-        format!("{command:?}"),
-        format!(r#"["some-boring-name"] "exciting-name" "1" "2" "3""#)
-    );
+    assert_eq!(format!("{command:?}"), format!("[some-boring-name] exciting-name 1 2 3"));
 
     assert_eq!(
         format!("{command:#?}"),
@@ -609,10 +606,7 @@ fn debug_print() {
 
     let mut command_with_env_and_cwd = Command::new("boring-name");
     command_with_env_and_cwd.current_dir("/some/path").env("FOO", "bar");
-    assert_eq!(
-        format!("{command_with_env_and_cwd:?}"),
-        r#"cd "/some/path" && FOO="bar" "boring-name""#
-    );
+    assert_eq!(format!("{command_with_env_and_cwd:?}"), "cd /some/path && FOO=bar boring-name");
     assert_eq!(
         format!("{command_with_env_and_cwd:#?}"),
         format!(
@@ -638,7 +632,7 @@ fn debug_print() {
 
     let mut command_with_removed_env = Command::new("boring-name");
     command_with_removed_env.env_remove("FOO").env_remove("BAR");
-    assert_eq!(format!("{command_with_removed_env:?}"), r#"env -u BAR -u FOO "boring-name""#);
+    assert_eq!(format!("{command_with_removed_env:?}"), "env -u BAR -u FOO boring-name");
     assert_eq!(
         format!("{command_with_removed_env:#?}"),
         format!(
@@ -660,7 +654,7 @@ fn debug_print() {
 
     let mut command_with_cleared_env = Command::new("boring-name");
     command_with_cleared_env.env_clear().env("BAR", "val").env_remove("FOO");
-    assert_eq!(format!("{command_with_cleared_env:?}"), r#"env -i BAR="val" "boring-name""#);
+    assert_eq!(format!("{command_with_cleared_env:?}"), "env -i BAR=val boring-name");
     assert_eq!(
         format!("{command_with_cleared_env:#?}"),
         format!(

library/std/src/sys/pal/unix/process/process_common.rs

@@ -580,8 +580,56 @@ impl fmt::Debug for Command {
 
             debug_command.finish()
         } else {
+            /// Only escape arguments when necessary, otherwise output it
+            /// unescaped to make the output easier to read.
+            ///
+            /// NOTE: This is best effort only!
+            fn relaxed_escaping(bytes: &[u8], is_arg: bool) -> impl fmt::Display + use<'_> {
+                // Don't escape if all the characters are likely
+                // to not be a problem when copied to the shell.
+                let can_print_safely = move |&c| {
+                    // See: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_02
+                    match c {
+                        // The documentation linked above says that `=`:
+                        // > may need to be quoted under certain circumstances
+                        //
+                        // Because they may be parsed as a variable assignment.
+                        // But in argument position to a command, they
+                        // shouldn't need escaping.
+                        b'=' if is_arg => true,
+                        // As per documentation linked above:
+                        // > The application shall quote the following characters
+                        b'|' | b'&' | b';' | b'<' | b'>' | b'(' | b')' | b'$' | b'`' | b'\\'
+                        | b'"' | b'\'' | b' ' | b'\t' | b'\n' => false,
+                        // As per documentation linked above:
+                        // > may need to be quoted under certain circumstances
+                        b'*' | b'?' | b'[' | b'#' | b'~' | b'=' | b'%' => false,
+                        // It'd be weird to quote `[` and not quote `]`.
+                        b']' => false,
+                        // ! does history expansion in Bash, require quoting for that as well.
+                        //
+                        // NOTE: This doesn't currently work properly, we'd need to backslash-escape
+                        // `!` as well (which `escape_ascii` doesn't do).
+                        b'!' => false,
+                        // Assume all other printable characters may be output.
+                        0x20..0x7e => true,
+                        // Unprintable or not ASCII.
+                        _ => false,
+                    }
+                };
+                fmt::from_fn(move |f| {
+                    if !bytes.is_empty() && bytes.iter().all(can_print_safely) {
+                        // Unwrap is fine, we've checked above that the bytes only contains ascii.
+                        let ascii = crate::str::from_utf8(bytes).unwrap();
+                        write!(f, "{ascii}")
+                    } else {
+                        write!(f, "\"{}\"", bytes.escape_ascii())
+                    }
+                })
+            }
+
             if let Some(ref cwd) = self.cwd {
-                write!(f, "cd {cwd:?} && ")?;
+                write!(f, "cd {} && ", relaxed_escaping(cwd.to_bytes(), true))?;
             }
             if self.env.does_clear() {
                 write!(f, "env -i ")?;
@@ -595,23 +643,29 @@ impl fmt::Debug for Command {
                             write!(f, "env ")?;
                             any_removed = true;
                         }
-                        write!(f, "-u {} ", key.to_string_lossy())?;
+                        write!(f, "-u {} ", relaxed_escaping(key.as_encoded_bytes(), false))?;
                     }
                 }
             }
             // Altered env vars can just be added in front of the program.
             for (key, value_opt) in self.get_envs() {
                 if let Some(value) = value_opt {
-                    write!(f, "{}={value:?} ", key.to_string_lossy())?;
+                    write!(
+                        f,
+                        "{}={} ",
+                        relaxed_escaping(key.as_encoded_bytes(), false),
+                        relaxed_escaping(value.as_encoded_bytes(), false)
+                    )?;
                 }
             }
             if self.program != self.args[0] {
-                write!(f, "[{:?}] ", self.program)?;
+                write!(f, "[{}] ", relaxed_escaping(self.program.to_bytes(), false))?;
             }
-            write!(f, "{:?}", self.args[0])?;
+
+            write!(f, "{}", relaxed_escaping(self.args[0].to_bytes(), false))?;
 
             for arg in &self.args[1..] {
-                write!(f, " {:?}", arg)?;
+                write!(f, " {}", relaxed_escaping(arg.to_bytes(), true))?;
             }
             Ok(())
         }

library/std/src/sys/pal/unix/process/process_common/tests.rs

@@ -190,3 +190,38 @@ fn unix_exit_statuses() {
         }
     }
 }
+
+#[test]
+fn command_debug_escaping() {
+    #[track_caller]
+    fn check(arg: impl AsRef<OsStr>, expected: &str) {
+        let cmd = Command::new(arg.as_ref());
+        assert_eq!(format!("{cmd:?}"), expected);
+    }
+
+    // Escaped.
+    check("", r#""""#);
+    check("é", r#""\xc3\xa9""#);
+    check("a'", r#""a\'""#);
+    check("a\"", r#""a\"""#);
+    check("'\"", r#""\'\"""#);
+    check(" ", r#"" ""#);
+    check(
+        "/Users/The user's name/.cargo/bin/cargo",
+        r#""/Users/The user\'s name/.cargo/bin/cargo""#,
+    );
+    check("!a", r#""!a""#);
+
+    // Simple enough that we don't escape them.
+    check("a", "a");
+    check("/my/simple/path", "/my/simple/path");
+    check("abc-defg_1234", "abc-defg_1234");
+
+    // Real-world use-case, immediately clear that two of the arguments are passed as one.
+    let mut cmd = crate::process::Command::new("ld");
+    cmd.arg("my/file.o");
+    cmd.arg("-macos_version_min");
+    cmd.arg("13.0");
+    cmd.arg("-arch arm64");
+    assert_eq!(format!("{cmd:?}"), r#"ld my/file.o -macos_version_min 13.0 "-arch arm64""#);
+}

tests/run-make/link-args-order/rmake.rs

@@ -13,17 +13,19 @@ fn main() {
         .linker_flavor(linker)
         .link_arg("a")
         .link_args("b c")
-        .link_args("d e")
-        .link_arg("f")
+        .link_arg("d e")
+        .link_args("f g")
+        .link_arg("h")
         .run_fail()
-        .assert_stderr_contains(r#""a" "b" "c" "d" "e" "f""#);
+        .assert_stderr_contains(r#"a b c "d e" f g h"#);
     rustc()
         .input("empty.rs")
         .linker_flavor(linker)
         .arg("-Zpre-link-arg=a")
         .arg("-Zpre-link-args=b c")
-        .arg("-Zpre-link-args=d e")
-        .arg("-Zpre-link-arg=f")
+        .arg("-Zpre-link-arg=d e")
+        .arg("-Zpre-link-args=f g")
+        .arg("-Zpre-link-arg=h")
         .run_fail()
-        .assert_stderr_contains(r#""a" "b" "c" "d" "e" "f""#);
+        .assert_stderr_contains(r#"a b c "d e" f g h"#);
 }

tests/run-make/link-dedup/rmake.rs

@@ -31,9 +31,9 @@ fn needle_from_libs(libs: &[&str]) -> String {
     let mut needle = String::new();
     for lib in libs {
         if is_msvc() {
-            let _ = needle.write_fmt(format_args!(r#""{lib}.lib" "#));
+            let _ = needle.write_fmt(format_args!("{lib}.lib "));
         } else {
-            let _ = needle.write_fmt(format_args!(r#""-l{lib}" "#));
+            let _ = needle.write_fmt(format_args!("-l{lib} "));
         }
     }
     needle.pop(); // remove trailing space

tests/run-make/pass-linker-flags-flavor/rmake.rs

@@ -72,8 +72,8 @@ fn main() {
         .stdout_utf8();
 
     let no_verbatim = regex::Regex::new("l1.*-Wl,a1.*l2.*-Wl,a2.*d1.*-Wl,a3").unwrap();
-    let one_verbatim = regex::Regex::new(r#"l1.*"a1".*l2.*-Wl,a2.*d1.*-Wl,a3"#).unwrap();
-    let ld = regex::Regex::new(r#"l1.*"a1".*l2.*"a2".*d1.*"a3""#).unwrap();
+    let one_verbatim = regex::Regex::new("l1.*a1.*l2.*-Wl,a2.*d1.*-Wl,a3").unwrap();
+    let ld = regex::Regex::new("l1.*a1.*l2.*a2.*d1.*a3").unwrap();
 
     assert!(no_verbatim.is_match(&out_gnu));
     assert!(no_verbatim.is_match(&out_att_gnu));