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

Add support for non-unix and non-windows platforms #13

Merged
merged 7 commits into from
May 17, 2019
Merged
Changes from 2 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
141 changes: 131 additions & 10 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,13 +254,16 @@ impl Client {
/// On Unix and Windows this will clobber the `CARGO_MAKEFLAGS` environment
/// variables for the child process, and on Unix this will also allow the
/// two file descriptors for this client to be inherited to the child.
///
/// On platforms other than Unix and Windows this does nothing.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like it may be more appropriate to panic on other platforms? It's not supported there so only in-process usage is expected to work

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would make impossible to use same code compiled to both native and WASI, without extra #[cfg(...)].

Copy link
Member Author

@bjorn3 bjorn3 May 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RReverser it would be possible, as WASI doesn't support spawning processes, so if you were to call configure you would likely panic shortly afterwards for spawning a thread anyway. Also because the function keeps existing no #[cfg(...)] is required to keep it compiling.

Edit: on other previously unsupported platforms, there would need to be a #[cfg(...)] in the client code, but at least that prevents them from accidentally overloading the processor due to missing cross-process jobserver support.

pub fn configure(&self, cmd: &mut Command) {
let arg = self.inner.string_arg();
// Older implementations of make use `--jobserver-fds` and newer
// implementations use `--jobserver-auth`, pass both to try to catch
// both implementations.
let value = format!("--jobserver-fds={0} --jobserver-auth={0}", arg);
cmd.env("CARGO_MAKEFLAGS", &value);
if let Some(arg) = self.inner.string_arg() {
// Older implementations of make use `--jobserver-fds` and newer
// implementations use `--jobserver-auth`, pass both to try to catch
// both implementations.
let value = format!("--jobserver-fds={0} --jobserver-auth={0}", arg);
cmd.env("CARGO_MAKEFLAGS", &value);
}
self.inner.configure(cmd);
}

Expand Down Expand Up @@ -573,8 +576,8 @@ mod imp {
}
}

pub fn string_arg(&self) -> String {
format!("{},{} -j", self.read.as_raw_fd(), self.write.as_raw_fd())
pub fn string_arg(&self) -> Option<String> {
Some(format!("{},{} -j", self.read.as_raw_fd(), self.write.as_raw_fd()))
}

pub fn configure(&self, cmd: &mut Command) {
Expand Down Expand Up @@ -881,8 +884,8 @@ mod imp {
}
}

pub fn string_arg(&self) -> String {
self.name.clone()
pub fn string_arg(&self) -> Option<String> {
Some(self.name.clone())
}

pub fn configure(&self, _cmd: &mut Command) {
Expand Down Expand Up @@ -961,3 +964,121 @@ mod imp {
}
}
}

#[cfg(not(any(unix, windows)))]
mod imp {
use std::io;
use std::sync::{Arc, Mutex};
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::mpsc::{self, SyncSender, Receiver, RecvTimeoutError};
use std::thread::{self, Builder, JoinHandle};
use std::time::Duration;
use std::process::Command;

#[derive(Debug)]
pub struct Client {
tx: SyncSender<()>,
rx: Mutex<Receiver<()>>,
}

#[derive(Debug)]
pub struct Acquired(());

impl Client {
pub fn new(limit: usize) -> io::Result<Client> {
let (tx, rx) = mpsc::sync_channel(limit);
for _ in 0..limit {
tx.send(()).unwrap();
}
Ok(Client {
tx,
rx: Mutex::new(rx),
})
}

pub unsafe fn open(_s: &str) -> Option<Client> {
None
}

pub fn acquire(&self) -> io::Result<Acquired> {
self.rx.lock().unwrap().recv().unwrap();
Ok(Acquired(()))
}

pub fn release(&self, _data: Option<&Acquired>) -> io::Result<()> {
self.tx.send(()).unwrap();
Ok(())
}

pub fn string_arg(&self) -> Option<String> {
None
}

pub fn configure(&self, _cmd: &mut Command) {}
}

#[derive(Debug)]
pub struct Helper {
thread: JoinHandle<()>,
quitting: Arc<AtomicBool>,
rx_done: Receiver<()>,
}

pub fn spawn_helper(client: ::Client,
rx: Receiver<()>,
mut f: Box<FnMut(io::Result<::Acquired>) + Send>)
-> io::Result<Helper>
{
let quitting = Arc::new(AtomicBool::new(false));
let quitting2 = quitting.clone();
let (tx_done, rx_done) = mpsc::channel();
let thread = Builder::new().spawn(move || {
'outer:
for () in rx {
loop {
let res = client.acquire();
if let Err(ref e) = res {
if e.kind() == io::ErrorKind::Interrupted {
if quitting2.load(Ordering::SeqCst) {
break 'outer
} else {
continue
}
}
}
f(res);
break
}
bjorn3 marked this conversation as resolved.
Show resolved Hide resolved
}
tx_done.send(()).unwrap();
})?;

Ok(Helper {
thread: thread,
quitting: quitting,
rx_done: rx_done,
})
}

impl Helper {
pub fn join(self) {
self.quitting.store(true, Ordering::SeqCst);
bjorn3 marked this conversation as resolved.
Show resolved Hide resolved
let dur = Duration::from_millis(10);
let mut done = false;
for _ in 0..100 {
bjorn3 marked this conversation as resolved.
Show resolved Hide resolved
match self.rx_done.recv_timeout(dur) {
Ok(()) |
Err(RecvTimeoutError::Disconnected) => {
done = true;
break
}
Err(RecvTimeoutError::Timeout) => {}
}
thread::yield_now();
}
if done {
drop(self.thread.join());
}
}
}
}