Skip to content

Commit b59ba27

Browse files
committed
Refactor routing output from rustc/rustdoc
This commit refactors slightly how we actually spawn rustc/rustdoc processes and how their output is routed. Over time lots has changed around this, but it turns out that we unconditionally capture all output from the compiler/rustdoc today, no exceptions. As a result simplify the various execution functions in the `Executor` trait as well as branches for emitting json messages. Instead throw everything in the same bucket and just always look for lines that start with `{` which indicate a JSON message. This also fixes a few issues where output printed in each thread is now routed through the main coordinator thread to handle updating the progress bar if necessary.
1 parent 22f2740 commit b59ba27

File tree

5 files changed

+119
-148
lines changed

5 files changed

+119
-148
lines changed

src/cargo/core/compiler/custom_build.rs

Lines changed: 47 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use std::path::{Path, PathBuf};
55
use std::str;
66
use std::sync::{Arc, Mutex};
77

8+
use crate::core::compiler::job_queue::JobState;
89
use crate::core::PackageId;
910
use crate::util::errors::{CargoResult, CargoResultExt};
1011
use crate::util::machine_message;
@@ -96,20 +97,21 @@ pub fn prepare<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRe
9697
}
9798
}
9899

99-
fn emit_build_output(output: &BuildOutput, package_id: PackageId) {
100+
fn emit_build_output(state: &JobState<'_>, output: &BuildOutput, package_id: PackageId) {
100101
let library_paths = output
101102
.library_paths
102103
.iter()
103104
.map(|l| l.display().to_string())
104105
.collect::<Vec<_>>();
105106

106-
machine_message::emit(&machine_message::BuildScript {
107+
let msg = machine_message::emit(&machine_message::BuildScript {
107108
package_id,
108109
linked_libs: &output.library_links,
109110
linked_paths: &library_paths,
110111
cfgs: &output.cfgs,
111112
env: &output.env,
112113
});
114+
state.stdout(&msg);
113115
}
114116

115117
fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult<Job> {
@@ -299,52 +301,58 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes
299301
}
300302
}
301303

302-
// And now finally, run the build command itself!
303304
if build_plan {
304305
state.build_plan(invocation_name, cmd.clone(), Arc::new(Vec::new()));
305-
} else {
306-
state.running(&cmd);
307-
let timestamp = paths::set_invocation_time(&script_run_dir)?;
308-
let output = if extra_verbose {
309-
let prefix = format!("[{} {}] ", id.name(), id.version());
310-
state.capture_output(&cmd, Some(prefix), true)
311-
} else {
312-
cmd.exec_with_output()
313-
};
314-
let output = output.map_err(|e| {
315-
failure::format_err!(
316-
"failed to run custom build command for `{}`\n{}",
317-
pkg_name,
318-
e
319-
)
320-
})?;
306+
return Ok(());
307+
}
321308

322-
// After the build command has finished running, we need to be sure to
323-
// remember all of its output so we can later discover precisely what it
324-
// was, even if we don't run the build command again (due to freshness).
325-
//
326-
// This is also the location where we provide feedback into the build
327-
// state informing what variables were discovered via our script as
328-
// well.
329-
paths::write(&output_file, &output.stdout)?;
330-
filetime::set_file_times(output_file, timestamp, timestamp)?;
331-
paths::write(&err_file, &output.stderr)?;
332-
paths::write(&root_output_file, util::path2bytes(&script_out_dir)?)?;
333-
let parsed_output =
334-
BuildOutput::parse(&output.stdout, &pkg_name, &script_out_dir, &script_out_dir)?;
335-
336-
if json_messages {
337-
emit_build_output(&parsed_output, id);
338-
}
339-
build_state.insert(id, kind, parsed_output);
309+
// And now finally, run the build command itself!
310+
state.running(&cmd);
311+
let timestamp = paths::set_invocation_time(&script_run_dir)?;
312+
let prefix = format!("[{} {}] ", id.name(), id.version());
313+
let output = cmd
314+
.exec_with_streaming(
315+
&mut |stdout| {
316+
if extra_verbose {
317+
state.stdout(&format!("{}{}", prefix, stdout));
318+
}
319+
Ok(())
320+
},
321+
&mut |stderr| {
322+
if extra_verbose {
323+
state.stderr(&format!("{}{}", prefix, stderr));
324+
}
325+
Ok(())
326+
},
327+
true,
328+
)
329+
.chain_err(|| format!("failed to run custom build command for `{}`", pkg_name))?;
330+
331+
// After the build command has finished running, we need to be sure to
332+
// remember all of its output so we can later discover precisely what it
333+
// was, even if we don't run the build command again (due to freshness).
334+
//
335+
// This is also the location where we provide feedback into the build
336+
// state informing what variables were discovered via our script as
337+
// well.
338+
paths::write(&output_file, &output.stdout)?;
339+
filetime::set_file_times(output_file, timestamp, timestamp)?;
340+
paths::write(&err_file, &output.stderr)?;
341+
paths::write(&root_output_file, util::path2bytes(&script_out_dir)?)?;
342+
let parsed_output =
343+
BuildOutput::parse(&output.stdout, &pkg_name, &script_out_dir, &script_out_dir)?;
344+
345+
if json_messages {
346+
emit_build_output(state, &parsed_output, id);
340347
}
348+
build_state.insert(id, kind, parsed_output);
341349
Ok(())
342350
});
343351

344352
// Now that we've prepared our work-to-do, we need to prepare the fresh work
345353
// itself to run when we actually end up just discarding what we calculated
346354
// above.
347-
let fresh = Work::new(move |_tx| {
355+
let fresh = Work::new(move |state| {
348356
let (id, pkg_name, build_state, output_file, script_out_dir) = all;
349357
let output = match prev_output {
350358
Some(output) => output,
@@ -357,7 +365,7 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes
357365
};
358366

359367
if json_messages {
360-
emit_build_output(&output, id);
368+
emit_build_output(state, &output, id);
361369
}
362370

363371
build_state.insert(id, kind, output);

src/cargo/core/compiler/job_queue.rs

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ use std::collections::{HashMap, HashSet};
22
use std::cell::Cell;
33
use std::io;
44
use std::marker;
5-
use std::process::Output;
65
use std::sync::mpsc::{channel, Receiver, Sender};
76
use std::sync::Arc;
87

@@ -106,24 +105,12 @@ impl<'a> JobState<'a> {
106105
.send(Message::BuildPlanMsg(module_name, cmd, filenames));
107106
}
108107

109-
pub fn capture_output(
110-
&self,
111-
cmd: &ProcessBuilder,
112-
prefix: Option<String>,
113-
capture_output: bool,
114-
) -> CargoResult<Output> {
115-
let prefix = prefix.unwrap_or_else(String::new);
116-
cmd.exec_with_streaming(
117-
&mut |out| {
118-
let _ = self.tx.send(Message::Stdout(format!("{}{}", prefix, out)));
119-
Ok(())
120-
},
121-
&mut |err| {
122-
let _ = self.tx.send(Message::Stderr(format!("{}{}", prefix, err)));
123-
Ok(())
124-
},
125-
capture_output,
126-
)
108+
pub fn stdout(&self, stdout: &str) {
109+
drop(self.tx.send(Message::Stdout(stdout.to_string())));
110+
}
111+
112+
pub fn stderr(&self, stderr: &str) {
113+
drop(self.tx.send(Message::Stderr(stderr.to_string())));
127114
}
128115

129116
/// A method used to signal to the coordinator thread that the rmeta file

0 commit comments

Comments
 (0)