From 0aa10904be8d0453d1fb4e2252cc91f4eba06a90 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 13 Feb 2024 23:41:36 +1100 Subject: [PATCH] Fix jobserver: Last `--jobserver-auth` wins `--jobserver-auth=` is the only documented makeflags. `--jobserver-fds=` is actually an internal only makeflags, so we should always prefer `--jobserver-auth=`. Also, according to doc of makeflags, if there are multiple `--jobserver-auth=` the last one is used ref: rust-lang/jobserver-rs#67 Signed-off-by: Jiahao XU --- src/parallel/job_token/mod.rs | 28 ++++++++++++++++++++++++++-- src/parallel/job_token/unix.rs | 23 +++++------------------ src/parallel/job_token/windows.rs | 16 ++++------------ 3 files changed, 35 insertions(+), 32 deletions(-) diff --git a/src/parallel/job_token/mod.rs b/src/parallel/job_token/mod.rs index 475b8f139..fda097da9 100644 --- a/src/parallel/job_token/mod.rs +++ b/src/parallel/job_token/mod.rs @@ -88,9 +88,33 @@ mod inherited_jobserver { .or_else(|| var_os("MAKEFLAGS")) .or_else(|| var_os("MFLAGS"))?; - let inner = sys::JobServerClient::open(var)?; + #[cfg(unix)] + let var = std::os::unix::ffi::OsStrExt::as_bytes(var.as_os_str()); + #[cfg(not(unix))] + let var = var.to_str()?.as_bytes(); - Some(Self { + let makeflags = var.split(u8::is_ascii_whitespace); + + // `--jobserver-auth=` is the only documented makeflags. + // `--jobserver-fds=` is actually an internal only makeflags, so we should + // always prefer `--jobserver-auth=`. + // + // Also, according to doc of makeflags, if there are multiple `--jobserver-auth=` + // the last one is used + if let Some(flag) = makeflags + .clone() + .filter_map(|s| s.strip_prefix(b"--jobserver-auth=")) + .last() + { + sys::JobServerClient::open(flag) + } else { + sys::JobServerClient::open( + makeflags + .filter_map(|s| s.strip_prefix(b"--jobserver-fds=")) + .last()?, + ) + } + .map(|inner| Self { inner, global_implicit_token: AtomicBool::new(true), }) diff --git a/src/parallel/job_token/unix.rs b/src/parallel/job_token/unix.rs index af626c24d..37f37e9d0 100644 --- a/src/parallel/job_token/unix.rs +++ b/src/parallel/job_token/unix.rs @@ -1,14 +1,11 @@ use std::{ - ffi::{OsStr, OsString}, + ffi::OsStr, fs::{self, File}, io::{self, Read, Write}, mem::ManuallyDrop, os::{ raw::c_int, - unix::{ - ffi::{OsStrExt, OsStringExt}, - prelude::*, - }, + unix::{ffi::OsStrExt, prelude::*}, }, path::Path, }; @@ -19,21 +16,11 @@ pub(super) struct JobServerClient { } impl JobServerClient { - pub(super) unsafe fn open(var: OsString) -> Option { - let bytes = var.into_vec(); - - let s = bytes - .split(u8::is_ascii_whitespace) - .filter_map(|arg| { - arg.strip_prefix(b"--jobserver-fds=") - .or_else(|| arg.strip_prefix(b"--jobserver-auth=")) - }) - .find(|bytes| !bytes.is_empty())?; - - if let Some(fifo) = s.strip_prefix(b"fifo:") { + pub(super) unsafe fn open(var: &[u8]) -> Option { + if let Some(fifo) = var.strip_prefix(b"fifo:") { Self::from_fifo(Path::new(OsStr::from_bytes(fifo))) } else { - Self::from_pipe(OsStr::from_bytes(s).to_str()?) + Self::from_pipe(OsStr::from_bytes(var).to_str()?) } } diff --git a/src/parallel/job_token/windows.rs b/src/parallel/job_token/windows.rs index b85b1a27d..434fe169e 100644 --- a/src/parallel/job_token/windows.rs +++ b/src/parallel/job_token/windows.rs @@ -1,7 +1,4 @@ -use std::{ - ffi::{CString, OsString}, - io, ptr, -}; +use std::{ffi::CString, io, ptr, str}; use crate::windows::windows_sys::{ OpenSemaphoreA, ReleaseSemaphore, WaitForSingleObject, FALSE, HANDLE, SEMAPHORE_MODIFY_STATE, @@ -16,8 +13,8 @@ unsafe impl Sync for JobServerClient {} unsafe impl Send for JobServerClient {} impl JobServerClient { - pub(super) unsafe fn open(var: OsString) -> Option { - let var = var.to_str()?; + pub(super) unsafe fn open(var: &[u8]) -> Option { + let var = str::from_utf8(var).ok()?; if !var.is_ascii() { // `OpenSemaphoreA` only accepts ASCII, not utf-8. // @@ -29,12 +26,7 @@ impl JobServerClient { return None; } - let s = var - .split_ascii_whitespace() - .filter_map(|arg| arg.strip_prefix("--jobserver-auth=")) - .find(|s| !s.is_empty())?; - - let name = CString::new(s).ok()?; + let name = CString::new(var).ok()?; let sem = OpenSemaphoreA( THREAD_SYNCHRONIZE | SEMAPHORE_MODIFY_STATE,