Auto merge of #6849 - johnbartholomew:issue-2511-cargo-non-utf8-args, r=alexcrichton

Pass OsStr/OsString args through to the process spawned by cargo run.

This is intended to fix #2511, allowing non-UTF8 arguments to be passed through from cargo run to the spawned process. I was not sure whether the interface of cargo::ops needs to remain unchanged - I assume it does, so I added cargo::ops::run_os() taking &[OsString], and retained cargo::ops::run() with its current signature. If it's ok to change the internal cargo API then it may be better to just pass &[OsString] to run().

I have not tried to pass through OsStr/OsString to other places that could reasonably use them. Just one test added covering this path. It is restricted to `cfg(unix)` due to use of `std::os::unix::ffi::OsStrExt`.

This is my first pull request so I expect there will be changes needed to make this mergeable.

Author: bors

src/bin/cargo/commands/run.rs

@@ -72,7 +72,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
             };
         }
     };
-    match ops::run(&ws, &compile_opts, &values(args, "args"))? {
+    match ops::run(&ws, &compile_opts, &values_os(args, "args"))? {
         None => Ok(()),
         Some(err) => {
             // If we never actually spawned the process then that sounds pretty

src/cargo/ops/cargo_run.rs

@@ -1,3 +1,4 @@
+use std::ffi::OsString;
 use std::iter;
 use std::path::Path;
 
@@ -8,7 +9,7 @@ use crate::util::{CargoResult, ProcessError};
 pub fn run(
     ws: &Workspace<'_>,
     options: &ops::CompileOptions<'_>,
-    args: &[String],
+    args: &[OsString],
 ) -> CargoResult<Option<ProcessError>> {
     let config = ws.config();
 

src/cargo/util/command_prelude.rs

@@ -1,3 +1,4 @@
+use std::ffi::{OsStr, OsString};
 use std::fs;
 use std::path::PathBuf;
 
@@ -463,6 +464,10 @@ about this warning.";
 
     fn _values_of(&self, name: &str) -> Vec<String>;
 
+    fn _value_of_os(&self, name: &str) -> Option<&OsStr>;
+
+    fn _values_of_os(&self, name: &str) -> Vec<OsString>;
+
     fn _is_present(&self, name: &str) -> bool;
 }
 
@@ -471,13 +476,24 @@ impl<'a> ArgMatchesExt for ArgMatches<'a> {
         self.value_of(name)
     }
 
+    fn _value_of_os(&self, name: &str) -> Option<&OsStr> {
+        self.value_of_os(name)
+    }
+
     fn _values_of(&self, name: &str) -> Vec<String> {
         self.values_of(name)
             .unwrap_or_default()
             .map(|s| s.to_string())
             .collect()
     }
 
+    fn _values_of_os(&self, name: &str) -> Vec<OsString> {
+        self.values_of_os(name)
+            .unwrap_or_default()
+            .map(|s| s.to_os_string())
+            .collect()
+    }
+
     fn _is_present(&self, name: &str) -> bool {
         self.is_present(name)
     }
@@ -487,6 +503,10 @@ pub fn values(args: &ArgMatches<'_>, name: &str) -> Vec<String> {
     args._values_of(name)
 }
 
+pub fn values_os(args: &ArgMatches<'_>, name: &str) -> Vec<OsString> {
+    args._values_of_os(name)
+}
+
 #[derive(PartialEq, PartialOrd, Eq, Ord)]
 pub enum CommandInfo {
     BuiltIn { name: String, about: Option<String> },

tests/testsuite/run.rs

@@ -75,6 +75,32 @@ fn simple_with_args() {
     p.cargo("run hello world").run();
 }
 
+#[cfg(unix)]
+#[test]
+fn simple_with_non_utf8_args() {
+    use std::os::unix::ffi::OsStrExt;
+
+    let p = project()
+        .file(
+            "src/main.rs",
+            r#"
+            use std::ffi::OsStr;
+            use std::os::unix::ffi::OsStrExt;
+
+            fn main() {
+                assert_eq!(std::env::args_os().nth(1).unwrap(), OsStr::from_bytes(b"hello"));
+                assert_eq!(std::env::args_os().nth(2).unwrap(), OsStr::from_bytes(b"ab\xffcd"));
+            }
+        "#,
+        )
+        .build();
+
+    p.cargo("run")
+        .arg("hello")
+        .arg(std::ffi::OsStr::from_bytes(b"ab\xFFcd"))
+        .run();
+}
+
 #[test]
 fn exit_code() {
     let p = project()