From 3ab8407c5a1732f3df9ff0bdbcc61a43b6601cd8 Mon Sep 17 00:00:00 2001 From: John Bartholomew Date: Sun, 14 Apr 2019 15:38:59 +0100 Subject: [PATCH 1/2] Pass OsStr/OsString args through to the process spawned by cargo run. To avoid breaking other (external) callers of ops::run(), this adds a new function ops::run_os() taking an &[OsString], and turns ops::run() into a wrapper (keeping its original signature) that calls run_os(). --- src/bin/cargo/commands/run.rs | 2 +- src/cargo/ops/cargo_run.rs | 10 ++++++++++ src/cargo/ops/mod.rs | 2 +- src/cargo/util/command_prelude.rs | 20 ++++++++++++++++++++ tests/testsuite/run.rs | 26 ++++++++++++++++++++++++++ 5 files changed, 58 insertions(+), 2 deletions(-) diff --git a/src/bin/cargo/commands/run.rs b/src/bin/cargo/commands/run.rs index 540513ca12a..ac53a6b872e 100644 --- a/src/bin/cargo/commands/run.rs +++ b/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_os(&ws, &compile_opts, &values_os(args, "args"))? { None => Ok(()), Some(err) => { // If we never actually spawned the process then that sounds pretty diff --git a/src/cargo/ops/cargo_run.rs b/src/cargo/ops/cargo_run.rs index fe3bcea952d..fa665b4f988 100644 --- a/src/cargo/ops/cargo_run.rs +++ b/src/cargo/ops/cargo_run.rs @@ -1,3 +1,4 @@ +use std::ffi::OsString; use std::iter; use std::path::Path; @@ -9,6 +10,15 @@ pub fn run( ws: &Workspace<'_>, options: &ops::CompileOptions<'_>, args: &[String], +) -> CargoResult> { + let osargs: Vec = args.iter().map(|s| OsString::from(s)).collect(); + run_os(ws, options, osargs.as_slice()) +} + +pub fn run_os( + ws: &Workspace<'_>, + options: &ops::CompileOptions<'_>, + args: &[OsString], ) -> CargoResult> { let config = ws.config(); diff --git a/src/cargo/ops/mod.rs b/src/cargo/ops/mod.rs index e60dcd7b85a..228a23a2c5a 100644 --- a/src/cargo/ops/mod.rs +++ b/src/cargo/ops/mod.rs @@ -12,7 +12,7 @@ pub use self::cargo_output_metadata::{output_metadata, ExportInfo, OutputMetadat pub use self::cargo_package::{package, PackageOpts}; pub use self::cargo_pkgid::pkgid; pub use self::cargo_read_manifest::{read_package, read_packages}; -pub use self::cargo_run::run; +pub use self::cargo_run::{run, run_os}; pub use self::cargo_test::{run_benches, run_tests, TestOptions}; pub use self::cargo_uninstall::uninstall; pub use self::fix::{fix, fix_maybe_exec_rustc, FixOptions}; diff --git a/src/cargo/util/command_prelude.rs b/src/cargo/util/command_prelude.rs index 21be7e86f4b..8d6963c6d9a 100644 --- a/src/cargo/util/command_prelude.rs +++ b/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; + fn _value_of_os(&self, name: &str) -> Option<&OsStr>; + + fn _values_of_os(&self, name: &str) -> Vec; + fn _is_present(&self, name: &str) -> bool; } @@ -471,6 +476,10 @@ 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 { self.values_of(name) .unwrap_or_default() @@ -478,6 +487,13 @@ impl<'a> ArgMatchesExt for ArgMatches<'a> { .collect() } + fn _values_of_os(&self, name: &str) -> Vec { + 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 { args._values_of(name) } +pub fn values_os(args: &ArgMatches<'_>, name: &str) -> Vec { + args._values_of_os(name) +} + #[derive(PartialEq, PartialOrd, Eq, Ord)] pub enum CommandInfo { BuiltIn { name: String, about: Option }, diff --git a/tests/testsuite/run.rs b/tests/testsuite/run.rs index 38ae8fe728c..e596dede15f 100644 --- a/tests/testsuite/run.rs +++ b/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() From 6d066a670795ef67b7ac777b068c9f7c75ddedf6 Mon Sep 17 00:00:00 2001 From: John Bartholomew Date: Mon, 15 Apr 2019 19:26:42 +0100 Subject: [PATCH 2/2] Remove ops::run_os, change ops::run to take &[OsString] directly. --- src/bin/cargo/commands/run.rs | 2 +- src/cargo/ops/cargo_run.rs | 9 --------- src/cargo/ops/mod.rs | 2 +- 3 files changed, 2 insertions(+), 11 deletions(-) diff --git a/src/bin/cargo/commands/run.rs b/src/bin/cargo/commands/run.rs index ac53a6b872e..f30e8570d01 100644 --- a/src/bin/cargo/commands/run.rs +++ b/src/bin/cargo/commands/run.rs @@ -72,7 +72,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { }; } }; - match ops::run_os(&ws, &compile_opts, &values_os(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 diff --git a/src/cargo/ops/cargo_run.rs b/src/cargo/ops/cargo_run.rs index fa665b4f988..7fc99211505 100644 --- a/src/cargo/ops/cargo_run.rs +++ b/src/cargo/ops/cargo_run.rs @@ -7,15 +7,6 @@ use crate::ops; use crate::util::{CargoResult, ProcessError}; pub fn run( - ws: &Workspace<'_>, - options: &ops::CompileOptions<'_>, - args: &[String], -) -> CargoResult> { - let osargs: Vec = args.iter().map(|s| OsString::from(s)).collect(); - run_os(ws, options, osargs.as_slice()) -} - -pub fn run_os( ws: &Workspace<'_>, options: &ops::CompileOptions<'_>, args: &[OsString], diff --git a/src/cargo/ops/mod.rs b/src/cargo/ops/mod.rs index 228a23a2c5a..e60dcd7b85a 100644 --- a/src/cargo/ops/mod.rs +++ b/src/cargo/ops/mod.rs @@ -12,7 +12,7 @@ pub use self::cargo_output_metadata::{output_metadata, ExportInfo, OutputMetadat pub use self::cargo_package::{package, PackageOpts}; pub use self::cargo_pkgid::pkgid; pub use self::cargo_read_manifest::{read_package, read_packages}; -pub use self::cargo_run::{run, run_os}; +pub use self::cargo_run::run; pub use self::cargo_test::{run_benches, run_tests, TestOptions}; pub use self::cargo_uninstall::uninstall; pub use self::fix::{fix, fix_maybe_exec_rustc, FixOptions};