Skip to content

Commit e4643b5

Browse files
committed
Don't panic while streaming compiler output
1 parent 9f98208 commit e4643b5

File tree

4 files changed

+33
-19
lines changed

4 files changed

+33
-19
lines changed

src/cargo/ops/cargo_rustc/custom_build.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,8 +206,8 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>)
206206
state.running(&p);
207207
let cmd = p.into_process_builder();
208208
let output = try!(cmd.exec_with_streaming(
209-
&mut |out_line| state.stdout(out_line),
210-
&mut |err_line| state.stderr(err_line),
209+
&mut |out_line| { state.stdout(out_line); Ok(()) },
210+
&mut |err_line| { state.stderr(err_line); Ok(()) },
211211
).map_err(|mut e| {
212212
e.desc = format!("failed to run custom build command for `{}`\n{}",
213213
pkg_name, e.desc);

src/cargo/ops/cargo_rustc/mod.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -280,11 +280,15 @@ fn rustc(cx: &mut Context, unit: &Unit) -> CargoResult<Work> {
280280
message: json::Json,
281281
}
282282
process_builder.exec_with_streaming(
283-
&mut |line| assert!(line.is_empty()),
283+
&mut |line| if !line.is_empty() {
284+
Err(internal(&format!("compiler stdout is not empty: `{}`", line)))
285+
} else {
286+
Ok(())
287+
},
284288
&mut |line| {
285-
let rustc_message = json::Json::from_str(line).unwrap_or_else(|_| {
286-
panic!("Compiler produced invalid json: `{}`", line)
287-
});
289+
let rustc_message = try!(json::Json::from_str(line).map_err(|_| {
290+
internal(&format!("compiler produced invalid json: `{}`", line))
291+
}));
288292

289293
let message = Message {
290294
reason: "rustc-message",
@@ -294,7 +298,7 @@ fn rustc(cx: &mut Context, unit: &Unit) -> CargoResult<Work> {
294298
};
295299
let encoded = json::encode(&message).unwrap();
296300
println!("{}", encoded);
297-
301+
Ok(())
298302
},
299303
).map(|_| ())
300304
} else {

src/cargo/util/errors.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,13 +110,13 @@ pub struct ProcessError {
110110
pub desc: String,
111111
pub exit: Option<ExitStatus>,
112112
pub output: Option<Output>,
113-
cause: Option<io::Error>,
113+
cause: Option<Box<Error + Send>>,
114114
}
115115

116116
impl Error for ProcessError {
117117
fn description(&self) -> &str { &self.desc }
118118
fn cause(&self) -> Option<&Error> {
119-
self.cause.as_ref().map(|s| s as &Error)
119+
self.cause.as_ref().map(|e| &**e as &Error)
120120
}
121121
}
122122

@@ -375,9 +375,10 @@ impl CargoError for str::ParseBoolError {}
375375
// Construction helpers
376376

377377
pub fn process_error(msg: &str,
378-
cause: Option<io::Error>,
378+
cause: Option<Box<Error + Send>>,
379379
status: Option<&ExitStatus>,
380-
output: Option<&Output>) -> ProcessError {
380+
output: Option<&Output>) -> ProcessError
381+
{
381382
let exit = match status {
382383
Some(s) => status_to_string(s),
383384
None => "never executed".to_string(),

src/cargo/util/process_builder.rs

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use std::fmt;
55
use std::path::Path;
66
use std::process::{Command, Stdio, Output};
77

8-
use util::{ProcessError, process_error, read2};
8+
use util::{CargoResult, ProcessError, process_error, read2};
99
use util::shell_escape::escape;
1010

1111
#[derive(Clone, PartialEq, Debug)]
@@ -77,7 +77,7 @@ impl ProcessBuilder {
7777
let exit = try!(command.status().map_err(|e| {
7878
process_error(&format!("could not execute process `{}`",
7979
self.debug_string()),
80-
Some(e), None, None)
80+
Some(Box::new(e)), None, None)
8181
}));
8282

8383
if exit.success() {
@@ -94,8 +94,8 @@ impl ProcessBuilder {
9494

9595
let output = try!(command.output().map_err(|e| {
9696
process_error(&format!("could not execute process `{}`",
97-
self.debug_string()),
98-
Some(e), None, None)
97+
self.debug_string()),
98+
Some(Box::new(e)), None, None)
9999
}));
100100

101101
if output.status.success() {
@@ -108,8 +108,8 @@ impl ProcessBuilder {
108108
}
109109

110110
pub fn exec_with_streaming(&self,
111-
on_stdout_line: &mut FnMut(&str),
112-
on_stderr_line: &mut FnMut(&str))
111+
on_stdout_line: &mut FnMut(&str) -> CargoResult<()>,
112+
on_stderr_line: &mut FnMut(&str) -> CargoResult<()>)
113113
-> Result<Output, ProcessError> {
114114
let mut stdout = Vec::new();
115115
let mut stderr = Vec::new();
@@ -119,6 +119,7 @@ impl ProcessBuilder {
119119
.stderr(Stdio::piped())
120120
.stdin(Stdio::null());
121121

122+
let mut callback_error = None;
122123
let status = try!((|| {
123124
let mut child = try!(cmd.spawn());
124125
let out = child.stdout.take().unwrap();
@@ -137,18 +138,22 @@ impl ProcessBuilder {
137138
let start = dst.len();
138139
dst.extend(data);
139140
for line in String::from_utf8_lossy(&dst[start..]).lines() {
140-
if is_out {
141+
if callback_error.is_some() { break }
142+
let callback_result = if is_out {
141143
on_stdout_line(line)
142144
} else {
143145
on_stderr_line(line)
146+
};
147+
if let Err(e) = callback_result {
148+
callback_error = Some(e);
144149
}
145150
}
146151
}));
147152
child.wait()
148153
})().map_err(|e| {
149154
process_error(&format!("could not execute process `{}`",
150155
self.debug_string()),
151-
Some(e), None, None)
156+
Some(Box::new(e)), None, None)
152157
}));
153158
let output = Output {
154159
stdout: stdout,
@@ -159,6 +164,10 @@ impl ProcessBuilder {
159164
Err(process_error(&format!("process didn't exit successfully: `{}`",
160165
self.debug_string()),
161166
None, Some(&output.status), Some(&output)))
167+
} else if let Some(e) = callback_error {
168+
Err(process_error(&format!("failed to parse process output: `{}`",
169+
self.debug_string()),
170+
Some(Box::new(e)), Some(&output.status), Some(&output)))
162171
} else {
163172
Ok(output)
164173
}

0 commit comments

Comments
 (0)