Fall back to direct cache write if tempfile creation on the same fs fails

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};
@@ -143,22 +143,54 @@ impl CacheRead {
                 optional,
             } in objects
             {
+                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;
+                }
                 let dir = match path.parent() {
                     Some(d) => d,
                     None => bail!("Output file without a parent directory!"),
                 };
                 // 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,
                 }
@@ -169,6 +201,23 @@ impl CacheRead {
     }
 }
 
+#[cfg(unix)]
+fn is_path_null(path: &Path) -> bool {
+    path == Path::new("/dev/null")
+}
+
+#[cfg(windows)]
+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.
 pub struct CacheWrite {
     zip: ZipWriter<Cursor<Vec<u8>>>,
@@ -268,3 +317,159 @@ 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 fd_path = PathBuf::from(format!("/dev/fd/{raw_fd}"));
+            let objects = vec![FileObjectSource {
+                key: "test_key".to_string(),
+                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(),
+                "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}");
+        });
+    }
+
+    #[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() {
+        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");
+    }
+}

tests/system.rs

@@ -195,6 +195,12 @@ 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";
+#[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.
 fn copy_to_tempdir(inputs: &[&str], tempdir: &Path) {
@@ -265,6 +271,110 @@ fn test_basic_compile(compiler: Compiler, tempdir: &Path) {
     });
 }
 
+fn test_basic_compile_into_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, NULL_PATH, 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, NULL_PATH, 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) {
+    info!("Not unix, skipping tests with /dev/stdout");
+}
+
 fn test_noncacheable_stats(compiler: Compiler, tempdir: &Path) {
     let Compiler {
         name,
@@ -629,6 +739,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_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" {