Fix formatting and add an integration test that validates that the Re…

…adOnlyStorage wrapper preempts the underlying storage for write scenarios

src/cache/mod.rs

@@ -25,12 +25,12 @@ pub mod gha;
 pub mod memcached;
 #[cfg(feature = "oss")]
 pub mod oss;
+pub mod readonly;
 #[cfg(feature = "redis")]
 pub mod redis;
 #[cfg(feature = "s3")]
 pub mod s3;
 #[cfg(feature = "webdav")]
 pub mod webdav;
-pub mod readonly;
 
 pub use crate::cache::cache::*;

src/cache/readonly.rs

@@ -1,5 +1,3 @@
-// Copyright 2016 Mozilla Foundation
-//
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
@@ -26,19 +24,16 @@ use super::PreprocessorCacheModeConfig;
 pub struct ReadOnlyStorage(pub Arc<dyn Storage>);
 
 #[async_trait]
-impl Storage for ReadOnlyStorage
-{
-    async fn get(&self, key: &str) -> Result<Cache>
-    {
+impl Storage for ReadOnlyStorage {
+    async fn get(&self, key: &str) -> Result<Cache> {
         self.0.get(key).await
     }
 
     /// Put `entry` in the cache under `key`.
     ///
     /// Returns a `Future` that will provide the result or error when the put is
     /// finished.
-    async fn put(&self, _key: &str, _entry: CacheWrite) -> Result<Duration>
-    {
+    async fn put(&self, _key: &str, _entry: CacheWrite) -> Result<Duration> {
         Err(anyhow!("Cannot write to read-only storage"))
     }
 
@@ -50,20 +45,17 @@ impl Storage for ReadOnlyStorage
     }
 
     /// Get the storage location.
-    fn location(&self) -> String
-    {
+    fn location(&self) -> String {
         self.0.location()
     }
 
     /// Get the current storage usage, if applicable.
-    async fn current_size(&self) -> Result<Option<u64>>
-    {
+    async fn current_size(&self) -> Result<Option<u64>> {
         self.0.current_size().await
     }
 
     /// Get the maximum storage size, if applicable.
-    async fn max_size(&self) -> Result<Option<u64>>
-    {
+    async fn max_size(&self) -> Result<Option<u64>> {
         self.0.max_size().await
     }
 
@@ -104,16 +96,29 @@ mod test {
     #[test]
     fn readonly_storage_is_readonly() {
         let storage = ReadOnlyStorage(Arc::new(MockStorage::new(None, false)));
-        assert_eq!(storage.check().now_or_never().unwrap().unwrap(), CacheMode::ReadOnly);
+        assert_eq!(
+            storage.check().now_or_never().unwrap().unwrap(),
+            CacheMode::ReadOnly
+        );
     }
 
     #[test]
     fn readonly_storage_forwards_preprocessor_cache_mode_config() {
-        let storage_no_preprocessor_cache = ReadOnlyStorage(Arc::new(MockStorage::new(None, false)));
-        assert!(!storage_no_preprocessor_cache.preprocessor_cache_mode_config().use_preprocessor_cache_mode);
-
-        let storage_with_preprocessor_cache = ReadOnlyStorage(Arc::new(MockStorage::new(None, true)));
-        assert!(storage_with_preprocessor_cache.preprocessor_cache_mode_config().use_preprocessor_cache_mode);
+        let storage_no_preprocessor_cache =
+            ReadOnlyStorage(Arc::new(MockStorage::new(None, false)));
+        assert!(
+            !storage_no_preprocessor_cache
+                .preprocessor_cache_mode_config()
+                .use_preprocessor_cache_mode
+        );
+
+        let storage_with_preprocessor_cache =
+            ReadOnlyStorage(Arc::new(MockStorage::new(None, true)));
+        assert!(
+            storage_with_preprocessor_cache
+                .preprocessor_cache_mode_config()
+                .use_preprocessor_cache_mode
+        );
     }
 
     #[test]
@@ -143,4 +148,4 @@ mod test {
             );
         });
     }
-}
+}

src/server.rs

@@ -13,7 +13,7 @@
 // limitations under the License.SCCACHE_MAX_FRAME_LENGTH
 
 use crate::cache::readonly::ReadOnlyStorage;
-use crate::cache::{storage_from_config, Storage, CacheMode};
+use crate::cache::{storage_from_config, CacheMode, Storage};
 use crate::compiler::{
     get_compiler_info, CacheControl, CompileResult, Compiler, CompilerArguments, CompilerHasher,
     CompilerKind, CompilerProxy, DistType, Language, MissType,
@@ -464,7 +464,7 @@ pub fn start_server(config: &Config, port: u16) -> Result<()> {
 
     let storage = match cache_mode {
         CacheMode::ReadOnly => Arc::new(ReadOnlyStorage(raw_storage)),
-        _ => raw_storage
+        _ => raw_storage,
     };
 
     let res =

tests/sccache_cargo.rs

@@ -440,3 +440,64 @@ fn test_rust_cargo_env_dep(test_info: SccacheTest) -> Result<()> {
     drop(test_info);
     Ok(())
 }
+
+/// Test that building a simple Rust crate with cargo using sccache in read-only mode with an empty cache results in
+/// a cache miss that is produced by the readonly storage wrapper (and does not attempt to write to the underlying cache).
+#[test]
+#[serial]
+fn test_rust_cargo_cmd_readonly_preemtive_block() -> Result<()> {
+    let test_info = SccacheTest::new(None)?;
+    // `cargo clean` first, just to be sure there's no leftover build objects.
+    cargo_clean(&test_info)?;
+
+    let sccache_log = test_info.tempdir.path().join("sccache.log");
+
+    stop_sccache()?;
+
+    restart_sccache(
+        &test_info,
+        Some(vec![
+            ("SCCACHE_LOCAL_RW_MODE".into(), "READ_ONLY".into()),
+            ("SCCACHE_LOG".into(), "trace".into()),
+            (
+                "SCCACHE_ERROR_LOG".into(),
+                sccache_log.to_str().unwrap().into(),
+            ),
+        ]),
+    )?;
+
+    // Now build the crate with cargo.
+    // Assert that our cache miss is due to the readonly storage wrapper, not due to the underlying disk cache.
+    Command::new(CARGO.as_os_str())
+        .args(["build", "--color=never"])
+        .envs(test_info.env.iter().cloned())
+        .current_dir(CRATE_DIR.as_os_str())
+        .assert()
+        .try_stderr(predicates::str::contains("\x1b[").from_utf8().not())?
+        .try_success()?;
+
+    let log_contents = fs::read_to_string(sccache_log)?;
+    assert!(predicates::str::contains("server has setup with ReadOnly").eval(log_contents.as_str()));
+    assert!(predicates::str::contains(
+        "Error executing cache write: Cannot write to read-only storage"
+    )
+    .eval(log_contents.as_str()));
+    assert!(predicates::str::contains("DiskCache::finish_put")
+        .not()
+        .eval(log_contents.as_str()));
+
+    // Stats reset on server restart, so this needs to be run for each build.
+    test_info
+        .show_stats()?
+        .try_stdout(
+            predicates::str::contains(r#""cache_hits":{"counts":{},"adv_counts":{}}"#).from_utf8(),
+        )?
+        .try_stdout(
+            predicates::str::contains(
+                r#""cache_misses":{"counts":{"Rust":2},"adv_counts":{"rust":2}}"#,
+            )
+            .from_utf8(),
+        )?
+        .try_success()?;
+    Ok(())
+}