diff --git a/Cargo.toml b/Cargo.toml index 802c9905e..e6062e0cc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,13 +19,17 @@ exclude = ["/.github", "tests", "src/bin"] edition = "2018" rust-version = "1.53" -[target.'cfg(unix)'.dependencies] -# Don't turn on the feature "std" for this, see https://github.com/rust-lang/cargo/issues/4866 +# Don't turn on the feature "std" for these dependencies, see https://github.com/rust-lang/cargo/issues/4866 # which is still an issue with `resolver = "1"`. -libc = { version = "0.2.62", default-features = false } + +[dependencies] +memchr = { version = "2.7.1", default-features = false, optional = true } + +[target.'cfg(unix)'.dependencies] +libc = { version = "0.2.62", default-features = false, optional = true } [features] -parallel = [] +parallel = ["libc", "memchr"] [dev-dependencies] tempfile = "3" diff --git a/src/command_helpers.rs b/src/command_helpers.rs index 3b3e923df..8e5cb8244 100644 --- a/src/command_helpers.rs +++ b/src/command_helpers.rs @@ -4,13 +4,12 @@ use std::{ collections::hash_map, ffi::OsString, fmt::Display, - fs::{self, File}, + fs, hash::Hasher, - io::{self, BufRead, BufReader, Read, Write}, + io::{self, BufRead, BufReader, Read, Stdout, Write}, path::Path, process::{Child, Command, Stdio}, sync::Arc, - thread::{self, JoinHandle}, }; use crate::{Error, ErrorKind, Object}; @@ -41,83 +40,55 @@ impl CargoOutput { } } - pub(crate) fn print_thread(&self) -> Result, Error> { - self.warnings.then(PrintThread::new).transpose() + fn stdio_for_warnings(&self) -> Stdio { + if self.warnings { + Stdio::piped() + } else { + Stdio::null() + } } -} - -pub(crate) struct PrintThread { - handle: Option>, - pipe_writer: Option, -} -impl PrintThread { - pub(crate) fn new() -> Result { - let (pipe_reader, pipe_writer) = crate::os_pipe::pipe()?; + #[cfg(feature = "parallel")] + pub(crate) fn reader_for_stderr_as_warnings( + &self, + child: &mut Child, + ) -> Option> { + self.warnings + .then(|| BufReader::with_capacity(4096, child.stderr.take().unwrap())) + } - // Capture the standard error coming from compilation, and write it out - // with cargo:warning= prefixes. Note that this is a bit wonky to avoid - // requiring the output to be UTF-8, we instead just ship bytes from one - // location to another. - let print = thread::spawn(move || { - let mut stderr = BufReader::with_capacity(4096, pipe_reader); + fn forward_stderr_as_warnings(&self, child: &mut Child) { + if self.warnings { + let mut stderr = BufReader::with_capacity(4096, child.stderr.as_mut().unwrap()); let mut line = Vec::with_capacity(20); let stdout = io::stdout(); // read_until returns 0 on Eof - while stderr.read_until(b'\n', &mut line).unwrap() != 0 { - { - let mut stdout = stdout.lock(); - - stdout.write_all(b"cargo:warning=").unwrap(); - stdout.write_all(&line).unwrap(); - stdout.write_all(b"\n").unwrap(); - } + while stderr.read_until(b'\n', &mut line).unwrap_or_default() != 0 { + write_warning(&stdout, &line); // read_until does not clear the buffer line.clear(); } - }); - - Ok(Self { - handle: Some(print), - pipe_writer: Some(pipe_writer), - }) - } - - /// # Panics - /// - /// Will panic if the pipe writer has already been taken. - pub(crate) fn take_pipe_writer(&mut self) -> File { - self.pipe_writer.take().unwrap() - } - - /// # Panics - /// - /// Will panic if the pipe writer has already been taken. - pub(crate) fn clone_pipe_writer(&self) -> Result { - self.try_clone_pipe_writer().map(Option::unwrap) - } - - pub(crate) fn try_clone_pipe_writer(&self) -> Result, Error> { - self.pipe_writer - .as_ref() - .map(File::try_clone) - .transpose() - .map_err(From::from) + } } } -impl Drop for PrintThread { - fn drop(&mut self) { - // Drop pipe_writer first to avoid deadlock - self.pipe_writer.take(); +fn write_warning(stdout: &Stdout, line: &[u8]) { + let mut stdout = stdout.lock(); - self.handle.take().unwrap().join().unwrap(); - } + stdout.write_all(b"cargo:warning=").unwrap(); + stdout.write_all(line).unwrap(); + stdout.write_all(b"\n").unwrap(); } -fn wait_on_child(cmd: &Command, program: &str, child: &mut Child) -> Result<(), Error> { +fn wait_on_child( + cmd: &Command, + program: &str, + child: &mut Child, + cargo_output: &CargoOutput, +) -> Result<(), Error> { + cargo_output.forward_stderr_as_warnings(child); let status = match child.wait() { Ok(s) => s, Err(e) => { @@ -193,20 +164,13 @@ pub(crate) fn objects_from_files(files: &[Arc], dst: &Path) -> Result) -> Result<(), Error> { - let mut child = spawn(cmd, program, pipe_writer)?; - wait_on_child(cmd, program, &mut child) -} - pub(crate) fn run( cmd: &mut Command, program: &str, - print: Option<&PrintThread>, + cargo_output: &CargoOutput, ) -> Result<(), Error> { - let pipe_writer = print.map(PrintThread::clone_pipe_writer).transpose()?; - run_inner(cmd, program, pipe_writer)?; - - Ok(()) + let mut child = spawn(cmd, program, cargo_output)?; + wait_on_child(cmd, program, &mut child, cargo_output) } pub(crate) fn run_output( @@ -216,12 +180,7 @@ pub(crate) fn run_output( ) -> Result, Error> { cmd.stdout(Stdio::piped()); - let mut print = cargo_output.print_thread()?; - let mut child = spawn( - cmd, - program, - print.as_mut().map(PrintThread::take_pipe_writer), - )?; + let mut child = spawn(cmd, program, cargo_output)?; let mut stdout = vec![]; child @@ -231,7 +190,7 @@ pub(crate) fn run_output( .read_to_end(&mut stdout) .unwrap(); - wait_on_child(cmd, program, &mut child)?; + wait_on_child(cmd, program, &mut child, cargo_output)?; Ok(stdout) } @@ -239,7 +198,7 @@ pub(crate) fn run_output( pub(crate) fn spawn( cmd: &mut Command, program: &str, - pipe_writer: Option, + cargo_output: &CargoOutput, ) -> Result { struct ResetStderr<'cmd>(&'cmd mut Command); @@ -254,10 +213,7 @@ pub(crate) fn spawn( println!("running: {:?}", cmd); let cmd = ResetStderr(cmd); - let child = cmd - .0 - .stderr(pipe_writer.map_or_else(Stdio::null, Stdio::from)) - .spawn(); + let child = cmd.0.stderr(cargo_output.stdio_for_warnings()).spawn(); match child { Ok(child) => Ok(child), Err(ref e) if e.kind() == io::ErrorKind::NotFound => { @@ -307,8 +263,27 @@ pub(crate) fn try_wait_on_child( program: &str, child: &mut Child, stdout: &mut dyn io::Write, + child_stderr_reader: &mut Option>, ) -> Result, Error> { - match child.try_wait() { + // Check the child status THEN forward stderr messages, so that we don't race + // between the child printing messages and then exiting. + let wait_result = child.try_wait(); + + if let Some(child_stderr_reader) = child_stderr_reader.as_mut() { + if let Ok(mut available) = child_stderr_reader.fill_buf() { + let stdout = io::stdout(); + let original_size = available.len(); + while let Some(i) = memchr::memchr(b'\n', available) { + let (line, remainder) = available.split_at(i); + write_warning(&stdout, line); + available = &remainder[1..]; + } + let consumed = original_size - available.len(); + child_stderr_reader.consume(consumed); + } + } + + match wait_result { Ok(Some(status)) => { let _ = writeln!(stdout, "{}", status); diff --git a/src/lib.rs b/src/lib.rs index c3237dd4b..a72f3531d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -66,7 +66,6 @@ use std::process::Child; use std::process::Command; use std::sync::{Arc, Mutex}; -mod os_pipe; #[cfg(feature = "parallel")] mod parallel; mod windows; @@ -1042,10 +1041,9 @@ impl Build { let dst = self.get_out_dir()?; let objects = objects_from_files(&self.files, &dst)?; - let print = self.cargo_output.print_thread()?; - self.compile_objects(&objects, print.as_ref())?; - self.assemble(lib_name, &dst.join(gnu_lib_name), &objects, print.as_ref())?; + self.compile_objects(&objects)?; + self.assemble(lib_name, &dst.join(gnu_lib_name), &objects)?; if self.get_target()?.contains("msvc") { let compiler = self.get_base_compiler()?; @@ -1207,23 +1205,22 @@ impl Build { pub fn try_compile_intermediates(&self) -> Result, Error> { let dst = self.get_out_dir()?; let objects = objects_from_files(&self.files, &dst)?; - let print = self.cargo_output.print_thread()?; - self.compile_objects(&objects, print.as_ref())?; + self.compile_objects(&objects)?; Ok(objects.into_iter().map(|v| v.dst).collect()) } #[cfg(feature = "parallel")] - fn compile_objects(&self, objs: &[Object], print: Option<&PrintThread>) -> Result<(), Error> { - use std::cell::Cell; + fn compile_objects(&self, objs: &[Object]) -> Result<(), Error> { + use std::{cell::Cell, io::BufReader, process::ChildStderr}; use parallel::async_executor::{block_on, YieldOnce}; if objs.len() <= 1 { for obj in objs { let (mut cmd, name) = self.create_compile_object_cmd(obj)?; - run(&mut cmd, &name, print)?; + run(&mut cmd, &name, &self.cargo_output)?; } return Ok(()); @@ -1280,7 +1277,13 @@ impl Build { parallel::retain_unordered_mut( &mut pendings, |(cmd, program, child, _token)| { - match try_wait_on_child(cmd, program, &mut child.0, &mut stdout) { + match try_wait_on_child( + cmd, + program, + &mut child.0, + &mut stdout, + &mut child.1, + ) { Ok(Some(())) => { // Task done, remove the entry has_made_progress.set(true); @@ -1328,11 +1331,11 @@ impl Build { YieldOnce::default().await } }; - let pipe_writer = print.map(PrintThread::clone_pipe_writer).transpose()?; - let child = spawn(&mut cmd, &program, pipe_writer)?; + let mut child = spawn(&mut cmd, &program, &self.cargo_output)?; + let stderr_reader = self.cargo_output.reader_for_stderr_as_warnings(&mut child); cell_update(&pendings, |mut pendings| { - pendings.push((cmd, program, KillOnDrop(child), token)); + pendings.push((cmd, program, KillOnDrop(child, stderr_reader), token)); pendings }); @@ -1345,7 +1348,7 @@ impl Build { return block_on(wait_future, spawn_future, &has_made_progress); - struct KillOnDrop(Child); + struct KillOnDrop(Child, Option>); impl Drop for KillOnDrop { fn drop(&mut self) { @@ -1367,10 +1370,10 @@ impl Build { } #[cfg(not(feature = "parallel"))] - fn compile_objects(&self, objs: &[Object], print: Option<&PrintThread>) -> Result<(), Error> { + fn compile_objects(&self, objs: &[Object]) -> Result<(), Error> { for obj in objs { let (mut cmd, name) = self.create_compile_object_cmd(obj)?; - run(&mut cmd, &name, print)?; + run(&mut cmd, &name, &self.cargo_output)?; } Ok(()) @@ -2145,13 +2148,7 @@ impl Build { Ok((cmd, tool.to_string())) } - fn assemble( - &self, - lib_name: &str, - dst: &Path, - objs: &[Object], - print: Option<&PrintThread>, - ) -> Result<(), Error> { + fn assemble(&self, lib_name: &str, dst: &Path, objs: &[Object]) -> Result<(), Error> { // Delete the destination if it exists as we want to // create on the first iteration instead of appending. let _ = fs::remove_file(dst); @@ -2165,7 +2162,7 @@ impl Build { .chain(self.objects.iter().map(std::ops::Deref::deref)) .collect(); for chunk in objs.chunks(100) { - self.assemble_progressive(dst, chunk, print)?; + self.assemble_progressive(dst, chunk)?; } if self.cuda && self.cuda_file_count() > 0 { @@ -2176,8 +2173,8 @@ impl Build { let dlink = out_dir.join(lib_name.to_owned() + "_dlink.o"); let mut nvcc = self.get_compiler().to_command(); nvcc.arg("--device-link").arg("-o").arg(&dlink).arg(dst); - run(&mut nvcc, "nvcc", print)?; - self.assemble_progressive(dst, &[dlink.as_path()], print)?; + run(&mut nvcc, "nvcc", &self.cargo_output)?; + self.assemble_progressive(dst, &[dlink.as_path()])?; } let target = self.get_target()?; @@ -2209,18 +2206,13 @@ impl Build { // NOTE: We add `s` even if flags were passed using $ARFLAGS/ar_flag, because `s` // here represents a _mode_, not an arbitrary flag. Further discussion of this choice // can be seen in https://github.com/rust-lang/cc-rs/pull/763. - run(ar.arg("s").arg(dst), &cmd, print)?; + run(ar.arg("s").arg(dst), &cmd, &self.cargo_output)?; } Ok(()) } - fn assemble_progressive( - &self, - dst: &Path, - objs: &[&Path], - print: Option<&PrintThread>, - ) -> Result<(), Error> { + fn assemble_progressive(&self, dst: &Path, objs: &[&Path]) -> Result<(), Error> { let target = self.get_target()?; if target.contains("msvc") { @@ -2241,7 +2233,7 @@ impl Build { cmd.arg(dst); } cmd.args(objs); - run(&mut cmd, &program, print)?; + run(&mut cmd, &program, &self.cargo_output)?; } else { let (mut ar, cmd, _any_flags) = self.get_ar()?; @@ -2272,7 +2264,7 @@ impl Build { // NOTE: We add cq here regardless of whether $ARFLAGS/ar_flag have been used because // it dictates the _mode_ ar runs in, which the setter of $ARFLAGS/ar_flag can't // dictate. See https://github.com/rust-lang/cc-rs/pull/763 for further discussion. - run(ar.arg("cq").arg(dst).args(objs), &cmd, print)?; + run(ar.arg("cq").arg(dst).args(objs), &cmd, &self.cargo_output)?; } Ok(()) diff --git a/src/os_pipe/mod.rs b/src/os_pipe/mod.rs deleted file mode 100644 index 0a6ad791f..000000000 --- a/src/os_pipe/mod.rs +++ /dev/null @@ -1,28 +0,0 @@ -//! Adapted from: -//! - -//! - -//! - -//! - -use std::fs::File; - -/// Open a new pipe and return a pair of [`File`] objects for the reader and writer. -/// -/// This corresponds to the `pipe2` library call on Posix and the -/// `CreatePipe` library call on Windows (though these implementation -/// details might change). These pipes are non-inheritable, so new child -/// processes won't receive a copy of them unless they're explicitly -/// passed as stdin/stdout/stderr. -pub fn pipe() -> std::io::Result<(File, File)> { - sys::pipe() -} - -#[cfg(unix)] -#[path = "unix.rs"] -mod sys; - -#[cfg(windows)] -#[path = "windows.rs"] -mod sys; - -#[cfg(all(not(unix), not(windows)))] -compile_error!("Only unix and windows support os_pipe!"); diff --git a/src/os_pipe/unix.rs b/src/os_pipe/unix.rs deleted file mode 100644 index ec4e547f0..000000000 --- a/src/os_pipe/unix.rs +++ /dev/null @@ -1,121 +0,0 @@ -use std::{ - fs::File, - io, - os::{raw::c_int, unix::io::FromRawFd}, -}; - -pub(super) fn pipe() -> io::Result<(File, File)> { - let mut fds = [0; 2]; - - // The only known way right now to create atomically set the CLOEXEC flag is - // to use the `pipe2` syscall. This was added to Linux in 2.6.27, glibc 2.9 - // and musl 0.9.3, and some other targets also have it. - #[cfg(any( - target_os = "dragonfly", - target_os = "freebsd", - target_os = "linux", - target_os = "netbsd", - target_os = "openbsd", - target_os = "redox" - ))] - { - unsafe { - cvt(libc::pipe2(fds.as_mut_ptr(), libc::O_CLOEXEC))?; - } - } - - #[cfg(not(any( - target_os = "dragonfly", - target_os = "freebsd", - target_os = "linux", - target_os = "netbsd", - target_os = "openbsd", - target_os = "redox" - )))] - { - unsafe { - cvt(libc::pipe(fds.as_mut_ptr()))?; - } - - cloexec::set_cloexec(fds[0])?; - cloexec::set_cloexec(fds[1])?; - } - - unsafe { Ok((File::from_raw_fd(fds[0]), File::from_raw_fd(fds[1]))) } -} - -fn cvt(t: c_int) -> io::Result { - if t == -1 { - Err(io::Error::last_os_error()) - } else { - Ok(t) - } -} - -#[cfg(not(any( - target_os = "dragonfly", - target_os = "freebsd", - target_os = "linux", - target_os = "netbsd", - target_os = "openbsd", - target_os = "redox" -)))] -mod cloexec { - use super::{c_int, cvt, io}; - - #[cfg(not(any( - target_env = "newlib", - target_os = "solaris", - target_os = "illumos", - target_os = "emscripten", - target_os = "fuchsia", - target_os = "l4re", - target_os = "linux", - target_os = "haiku", - target_os = "redox", - target_os = "vxworks", - target_os = "nto", - )))] - pub(super) fn set_cloexec(fd: c_int) -> io::Result<()> { - unsafe { - cvt(libc::ioctl(fd, libc::FIOCLEX))?; - } - - Ok(()) - } - - #[cfg(any( - all( - target_env = "newlib", - not(any(target_os = "espidf", target_os = "horizon")) - ), - target_os = "solaris", - target_os = "illumos", - target_os = "emscripten", - target_os = "fuchsia", - target_os = "l4re", - target_os = "linux", - target_os = "haiku", - target_os = "redox", - target_os = "vxworks", - target_os = "nto", - ))] - pub(super) fn set_cloexec(fd: c_int) -> io::Result<()> { - unsafe { - let previous = cvt(libc::fcntl(fd, libc::F_GETFD))?; - let new = previous | libc::FD_CLOEXEC; - if new != previous { - cvt(libc::fcntl(fd, libc::F_SETFD, new))?; - } - } - - Ok(()) - } - - // FD_CLOEXEC is not supported in ESP-IDF and Horizon OS but there's no need to, - // because neither supports spawning processes. - #[cfg(any(target_os = "espidf", target_os = "horizon"))] - pub(super) fn set_cloexec(_fd: c_int) -> io::Result<()> { - Ok(()) - } -} diff --git a/src/os_pipe/windows.rs b/src/os_pipe/windows.rs deleted file mode 100644 index de0106a2a..000000000 --- a/src/os_pipe/windows.rs +++ /dev/null @@ -1,24 +0,0 @@ -use crate::windows::windows_sys::{CreatePipe, INVALID_HANDLE_VALUE}; -use std::{fs::File, io, os::windows::prelude::*, ptr}; - -/// NOTE: These pipes do not support IOCP. -/// -/// If IOCP is needed, then you might want to emulate -/// anonymous pipes with `CreateNamedPipe`, as Rust's stdlib does. -pub(super) fn pipe() -> io::Result<(File, File)> { - let mut read_pipe = INVALID_HANDLE_VALUE; - let mut write_pipe = INVALID_HANDLE_VALUE; - - let ret = unsafe { CreatePipe(&mut read_pipe, &mut write_pipe, ptr::null_mut(), 0) }; - - if ret == 0 { - Err(io::Error::last_os_error()) - } else { - unsafe { - Ok(( - File::from_raw_handle(read_pipe as RawHandle), - File::from_raw_handle(write_pipe as RawHandle), - )) - } - } -}