Skip to content

Commit

Permalink
Fix jobserver: Last --jobserver-auth wins (#949)
Browse files Browse the repository at this point in the history
  • Loading branch information
NobodyXu authored Feb 16, 2024
1 parent 2dcfa14 commit a891a57
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 32 deletions.
28 changes: 26 additions & 2 deletions src/parallel/job_token/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
})
Expand Down
23 changes: 5 additions & 18 deletions src/parallel/job_token/unix.rs
Original file line number Diff line number Diff line change
@@ -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,
};
Expand All @@ -19,21 +16,11 @@ pub(super) struct JobServerClient {
}

impl JobServerClient {
pub(super) unsafe fn open(var: OsString) -> Option<Self> {
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<Self> {
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()?)
}
}

Expand Down
16 changes: 4 additions & 12 deletions src/parallel/job_token/windows.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -16,8 +13,8 @@ unsafe impl Sync for JobServerClient {}
unsafe impl Send for JobServerClient {}

impl JobServerClient {
pub(super) unsafe fn open(var: OsString) -> Option<Self> {
let var = var.to_str()?;
pub(super) unsafe fn open(var: &[u8]) -> Option<Self> {
let var = str::from_utf8(var).ok()?;
if !var.is_ascii() {
// `OpenSemaphoreA` only accepts ASCII, not utf-8.
//
Expand All @@ -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,
Expand Down

0 comments on commit a891a57

Please sign in to comment.