From 081c4f2baa99d32e5050e8afbce9a5c81581ee60 Mon Sep 17 00:00:00 2001 From: uellenberg Date: Sat, 20 Jan 2024 01:06:07 -0800 Subject: [PATCH 01/11] Implement read-only local cache --- docs/Configuration.md | 1 + docs/Local.md | 4 +++ src/cache/cache.rs | 22 ++++++++----- src/cache/disk.rs | 19 ++++++++++- src/cache/gcs.rs | 10 ++++++ src/compiler/c.rs | 71 ++++++++++++++++++++++++---------------- src/compiler/compiler.rs | 6 ++++ src/config.rs | 41 ++++++++++++++++++----- src/test/tests.rs | 2 ++ 9 files changed, 129 insertions(+), 47 deletions(-) diff --git a/docs/Configuration.md b/docs/Configuration.md index 7f5869998..eae7e9de6 100644 --- a/docs/Configuration.md +++ b/docs/Configuration.md @@ -100,6 +100,7 @@ configuration variables * `SCCACHE_DIR` local on disk artifact cache directory * `SCCACHE_CACHE_SIZE` maximum size of the local on disk cache i.e. `2G` - default is 10G * `SCCACHE_DIRECT` enable/disable preprocessor caching (see [the local doc](Local.md)) +* `SCCACHE_LOCAL_RW_MODE` the mode that the cache will operate in (`READ_ONLY` or `READ_WRITE`) #### s3 compatible diff --git a/docs/Local.md b/docs/Local.md index 89525fbf0..6f8e4382b 100644 --- a/docs/Local.md +++ b/docs/Local.md @@ -43,3 +43,7 @@ Configuration options and their default values: See where to write the config in [the configuration doc](Configuration.md). *Note that preprocessor caching is currently only implemented for GCC and Clang and when using local storage.* + +## Read-only cache mode + +By default, the local cache operates in read/write mode. The `SCCACHE_LOCAL_RW_MODE` environment variable can be set to `READ_ONLY` (or `READ_WRITE`) to modify this behavior. \ No newline at end of file diff --git a/src/cache/cache.rs b/src/cache/cache.rs index db1bee7bb..065a588ae 100644 --- a/src/cache/cache.rs +++ b/src/cache/cache.rs @@ -16,7 +16,7 @@ use crate::cache::azure::AzureBlobCache; use crate::cache::disk::DiskCache; #[cfg(feature = "gcs")] -use crate::cache::gcs::{GCSCache, RWMode}; +use crate::cache::gcs::GCSCache; #[cfg(feature = "gha")] use crate::cache::gha::GHACache; #[cfg(feature = "memcached")] @@ -112,7 +112,7 @@ impl fmt::Debug for Cache { } } -/// CacheMode is used to repreent which mode we are using. +/// CacheMode is used to represent which mode we are using. #[derive(Debug)] pub enum CacheMode { /// Only read cache from storage. @@ -121,6 +121,15 @@ pub enum CacheMode { ReadWrite, } +impl From for CacheMode { + fn from(value: config::CacheRWMode) -> Self { + match value { + config::CacheRWMode::ReadOnly => CacheMode::ReadOnly, + config::CacheRWMode::ReadWrite => CacheMode::ReadWrite, + } + } +} + /// Trait objects can't be bounded by more than one non-builtin trait. pub trait ReadSeek: Read + Seek + Send {} @@ -575,17 +584,12 @@ pub fn storage_from_config( }) => { debug!("Init gcs cache with bucket {bucket}, key_prefix {key_prefix}"); - let gcs_read_write_mode = match rw_mode { - config::GCSCacheRWMode::ReadOnly => RWMode::ReadOnly, - config::GCSCacheRWMode::ReadWrite => RWMode::ReadWrite, - }; - let storage = GCSCache::build( bucket, key_prefix, cred_path.as_deref(), service_account.as_deref(), - gcs_read_write_mode, + (*rw_mode).into(), credential_url.as_deref(), ) .map_err(|err| anyhow!("create gcs cache failed: {err:?}"))?; @@ -660,12 +664,14 @@ pub fn storage_from_config( let (dir, size) = (&config.fallback_cache.dir, config.fallback_cache.size); let preprocessor_cache_mode_config = config.fallback_cache.preprocessor_cache_mode; + let rw_mode = config.fallback_cache.rw_mode; debug!("Init disk cache with dir {:?}, size {}", dir, size); Ok(Arc::new(DiskCache::new( dir, size, pool, preprocessor_cache_mode_config, + rw_mode, ))) } diff --git a/src/cache/disk.rs b/src/cache/disk.rs index 5f63e01ed..fcd95403f 100644 --- a/src/cache/disk.rs +++ b/src/cache/disk.rs @@ -12,8 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::cache::{Cache, CacheRead, CacheWrite, Storage}; +use crate::cache::{Cache, CacheMode, CacheRead, CacheWrite, Storage}; use crate::compiler::PreprocessorCacheEntry; +use crate::config::CacheRWMode; use crate::lru_disk_cache::LruDiskCache; use crate::lru_disk_cache::{Error as LruError, ReadSeek}; use async_trait::async_trait; @@ -65,6 +66,7 @@ pub struct DiskCache { pool: tokio::runtime::Handle, preprocessor_cache_mode_config: PreprocessorCacheModeConfig, preprocessor_cache: Arc>, + rw_mode: CacheRWMode, } impl DiskCache { @@ -74,6 +76,7 @@ impl DiskCache { max_size: u64, pool: &tokio::runtime::Handle, preprocessor_cache_mode_config: PreprocessorCacheModeConfig, + rw_mode: CacheRWMode, ) -> DiskCache { DiskCache { lru: Arc::new(Mutex::new(LazyDiskCache::Uninit { @@ -88,6 +91,7 @@ impl DiskCache { .into_os_string(), max_size, })), + rw_mode, } } } @@ -130,6 +134,11 @@ impl Storage for DiskCache { // We should probably do this on a background thread if we're going to buffer // everything in memory... trace!("DiskCache::finish_put({})", key); + + if self.rw_mode == CacheRWMode::ReadOnly { + return Err(anyhow!("Cannot write to a read-only cache1")); + } + let lru = self.lru.clone(); let key = make_key_path(key); @@ -143,6 +152,10 @@ impl Storage for DiskCache { .await? } + async fn check(&self) -> Result { + Ok(self.rw_mode.into()) + } + fn location(&self) -> String { format!("Local disk: {:?}", self.lru.lock().unwrap().path()) } @@ -171,6 +184,10 @@ impl Storage for DiskCache { key: &str, preprocessor_cache_entry: PreprocessorCacheEntry, ) -> Result<()> { + if self.rw_mode == CacheRWMode::ReadOnly { + return Err(anyhow!("Cannot write to a read-only cache2")); + } + let key = normalize_key(key); let mut buf = vec![]; preprocessor_cache_entry.serialize_to(&mut buf)?; diff --git a/src/cache/gcs.rs b/src/cache/gcs.rs index 8ec637567..d834540f4 100644 --- a/src/cache/gcs.rs +++ b/src/cache/gcs.rs @@ -13,6 +13,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use crate::config; use crate::errors::*; use opendal::Operator; use opendal::{layers::LoggingLayer, services::Gcs}; @@ -36,6 +37,15 @@ impl RWMode { } } +impl From for RWMode { + fn from(value: config::CacheRWMode) -> Self { + match value { + config::CacheRWMode::ReadOnly => RWMode::ReadOnly, + config::CacheRWMode::ReadWrite => RWMode::ReadWrite, + } + } +} + /// A cache that stores entries in Google Cloud Storage pub struct GCSCache; diff --git a/src/compiler/c.rs b/src/compiler/c.rs index 35d947715..769a32991 100644 --- a/src/compiler/c.rs +++ b/src/compiler/c.rs @@ -340,42 +340,51 @@ where let mut updated = false; let hit = preprocessor_cache_entry .lookup_result_digest(preprocessor_cache_mode_config, &mut updated); + + let mut update_failed = false; if updated { // Time macros have been found, we need to update // the preprocessor cache entry. See [`PreprocessorCacheEntry::result_matches`]. debug!( "Preprocessor cache updated because of time macros: {preprocessor_key}" ); - storage.put_preprocessor_cache_entry( + + if let Err(e) = storage.put_preprocessor_cache_entry( preprocessor_key, preprocessor_cache_entry, - )?; + ) { + debug!("Failed to update preprocessor cache: {}", e); + update_failed = true; + } } - if let Some(key) = hit { - debug!("Preprocessor cache hit: {preprocessor_key}"); - // A compiler binary may be a symlink to another and - // so has the same digest, but that means - // the toolchain will not contain the correct path - // to invoke the compiler! Add the compiler - // executable path to try and prevent this - let weak_toolchain_key = - format!("{}-{}", executable.to_string_lossy(), executable_digest); - return Ok(HashResult { - key, - compilation: Box::new(CCompilation { - parsed_args: parsed_args.to_owned(), - #[cfg(feature = "dist-client")] - // TODO or is it never relevant since dist? - preprocessed_input: vec![], - executable: executable.to_owned(), - compiler: compiler.to_owned(), - cwd: cwd.to_owned(), - env_vars: env_vars.to_owned(), - }), - weak_toolchain_key, - }); - } else { - debug!("Preprocessor cache miss: {preprocessor_key}"); + + if !update_failed { + if let Some(key) = hit { + debug!("Preprocessor cache hit: {preprocessor_key}"); + // A compiler binary may be a symlink to another and + // so has the same digest, but that means + // the toolchain will not contain the correct path + // to invoke the compiler! Add the compiler + // executable path to try and prevent this + let weak_toolchain_key = + format!("{}-{}", executable.to_string_lossy(), executable_digest); + return Ok(HashResult { + key, + compilation: Box::new(CCompilation { + parsed_args: parsed_args.to_owned(), + #[cfg(feature = "dist-client")] + // TODO or is it never relevant since dist? + preprocessed_input: vec![], + executable: executable.to_owned(), + compiler: compiler.to_owned(), + cwd: cwd.to_owned(), + env_vars: env_vars.to_owned(), + }), + weak_toolchain_key, + }); + } else { + debug!("Preprocessor cache miss: {preprocessor_key}"); + } } } } @@ -491,8 +500,12 @@ where .collect(); files.sort_unstable_by(|a, b| a.1.cmp(&b.1)); preprocessor_cache_entry.add_result(start_of_compilation, &key, files); - storage - .put_preprocessor_cache_entry(&preprocessor_key, preprocessor_cache_entry)?; + + if let Err(e) = storage + .put_preprocessor_cache_entry(&preprocessor_key, preprocessor_cache_entry) + { + debug!("Failed to update preprocessor cache: {}", e); + } } } diff --git a/src/compiler/compiler.rs b/src/compiler/compiler.rs index c12563a3c..c7deecc0c 100644 --- a/src/compiler/compiler.rs +++ b/src/compiler/compiler.rs @@ -1382,6 +1382,7 @@ mod test { use super::*; use crate::cache::disk::DiskCache; use crate::cache::{CacheRead, PreprocessorCacheModeConfig}; + use crate::config::CacheRWMode; use crate::mock_command::*; use crate::test::mock_storage::MockStorage; use crate::test::utils::*; @@ -1824,6 +1825,7 @@ LLVM version: 6.0", use_preprocessor_cache_mode: preprocessor_cache_mode, ..Default::default() }, + CacheRWMode::ReadWrite, ); // Write a dummy input file so the preprocessor cache mode can work std::fs::write(f.tempdir.path().join("foo.c"), "whatever").unwrap(); @@ -1949,6 +1951,7 @@ LLVM version: 6.0", use_preprocessor_cache_mode: preprocessor_cache_mode, ..Default::default() }, + CacheRWMode::ReadWrite, ); // Write a dummy input file so the preprocessor cache mode can work std::fs::write(f.tempdir.path().join("foo.c"), "whatever").unwrap(); @@ -2236,6 +2239,7 @@ LLVM version: 6.0", use_preprocessor_cache_mode: preprocessor_cache_mode, ..Default::default() }, + CacheRWMode::ReadWrite, ); let storage = Arc::new(storage); // Write a dummy input file so the preprocessor cache mode can work @@ -2361,6 +2365,7 @@ LLVM version: 6.0", use_preprocessor_cache_mode: preprocessor_cache_mode, ..Default::default() }, + CacheRWMode::ReadWrite, ); let storage = Arc::new(storage); // Pretend to be GCC. Also inject a fake object file that the subsequent @@ -2454,6 +2459,7 @@ LLVM version: 6.0", use_preprocessor_cache_mode: preprocessor_cache_mode, ..Default::default() }, + CacheRWMode::ReadWrite, ); let storage = Arc::new(storage); // Pretend to be GCC. diff --git a/src/config.rs b/src/config.rs index b39b5547b..10c6f993a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -162,6 +162,7 @@ pub struct DiskCacheConfig { // TODO: use deserialize_with to allow human-readable sizes in toml pub size: u64, pub preprocessor_cache_mode: PreprocessorCacheModeConfig, + pub rw_mode: CacheRWMode, } impl Default for DiskCacheConfig { @@ -170,13 +171,14 @@ impl Default for DiskCacheConfig { dir: default_disk_cache_dir(), size: default_disk_cache_size(), preprocessor_cache_mode: PreprocessorCacheModeConfig::activated(), + rw_mode: CacheRWMode::ReadWrite, } } } #[derive(Copy, Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] #[serde(deny_unknown_fields)] -pub enum GCSCacheRWMode { +pub enum CacheRWMode { #[serde(rename = "READ_ONLY")] ReadOnly, #[serde(rename = "READ_WRITE")] @@ -190,7 +192,7 @@ pub struct GCSCacheConfig { pub key_prefix: String, pub cred_path: Option, pub service_account: Option, - pub rw_mode: GCSCacheRWMode, + pub rw_mode: CacheRWMode, pub credential_url: Option, } @@ -628,17 +630,17 @@ fn config_from_env() -> Result { let service_account = env::var("SCCACHE_GCS_SERVICE_ACCOUNT").ok(); let rw_mode = match env::var("SCCACHE_GCS_RW_MODE").as_ref().map(String::as_str) { - Ok("READ_ONLY") => GCSCacheRWMode::ReadOnly, - Ok("READ_WRITE") => GCSCacheRWMode::ReadWrite, + Ok("READ_ONLY") => CacheRWMode::ReadOnly, + Ok("READ_WRITE") => CacheRWMode::ReadWrite, // TODO: unsure if these should warn during the configuration loading // or at the time when they're actually used to connect to GCS Ok(_) => { warn!("Invalid SCCACHE_GCS_RW_MODE-- defaulting to READ_ONLY."); - GCSCacheRWMode::ReadOnly + CacheRWMode::ReadOnly } _ => { warn!("No SCCACHE_GCS_RW_MODE specified-- defaulting to READ_ONLY."); - GCSCacheRWMode::ReadOnly + CacheRWMode::ReadOnly } }; @@ -739,12 +741,29 @@ fn config_from_env() -> Result { _ => false, }; - let any_overridden = disk_dir.is_some() || disk_sz.is_some() || preprocessor_mode_overridden; + let (disk_rw_mode, disk_rw_mode_overridden) = match env::var("SCCACHE_LOCAL_RW_MODE") + .as_ref() + .map(String::as_str) + { + Ok("READ_ONLY") => (CacheRWMode::ReadOnly, true), + Ok("READ_WRITE") => (CacheRWMode::ReadWrite, true), + Ok(_) => { + warn!("Invalid SCCACHE_LOCAL_RW_MODE-- defaulting to READ_WRITE."); + (CacheRWMode::ReadWrite, false) + } + _ => (CacheRWMode::ReadWrite, false), + }; + + let any_overridden = disk_dir.is_some() + || disk_sz.is_some() + || preprocessor_mode_overridden + || disk_rw_mode_overridden; let disk = if any_overridden { Some(DiskCacheConfig { dir: disk_dir.unwrap_or_else(default_disk_cache_dir), size: disk_sz.unwrap_or_else(default_disk_cache_size), preprocessor_cache_mode: preprocessor_mode_config, + rw_mode: disk_rw_mode, }) } else { None @@ -1088,6 +1107,7 @@ fn config_overrides() { dir: "/env-cache".into(), size: 5, preprocessor_cache_mode: Default::default(), + rw_mode: CacheRWMode::ReadWrite, }), redis: Some(RedisCacheConfig { url: "myotherredisurl".to_owned(), @@ -1103,6 +1123,7 @@ fn config_overrides() { dir: "/file-cache".into(), size: 15, preprocessor_cache_mode: Default::default(), + rw_mode: CacheRWMode::ReadWrite, }), memcached: Some(MemcachedCacheConfig { url: "memurl".to_owned(), @@ -1129,6 +1150,7 @@ fn config_overrides() { dir: "/env-cache".into(), size: 5, preprocessor_cache_mode: Default::default(), + rw_mode: CacheRWMode::ReadWrite, }, dist: Default::default(), server_startup_timeout: None, @@ -1234,7 +1256,7 @@ fn test_gcs_service_account() { }) => { assert_eq!(bucket, "my-bucket"); assert_eq!(service_account, Some("my@example.com".to_string())); - assert_eq!(rw_mode, GCSCacheRWMode::ReadWrite); + assert_eq!(rw_mode, CacheRWMode::ReadWrite); } None => unreachable!(), }; @@ -1317,12 +1339,13 @@ token = "webdavtoken" dir: PathBuf::from("/tmp/.cache/sccache"), size: 7 * 1024 * 1024 * 1024, preprocessor_cache_mode: PreprocessorCacheModeConfig::activated(), + rw_mode: CacheRWMode::ReadWrite, }), gcs: Some(GCSCacheConfig { bucket: "bucket".to_owned(), cred_path: Some("/psst/secret/cred".to_string()), service_account: Some("example_service_account".to_string()), - rw_mode: GCSCacheRWMode::ReadOnly, + rw_mode: CacheRWMode::ReadOnly, key_prefix: "prefix".into(), credential_url: None, }), diff --git a/src/test/tests.rs b/src/test/tests.rs index 54ad27f62..49d65f13a 100644 --- a/src/test/tests.rs +++ b/src/test/tests.rs @@ -16,6 +16,7 @@ use crate::cache::disk::DiskCache; use crate::cache::PreprocessorCacheModeConfig; use crate::client::connect_to_server; use crate::commands::{do_compile, request_shutdown, request_stats}; +use crate::config::CacheRWMode; use crate::jobserver::Client; use crate::mock_command::*; use crate::server::{DistClientContainer, SccacheServer, ServerMessage}; @@ -85,6 +86,7 @@ where cache_size, runtime.handle(), PreprocessorCacheModeConfig::default(), + CacheRWMode::ReadWrite, )); let client = unsafe { Client::new() }; From e910ff421f41c68df5f932f2ebe8fcd056fb4a43 Mon Sep 17 00:00:00 2001 From: uellenberg Date: Sat, 20 Jan 2024 02:58:50 -0800 Subject: [PATCH 02/11] Fix build errors with dist-server --- src/cache/cache.rs | 9 --------- src/cache/gcs.rs | 10 ---------- src/config.rs | 20 ++++++++++++++++++++ 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/cache/cache.rs b/src/cache/cache.rs index 065a588ae..2beabaed2 100644 --- a/src/cache/cache.rs +++ b/src/cache/cache.rs @@ -121,15 +121,6 @@ pub enum CacheMode { ReadWrite, } -impl From for CacheMode { - fn from(value: config::CacheRWMode) -> Self { - match value { - config::CacheRWMode::ReadOnly => CacheMode::ReadOnly, - config::CacheRWMode::ReadWrite => CacheMode::ReadWrite, - } - } -} - /// Trait objects can't be bounded by more than one non-builtin trait. pub trait ReadSeek: Read + Seek + Send {} diff --git a/src/cache/gcs.rs b/src/cache/gcs.rs index d834540f4..8ec637567 100644 --- a/src/cache/gcs.rs +++ b/src/cache/gcs.rs @@ -13,7 +13,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::config; use crate::errors::*; use opendal::Operator; use opendal::{layers::LoggingLayer, services::Gcs}; @@ -37,15 +36,6 @@ impl RWMode { } } -impl From for RWMode { - fn from(value: config::CacheRWMode) -> Self { - match value { - config::CacheRWMode::ReadOnly => RWMode::ReadOnly, - config::CacheRWMode::ReadWrite => RWMode::ReadWrite, - } - } -} - /// A cache that stores entries in Google Cloud Storage pub struct GCSCache; diff --git a/src/config.rs b/src/config.rs index 10c6f993a..105bc871e 100644 --- a/src/config.rs +++ b/src/config.rs @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +use crate::cache::gcs::RWMode; +use crate::cache::CacheMode; use directories::ProjectDirs; use fs::File; use fs_err as fs; @@ -185,6 +187,24 @@ pub enum CacheRWMode { ReadWrite, } +impl From for CacheMode { + fn from(value: CacheRWMode) -> Self { + match value { + CacheRWMode::ReadOnly => CacheMode::ReadOnly, + CacheRWMode::ReadWrite => CacheMode::ReadWrite, + } + } +} + +impl From for RWMode { + fn from(value: CacheRWMode) -> Self { + match value { + CacheRWMode::ReadOnly => RWMode::ReadOnly, + CacheRWMode::ReadWrite => RWMode::ReadWrite, + } + } +} + #[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] #[serde(deny_unknown_fields)] pub struct GCSCacheConfig { From 843448a481f6ec97b2451699974514b6b2f6966a Mon Sep 17 00:00:00 2001 From: uellenberg Date: Sat, 20 Jan 2024 15:04:34 -0800 Subject: [PATCH 03/11] Add test for local read-only cache --- tests/sccache_cargo.rs | 161 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 161 insertions(+) diff --git a/tests/sccache_cargo.rs b/tests/sccache_cargo.rs index 2af7af89d..30c6c6f87 100644 --- a/tests/sccache_cargo.rs +++ b/tests/sccache_cargo.rs @@ -131,12 +131,24 @@ fn test_rust_cargo_check() -> Result<()> { test_rust_cargo_cmd("check", SccacheTest::new(None)?) } +#[test] +#[serial] +fn test_rust_cargo_check_readonly() -> Result<()> { + test_rust_cargo_cmd_readonly("check", SccacheTest::new(None)?) +} + #[test] #[serial] fn test_rust_cargo_build() -> Result<()> { test_rust_cargo_cmd("build", SccacheTest::new(None)?) } +#[test] +#[serial] +fn test_rust_cargo_build_readonly() -> Result<()> { + test_rust_cargo_cmd_readonly("build", SccacheTest::new(None)?) +} + #[test] #[serial] #[cfg(unix)] @@ -193,6 +205,16 @@ fn test_rust_cargo_check_nightly() -> Result<()> { ) } +#[cfg(feature = "unstable")] +#[test] +#[serial] +fn test_rust_cargo_check_nightly_readonly() -> Result<()> { + test_rust_cargo_cmd_readonly( + "check", + SccacheTest::new(Some(&[("RUSTFLAGS", OsString::from("-Zprofile"))]))?, + ) +} + #[cfg(feature = "unstable")] #[test] #[serial] @@ -203,6 +225,16 @@ fn test_rust_cargo_build_nightly() -> Result<()> { ) } +#[cfg(feature = "unstable")] +#[test] +#[serial] +fn test_rust_cargo_build_nightly_readonly() -> Result<()> { + test_rust_cargo_cmd_readonly( + "build", + SccacheTest::new(Some(&[("RUSTFLAGS", OsString::from("-Zprofile"))]))?, + ) +} + fn cargo_clean(test_info: &SccacheTest) -> Result<()> { Command::new(CARGO.as_os_str()) .args(["clean"]) @@ -251,6 +283,135 @@ fn test_rust_cargo_cmd(cmd: &str, test_info: SccacheTest) -> Result<()> { Ok(()) } +fn restart_sccache( + test_info: &SccacheTest, + additional_envs: Option>, +) -> Result<()> { + let cache_dir = test_info.tempdir.path().join("cache"); + + stop_sccache()?; + + trace!("sccache --start-server"); + + let mut cmd = Command::new(SCCACHE_BIN.as_os_str()); + cmd.arg("--start-server"); + cmd.env("SCCACHE_DIR", &cache_dir); + + if let Some(additional_envs) = additional_envs { + cmd.envs(additional_envs); + } + + cmd.assert() + .try_success() + .context("Failed to start sccache server")?; + + Ok(()) +} + +/// Test that building a simple Rust crate with cargo using sccache results in the following behaviors (for three different runs): +/// - In read-only mode, a cache miss. +/// - In read-write mode, a cache miss. +/// - In read-only mode, a cache hit. +/// +/// The environment variable for read/write mode is added by this function. +fn test_rust_cargo_cmd_readonly(cmd: &str, test_info: SccacheTest) -> Result<()> { + // `cargo clean` first, just to be sure there's no leftover build objects. + cargo_clean(&test_info)?; + + // The cache must be put into read-only mode, and that can only be configured + // when the server starts up, so we need to restart it. + restart_sccache( + &test_info, + Some(vec![("SCCACHE_LOCAL_RW_MODE".into(), "READ_ONLY".into())]), + )?; + + // Now build the crate with cargo. + Command::new(CARGO.as_os_str()) + .args([cmd, "--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()?; + + // 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()?; + + cargo_clean(&test_info)?; + restart_sccache( + &test_info, + Some(vec![("SCCACHE_LOCAL_RW_MODE".into(), "READ_WRITE".into())]), + )?; + Command::new(CARGO.as_os_str()) + .args([cmd, "--color=always"]) + .envs(test_info.env.iter().cloned()) + .current_dir(CRATE_DIR.as_os_str()) + .assert() + .try_stderr(predicates::str::contains("\x1b[").from_utf8())? + .try_success()?; + + 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()?; + + cargo_clean(&test_info)?; + restart_sccache( + &test_info, + Some(vec![("SCCACHE_LOCAL_RW_MODE".into(), "READ_ONLY".into())]), + )?; + Command::new(CARGO.as_os_str()) + .args([cmd, "--color=always"]) + .envs(test_info.env.iter().cloned()) + .current_dir(CRATE_DIR.as_os_str()) + .assert() + .try_stderr(predicates::str::contains("\x1b[").from_utf8())? + .try_success()?; + + test_info + .show_stats()? + .try_stdout( + predicates::str::contains( + r#""cache_hits":{"counts":{"Rust":2},"adv_counts":{"rust":2}}"#, + ) + .from_utf8(), + )? + .try_stdout( + predicates::str::contains( + r#""cache_misses":{"counts":{},"adv_counts":{}}"#, + ) + .from_utf8(), + )? + .try_success()?; + + Ok(()) +} + fn test_rust_cargo_env_dep(test_info: SccacheTest) -> Result<()> { cargo_clean(&test_info)?; // Now build the crate with cargo. From beac2ecc735d262bdc114eb8f9ec4bf6d655d3ed Mon Sep 17 00:00:00 2001 From: uellenberg Date: Sat, 20 Jan 2024 15:05:03 -0800 Subject: [PATCH 04/11] Fix local read-only mode error messages --- src/cache/disk.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cache/disk.rs b/src/cache/disk.rs index fcd95403f..11c571c5f 100644 --- a/src/cache/disk.rs +++ b/src/cache/disk.rs @@ -136,7 +136,7 @@ impl Storage for DiskCache { trace!("DiskCache::finish_put({})", key); if self.rw_mode == CacheRWMode::ReadOnly { - return Err(anyhow!("Cannot write to a read-only cache1")); + return Err(anyhow!("Cannot write to a read-only cache")); } let lru = self.lru.clone(); @@ -185,7 +185,7 @@ impl Storage for DiskCache { preprocessor_cache_entry: PreprocessorCacheEntry, ) -> Result<()> { if self.rw_mode == CacheRWMode::ReadOnly { - return Err(anyhow!("Cannot write to a read-only cache2")); + return Err(anyhow!("Cannot write to a read-only cache")); } let key = normalize_key(key); From 1bf9f6175c97deca3b3c2b1668a4667616b7c817 Mon Sep 17 00:00:00 2001 From: uellenberg Date: Sat, 20 Jan 2024 20:41:01 -0800 Subject: [PATCH 05/11] Merge RWMode into CacheMode --- src/cache/cache.rs | 4 ++-- src/cache/disk.rs | 11 +++++------ src/cache/gcs.rs | 23 ++++++++--------------- src/compiler/compiler.rs | 13 ++++++------- src/config.rs | 10 ---------- src/test/tests.rs | 5 ++--- tests/sccache_cargo.rs | 18 +++++------------- 7 files changed, 28 insertions(+), 56 deletions(-) diff --git a/src/cache/cache.rs b/src/cache/cache.rs index 2beabaed2..d90b309a8 100644 --- a/src/cache/cache.rs +++ b/src/cache/cache.rs @@ -113,7 +113,7 @@ impl fmt::Debug for Cache { } /// CacheMode is used to represent which mode we are using. -#[derive(Debug)] +#[derive(Copy, Clone, Debug, PartialEq, Eq)] pub enum CacheMode { /// Only read cache from storage. ReadOnly, @@ -655,7 +655,7 @@ pub fn storage_from_config( let (dir, size) = (&config.fallback_cache.dir, config.fallback_cache.size); let preprocessor_cache_mode_config = config.fallback_cache.preprocessor_cache_mode; - let rw_mode = config.fallback_cache.rw_mode; + let rw_mode = config.fallback_cache.rw_mode.into(); debug!("Init disk cache with dir {:?}, size {}", dir, size); Ok(Arc::new(DiskCache::new( dir, diff --git a/src/cache/disk.rs b/src/cache/disk.rs index 11c571c5f..2cc4e5c8f 100644 --- a/src/cache/disk.rs +++ b/src/cache/disk.rs @@ -14,7 +14,6 @@ use crate::cache::{Cache, CacheMode, CacheRead, CacheWrite, Storage}; use crate::compiler::PreprocessorCacheEntry; -use crate::config::CacheRWMode; use crate::lru_disk_cache::LruDiskCache; use crate::lru_disk_cache::{Error as LruError, ReadSeek}; use async_trait::async_trait; @@ -66,7 +65,7 @@ pub struct DiskCache { pool: tokio::runtime::Handle, preprocessor_cache_mode_config: PreprocessorCacheModeConfig, preprocessor_cache: Arc>, - rw_mode: CacheRWMode, + rw_mode: CacheMode, } impl DiskCache { @@ -76,7 +75,7 @@ impl DiskCache { max_size: u64, pool: &tokio::runtime::Handle, preprocessor_cache_mode_config: PreprocessorCacheModeConfig, - rw_mode: CacheRWMode, + rw_mode: CacheMode, ) -> DiskCache { DiskCache { lru: Arc::new(Mutex::new(LazyDiskCache::Uninit { @@ -135,7 +134,7 @@ impl Storage for DiskCache { // everything in memory... trace!("DiskCache::finish_put({})", key); - if self.rw_mode == CacheRWMode::ReadOnly { + if self.rw_mode == CacheMode::ReadOnly { return Err(anyhow!("Cannot write to a read-only cache")); } @@ -153,7 +152,7 @@ impl Storage for DiskCache { } async fn check(&self) -> Result { - Ok(self.rw_mode.into()) + Ok(self.rw_mode) } fn location(&self) -> String { @@ -184,7 +183,7 @@ impl Storage for DiskCache { key: &str, preprocessor_cache_entry: PreprocessorCacheEntry, ) -> Result<()> { - if self.rw_mode == CacheRWMode::ReadOnly { + if self.rw_mode == CacheMode::ReadOnly { return Err(anyhow!("Cannot write to a read-only cache")); } diff --git a/src/cache/gcs.rs b/src/cache/gcs.rs index 8ec637567..79fbbd4ce 100644 --- a/src/cache/gcs.rs +++ b/src/cache/gcs.rs @@ -13,6 +13,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use crate::cache::CacheMode; use crate::errors::*; use opendal::Operator; use opendal::{layers::LoggingLayer, services::Gcs}; @@ -21,18 +22,10 @@ use reqwest::Client; use serde::Deserialize; use url::Url; -#[derive(Copy, Clone)] -pub enum RWMode { - ReadOnly, - ReadWrite, -} - -impl RWMode { - fn to_scope(self) -> &'static str { - match self { - RWMode::ReadOnly => "https://www.googleapis.com/auth/devstorage.read_only", - RWMode::ReadWrite => "https://www.googleapis.com/auth/devstorage.read_write", - } +fn rw_to_scope(mode: CacheMode) -> &'static str { + match mode { + CacheMode::ReadOnly => "https://www.googleapis.com/auth/devstorage.read_only", + CacheMode::ReadWrite => "https://www.googleapis.com/auth/devstorage.read_write", } } @@ -46,13 +39,13 @@ impl GCSCache { key_prefix: &str, cred_path: Option<&str>, service_account: Option<&str>, - rw_mode: RWMode, + rw_mode: CacheMode, credential_url: Option<&str>, ) -> Result { let mut builder = Gcs::default(); builder.bucket(bucket); builder.root(key_prefix); - builder.scope(rw_mode.to_scope()); + builder.scope(rw_to_scope(rw_mode)); if let Some(service_account) = service_account { builder.service_account(service_account); @@ -67,7 +60,7 @@ impl GCSCache { .map_err(|err| anyhow!("gcs credential url is invalid: {err:?}"))?; builder.customed_token_loader(Box::new(TaskClusterTokenLoader { - scope: rw_mode.to_scope().to_string(), + scope: rw_to_scope(rw_mode).to_string(), url: cred_url.to_string(), })); } diff --git a/src/compiler/compiler.rs b/src/compiler/compiler.rs index c7deecc0c..763c81226 100644 --- a/src/compiler/compiler.rs +++ b/src/compiler/compiler.rs @@ -1381,8 +1381,7 @@ where mod test { use super::*; use crate::cache::disk::DiskCache; - use crate::cache::{CacheRead, PreprocessorCacheModeConfig}; - use crate::config::CacheRWMode; + use crate::cache::{CacheMode, CacheRead, PreprocessorCacheModeConfig}; use crate::mock_command::*; use crate::test::mock_storage::MockStorage; use crate::test::utils::*; @@ -1825,7 +1824,7 @@ LLVM version: 6.0", use_preprocessor_cache_mode: preprocessor_cache_mode, ..Default::default() }, - CacheRWMode::ReadWrite, + CacheMode::ReadWrite, ); // Write a dummy input file so the preprocessor cache mode can work std::fs::write(f.tempdir.path().join("foo.c"), "whatever").unwrap(); @@ -1951,7 +1950,7 @@ LLVM version: 6.0", use_preprocessor_cache_mode: preprocessor_cache_mode, ..Default::default() }, - CacheRWMode::ReadWrite, + CacheMode::ReadWrite, ); // Write a dummy input file so the preprocessor cache mode can work std::fs::write(f.tempdir.path().join("foo.c"), "whatever").unwrap(); @@ -2239,7 +2238,7 @@ LLVM version: 6.0", use_preprocessor_cache_mode: preprocessor_cache_mode, ..Default::default() }, - CacheRWMode::ReadWrite, + CacheMode::ReadWrite, ); let storage = Arc::new(storage); // Write a dummy input file so the preprocessor cache mode can work @@ -2365,7 +2364,7 @@ LLVM version: 6.0", use_preprocessor_cache_mode: preprocessor_cache_mode, ..Default::default() }, - CacheRWMode::ReadWrite, + CacheMode::ReadWrite, ); let storage = Arc::new(storage); // Pretend to be GCC. Also inject a fake object file that the subsequent @@ -2459,7 +2458,7 @@ LLVM version: 6.0", use_preprocessor_cache_mode: preprocessor_cache_mode, ..Default::default() }, - CacheRWMode::ReadWrite, + CacheMode::ReadWrite, ); let storage = Arc::new(storage); // Pretend to be GCC. diff --git a/src/config.rs b/src/config.rs index 105bc871e..a8f366550 100644 --- a/src/config.rs +++ b/src/config.rs @@ -12,7 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::cache::gcs::RWMode; use crate::cache::CacheMode; use directories::ProjectDirs; use fs::File; @@ -196,15 +195,6 @@ impl From for CacheMode { } } -impl From for RWMode { - fn from(value: CacheRWMode) -> Self { - match value { - CacheRWMode::ReadOnly => RWMode::ReadOnly, - CacheRWMode::ReadWrite => RWMode::ReadWrite, - } - } -} - #[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] #[serde(deny_unknown_fields)] pub struct GCSCacheConfig { diff --git a/src/test/tests.rs b/src/test/tests.rs index 49d65f13a..3eae4217c 100644 --- a/src/test/tests.rs +++ b/src/test/tests.rs @@ -13,10 +13,9 @@ // limitations under the License. use crate::cache::disk::DiskCache; -use crate::cache::PreprocessorCacheModeConfig; +use crate::cache::{CacheMode, PreprocessorCacheModeConfig}; use crate::client::connect_to_server; use crate::commands::{do_compile, request_shutdown, request_stats}; -use crate::config::CacheRWMode; use crate::jobserver::Client; use crate::mock_command::*; use crate::server::{DistClientContainer, SccacheServer, ServerMessage}; @@ -86,7 +85,7 @@ where cache_size, runtime.handle(), PreprocessorCacheModeConfig::default(), - CacheRWMode::ReadWrite, + CacheMode::ReadWrite, )); let client = unsafe { Client::new() }; diff --git a/tests/sccache_cargo.rs b/tests/sccache_cargo.rs index 30c6c6f87..0e9455ad3 100644 --- a/tests/sccache_cargo.rs +++ b/tests/sccache_cargo.rs @@ -338,16 +338,13 @@ fn test_rust_cargo_cmd_readonly(cmd: &str, test_info: SccacheTest) -> Result<()> test_info .show_stats()? .try_stdout( - predicates::str::contains( - r#""cache_hits":{"counts":{},"adv_counts":{}}"#, - ) - .from_utf8(), + 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(), + .from_utf8(), )? .try_success()?; @@ -367,16 +364,13 @@ fn test_rust_cargo_cmd_readonly(cmd: &str, test_info: SccacheTest) -> Result<()> test_info .show_stats()? .try_stdout( - predicates::str::contains( - r#""cache_hits":{"counts":{},"adv_counts":{}}"#, - ) - .from_utf8(), + 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(), + .from_utf8(), )? .try_success()?; @@ -402,9 +396,7 @@ fn test_rust_cargo_cmd_readonly(cmd: &str, test_info: SccacheTest) -> Result<()> .from_utf8(), )? .try_stdout( - predicates::str::contains( - r#""cache_misses":{"counts":{},"adv_counts":{}}"#, - ) + predicates::str::contains(r#""cache_misses":{"counts":{},"adv_counts":{}}"#) .from_utf8(), )? .try_success()?; From 609b405df4ad7e1fcaa993b9d6eced01bf23ee09 Mon Sep 17 00:00:00 2001 From: uellenberg Date: Sat, 20 Jan 2024 20:44:00 -0800 Subject: [PATCH 06/11] Rename CacheRWMode to CacheModeConfig --- src/config.rs | 44 ++++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/config.rs b/src/config.rs index a8f366550..3c7882bb8 100644 --- a/src/config.rs +++ b/src/config.rs @@ -163,7 +163,7 @@ pub struct DiskCacheConfig { // TODO: use deserialize_with to allow human-readable sizes in toml pub size: u64, pub preprocessor_cache_mode: PreprocessorCacheModeConfig, - pub rw_mode: CacheRWMode, + pub rw_mode: CacheModeConfig, } impl Default for DiskCacheConfig { @@ -172,25 +172,25 @@ impl Default for DiskCacheConfig { dir: default_disk_cache_dir(), size: default_disk_cache_size(), preprocessor_cache_mode: PreprocessorCacheModeConfig::activated(), - rw_mode: CacheRWMode::ReadWrite, + rw_mode: CacheModeConfig::ReadWrite, } } } #[derive(Copy, Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] #[serde(deny_unknown_fields)] -pub enum CacheRWMode { +pub enum CacheModeConfig { #[serde(rename = "READ_ONLY")] ReadOnly, #[serde(rename = "READ_WRITE")] ReadWrite, } -impl From for CacheMode { - fn from(value: CacheRWMode) -> Self { +impl From for CacheMode { + fn from(value: CacheModeConfig) -> Self { match value { - CacheRWMode::ReadOnly => CacheMode::ReadOnly, - CacheRWMode::ReadWrite => CacheMode::ReadWrite, + CacheModeConfig::ReadOnly => CacheMode::ReadOnly, + CacheModeConfig::ReadWrite => CacheMode::ReadWrite, } } } @@ -202,7 +202,7 @@ pub struct GCSCacheConfig { pub key_prefix: String, pub cred_path: Option, pub service_account: Option, - pub rw_mode: CacheRWMode, + pub rw_mode: CacheModeConfig, pub credential_url: Option, } @@ -640,17 +640,17 @@ fn config_from_env() -> Result { let service_account = env::var("SCCACHE_GCS_SERVICE_ACCOUNT").ok(); let rw_mode = match env::var("SCCACHE_GCS_RW_MODE").as_ref().map(String::as_str) { - Ok("READ_ONLY") => CacheRWMode::ReadOnly, - Ok("READ_WRITE") => CacheRWMode::ReadWrite, + Ok("READ_ONLY") => CacheModeConfig::ReadOnly, + Ok("READ_WRITE") => CacheModeConfig::ReadWrite, // TODO: unsure if these should warn during the configuration loading // or at the time when they're actually used to connect to GCS Ok(_) => { warn!("Invalid SCCACHE_GCS_RW_MODE-- defaulting to READ_ONLY."); - CacheRWMode::ReadOnly + CacheModeConfig::ReadOnly } _ => { warn!("No SCCACHE_GCS_RW_MODE specified-- defaulting to READ_ONLY."); - CacheRWMode::ReadOnly + CacheModeConfig::ReadOnly } }; @@ -755,13 +755,13 @@ fn config_from_env() -> Result { .as_ref() .map(String::as_str) { - Ok("READ_ONLY") => (CacheRWMode::ReadOnly, true), - Ok("READ_WRITE") => (CacheRWMode::ReadWrite, true), + Ok("READ_ONLY") => (CacheModeConfig::ReadOnly, true), + Ok("READ_WRITE") => (CacheModeConfig::ReadWrite, true), Ok(_) => { warn!("Invalid SCCACHE_LOCAL_RW_MODE-- defaulting to READ_WRITE."); - (CacheRWMode::ReadWrite, false) + (CacheModeConfig::ReadWrite, false) } - _ => (CacheRWMode::ReadWrite, false), + _ => (CacheModeConfig::ReadWrite, false), }; let any_overridden = disk_dir.is_some() @@ -1117,7 +1117,7 @@ fn config_overrides() { dir: "/env-cache".into(), size: 5, preprocessor_cache_mode: Default::default(), - rw_mode: CacheRWMode::ReadWrite, + rw_mode: CacheModeConfig::ReadWrite, }), redis: Some(RedisCacheConfig { url: "myotherredisurl".to_owned(), @@ -1133,7 +1133,7 @@ fn config_overrides() { dir: "/file-cache".into(), size: 15, preprocessor_cache_mode: Default::default(), - rw_mode: CacheRWMode::ReadWrite, + rw_mode: CacheModeConfig::ReadWrite, }), memcached: Some(MemcachedCacheConfig { url: "memurl".to_owned(), @@ -1160,7 +1160,7 @@ fn config_overrides() { dir: "/env-cache".into(), size: 5, preprocessor_cache_mode: Default::default(), - rw_mode: CacheRWMode::ReadWrite, + rw_mode: CacheModeConfig::ReadWrite, }, dist: Default::default(), server_startup_timeout: None, @@ -1266,7 +1266,7 @@ fn test_gcs_service_account() { }) => { assert_eq!(bucket, "my-bucket"); assert_eq!(service_account, Some("my@example.com".to_string())); - assert_eq!(rw_mode, CacheRWMode::ReadWrite); + assert_eq!(rw_mode, CacheModeConfig::ReadWrite); } None => unreachable!(), }; @@ -1349,13 +1349,13 @@ token = "webdavtoken" dir: PathBuf::from("/tmp/.cache/sccache"), size: 7 * 1024 * 1024 * 1024, preprocessor_cache_mode: PreprocessorCacheModeConfig::activated(), - rw_mode: CacheRWMode::ReadWrite, + rw_mode: CacheModeConfig::ReadWrite, }), gcs: Some(GCSCacheConfig { bucket: "bucket".to_owned(), cred_path: Some("/psst/secret/cred".to_string()), service_account: Some("example_service_account".to_string()), - rw_mode: CacheRWMode::ReadOnly, + rw_mode: CacheModeConfig::ReadOnly, key_prefix: "prefix".into(), credential_url: None, }), From 8c54c2f2d659d11cbd2de9aec95a724509e8f317 Mon Sep 17 00:00:00 2001 From: Jonah Uellenberg Date: Mon, 22 Jan 2024 12:46:08 -0800 Subject: [PATCH 07/11] Expand local read-only documentation --- docs/Local.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/Local.md b/docs/Local.md index 6f8e4382b..4b601080c 100644 --- a/docs/Local.md +++ b/docs/Local.md @@ -46,4 +46,8 @@ See where to write the config in [the configuration doc](Configuration.md). ## Read-only cache mode -By default, the local cache operates in read/write mode. The `SCCACHE_LOCAL_RW_MODE` environment variable can be set to `READ_ONLY` (or `READ_WRITE`) to modify this behavior. \ No newline at end of file +By default, the local cache operates in read/write mode. The `SCCACHE_LOCAL_RW_MODE` environment variable can be set to `READ_ONLY` (or `READ_WRITE`) to modify this behavior. + +You can use read-only mode to prevent sccache from writing new cache items to the disk. This can be useful, for example, if you want to use items that have already been cached, but not add new ones to the cache. + +Note that this feature is only effective if you already have items in your cache. Using this option on an empty cache will cause sccache to simply do nothing. From 805e4ef6e52b49fc4718dcab2c64a0c67e63d3fe Mon Sep 17 00:00:00 2001 From: Jonah Uellenberg Date: Mon, 22 Jan 2024 12:51:23 -0800 Subject: [PATCH 08/11] Clarify how local read-only mode can be misused Co-authored-by: Sylvestre Ledru --- docs/Local.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Local.md b/docs/Local.md index 4b601080c..5b05d0f70 100644 --- a/docs/Local.md +++ b/docs/Local.md @@ -50,4 +50,4 @@ By default, the local cache operates in read/write mode. The `SCCACHE_LOCAL_RW_M You can use read-only mode to prevent sccache from writing new cache items to the disk. This can be useful, for example, if you want to use items that have already been cached, but not add new ones to the cache. -Note that this feature is only effective if you already have items in your cache. Using this option on an empty cache will cause sccache to simply do nothing. +Note that this feature is only effective if you already have items in your cache. Using this option on an empty cache will cause sccache to simply do nothing, just add overhead. From b065f2086d5d7b415ba068dd901fb9e72c73412e Mon Sep 17 00:00:00 2001 From: uellenberg Date: Thu, 25 Jan 2024 10:29:08 -0800 Subject: [PATCH 09/11] Test that the disk cache will not accept writes in read-only mode --- src/cache/cache.rs | 58 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/src/cache/cache.rs b/src/cache/cache.rs index d90b309a8..a45e026ef 100644 --- a/src/cache/cache.rs +++ b/src/cache/cache.rs @@ -669,6 +669,7 @@ pub fn storage_from_config( #[cfg(test)] mod test { use super::*; + use crate::config::CacheModeConfig; #[test] fn test_normalize_key() { @@ -677,4 +678,61 @@ mod test { "0/1/2/0123456789abcdef0123456789abcdef" ); } + + #[test] + fn test_read_write_mode_local() { + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_all() + .worker_threads(1) + .build() + .unwrap(); + + let mut config = Config::default(); + // Use disk cache. + config.cache = None; + + let tempdir = tempfile::Builder::new() + .prefix("sccache_test_rust_cargo") + .tempdir() + .context("Failed to create tempdir") + .unwrap(); + let cache_dir = tempdir.path().join("cache"); + fs::create_dir(&cache_dir).unwrap(); + + config.fallback_cache.dir = cache_dir; + + // Test Read Write + config.fallback_cache.rw_mode = CacheModeConfig::ReadWrite; + + { + let cache = storage_from_config(&config, runtime.handle()).unwrap(); + + runtime.block_on(async move { + cache + .put("test1".into(), CacheWrite::default()) + .await + .unwrap(); + cache + .put_preprocessor_cache_entry("test1".into(), PreprocessorCacheEntry::default()) + .unwrap(); + }); + } + + // Test Read-only + config.fallback_cache.rw_mode = CacheModeConfig::ReadOnly; + + { + let cache = storage_from_config(&config, runtime.handle()).unwrap(); + + runtime.block_on(async move { + cache + .put("test1".into(), CacheWrite::default()) + .await + .unwrap_err(); + cache + .put_preprocessor_cache_entry("test1".into(), PreprocessorCacheEntry::default()) + .unwrap_err(); + }); + } + } } From c765ec109c3ee7e2c92c801207a26d85e519513a Mon Sep 17 00:00:00 2001 From: Jonah Uellenberg Date: Thu, 25 Jan 2024 17:57:16 -0800 Subject: [PATCH 10/11] Fix Clippy --- src/cache/cache.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/cache/cache.rs b/src/cache/cache.rs index a45e026ef..125ff7874 100644 --- a/src/cache/cache.rs +++ b/src/cache/cache.rs @@ -687,9 +687,8 @@ mod test { .build() .unwrap(); - let mut config = Config::default(); // Use disk cache. - config.cache = None; + let mut config = Config { cache: None, ..Default::default() }; let tempdir = tempfile::Builder::new() .prefix("sccache_test_rust_cargo") @@ -709,11 +708,11 @@ mod test { runtime.block_on(async move { cache - .put("test1".into(), CacheWrite::default()) + .put("test1", CacheWrite::default()) .await .unwrap(); cache - .put_preprocessor_cache_entry("test1".into(), PreprocessorCacheEntry::default()) + .put_preprocessor_cache_entry("test1", PreprocessorCacheEntry::default()) .unwrap(); }); } @@ -726,11 +725,11 @@ mod test { runtime.block_on(async move { cache - .put("test1".into(), CacheWrite::default()) + .put("test1", CacheWrite::default()) .await .unwrap_err(); cache - .put_preprocessor_cache_entry("test1".into(), PreprocessorCacheEntry::default()) + .put_preprocessor_cache_entry("test1", PreprocessorCacheEntry::default()) .unwrap_err(); }); } From 6364e35c82bdb5ee38f2179264a5a4b2bcbff9cf Mon Sep 17 00:00:00 2001 From: uellenberg Date: Fri, 26 Jan 2024 00:30:11 -0800 Subject: [PATCH 11/11] Check error message for read-only mode test --- src/cache/cache.rs | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/src/cache/cache.rs b/src/cache/cache.rs index 125ff7874..3abb7c5a8 100644 --- a/src/cache/cache.rs +++ b/src/cache/cache.rs @@ -688,7 +688,10 @@ mod test { .unwrap(); // Use disk cache. - let mut config = Config { cache: None, ..Default::default() }; + let mut config = Config { + cache: None, + ..Default::default() + }; let tempdir = tempfile::Builder::new() .prefix("sccache_test_rust_cargo") @@ -707,10 +710,7 @@ mod test { let cache = storage_from_config(&config, runtime.handle()).unwrap(); runtime.block_on(async move { - cache - .put("test1", CacheWrite::default()) - .await - .unwrap(); + cache.put("test1", CacheWrite::default()).await.unwrap(); cache .put_preprocessor_cache_entry("test1", PreprocessorCacheEntry::default()) .unwrap(); @@ -724,13 +724,21 @@ mod test { let cache = storage_from_config(&config, runtime.handle()).unwrap(); runtime.block_on(async move { - cache - .put("test1", CacheWrite::default()) - .await - .unwrap_err(); - cache - .put_preprocessor_cache_entry("test1", PreprocessorCacheEntry::default()) - .unwrap_err(); + assert_eq!( + cache + .put("test1", CacheWrite::default()) + .await + .unwrap_err() + .to_string(), + "Cannot write to a read-only cache" + ); + assert_eq!( + cache + .put_preprocessor_cache_entry("test1", PreprocessorCacheEntry::default()) + .unwrap_err() + .to_string(), + "Cannot write to a read-only cache" + ); }); } }