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 <[email protected]>
rust-lang · Feb 13, 2024 · 0aa1090 · 0aa1090
1 parent 0042c70
commit 0aa1090
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 32 deletions.
diff --git 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
@@ -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<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()?)
         }
     }
 

diff --git 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<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.
             //
@@ -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,