From 5eb80b741367febb5db5db7bba2ccc123c11503e Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 14 Sep 2016 21:10:30 +0300 Subject: [PATCH] Move stream_output to ProcessBuilder --- src/cargo/ops/cargo_rustc/custom_build.rs | 63 ++------------------- src/cargo/ops/cargo_rustc/job_queue.rs | 4 +- src/cargo/util/process_builder.rs | 69 +++++++++++++++++++++-- tests/build-script.rs | 2 +- tests/build.rs | 2 +- tests/run.rs | 4 +- 6 files changed, 74 insertions(+), 70 deletions(-) diff --git a/src/cargo/ops/cargo_rustc/custom_build.rs b/src/cargo/ops/cargo_rustc/custom_build.rs index ffc1aeaab8f..8835824898c 100644 --- a/src/cargo/ops/cargo_rustc/custom_build.rs +++ b/src/cargo/ops/cargo_rustc/custom_build.rs @@ -3,16 +3,12 @@ use std::fs; use std::path::{PathBuf, Path}; use std::str; use std::sync::{Mutex, Arc}; -use std::process::{Stdio, Output}; use core::PackageId; -use util::{CargoResult, Human}; +use util::{CargoResult, Human, Freshness}; use util::{internal, ChainError, profile, paths}; -use util::{Freshness, ProcessBuilder, read2}; -use util::errors::{process_error, ProcessError}; use super::job::Work; -use super::job_queue::JobState; use super::{fingerprint, Kind, Context, Unit}; use super::CommandType; @@ -209,7 +205,10 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) // And now finally, run the build command itself! state.running(&p); let cmd = p.into_process_builder(); - let output = try!(stream_output(state, &cmd).map_err(|mut e| { + let output = try!(cmd.exec_with_streaming( + &mut |out_line| state.stdout(out_line), + &mut |err_line| state.stderr(err_line), + ).map_err(|mut e| { e.desc = format!("failed to run custom build command for `{}`\n{}", pkg_name, e.desc); Human(e) @@ -443,55 +442,3 @@ pub fn build_map<'b, 'cfg>(cx: &mut Context<'b, 'cfg>, } } } - -fn stream_output(state: &JobState, cmd: &ProcessBuilder) - -> Result { - let mut stdout = Vec::new(); - let mut stderr = Vec::new(); - - let status = try!((|| { - let mut cmd = cmd.build_command(); - cmd.stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .stdin(Stdio::null()); - let mut child = try!(cmd.spawn()); - let out = child.stdout.take().unwrap(); - let err = child.stderr.take().unwrap(); - - try!(read2(out, err, &mut |is_out, data, eof| { - let idx = if eof { - data.len() - } else { - match data.iter().rposition(|b| *b == b'\n') { - Some(i) => i + 1, - None => return, - } - }; - let data = data.drain(..idx); - let dst = if is_out {&mut stdout} else {&mut stderr}; - let start = dst.len(); - dst.extend(data); - let s = String::from_utf8_lossy(&dst[start..]); - if is_out { - state.stdout(&s); - } else { - state.stderr(&s); - } - })); - child.wait() - })().map_err(|e| { - let msg = format!("could not exeute process {}", cmd); - process_error(&msg, Some(e), None, None) - })); - let output = Output { - stdout: stdout, - stderr: stderr, - status: status, - }; - if !output.status.success() { - let msg = format!("process didn't exit successfully: {}", cmd); - Err(process_error(&msg, None, Some(&status), Some(&output))) - } else { - Ok(output) - } -} diff --git a/src/cargo/ops/cargo_rustc/job_queue.rs b/src/cargo/ops/cargo_rustc/job_queue.rs index a0cb1dddee9..53bad6b786c 100644 --- a/src/cargo/ops/cargo_rustc/job_queue.rs +++ b/src/cargo/ops/cargo_rustc/job_queue.rs @@ -170,12 +170,12 @@ impl<'a> JobQueue<'a> { } Message::Stdout(out) => { if cx.config.extra_verbose() { - try!(write!(cx.config.shell().out(), "{}", out)); + try!(writeln!(cx.config.shell().out(), "{}", out)); } } Message::Stderr(err) => { if cx.config.extra_verbose() { - try!(write!(cx.config.shell().err(), "{}", err)); + try!(writeln!(cx.config.shell().err(), "{}", err)); } } Message::Finish(result) => { diff --git a/src/cargo/util/process_builder.rs b/src/cargo/util/process_builder.rs index d86df727fa3..856e337e200 100644 --- a/src/cargo/util/process_builder.rs +++ b/src/cargo/util/process_builder.rs @@ -3,9 +3,9 @@ use std::env; use std::ffi::{OsString, OsStr}; use std::fmt; use std::path::Path; -use std::process::{Command, Output}; +use std::process::{Command, Stdio, Output}; -use util::{ProcessError, process_error}; +use util::{ProcessError, process_error, read2}; use util::shell_escape::escape; #[derive(Clone, PartialEq, Debug)] @@ -75,7 +75,7 @@ impl ProcessBuilder { pub fn exec(&self) -> Result<(), ProcessError> { let mut command = self.build_command(); let exit = try!(command.status().map_err(|e| { - process_error(&format!("Could not execute process `{}`", + process_error(&format!("could not execute process `{}`", self.debug_string()), Some(e), None, None) })); @@ -83,7 +83,7 @@ impl ProcessBuilder { if exit.success() { Ok(()) } else { - Err(process_error(&format!("Process didn't exit successfully: `{}`", + Err(process_error(&format!("process didn't exit successfully: `{}`", self.debug_string()), None, Some(&exit), None)) } @@ -93,7 +93,7 @@ impl ProcessBuilder { let mut command = self.build_command(); let output = try!(command.output().map_err(|e| { - process_error(&format!("Could not execute process `{}`", + process_error(&format!("could not execute process `{}`", self.debug_string()), Some(e), None, None) })); @@ -101,12 +101,69 @@ impl ProcessBuilder { if output.status.success() { Ok(output) } else { - Err(process_error(&format!("Process didn't exit successfully: `{}`", + Err(process_error(&format!("process didn't exit successfully: `{}`", self.debug_string()), None, Some(&output.status), Some(&output))) } } + pub fn exec_with_streaming(&self, + on_stdout_line: &mut FnMut(&str), + on_stderr_line: &mut FnMut(&str)) + -> Result { + let mut stdout = Vec::new(); + let mut stderr = Vec::new(); + + let mut cmd = self.build_command(); + cmd.stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .stdin(Stdio::null()); + + let status = try!((|| { + let mut child = try!(cmd.spawn()); + let out = child.stdout.take().unwrap(); + let err = child.stderr.take().unwrap(); + try!(read2(out, err, &mut |is_out, data, eof| { + let idx = if eof { + data.len() + } else { + match data.iter().rposition(|b| *b == b'\n') { + Some(i) => i + 1, + None => return, + } + }; + let data = data.drain(..idx); + let dst = if is_out {&mut stdout} else {&mut stderr}; + let start = dst.len(); + dst.extend(data); + for line in String::from_utf8_lossy(&dst[start..]).lines() { + if is_out { + on_stdout_line(line) + } else { + on_stderr_line(line) + } + } + })); + child.wait() + })().map_err(|e| { + process_error(&format!("could not execute process `{}`", + self.debug_string()), + Some(e), None, None) + })); + let output = Output { + stdout: stdout, + stderr: stderr, + status: status, + }; + if !output.status.success() { + Err(process_error(&format!("process didn't exit successfully: `{}`", + self.debug_string()), + None, Some(&output.status), Some(&output))) + } else { + Ok(output) + } + } + pub fn build_command(&self) -> Command { let mut command = Command::new(&self.program); if let Some(cwd) = self.get_cwd() { diff --git a/tests/build-script.rs b/tests/build-script.rs index f471f5475b9..e0669311039 100644 --- a/tests/build-script.rs +++ b/tests/build-script.rs @@ -715,7 +715,7 @@ fn build_deps_not_for_normal() { [ERROR] Could not compile `foo`. Caused by: - Process didn't exit successfully: [..] + process didn't exit successfully: [..] ")); } diff --git a/tests/build.rs b/tests/build.rs index 4844eae5362..5657396382c 100644 --- a/tests/build.rs +++ b/tests/build.rs @@ -1967,7 +1967,7 @@ fn rustc_env_var() { .env("RUSTC", "rustc-that-does-not-exist").arg("-v"), execs().with_status(101) .with_stderr("\ -[ERROR] Could not execute process `rustc-that-does-not-exist -vV` ([..]) +[ERROR] could not execute process `rustc-that-does-not-exist -vV` ([..]) Caused by: [..] diff --git a/tests/run.rs b/tests/run.rs index 1f76df20dcd..7c769c24515 100644 --- a/tests/run.rs +++ b/tests/run.rs @@ -132,7 +132,7 @@ fn exit_code() { [COMPILING] foo v0.0.1 (file[..]) [FINISHED] debug [unoptimized + debuginfo] target(s) in [..] [RUNNING] `target[..]` -[ERROR] Process didn't exit successfully: `target[..]foo[..]` (exit code: 2) +[ERROR] process didn't exit successfully: `target[..]foo[..]` (exit code: 2) ")); } @@ -156,7 +156,7 @@ fn exit_code_verbose() { [RUNNING] `rustc [..]` [FINISHED] debug [unoptimized + debuginfo] target(s) in [..] [RUNNING] `target[..]` -[ERROR] Process didn't exit successfully: `target[..]foo[..]` (exit code: 2) +[ERROR] process didn't exit successfully: `target[..]foo[..]` (exit code: 2) ")); }