itsscb/cargo
1
0
Fork 0
mirror of https://github.com/rust-lang/cargo.git synced 2025-10-01 11:30:39 +00:00
Code Issues Packages Projects Releases Wiki Activity

Auto merge of #5666 - mikeyhew:fix-shell-quoting, r=alexcrichton

Browse Source 
[needs test] always shell-escape command arguments after failure

cc @joshtriplett
fixes #5665

Removes the `debug_string` method, in favour of always using the
`fmt::Display` impl. `debug_string` didn’t escape the command
arguments, so that’s why we ended up with unescaped arguments after
compilation failed.

Don't merge this yet, because I still want to add a regression test. I don't think there's any tests on shell quoting yet, so I'll probably add a new test file including tests for the "Running <command>" output too. I still have to figure out how to write tests, and don't have the energy to do that tonight 😴

I just wanted to say, by the way, I was pleasantly surprised with how clean the code is in this repo, especially compared to rustc. It made it really easy to find the relevant part of the code and implement these changes.
This commit is contained in:
bors 2018-07-03 22:18:05 +00:00
commit 592b253634
2 changed files with 51 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
src/cargo/util/process_builder.rs
View File

@ -133,7 +133,7 @@ impl ProcessBuilder {
let mut command = self.build_command();
let exit = command.status().chain_err(|| {
process_error(
&format!("could not execute process `{}`", self.debug_string()),
&format!("could not execute process {}", self),
None,
None,
)
@ -143,10 +143,7 @@ impl ProcessBuilder {
Ok(())
} else {
Err(process_error(
&format!(
"process didn't exit successfully: `{}`",
self.debug_string()
),
&format!("process didn't exit successfully: {}", self),
Some(&exit),
None,
).into())
@ -164,7 +161,7 @@ impl ProcessBuilder {
let error = command.exec();
Err(CargoError::from(error)
.context(process_error(
&format!("could not execute process `{}`", self.debug_string()),
&format!("could not execute process {}", self),
None,
None,
))
@ -185,7 +182,7 @@ impl ProcessBuilder {
let output = command.output().chain_err(|| {
process_error(
&format!("could not execute process `{}`", self.debug_string()),
&format!("could not execute process {}", self),
None,
None,
)
@ -195,10 +192,7 @@ impl ProcessBuilder {
Ok(output)
} else {
Err(process_error(
&format!(
"process didn't exit successfully: `{}`",
self.debug_string()
),
&format!("process didn't exit successfully: {}", self),
Some(&output.status),
Some(&output),
).into())
@ -261,7 +255,7 @@ impl ProcessBuilder {
})()
.chain_err(|| {
process_error(
&format!("could not execute process `{}`", self.debug_string()),
&format!("could not execute process {}", self),
None,
None,
)
@ -276,16 +270,13 @@ impl ProcessBuilder {
let to_print = if print_output { Some(&output) } else { None };
if !output.status.success() {
return Err(process_error(
&format!(
"process didn't exit successfully: `{}`",
self.debug_string()
),
&format!("process didn't exit successfully: {}", self),
Some(&output.status),
to_print,
).into());
} else if let Some(e) = callback_error {
let cx = process_error(
&format!("failed to parse process output: `{}`", self.debug_string()),
&format!("failed to parse process output: {}", self),
Some(&output.status),
to_print,
);
@ -321,16 +312,6 @@ impl ProcessBuilder {
}
command
}
/// Get the command line for the process as a string.
fn debug_string(&self) -> String {
let mut program = format!("{}", self.program.to_string_lossy());
for arg in &self.args {
program.push(' ');
program.push_str(&format!("{}", arg.to_string_lossy()));
}
program
}
}
/// A helper function to create a `ProcessBuilder`.

43
tests/testsuite/shell_quoting.rs Normal file
View File

@ -0,0 +1,43 @@
//! this file tests that when the commands being run are shown
//! in the output, their arguments are quoted properly
//! so that the command can be run in a terminal
use cargotest::support::{
execs,
project,
};
use hamcrest::assert_that
#[test]
fn features_are_quoted() {
let p = project("foo")
.file(
"Cargo.toml",
r#"
[project]
name = "foo"
version = "0.1.0"
authors = ["mikeyhew@example.com"]
[features]
some_feature = []
default = ["some_feature"]
"#,
)
.file("src/main.rs", "fn main() {error}")
.build();
assert_that(
p.cargo("check -v"),
execs()
.with_status(101)
.with_stderr_contains(
r#"\
[CHECKING] foo [..]
[RUNNING] `rustc [..] --cfg 'feature="default"' --cfg 'feature="some_feature"' [..]`
[ERROR] [..]
process didn't exit successfully: `rustc [..] --cfg 'feature="default"' --cfg 'feature="some_feature"' [..]`
"#
)
);
}