From 8590e673f788136664cd67497f8ea6c192c20a04 Mon Sep 17 00:00:00 2001 From: Zhang Maiyun Date: Thu, 20 Mar 2025 15:53:57 -0400 Subject: [PATCH 01/11] Skip cache output to /dev/null Signed-off-by: Zhang Maiyun --- src/cache/cache_io.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/cache/cache_io.rs b/src/cache/cache_io.rs index c4c2fa1347..9cd006453e 100644 --- a/src/cache/cache_io.rs +++ b/src/cache/cache_io.rs @@ -143,6 +143,10 @@ impl CacheRead { optional, } in objects { + if path == Path::new("/dev/null") { + debug!("Skipping output to /dev/null"); + continue; + } let dir = match path.parent() { Some(d) => d, None => bail!("Output file without a parent directory!"), From 34b2c1bb0387c9706355d061d042692722fb12fa Mon Sep 17 00:00:00 2001 From: Zhang Maiyun Date: Thu, 20 Mar 2025 15:55:16 -0400 Subject: [PATCH 02/11] Fall back to direct write if tmp creation fails Fixes #2288 The new logic tries to write to a temp file and atomically move it as before, but if a temp file cannot be created, it falls back to writing directly to the final location. In the latter case, we also ignore errors from `set_file_mode`. Signed-off-by: Zhang Maiyun --- src/cache/cache_io.rs | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/src/cache/cache_io.rs b/src/cache/cache_io.rs index 9cd006453e..02eb4c14c8 100644 --- a/src/cache/cache_io.rs +++ b/src/cache/cache_io.rs @@ -15,7 +15,7 @@ use crate::errors::*; use fs_err as fs; use std::fmt; use std::io::{Cursor, Read, Seek, Write}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use tempfile::NamedTempFile; use zip::write::FileOptions; use zip::{CompressionMethod, ZipArchive, ZipWriter}; @@ -154,15 +154,35 @@ impl CacheRead { // Write the cache entry to a tempfile and then atomically // move it to its final location so that other rustc invocations // happening in parallel don't see a partially-written file. - let mut tmp = NamedTempFile::new_in(dir)?; - match (self.get_object(&key, &mut tmp), optional) { - (Ok(mode), _) => { - tmp.persist(&path)?; + match (NamedTempFile::new_in(dir), optional) { + (Ok(mut tmp), _) => { + match (self.get_object(&key, &mut tmp), optional) { + (Ok(mode), _) => { + tmp.persist(&path)?; + if let Some(mode) = mode { + set_file_mode(path.as_path(), mode)?; + } + } + (Err(e), false) => return Err(e), + // skip if no object found and it's optional + (Err(_), true) => continue, + } + } + (Err(e), false) => { + // Fall back to writing directly to the final location + warn!("Failed to create temp file on the same file system: {e}"); + let mut f = std::fs::File::create(&path)?; + // `optional`` is false in this branch, so do not ignore errors + let mode = self.get_object(&key, &mut f)?; if let Some(mode) = mode { - set_file_mode(&path, mode)?; + if let Err(e) = set_file_mode(path.as_path(), mode) { + // Here we ignore errors from setting file mode because + // if we could not create a temp file in the same directory, + // we probably can't set the mode either (e.g. /dev/stuff) + warn!("Failed to reset file mode: {e}"); + } } } - (Err(e), false) => return Err(e), // skip if no object found and it's optional (Err(_), true) => continue, } From 032282c1d83542805f86361ad58bbe232c10218f Mon Sep 17 00:00:00 2001 From: Zhang Maiyun Date: Thu, 10 Apr 2025 05:54:34 -0700 Subject: [PATCH 03/11] Fix typo Co-authored-by: Alex Overchenko --- src/cache/cache_io.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cache/cache_io.rs b/src/cache/cache_io.rs index 02eb4c14c8..294519b6db 100644 --- a/src/cache/cache_io.rs +++ b/src/cache/cache_io.rs @@ -172,7 +172,7 @@ impl CacheRead { // Fall back to writing directly to the final location warn!("Failed to create temp file on the same file system: {e}"); let mut f = std::fs::File::create(&path)?; - // `optional`` is false in this branch, so do not ignore errors + // `optional` is false in this branch, so do not ignore errors let mode = self.get_object(&key, &mut f)?; if let Some(mode) = mode { if let Err(e) = set_file_mode(path.as_path(), mode) { From eb34be447c5a836ff6b88b62dc19993883452341 Mon Sep 17 00:00:00 2001 From: Zhang Maiyun Date: Mon, 26 May 2025 07:55:35 -0700 Subject: [PATCH 04/11] Test for cache extraction to /dev/(null|fd) --- src/cache/cache_io.rs | 68 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/src/cache/cache_io.rs b/src/cache/cache_io.rs index 294519b6db..6cb3e97394 100644 --- a/src/cache/cache_io.rs +++ b/src/cache/cache_io.rs @@ -292,3 +292,71 @@ impl Default for CacheWrite { Self::new() } } + +#[cfg(test)] +mod tests { + use super::*; + + #[cfg(unix)] + #[test] + fn test_extract_object_to_devnull_works() { + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_all() + .worker_threads(1) + .build() + .unwrap(); + + let pool = runtime.handle(); + + let cache_data = CacheWrite::new(); + let cache_read = + CacheRead::from(std::io::Cursor::new(cache_data.finish().unwrap())).unwrap(); + + let objects = vec![FileObjectSource { + key: "test_key".to_string(), + path: PathBuf::from("/dev/null"), + optional: false, + }]; + + let result = runtime.block_on(cache_read.extract_objects(objects, pool)); + assert!(result.is_ok(), "Extracting to /dev/null should succeed"); + } + + #[cfg(unix)] + #[test] + fn test_extract_object_to_dev_fd_something() { + // Open a pipe, write to `/dev/fd/{fd}` and check the other end that the correct data was written. + use std::os::fd::AsRawFd; + use tokio::io::AsyncReadExt; + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_all() + .worker_threads(1) + .build() + .unwrap(); + let pool = runtime.handle(); + let mut cache_data = CacheWrite::new(); + let data = b"test data"; + cache_data.put_bytes("test_key", data).unwrap(); + let cache_read = + CacheRead::from(std::io::Cursor::new(cache_data.finish().unwrap())).unwrap(); + runtime.block_on(async { + let (sender, mut receiver) = tokio::net::unix::pipe::pipe().unwrap(); + let sender_fd = sender.into_blocking_fd().unwrap(); + let raw_fd = sender_fd.as_raw_fd(); + let objects = vec![FileObjectSource { + key: "test_key".to_string(), + path: PathBuf::from(format!("/dev/fd/{raw_fd}")), + optional: false, + }]; + let result = cache_read.extract_objects(objects, pool).await; + assert!( + result.is_ok(), + "Extracting to /dev/fd/{raw_fd} should succeed" + ); + let mut buf = vec![0; data.len()]; + let n = receiver.read_exact(&mut buf).await.unwrap(); + assert_eq!(n, data.len(), "Read the correct number of bytes"); + assert_eq!(buf, data, "Read the correct data from /dev/fd/{raw_fd}"); + }); + } +} From 17cd493747610d982ea44f21f6d84d8b76e5f7a3 Mon Sep 17 00:00:00 2001 From: Zhang Maiyun Date: Sun, 15 Mar 2026 10:55:43 -0400 Subject: [PATCH 05/11] Skip output to `NUL` on Windows targets --- src/cache/cache_io.rs | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/cache/cache_io.rs b/src/cache/cache_io.rs index 6cb3e97394..16ac9c0455 100644 --- a/src/cache/cache_io.rs +++ b/src/cache/cache_io.rs @@ -143,10 +143,16 @@ impl CacheRead { optional, } in objects { + #[cfg(unix)] if path == Path::new("/dev/null") { debug!("Skipping output to /dev/null"); continue; } + #[cfg(windows)] + if path == Path::new("NUL") { + debug!("Skipping output to NUL"); + continue; + } let dir = match path.parent() { Some(d) => d, None => bail!("Output file without a parent directory!"), @@ -359,4 +365,28 @@ mod tests { assert_eq!(buf, data, "Read the correct data from /dev/fd/{raw_fd}"); }); } + #[cfg(windows)] + #[test] + fn test_extract_object_to_nul_works() { + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_all() + .worker_threads(1) + .build() + .unwrap(); + + let pool = runtime.handle(); + + let cache_data = CacheWrite::new(); + let cache_read = + CacheRead::from(std::io::Cursor::new(cache_data.finish().unwrap())).unwrap(); + + let objects = vec![FileObjectSource { + key: "test_key".to_string(), + path: PathBuf::from("NUL"), + optional: false, + }]; + + let result = runtime.block_on(cache_read.extract_objects(objects, pool)); + assert!(result.is_ok(), "Extracting to NUL should succeed"); + } } From 01fdde4faa33d9fe82010d5e746a348704a8578c Mon Sep 17 00:00:00 2001 From: Zhang Maiyun Date: Sun, 15 Mar 2026 12:09:31 -0400 Subject: [PATCH 06/11] Portable unit test: create ro dir and write there In addition, when `/dev/fd/{fd}` doesn't actually exist (e.g. FreeBSD without `fdescfs`), skip the test to extract to `/dev/fd/{fd}`. Signed-off-by: Zhang Maiyun --- src/cache/cache_io.rs | 66 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-) diff --git a/src/cache/cache_io.rs b/src/cache/cache_io.rs index 16ac9c0455..202655f0b1 100644 --- a/src/cache/cache_io.rs +++ b/src/cache/cache_io.rs @@ -349,11 +349,17 @@ mod tests { let (sender, mut receiver) = tokio::net::unix::pipe::pipe().unwrap(); let sender_fd = sender.into_blocking_fd().unwrap(); let raw_fd = sender_fd.as_raw_fd(); + let fd_path = PathBuf::from(format!("/dev/fd/{raw_fd}")); let objects = vec![FileObjectSource { key: "test_key".to_string(), - path: PathBuf::from(format!("/dev/fd/{raw_fd}")), + path: fd_path.clone(), optional: false, }]; + // On FreeBSD, `/dev/fd/{fd}` does not always exist (i.e. without mounting `fdescfs`), so we skip this test if we get `ENOENT`. + if ! fd_path.exists() { + info!("Skipping test_extract_object_to_dev_fd_something because /dev/fd/{raw_fd} does not exist"); + return; + } let result = cache_read.extract_objects(objects, pool).await; assert!( result.is_ok(), @@ -365,6 +371,64 @@ mod tests { assert_eq!(buf, data, "Read the correct data from /dev/fd/{raw_fd}"); }); } + + #[test] + fn test_extract_object_to_non_writable_path() { + // See `test_extract_object_to_dev_fd_something`: we still cannot cover all platforms by the other tests. Here we test a more portable case of creating a file and making its parent directory non-writable, in which case we should still be able to extract the object successfully. + + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_all() + .worker_threads(1) + .build() + .unwrap(); + + let pool = runtime.handle(); + + let mut cache_data = CacheWrite::new(); + cache_data.put_bytes("test_key", b"real_test_data").unwrap(); + let cache_read = + CacheRead::from(std::io::Cursor::new(cache_data.finish().unwrap())).unwrap(); + + let tmpdir = tempfile::tempdir().unwrap(); + let target_path = tmpdir.path().join("test_file"); + std::fs::write(&target_path, b"test").unwrap(); + // The current Rust fs permissions API is kind of awkward... + let mut perm = tmpdir.path().metadata().unwrap().permissions(); + perm.set_readonly(true); + std::fs::set_permissions(tmpdir.path(), perm.clone()).unwrap(); + // Note that this doesn't guarantee that the a new file cannot be created anymore. + // For example, as documented in `std::fs::Permissions::set_readonly`, the + // `FILE_ATTRIBUTE_READONLY` attribute on Windows is entirely ignored for directories. + // std::fs::File::create(tmpdir.path().join("another_file")).unwrap_err(); + + let objects = vec![FileObjectSource { + key: "test_key".to_string(), + path: target_path.clone(), + optional: false, + }]; + + let result = runtime.block_on(cache_read.extract_objects(objects, pool)); + assert!( + result.is_ok(), + "Extracting to the target path should succeed" + ); + // Test the content; make sure the old content is overwritten + let content = std::fs::read(&target_path).unwrap(); + assert_eq!( + content, b"real_test_data", + "Extracted content should be correct" + ); + + // `tempfile` needs us to reset permissions for cleanup to work + #[allow( + clippy::permissions_set_readonly_false, + reason = "The affected directory is immediately deleted with no security implications" + )] + perm.set_readonly(false); + std::fs::set_permissions(tmpdir.path(), perm).unwrap(); + tmpdir.close().unwrap(); + } + #[cfg(windows)] #[test] fn test_extract_object_to_nul_works() { From a8f6318781f29173ce3ffbae1c8e9a0edeaac0ad Mon Sep 17 00:00:00 2001 From: Zhang Maiyun Date: Sun, 15 Jun 2025 20:06:27 -0700 Subject: [PATCH 07/11] High-level tests into /dev/(null|stdout) --- tests/system.rs | 108 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/tests/system.rs b/tests/system.rs index 2a2207f6ba..f3f19823e1 100644 --- a/tests/system.rs +++ b/tests/system.rs @@ -195,6 +195,8 @@ const INPUT_FOR_HIP_A: &str = "test_a.hip"; const INPUT_FOR_HIP_B: &str = "test_b.hip"; const INPUT_FOR_HIP_C: &str = "test_c.hip"; const OUTPUT: &str = "test.o"; +const DEV_NULL: &str = "/dev/null"; +const DEV_STDOUT: &str = "/dev/stdout"; // Copy the source files into the tempdir so we can compile with relative paths, since the commandline winds up in the hash key. fn copy_to_tempdir(inputs: &[&str], tempdir: &Path) { @@ -265,6 +267,110 @@ fn test_basic_compile(compiler: Compiler, tempdir: &Path) { }); } +fn test_basic_compile_into_dev_null(compiler: Compiler, tempdir: &Path) { + let Compiler { + name, + exe, + env_vars, + } = compiler; + println!("test_basic_compile_into_dev_null: {}", name); + zero_stats(); + // Compile a source file. + copy_to_tempdir(&[INPUT, INPUT_ERR], tempdir); + + trace!("compile"); + sccache_command() + .args(compile_cmdline(name, &exe, INPUT, DEV_NULL, Vec::new())) + .current_dir(tempdir) + .envs(env_vars.clone()) + .assert() + .success(); + trace!("request stats"); + get_stats(|info| { + assert_eq!(1, info.stats.compile_requests); + assert_eq!(1, info.stats.requests_executed); + assert_eq!(1, info.stats.cache_hits.all()); + assert_eq!(0, info.stats.cache_misses.all()); + assert!(info.stats.cache_misses.get("C/C++").is_none()); + let adv_key = adv_key_kind("c", compiler.name); + assert!(info.stats.cache_misses.get_adv(&adv_key).is_none()); + }); + trace!("compile"); + sccache_command() + .args(compile_cmdline(name, &exe, INPUT, DEV_NULL, Vec::new())) + .current_dir(tempdir) + .envs(env_vars) + .assert() + .success(); + trace!("request stats"); + get_stats(|info| { + assert_eq!(2, info.stats.compile_requests); + assert_eq!(2, info.stats.requests_executed); + assert_eq!(2, info.stats.cache_hits.all()); + assert_eq!(0, info.stats.cache_misses.all()); + assert_eq!(&2, info.stats.cache_hits.get("C/C++").unwrap()); + assert!(info.stats.cache_misses.get("C/C++").is_none()); + let adv_key = adv_key_kind("c", compiler.name); + assert_eq!(&2, info.stats.cache_hits.get_adv(&adv_key).unwrap()); + assert!(info.stats.cache_misses.get_adv(&adv_key).is_none()); + }); +} + +#[cfg(unix)] +fn test_basic_compile_into_dev_stdout(compiler: Compiler, tempdir: &Path) { + let Compiler { + name, + exe, + env_vars, + } = compiler; + println!("test_basic_compile_into_dev_stdout: {}", name); + zero_stats(); + // Compile a source file. + copy_to_tempdir(&[INPUT, INPUT_ERR], tempdir); + + trace!("compile"); + sccache_command() + .args(compile_cmdline(name, &exe, INPUT, DEV_STDOUT, Vec::new())) + .current_dir(tempdir) + .envs(env_vars.clone()) + .assert() + .success(); + trace!("request stats"); + get_stats(|info| { + assert_eq!(1, info.stats.compile_requests); + assert_eq!(1, info.stats.requests_executed); + assert_eq!(1, info.stats.cache_hits.all()); + assert_eq!(0, info.stats.cache_misses.all()); + assert!(info.stats.cache_misses.get("C/C++").is_none()); + let adv_key = adv_key_kind("c", compiler.name); + assert!(info.stats.cache_misses.get_adv(&adv_key).is_none()); + }); + trace!("compile"); + sccache_command() + .args(compile_cmdline(name, &exe, INPUT, DEV_STDOUT, Vec::new())) + .current_dir(tempdir) + .envs(env_vars) + .assert() + .success(); + trace!("request stats"); + get_stats(|info| { + assert_eq!(2, info.stats.compile_requests); + assert_eq!(2, info.stats.requests_executed); + assert_eq!(2, info.stats.cache_hits.all()); + assert_eq!(0, info.stats.cache_misses.all()); + assert_eq!(&2, info.stats.cache_hits.get("C/C++").unwrap()); + assert!(info.stats.cache_misses.get("C/C++").is_none()); + let adv_key = adv_key_kind("c", compiler.name); + assert_eq!(&2, info.stats.cache_hits.get_adv(&adv_key).unwrap()); + assert!(info.stats.cache_misses.get_adv(&adv_key).is_none()); + }); +} + +#[cfg(not(unix))] +fn test_basic_compile_into_dev_stdout(_: Compiler, _: &Path) { + warn!("Not unix, skipping tests with /dev/stdout"); +} + fn test_noncacheable_stats(compiler: Compiler, tempdir: &Path) { let Compiler { name, @@ -629,6 +735,8 @@ fn test_gcc_clang_depfile(compiler: Compiler, tempdir: &Path) { fn run_sccache_command_tests(compiler: Compiler, tempdir: &Path, preprocessor_cache_mode: bool) { if compiler.name != "clang++" { test_basic_compile(compiler.clone(), tempdir); + test_basic_compile_into_dev_null(compiler.clone(), tempdir); + test_basic_compile_into_dev_stdout(compiler.clone(), tempdir); } test_compile_with_define(compiler.clone(), tempdir); if compiler.name == "cl.exe" { From b8e40b0d6492e9dcc816387177fe1b6334b678bd Mon Sep 17 00:00:00 2001 From: Zhang Maiyun Date: Sat, 25 Apr 2026 15:43:11 -0400 Subject: [PATCH 08/11] Change warn to info in test --- tests/system.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/system.rs b/tests/system.rs index f3f19823e1..223d8c329b 100644 --- a/tests/system.rs +++ b/tests/system.rs @@ -368,7 +368,7 @@ fn test_basic_compile_into_dev_stdout(compiler: Compiler, tempdir: &Path) { #[cfg(not(unix))] fn test_basic_compile_into_dev_stdout(_: Compiler, _: &Path) { - warn!("Not unix, skipping tests with /dev/stdout"); + info!("Not unix, skipping tests with /dev/stdout"); } fn test_noncacheable_stats(compiler: Compiler, tempdir: &Path) { From 91b5128b8d2b008ddbb367fa457f0ce71997557f Mon Sep 17 00:00:00 2001 From: Zhang Maiyun Date: Sat, 25 Apr 2026 16:09:32 -0400 Subject: [PATCH 09/11] Extract null path from logic --- src/cache/cache_io.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/cache/cache_io.rs b/src/cache/cache_io.rs index 202655f0b1..42bc3f4ba6 100644 --- a/src/cache/cache_io.rs +++ b/src/cache/cache_io.rs @@ -143,14 +143,8 @@ impl CacheRead { optional, } in objects { - #[cfg(unix)] - if path == Path::new("/dev/null") { - debug!("Skipping output to /dev/null"); - continue; - } - #[cfg(windows)] - if path == Path::new("NUL") { - debug!("Skipping output to NUL"); + if path == null_path() { + debug!("Skipping output to {}", path.display()); continue; } let dir = match path.parent() { @@ -199,6 +193,15 @@ impl CacheRead { } } +#[cfg(unix)] +fn null_path() -> &'static Path { + Path::new("/dev/null") +} +#[cfg(windows)] +fn null_path() -> &'static Path { + Path::new("NUL") +} + /// Data to be stored in the compiler cache. pub struct CacheWrite { zip: ZipWriter>>, From 85de72bef903faa7b8c7a76f14fadf00bc9a2067 Mon Sep 17 00:00:00 2001 From: Zhang Maiyun Date: Sat, 25 Apr 2026 16:25:10 -0400 Subject: [PATCH 10/11] Gate windows/unix null paths in integration tests --- tests/system.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/system.rs b/tests/system.rs index 223d8c329b..fbbaeca96b 100644 --- a/tests/system.rs +++ b/tests/system.rs @@ -195,7 +195,11 @@ const INPUT_FOR_HIP_A: &str = "test_a.hip"; const INPUT_FOR_HIP_B: &str = "test_b.hip"; const INPUT_FOR_HIP_C: &str = "test_c.hip"; const OUTPUT: &str = "test.o"; -const DEV_NULL: &str = "/dev/null"; +#[cfg(unix)] +const NULL_PATH: &str = "/dev/null"; +#[cfg(windows)] +const NULL_PATH: &str = "NUL"; +#[cfg(unix)] const DEV_STDOUT: &str = "/dev/stdout"; // Copy the source files into the tempdir so we can compile with relative paths, since the commandline winds up in the hash key. @@ -267,7 +271,7 @@ fn test_basic_compile(compiler: Compiler, tempdir: &Path) { }); } -fn test_basic_compile_into_dev_null(compiler: Compiler, tempdir: &Path) { +fn test_basic_compile_into_null(compiler: Compiler, tempdir: &Path) { let Compiler { name, exe, @@ -280,7 +284,7 @@ fn test_basic_compile_into_dev_null(compiler: Compiler, tempdir: &Path) { trace!("compile"); sccache_command() - .args(compile_cmdline(name, &exe, INPUT, DEV_NULL, Vec::new())) + .args(compile_cmdline(name, &exe, INPUT, NULL_PATH, Vec::new())) .current_dir(tempdir) .envs(env_vars.clone()) .assert() @@ -297,7 +301,7 @@ fn test_basic_compile_into_dev_null(compiler: Compiler, tempdir: &Path) { }); trace!("compile"); sccache_command() - .args(compile_cmdline(name, &exe, INPUT, DEV_NULL, Vec::new())) + .args(compile_cmdline(name, &exe, INPUT, NULL_PATH, Vec::new())) .current_dir(tempdir) .envs(env_vars) .assert() @@ -735,7 +739,7 @@ fn test_gcc_clang_depfile(compiler: Compiler, tempdir: &Path) { fn run_sccache_command_tests(compiler: Compiler, tempdir: &Path, preprocessor_cache_mode: bool) { if compiler.name != "clang++" { test_basic_compile(compiler.clone(), tempdir); - test_basic_compile_into_dev_null(compiler.clone(), tempdir); + test_basic_compile_into_null(compiler.clone(), tempdir); test_basic_compile_into_dev_stdout(compiler.clone(), tempdir); } test_compile_with_define(compiler.clone(), tempdir); From 3804ba54b2c8d9f28a285e541f923b73114325a0 Mon Sep 17 00:00:00 2001 From: Zhang Maiyun Date: Sat, 25 Apr 2026 17:00:30 -0400 Subject: [PATCH 11/11] Test and fix NUL handling on windows --- src/cache/cache_io.rs | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/src/cache/cache_io.rs b/src/cache/cache_io.rs index 42bc3f4ba6..9e29743e90 100644 --- a/src/cache/cache_io.rs +++ b/src/cache/cache_io.rs @@ -143,7 +143,15 @@ impl CacheRead { optional, } in objects { - if path == null_path() { + if is_path_null(&path) { + // For unix, this is just a fast path to discard such outputs, + // so it is not an issue if `is_path_null` has false-negatives. + // But for Windows, since `NUL` looks like a relative path, the + // temporary file creation logic would happily succeed, creating + // a temp file in the CWD, but then the subsequent `persist` + // would fail with `ERROR_ALREADY_EXISTS`, since `NUL` always + // exists and cannot be `replaced`, so we really need to + // short-circuit more of such cases on Windows. debug!("Skipping output to {}", path.display()); continue; } @@ -194,12 +202,20 @@ impl CacheRead { } #[cfg(unix)] -fn null_path() -> &'static Path { - Path::new("/dev/null") +fn is_path_null(path: &Path) -> bool { + path == Path::new("/dev/null") } + #[cfg(windows)] -fn null_path() -> &'static Path { - Path::new("NUL") +fn is_path_null(path: &Path) -> bool { + // For Windows, it appears that `NUL` with whatever extension is also a blackhole + // (at least for `CreateFileX`), so it does not suffice to check for an exact match + // Also note that gcc, cl.exe, et al. append a correct extension automatically even + // if the user asks for output to `NUL`. + let Some(stem) = path.file_stem() else { + return false; + }; + stem.eq_ignore_ascii_case("NUL") } /// Data to be stored in the compiler cache.