From 52e942827b83e93df06f384b3eb4b0fd3f835123 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 18 Jul 2023 18:15:37 +1000 Subject: [PATCH 1/4] Vendor dependency `os_pipe` Signed-off-by: Jiahao XU --- Cargo.toml | 7 +- src/bin/cat.rs | 11 ++ src/bin/cat_both.rs | 18 +++ src/bin/swap.rs | 48 +++++++ src/lib.rs | 2 + src/os_pipe.rs | 305 +++++++++++++++++++++++++++++++++++++++++ src/os_pipe/unix.rs | 121 ++++++++++++++++ src/os_pipe/windows.rs | 24 ++++ tests/support/mod.rs | 6 +- 9 files changed, 538 insertions(+), 4 deletions(-) create mode 100644 src/bin/cat.rs create mode 100644 src/bin/cat_both.rs create mode 100644 src/bin/swap.rs create mode 100644 src/os_pipe.rs create mode 100644 src/os_pipe/unix.rs create mode 100644 src/os_pipe/windows.rs diff --git a/Cargo.toml b/Cargo.toml index 3117b8e2..d357903d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,7 +19,12 @@ edition = "2018" [dependencies] jobserver = { version = "0.1.16", optional = true } -os_pipe = "1" + +[target.'cfg(unix)'.dependencies] +libc = "0.2.62" + +[target.'cfg(windows)'.dependencies] +windows-sys = { version = "0.48.0", features = ["Win32_Foundation", "Win32_System_Pipes", "Win32_Security", "Win32_System_Threading"] } [features] parallel = ["jobserver"] diff --git a/src/bin/cat.rs b/src/bin/cat.rs new file mode 100644 index 00000000..a71a58b9 --- /dev/null +++ b/src/bin/cat.rs @@ -0,0 +1,11 @@ +#![cfg_attr(test, allow(dead_code))] + +/// Windows doesn't have a native equivalent for cat, so we use this little +/// Rust implementation instead. +use std::io::{copy, stdin, stdout}; + +fn main() { + let stdin_handle = stdin(); + let stdout_handle = stdout(); + copy(&mut stdin_handle.lock(), &mut stdout_handle.lock()).unwrap(); +} diff --git a/src/bin/cat_both.rs b/src/bin/cat_both.rs new file mode 100644 index 00000000..0f30b61d --- /dev/null +++ b/src/bin/cat_both.rs @@ -0,0 +1,18 @@ +//! This little test binary reads stdin, and then writes what it read to both +//! stdout and stderr, with a little tag to differentiate them. We use it to +//! test duping the standard file descriptors. + +#![cfg_attr(test, allow(dead_code))] + +use std::io::{self, prelude::*}; + +fn main() { + let mut input = Vec::new(); + io::stdin().read_to_end(&mut input).unwrap(); + + print!("stdout: "); + io::stdout().write_all(&input).unwrap(); + + eprint!("stderr: "); + io::stderr().write_all(&input).unwrap(); +} diff --git a/src/bin/swap.rs b/src/bin/swap.rs new file mode 100644 index 00000000..ecca345c --- /dev/null +++ b/src/bin/swap.rs @@ -0,0 +1,48 @@ +#![cfg_attr(test, allow(dead_code))] + +/// This little test binary reads stdin and write what it reads to both +/// stdout and stderr. It depends on os_pipe's parent_* functions, and +/// we use it to test them. +use std::{env::args_os, fs::File, io, mem::ManuallyDrop, process::Command}; + +#[cfg(windows)] +use std::os::windows::prelude::*; + +#[cfg(unix)] +use std::os::unix::prelude::*; + +#[cfg(windows)] +fn dup(f: &dyn AsRawHandle) -> File { + let handle = f.as_raw_handle(); + ManuallyDrop::new(unsafe { File::from_raw_handle(handle) }) + .try_clone() + .unwrap() +} + +#[cfg(unix)] +fn dup(f: &dyn AsRawFd) -> File { + let handle = f.as_raw_fd(); + ManuallyDrop::new(unsafe { File::from_raw_fd(handle) }) + .try_clone() + .unwrap() +} + +fn main() { + let stdin = dup(&io::stdin()); + let stdout = dup(&io::stdout()); + let stderr = dup(&io::stderr()); + + let mut args = args_os(); + args.next().unwrap(); // Ignore args[0] + let mut child = Command::new(args.next().unwrap()); // Run args[1] + child.args(args); // Feed rest of the arg into the program + + // Swap stdout and stderr in the child. Set stdin too, just for testing, + // though this should be the same as the default behavior. + child.stdin(stdin); + child.stdout(stderr); + child.stderr(stdout); + + // Run the child. This method is kind of confusingly named... + child.status().unwrap(); +} diff --git a/src/lib.rs b/src/lib.rs index 26242428..e0be340c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -69,6 +69,8 @@ use std::process::{Child, Command, Stdio}; use std::sync::{Arc, Mutex}; use std::thread::{self, JoinHandle}; +mod os_pipe; + // These modules are all glue to support reading the MSVC version from // the registry and from COM interfaces #[cfg(windows)] diff --git a/src/os_pipe.rs b/src/os_pipe.rs new file mode 100644 index 00000000..bd799b8e --- /dev/null +++ b/src/os_pipe.rs @@ -0,0 +1,305 @@ +//! Adapted from: +//! - https://doc.rust-lang.org/src/std/sys/unix/pipe.rs.html +//! - https://doc.rust-lang.org/src/std/sys/unix/fd.rs.html#385 +//! - https://github.com/rust-lang/rust/blob/master/library/std/src/sys/mod.rs#L57 +//! - https://github.com/oconnor663/os_pipe.rs +use std::{fmt, fs::File, io, process::Stdio}; + +macro_rules! impl_Read_by_forward { + ($type:ty) => { + impl io::Read for $type { + fn read(&mut self, buf: &mut [u8]) -> io::Result { + (&self.0).read(buf) + } + + fn read_vectored(&mut self, bufs: &mut [io::IoSliceMut<'_>]) -> io::Result { + (&self.0).read_vectored(bufs) + } + + fn read_to_end(&mut self, buf: &mut Vec) -> io::Result { + (&self.0).read_to_end(buf) + } + fn read_to_string(&mut self, buf: &mut String) -> io::Result { + (&self.0).read_to_string(buf) + } + fn read_exact(&mut self, buf: &mut [u8]) -> io::Result<()> { + (&self.0).read_exact(buf) + } + } + }; +} + +macro_rules! impl_Write_by_forward { + ($type:ty) => { + impl io::Write for $type { + fn write(&mut self, buf: &[u8]) -> io::Result { + (&self.0).write(buf) + } + fn flush(&mut self) -> io::Result<()> { + (&self.0).flush() + } + + fn write_vectored(&mut self, bufs: &[io::IoSlice<'_>]) -> io::Result { + (&self.0).write_vectored(bufs) + } + fn write_all(&mut self, buf: &[u8]) -> io::Result<()> { + (&self.0).write_all(buf) + } + fn write_fmt(&mut self, fmt: fmt::Arguments<'_>) -> io::Result<()> { + (&self.0).write_fmt(fmt) + } + } + }; +} + +/// The reading end of a pipe, returned by [`pipe`](fn.pipe.html). +/// +/// `PipeReader` implements `Into`, so you can pass it as an argument to +/// `Command::stdin` to spawn a child process that reads from the pipe. +#[derive(Debug)] +pub struct PipeReader(File); + +impl PipeReader { + pub fn try_clone(&self) -> io::Result { + self.0.try_clone().map(Self) + } +} + +impl_Read_by_forward!(PipeReader); +impl_Read_by_forward!(&PipeReader); + +impl From for Stdio { + fn from(p: PipeReader) -> Stdio { + p.0.into() + } +} + +/// The writing end of a pipe, returned by [`pipe`](fn.pipe.html). +/// +/// `PipeWriter` implements `Into`, so you can pass it as an argument to +/// `Command::stdout` or `Command::stderr` to spawn a child process that writes +/// to the pipe. +#[derive(Debug)] +pub struct PipeWriter(File); + +impl PipeWriter { + pub fn try_clone(&self) -> io::Result { + self.0.try_clone().map(Self) + } +} + +impl_Write_by_forward!(PipeWriter); +impl_Write_by_forward!(&PipeWriter); + +impl From for Stdio { + fn from(p: PipeWriter) -> Stdio { + p.0.into() + } +} + +/// Open a new pipe and return a [`PipeReader`] and [`PipeWriter`] pair. +/// +/// 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. +/// +/// [`PipeReader`]: struct.PipeReader.html +/// [`PipeWriter`]: struct.PipeWriter.html +pub fn pipe() -> io::Result<(PipeReader, PipeWriter)> { + sys::pipe().map(|(r, w)| (PipeReader(r), PipeWriter(w))) +} + +#[cfg(unix)] +#[path = "os_pipe/unix.rs"] +mod sys; + +#[cfg(windows)] +#[path = "os_pipe/windows.rs"] +mod sys; + +#[cfg(all(not(unix), not(windows)))] +compile_error!("Only unix and windows support os_pipe!"); + +#[cfg(test)] +mod tests { + use super::*; + use std::{ + env::{self, consts::EXE_EXTENSION}, + io::prelude::*, + path::PathBuf, + process::Command, + thread, + }; + + fn path_to_exe(name: &str) -> PathBuf { + let mut p = env::current_exe().unwrap(); + p.pop(); + if p.ends_with("deps") { + p.pop(); + } + p.push(name); + p.set_extension(EXE_EXTENSION); + + p + } + + #[test] + fn test_pipe_some_data() { + let (mut reader, mut writer) = pipe().unwrap(); + // A small write won't fill the pipe buffer, so it won't block this thread. + writer.write_all(b"some stuff").unwrap(); + drop(writer); + let mut out = String::new(); + reader.read_to_string(&mut out).unwrap(); + assert_eq!(out, "some stuff"); + } + + #[test] + fn test_pipe_some_data_with_refs() { + // As with `File`, there's a second set of impls for shared + // refs. Test those. + let (reader, writer) = pipe().unwrap(); + let mut reader_ref = &reader; + { + let mut writer_ref = &writer; + // A small write won't fill the pipe buffer, so it won't block this thread. + writer_ref.write_all(b"some stuff").unwrap(); + } + drop(writer); + let mut out = String::new(); + reader_ref.read_to_string(&mut out).unwrap(); + assert_eq!(out, "some stuff"); + } + + #[test] + fn test_pipe_no_data() { + let (mut reader, writer) = pipe().unwrap(); + drop(writer); + let mut out = String::new(); + reader.read_to_string(&mut out).unwrap(); + assert_eq!(out, ""); + } + + #[test] + fn test_pipe_a_megabyte_of_data_from_another_thread() { + let data = vec![0xff; 1_000_000]; + let data_copy = data.clone(); + let (mut reader, mut writer) = pipe().unwrap(); + let joiner = thread::spawn(move || { + writer.write_all(&data_copy).unwrap(); + // This drop happens automatically, so writing it out here is mostly + // just for clarity. For what it's worth, it also guards against + // accidentally forgetting to drop if we switch to scoped threads or + // something like that and change this to a non-moving closure. The + // explicit drop forces `writer` to move. + drop(writer); + }); + let mut out = Vec::new(); + reader.read_to_end(&mut out).unwrap(); + joiner.join().unwrap(); + assert_eq!(out, data); + } + + #[test] + fn test_pipes_are_not_inheritable() { + // Create pipes for a child process. + let (input_reader, mut input_writer) = pipe().unwrap(); + let (mut output_reader, output_writer) = pipe().unwrap(); + + // Create a bunch of duplicated copies, which we'll close later. This + // tests that duplication preserves non-inheritability. + let ir_dup = input_reader.try_clone().unwrap(); + let iw_dup = input_writer.try_clone().unwrap(); + let or_dup = output_reader.try_clone().unwrap(); + let ow_dup = output_writer.try_clone().unwrap(); + + // Spawn the child. Note that this temporary Command object takes + // ownership of our copies of the child's stdin and stdout, and then + // closes them immediately when it drops. That stops us from blocking + // our own read below. We use our own simple implementation of cat for + // compatibility with Windows. + let mut child = Command::new(path_to_exe("cat")) + .stdin(input_reader) + .stdout(output_writer) + .spawn() + .unwrap(); + + // Drop all the dups now that the child is spawned. + drop(ir_dup); + drop(iw_dup); + drop(or_dup); + drop(ow_dup); + + // Write to the child's stdin. This is a small write, so it shouldn't + // block. + input_writer.write_all(b"hello").unwrap(); + drop(input_writer); + + // Read from the child's stdout. If this child has accidentally + // inherited the write end of its own stdin, then it will never exit, + // and this read will block forever. That's what this test is all + // about. + let mut output = Vec::new(); + output_reader.read_to_end(&mut output).unwrap(); + child.wait().unwrap(); + + // Confirm that we got the right bytes. + assert_eq!(b"hello", &*output); + } + + #[test] + fn test_parent_handles() { + // This test invokes the `swap` test program, which uses parent_stdout() and + // parent_stderr() to swap the outputs for another child that it spawns. + + // Create pipes for a child process. + let (reader, mut writer) = pipe().unwrap(); + + // Write input. This shouldn't block because it's small. Then close the write end, or else + // the child will hang. + writer.write_all(b"quack").unwrap(); + drop(writer); + + // Use `swap` to run `cat_both`. `cat_both will read "quack" from stdin + // and write it to stdout and stderr with different tags. But because we + // run it inside `swap`, the tags in the output should be backwards. + let output = Command::new(path_to_exe("swap")) + .arg(path_to_exe("cat_both")) + .stdin(reader) + .output() + .unwrap(); + + // Check for a clean exit. + assert!( + output.status.success(), + "child process returned {:#?}", + output + ); + + // Confirm that we got the right bytes. + assert_eq!(b"stderr: quack", &*output.stdout); + assert_eq!(b"stdout: quack", &*output.stderr); + } + + #[test] + fn test_try_clone() { + let (reader, writer) = pipe().unwrap(); + let mut reader_clone = reader.try_clone().unwrap(); + let mut writer_clone = writer.try_clone().unwrap(); + // A small write won't fill the pipe buffer, so it won't block this thread. + writer_clone.write_all(b"some stuff").unwrap(); + drop(writer); + drop(writer_clone); + let mut out = String::new(); + reader_clone.read_to_string(&mut out).unwrap(); + assert_eq!(out, "some stuff"); + } + + #[test] + fn test_debug() { + let (reader, writer) = pipe().unwrap(); + format!("{:?} {:?}", reader, writer); + } +} diff --git a/src/os_pipe/unix.rs b/src/os_pipe/unix.rs new file mode 100644 index 00000000..ec4e547f --- /dev/null +++ b/src/os_pipe/unix.rs @@ -0,0 +1,121 @@ +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 new file mode 100644 index 00000000..4f7a5709 --- /dev/null +++ b/src/os_pipe/windows.rs @@ -0,0 +1,24 @@ +use std::{fs::File, io, os::windows::prelude::*, ptr}; +use windows_sys::Win32::{Foundation::INVALID_HANDLE_VALUE, System::Pipes::CreatePipe}; + +/// 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), + )) + } + } +} diff --git a/tests/support/mod.rs b/tests/support/mod.rs index f3c04405..dd1e38b6 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -34,7 +34,7 @@ impl Test { // lesser of the two evils. env::remove_var("RUSTC_WRAPPER"); - let mut gcc = PathBuf::from(env::current_exe().unwrap()); + let mut gcc = env::current_exe().unwrap(); gcc.pop(); if gcc.ends_with("deps") { gcc.pop(); @@ -42,8 +42,8 @@ impl Test { let td = Builder::new().prefix("gcc-test").tempdir_in(&gcc).unwrap(); gcc.push(format!("gcc-shim{}", env::consts::EXE_SUFFIX)); Test { - td: td, - gcc: gcc, + td, + gcc, msvc: false, } } From 2dceba2667f1b20f37874f5c33135576e02c7dcb Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Wed, 19 Jul 2023 18:47:24 -0700 Subject: [PATCH 2/4] Simplify os_pipe substantially (#1) Eliminate the separate PipeReader and PipeWriter types, and just use File. Remove unused windows-sys features. Remove added test infrastructure. Revert unrelated changes. --- Cargo.toml | 2 +- src/bin/cat.rs | 11 -- src/bin/cat_both.rs | 18 --- src/bin/swap.rs | 48 -------- src/lib.rs | 14 +-- src/os_pipe.rs | 285 +------------------------------------------ tests/support/mod.rs | 6 +- 7 files changed, 15 insertions(+), 369 deletions(-) delete mode 100644 src/bin/cat.rs delete mode 100644 src/bin/cat_both.rs delete mode 100644 src/bin/swap.rs diff --git a/Cargo.toml b/Cargo.toml index d357903d..d4f2a854 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,7 +24,7 @@ jobserver = { version = "0.1.16", optional = true } libc = "0.2.62" [target.'cfg(windows)'.dependencies] -windows-sys = { version = "0.48.0", features = ["Win32_Foundation", "Win32_System_Pipes", "Win32_Security", "Win32_System_Threading"] } +windows-sys = { version = "0.48.0", features = ["Win32_Foundation", "Win32_System_Pipes"] } [features] parallel = ["jobserver"] diff --git a/src/bin/cat.rs b/src/bin/cat.rs deleted file mode 100644 index a71a58b9..00000000 --- a/src/bin/cat.rs +++ /dev/null @@ -1,11 +0,0 @@ -#![cfg_attr(test, allow(dead_code))] - -/// Windows doesn't have a native equivalent for cat, so we use this little -/// Rust implementation instead. -use std::io::{copy, stdin, stdout}; - -fn main() { - let stdin_handle = stdin(); - let stdout_handle = stdout(); - copy(&mut stdin_handle.lock(), &mut stdout_handle.lock()).unwrap(); -} diff --git a/src/bin/cat_both.rs b/src/bin/cat_both.rs deleted file mode 100644 index 0f30b61d..00000000 --- a/src/bin/cat_both.rs +++ /dev/null @@ -1,18 +0,0 @@ -//! This little test binary reads stdin, and then writes what it read to both -//! stdout and stderr, with a little tag to differentiate them. We use it to -//! test duping the standard file descriptors. - -#![cfg_attr(test, allow(dead_code))] - -use std::io::{self, prelude::*}; - -fn main() { - let mut input = Vec::new(); - io::stdin().read_to_end(&mut input).unwrap(); - - print!("stdout: "); - io::stdout().write_all(&input).unwrap(); - - eprint!("stderr: "); - io::stderr().write_all(&input).unwrap(); -} diff --git a/src/bin/swap.rs b/src/bin/swap.rs deleted file mode 100644 index ecca345c..00000000 --- a/src/bin/swap.rs +++ /dev/null @@ -1,48 +0,0 @@ -#![cfg_attr(test, allow(dead_code))] - -/// This little test binary reads stdin and write what it reads to both -/// stdout and stderr. It depends on os_pipe's parent_* functions, and -/// we use it to test them. -use std::{env::args_os, fs::File, io, mem::ManuallyDrop, process::Command}; - -#[cfg(windows)] -use std::os::windows::prelude::*; - -#[cfg(unix)] -use std::os::unix::prelude::*; - -#[cfg(windows)] -fn dup(f: &dyn AsRawHandle) -> File { - let handle = f.as_raw_handle(); - ManuallyDrop::new(unsafe { File::from_raw_handle(handle) }) - .try_clone() - .unwrap() -} - -#[cfg(unix)] -fn dup(f: &dyn AsRawFd) -> File { - let handle = f.as_raw_fd(); - ManuallyDrop::new(unsafe { File::from_raw_fd(handle) }) - .try_clone() - .unwrap() -} - -fn main() { - let stdin = dup(&io::stdin()); - let stdout = dup(&io::stdout()); - let stderr = dup(&io::stderr()); - - let mut args = args_os(); - args.next().unwrap(); // Ignore args[0] - let mut child = Command::new(args.next().unwrap()); // Run args[1] - child.args(args); // Feed rest of the arg into the program - - // Swap stdout and stderr in the child. Set stdin too, just for testing, - // though this should be the same as the default behavior. - child.stdin(stdin); - child.stdout(stderr); - child.stderr(stdout); - - // Run the child. This method is kind of confusingly named... - child.status().unwrap(); -} diff --git a/src/lib.rs b/src/lib.rs index e0be340c..c5d022e7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -61,7 +61,7 @@ use std::collections::{hash_map, HashMap}; use std::env; use std::ffi::{OsStr, OsString}; use std::fmt::{self, Display, Formatter}; -use std::fs; +use std::fs::{self, File}; use std::hash::Hasher; use std::io::{self, BufRead, BufReader, Read, Write}; use std::path::{Component, Path, PathBuf}; @@ -3499,7 +3499,7 @@ fn wait_on_child(cmd: &Command, program: &str, child: &mut Child) -> Result<(), fn run_inner( cmd: &mut Command, program: &str, - pipe_writer: os_pipe::PipeWriter, + pipe_writer: File, ) -> Result<(), Error> { let mut child = spawn(cmd, program, pipe_writer)?; wait_on_child(cmd, program, &mut child) @@ -3534,7 +3534,7 @@ fn run_output(cmd: &mut Command, program: &str) -> Result, Error> { fn spawn( cmd: &mut Command, program: &str, - pipe_writer: os_pipe::PipeWriter, + pipe_writer: File, ) -> Result { struct ResetStderr<'cmd>(&'cmd mut Command); @@ -3775,7 +3775,7 @@ impl AsmFileExt { struct PrintThread { handle: Option>, - pipe_writer: Option, + pipe_writer: Option, } impl PrintThread { @@ -3806,14 +3806,14 @@ impl PrintThread { }) } - fn pipe_writer(&mut self) -> &mut Option { + fn pipe_writer(&mut self) -> &mut Option { &mut self.pipe_writer } - fn pipe_writer_cloned(&self) -> Result, Error> { + fn pipe_writer_cloned(&self) -> Result, Error> { self.pipe_writer .as_ref() - .map(os_pipe::PipeWriter::try_clone) + .map(File::try_clone) .transpose() .map_err(From::from) } diff --git a/src/os_pipe.rs b/src/os_pipe.rs index bd799b8e..218c9bc9 100644 --- a/src/os_pipe.rs +++ b/src/os_pipe.rs @@ -3,112 +3,17 @@ //! - https://doc.rust-lang.org/src/std/sys/unix/fd.rs.html#385 //! - https://github.com/rust-lang/rust/blob/master/library/std/src/sys/mod.rs#L57 //! - https://github.com/oconnor663/os_pipe.rs -use std::{fmt, fs::File, io, process::Stdio}; +use std::fs::File; -macro_rules! impl_Read_by_forward { - ($type:ty) => { - impl io::Read for $type { - fn read(&mut self, buf: &mut [u8]) -> io::Result { - (&self.0).read(buf) - } - - fn read_vectored(&mut self, bufs: &mut [io::IoSliceMut<'_>]) -> io::Result { - (&self.0).read_vectored(bufs) - } - - fn read_to_end(&mut self, buf: &mut Vec) -> io::Result { - (&self.0).read_to_end(buf) - } - fn read_to_string(&mut self, buf: &mut String) -> io::Result { - (&self.0).read_to_string(buf) - } - fn read_exact(&mut self, buf: &mut [u8]) -> io::Result<()> { - (&self.0).read_exact(buf) - } - } - }; -} - -macro_rules! impl_Write_by_forward { - ($type:ty) => { - impl io::Write for $type { - fn write(&mut self, buf: &[u8]) -> io::Result { - (&self.0).write(buf) - } - fn flush(&mut self) -> io::Result<()> { - (&self.0).flush() - } - - fn write_vectored(&mut self, bufs: &[io::IoSlice<'_>]) -> io::Result { - (&self.0).write_vectored(bufs) - } - fn write_all(&mut self, buf: &[u8]) -> io::Result<()> { - (&self.0).write_all(buf) - } - fn write_fmt(&mut self, fmt: fmt::Arguments<'_>) -> io::Result<()> { - (&self.0).write_fmt(fmt) - } - } - }; -} - -/// The reading end of a pipe, returned by [`pipe`](fn.pipe.html). -/// -/// `PipeReader` implements `Into`, so you can pass it as an argument to -/// `Command::stdin` to spawn a child process that reads from the pipe. -#[derive(Debug)] -pub struct PipeReader(File); - -impl PipeReader { - pub fn try_clone(&self) -> io::Result { - self.0.try_clone().map(Self) - } -} - -impl_Read_by_forward!(PipeReader); -impl_Read_by_forward!(&PipeReader); - -impl From for Stdio { - fn from(p: PipeReader) -> Stdio { - p.0.into() - } -} - -/// The writing end of a pipe, returned by [`pipe`](fn.pipe.html). -/// -/// `PipeWriter` implements `Into`, so you can pass it as an argument to -/// `Command::stdout` or `Command::stderr` to spawn a child process that writes -/// to the pipe. -#[derive(Debug)] -pub struct PipeWriter(File); - -impl PipeWriter { - pub fn try_clone(&self) -> io::Result { - self.0.try_clone().map(Self) - } -} - -impl_Write_by_forward!(PipeWriter); -impl_Write_by_forward!(&PipeWriter); - -impl From for Stdio { - fn from(p: PipeWriter) -> Stdio { - p.0.into() - } -} - -/// Open a new pipe and return a [`PipeReader`] and [`PipeWriter`] pair. +/// 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. -/// -/// [`PipeReader`]: struct.PipeReader.html -/// [`PipeWriter`]: struct.PipeWriter.html -pub fn pipe() -> io::Result<(PipeReader, PipeWriter)> { - sys::pipe().map(|(r, w)| (PipeReader(r), PipeWriter(w))) +pub fn pipe() -> std::io::Result<(File, File)> { + sys::pipe() } #[cfg(unix)] @@ -121,185 +26,3 @@ mod sys; #[cfg(all(not(unix), not(windows)))] compile_error!("Only unix and windows support os_pipe!"); - -#[cfg(test)] -mod tests { - use super::*; - use std::{ - env::{self, consts::EXE_EXTENSION}, - io::prelude::*, - path::PathBuf, - process::Command, - thread, - }; - - fn path_to_exe(name: &str) -> PathBuf { - let mut p = env::current_exe().unwrap(); - p.pop(); - if p.ends_with("deps") { - p.pop(); - } - p.push(name); - p.set_extension(EXE_EXTENSION); - - p - } - - #[test] - fn test_pipe_some_data() { - let (mut reader, mut writer) = pipe().unwrap(); - // A small write won't fill the pipe buffer, so it won't block this thread. - writer.write_all(b"some stuff").unwrap(); - drop(writer); - let mut out = String::new(); - reader.read_to_string(&mut out).unwrap(); - assert_eq!(out, "some stuff"); - } - - #[test] - fn test_pipe_some_data_with_refs() { - // As with `File`, there's a second set of impls for shared - // refs. Test those. - let (reader, writer) = pipe().unwrap(); - let mut reader_ref = &reader; - { - let mut writer_ref = &writer; - // A small write won't fill the pipe buffer, so it won't block this thread. - writer_ref.write_all(b"some stuff").unwrap(); - } - drop(writer); - let mut out = String::new(); - reader_ref.read_to_string(&mut out).unwrap(); - assert_eq!(out, "some stuff"); - } - - #[test] - fn test_pipe_no_data() { - let (mut reader, writer) = pipe().unwrap(); - drop(writer); - let mut out = String::new(); - reader.read_to_string(&mut out).unwrap(); - assert_eq!(out, ""); - } - - #[test] - fn test_pipe_a_megabyte_of_data_from_another_thread() { - let data = vec![0xff; 1_000_000]; - let data_copy = data.clone(); - let (mut reader, mut writer) = pipe().unwrap(); - let joiner = thread::spawn(move || { - writer.write_all(&data_copy).unwrap(); - // This drop happens automatically, so writing it out here is mostly - // just for clarity. For what it's worth, it also guards against - // accidentally forgetting to drop if we switch to scoped threads or - // something like that and change this to a non-moving closure. The - // explicit drop forces `writer` to move. - drop(writer); - }); - let mut out = Vec::new(); - reader.read_to_end(&mut out).unwrap(); - joiner.join().unwrap(); - assert_eq!(out, data); - } - - #[test] - fn test_pipes_are_not_inheritable() { - // Create pipes for a child process. - let (input_reader, mut input_writer) = pipe().unwrap(); - let (mut output_reader, output_writer) = pipe().unwrap(); - - // Create a bunch of duplicated copies, which we'll close later. This - // tests that duplication preserves non-inheritability. - let ir_dup = input_reader.try_clone().unwrap(); - let iw_dup = input_writer.try_clone().unwrap(); - let or_dup = output_reader.try_clone().unwrap(); - let ow_dup = output_writer.try_clone().unwrap(); - - // Spawn the child. Note that this temporary Command object takes - // ownership of our copies of the child's stdin and stdout, and then - // closes them immediately when it drops. That stops us from blocking - // our own read below. We use our own simple implementation of cat for - // compatibility with Windows. - let mut child = Command::new(path_to_exe("cat")) - .stdin(input_reader) - .stdout(output_writer) - .spawn() - .unwrap(); - - // Drop all the dups now that the child is spawned. - drop(ir_dup); - drop(iw_dup); - drop(or_dup); - drop(ow_dup); - - // Write to the child's stdin. This is a small write, so it shouldn't - // block. - input_writer.write_all(b"hello").unwrap(); - drop(input_writer); - - // Read from the child's stdout. If this child has accidentally - // inherited the write end of its own stdin, then it will never exit, - // and this read will block forever. That's what this test is all - // about. - let mut output = Vec::new(); - output_reader.read_to_end(&mut output).unwrap(); - child.wait().unwrap(); - - // Confirm that we got the right bytes. - assert_eq!(b"hello", &*output); - } - - #[test] - fn test_parent_handles() { - // This test invokes the `swap` test program, which uses parent_stdout() and - // parent_stderr() to swap the outputs for another child that it spawns. - - // Create pipes for a child process. - let (reader, mut writer) = pipe().unwrap(); - - // Write input. This shouldn't block because it's small. Then close the write end, or else - // the child will hang. - writer.write_all(b"quack").unwrap(); - drop(writer); - - // Use `swap` to run `cat_both`. `cat_both will read "quack" from stdin - // and write it to stdout and stderr with different tags. But because we - // run it inside `swap`, the tags in the output should be backwards. - let output = Command::new(path_to_exe("swap")) - .arg(path_to_exe("cat_both")) - .stdin(reader) - .output() - .unwrap(); - - // Check for a clean exit. - assert!( - output.status.success(), - "child process returned {:#?}", - output - ); - - // Confirm that we got the right bytes. - assert_eq!(b"stderr: quack", &*output.stdout); - assert_eq!(b"stdout: quack", &*output.stderr); - } - - #[test] - fn test_try_clone() { - let (reader, writer) = pipe().unwrap(); - let mut reader_clone = reader.try_clone().unwrap(); - let mut writer_clone = writer.try_clone().unwrap(); - // A small write won't fill the pipe buffer, so it won't block this thread. - writer_clone.write_all(b"some stuff").unwrap(); - drop(writer); - drop(writer_clone); - let mut out = String::new(); - reader_clone.read_to_string(&mut out).unwrap(); - assert_eq!(out, "some stuff"); - } - - #[test] - fn test_debug() { - let (reader, writer) = pipe().unwrap(); - format!("{:?} {:?}", reader, writer); - } -} diff --git a/tests/support/mod.rs b/tests/support/mod.rs index dd1e38b6..f3c04405 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -34,7 +34,7 @@ impl Test { // lesser of the two evils. env::remove_var("RUSTC_WRAPPER"); - let mut gcc = env::current_exe().unwrap(); + let mut gcc = PathBuf::from(env::current_exe().unwrap()); gcc.pop(); if gcc.ends_with("deps") { gcc.pop(); @@ -42,8 +42,8 @@ impl Test { let td = Builder::new().prefix("gcc-test").tempdir_in(&gcc).unwrap(); gcc.push(format!("gcc-shim{}", env::consts::EXE_SUFFIX)); Test { - td, - gcc, + td: td, + gcc: gcc, msvc: false, } } From 8a4de6d035d3a9d82ae2ce8d19773c49635a9beb Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Thu, 20 Jul 2023 11:47:50 +1000 Subject: [PATCH 3/4] Fix cargo fmt Signed-off-by: Jiahao XU --- src/lib.rs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index c5d022e7..0532e528 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -3496,11 +3496,7 @@ fn wait_on_child(cmd: &Command, program: &str, child: &mut Child) -> Result<(), } } -fn run_inner( - cmd: &mut Command, - program: &str, - pipe_writer: File, -) -> Result<(), Error> { +fn run_inner(cmd: &mut Command, program: &str, pipe_writer: File) -> Result<(), Error> { let mut child = spawn(cmd, program, pipe_writer)?; wait_on_child(cmd, program, &mut child) } @@ -3531,11 +3527,7 @@ fn run_output(cmd: &mut Command, program: &str) -> Result, Error> { Ok(stdout) } -fn spawn( - cmd: &mut Command, - program: &str, - pipe_writer: File, -) -> Result { +fn spawn(cmd: &mut Command, program: &str, pipe_writer: File) -> Result { struct ResetStderr<'cmd>(&'cmd mut Command); impl Drop for ResetStderr<'_> { From 7d0eb1f05489e47c02105e5d11db7acdf70437f3 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Thu, 20 Jul 2023 12:03:05 +1000 Subject: [PATCH 4/4] Fix `windows-sys` feature: Enable `Win32_Security` Signed-off-by: Jiahao XU --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index d4f2a854..4158cd27 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,7 +24,7 @@ jobserver = { version = "0.1.16", optional = true } libc = "0.2.62" [target.'cfg(windows)'.dependencies] -windows-sys = { version = "0.48.0", features = ["Win32_Foundation", "Win32_System_Pipes"] } +windows-sys = { version = "0.48.0", features = ["Win32_Foundation", "Win32_System_Pipes", "Win32_Security"] } [features] parallel = ["jobserver"]