Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add cargo_output to eliminate last vestiges of stdout pollution #1141

Merged
merged 1 commit into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 31 additions & 4 deletions src/command_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,28 @@ pub(crate) struct CargoOutput {
pub(crate) metadata: bool,
pub(crate) warnings: bool,
pub(crate) debug: bool,
pub(crate) output: OutputKind,
checked_dbg_var: Arc<AtomicBool>,
}

/// Different strategies for handling compiler output (to stdout)
#[derive(Clone, Debug)]
pub(crate) enum OutputKind {
/// Forward the output to this process' stdout (Stdio::inherit)
Forward,
/// Discard the output (Stdio::null)
Discard,
/// Capture the result
Capture,
}

impl CargoOutput {
pub(crate) fn new() -> Self {
#[allow(clippy::disallowed_methods)]
Self {
metadata: true,
warnings: true,
output: OutputKind::Forward,
debug: std::env::var_os("CC_ENABLE_DEBUG_OUTPUT").is_some(),
checked_dbg_var: Arc::new(AtomicBool::new(false)),
}
Expand Down Expand Up @@ -65,6 +78,14 @@ impl CargoOutput {
Stdio::null()
}
}

fn stdio_for_output(&self) -> Stdio {
match self.output {
OutputKind::Capture => Stdio::piped(),
OutputKind::Forward => Stdio::inherit(),
OutputKind::Discard => Stdio::null(),
}
}
}

pub(crate) struct StderrForwarder {
Expand Down Expand Up @@ -321,9 +342,10 @@ pub(crate) fn run_output(
) -> Result<Vec<u8>, Error> {
let program = program.as_ref();

cmd.stdout(Stdio::piped());

let mut child = spawn(cmd, program, cargo_output)?;
// We specifically need the output to be captured, so override default
let mut captured_cargo_output = cargo_output.clone();
captured_cargo_output.output = OutputKind::Capture;
let mut child = spawn(cmd, program, &captured_cargo_output)?;

let mut stdout = vec![];
child
Expand All @@ -333,6 +355,7 @@ pub(crate) fn run_output(
.read_to_end(&mut stdout)
.unwrap();

// Don't care about this output, use the normal settings
wait_on_child(cmd, program, &mut child, cargo_output)?;

Ok(stdout)
Expand All @@ -356,7 +379,11 @@ pub(crate) fn spawn(
cargo_output.print_debug(&format_args!("running: {:?}", cmd));

let cmd = ResetStderr(cmd);
let child = cmd.0.stderr(cargo_output.stdio_for_warnings()).spawn();
let child = cmd
.0
.stderr(cargo_output.stdio_for_warnings())
.stdout(cargo_output.stdio_for_output())
.spawn();
match child {
Ok(child) => Ok(child),
Err(ref e) if e.kind() == io::ErrorKind::NotFound => {
Expand Down
14 changes: 14 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1196,6 +1196,20 @@ impl Build {
self
}

/// Define whether compiler output (to stdout) should be emitted. Defaults to `true`
/// (forward compiler stdout to this process' stdout)
///
/// Some compilers emit errors to stdout, so if you *really* need stdout to be clean
/// you should also set this to `false`.
pub fn cargo_output(&mut self, cargo_output: bool) -> &mut Build {
self.cargo_output.output = if cargo_output {
OutputKind::Forward
} else {
OutputKind::Discard
};
self
}

/// Adds a native library modifier that will be added to the
/// `rustc-link-lib=static:MODIFIERS=LIBRARY_NAME` metadata line
/// emitted for cargo if `cargo_metadata` is enabled.
Expand Down
22 changes: 11 additions & 11 deletions src/tool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@ use std::{
ffi::{OsStr, OsString},
io::Write,
path::{Path, PathBuf},
process::{Command, Stdio},
process::Command,
sync::RwLock,
};

use crate::{
command_helpers::{run_output, CargoOutput},
run,
tempfile::NamedTempfile,
Error, ErrorKind,
Error, ErrorKind, OutputKind,
};

/// Configuration used to represent an invocation of a C compiler.
Expand Down Expand Up @@ -158,14 +158,14 @@ impl Tool {
cargo_output.print_debug(&stdout);

// https://gitlab.kitware.com/cmake/cmake/-/blob/69a2eeb9dff5b60f2f1e5b425002a0fd45b7cadb/Modules/CMakeDetermineCompilerId.cmake#L267-271
let accepts_cl_style_flags =
run(Command::new(path).arg("-?").stdout(Stdio::null()), path, &{
// the errors are not errors!
let mut cargo_output = cargo_output.clone();
cargo_output.warnings = cargo_output.debug;
cargo_output
})
.is_ok();
let accepts_cl_style_flags = run(Command::new(path).arg("-?"), path, &{
// the errors are not errors!
let mut cargo_output = cargo_output.clone();
cargo_output.warnings = cargo_output.debug;
cargo_output.output = OutputKind::Discard;
cargo_output
})
.is_ok();

let clang = stdout.contains(r#""clang""#);
let gcc = stdout.contains(r#""gcc""#);
Expand Down Expand Up @@ -283,7 +283,7 @@ impl Tool {
/// Don't push optimization arg if it conflicts with existing args.
pub(crate) fn push_opt_unless_duplicate(&mut self, flag: OsString) {
if self.is_duplicate_opt_arg(&flag) {
println!("Info: Ignoring duplicate arg {:?}", &flag);
eprintln!("Info: Ignoring duplicate arg {:?}", &flag);
} else {
self.push_cc_arg(flag);
}
Expand Down
2 changes: 1 addition & 1 deletion src/windows/find_tools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ mod impl_ {
impl LibraryHandle {
fn new(name: &[u8]) -> Option<Self> {
let handle = unsafe { LoadLibraryA(name.as_ptr() as _) };
(!handle.is_null()).then(|| Self(handle))
(!handle.is_null()).then_some(Self(handle))
}

/// Get a function pointer to a function in the library.
Expand Down