CacheConfigs are always enabled; clarify directory validation (#11376)

Author: jder

crates/cache/src/config.rs

@@ -305,22 +305,17 @@ generate_deserializer!(deserialize_percent(num: u8, unit: &str) -> u8 {
     }
 });
 
-static CACHE_IMPROPER_CONFIG_ERROR_MSG: &str =
-    "Cache system should be enabled and all settings must be validated or defaulted";
-
 macro_rules! generate_setting_getter {
     ($setting:ident: $setting_type:ty) => {
         #[doc = concat!("Returns ", "`", stringify!($setting), "`.")]
-        ///
-        /// Panics if the cache is disabled.
         pub fn $setting(&self) -> $setting_type {
             self.$setting
         }
     };
 }
 
 impl CacheConfig {
-    /// Creates a new set of configuration which represents a disabled cache
+    /// Creates a cache configuration with default settings.
     pub fn new() -> Self {
         Self::default()
     }
@@ -385,13 +380,9 @@ impl CacheConfig {
     generate_setting_getter!(file_count_limit_percent_if_deleting: u8);
     generate_setting_getter!(files_total_size_limit_percent_if_deleting: u8);
 
-    /// Returns path to the cache directory.
-    ///
-    /// Panics if the cache is disabled.
-    pub fn directory(&self) -> &PathBuf {
-        self.directory
-            .as_ref()
-            .expect(CACHE_IMPROPER_CONFIG_ERROR_MSG)
+    /// Returns path to the cache directory if one is set.
+    pub fn directory(&self) -> Option<&PathBuf> {
+        self.directory.as_ref()
     }
 
     /// Specify where the cache directory is. Must be an absolute path.

crates/cache/src/config/tests.rs

@@ -99,7 +99,7 @@ fn test_all_settings() {
     fn check_conf(conf: &CacheConfig, cd: &PathBuf) {
         assert_eq!(
             conf.directory(),
-            &fs::canonicalize(cd).expect("canonicalize failed")
+            Some(&fs::canonicalize(cd).expect("canonicalize failed"))
         );
         assert_eq!(conf.worker_event_queue_size(), 0x10);
         assert_eq!(conf.baseline_compression_level(), 3);
@@ -536,7 +536,7 @@ fn test_builder_all_settings() {
     fn check_conf(conf: &CacheConfig, cd: &PathBuf) {
         assert_eq!(
             conf.directory(),
-            &fs::canonicalize(cd).expect("canonicalize failed")
+            Some(&fs::canonicalize(cd).expect("canonicalize failed"))
         );
         assert_eq!(conf.worker_event_queue_size(), 0x10);
         assert_eq!(conf.baseline_compression_level(), 3);

crates/cache/src/lib.rs

@@ -37,8 +37,6 @@ pub struct Cache {
 macro_rules! generate_config_setting_getter {
     ($setting:ident: $setting_type:ty) => {
         #[doc = concat!("Returns ", "`", stringify!($setting), "`.")]
-        ///
-        /// Panics if the cache is disabled.
         pub fn $setting(&self) -> $setting_type {
             self.config.$setting()
         }
@@ -97,10 +95,11 @@ impl Cache {
     generate_config_setting_getter!(files_total_size_limit_percent_if_deleting: u8);
 
     /// Returns path to the cache directory.
-    ///
-    /// Panics if the cache directory is not set.
     pub fn directory(&self) -> &PathBuf {
-        &self.config.directory()
+        &self
+            .config
+            .directory()
+            .expect("directory should be validated in Config::new")
     }
 
     #[cfg(test)]

crates/cache/src/tests.rs

@@ -24,8 +24,8 @@ fn test_cache_init() {
 
     // assumption: config init creates cache directory and returns canonicalized path
     assert_eq!(
-        *cache_config.directory(),
-        fs::canonicalize(cache_dir).unwrap()
+        cache_config.directory(),
+        Some(&fs::canonicalize(cache_dir).unwrap())
     );
     assert_eq!(
         cache_config.baseline_compression_level(),
@@ -53,8 +53,8 @@ fn test_write_read_cache() {
 
     // assumption: config load creates cache directory and returns canonicalized path
     assert_eq!(
-        *cache_config.directory(),
-        fs::canonicalize(cache_dir).unwrap()
+        cache_config.directory(),
+        Some(&fs::canonicalize(cache_dir).unwrap())
     );
 
     let compiler1 = "test-1";

crates/cache/src/worker.rs

@@ -393,6 +393,12 @@ impl WorkerThread {
         trace!("Task finished: recompress file: {}", path.display());
     }
 
+    fn directory(&self) -> &PathBuf {
+        self.cache_config
+            .directory()
+            .expect("CacheConfig should be validated before being passed to a WorkerThread")
+    }
+
     fn handle_on_cache_update(&self, path: PathBuf) {
         trace!("handle_on_cache_update() for path: {}", path.display());
 
@@ -416,7 +422,7 @@ impl WorkerThread {
         // acquire lock for cleanup task
         // Lock is a proof of recent cleanup task, so we don't want to delete them.
         // Expired locks will be deleted by the cleanup task.
-        let cleanup_file = self.cache_config.directory().join(".cleanup"); // some non existing marker file
+        let cleanup_file = self.directory().join(".cleanup"); // some non existing marker file
         if acquire_task_fs_lock(
             &cleanup_file,
             self.cache_config.cleanup_interval(),
@@ -722,12 +728,7 @@ impl WorkerThread {
         }
 
         let mut vec = Vec::new();
-        enter_dir(
-            &mut vec,
-            self.cache_config.directory(),
-            0,
-            &self.cache_config,
-        );
+        enter_dir(&mut vec, self.directory(), 0, &self.cache_config);
         vec
     }
 }

crates/wasmtime/src/config.rs

@@ -1398,8 +1398,8 @@ impl Config {
 
     /// Set a custom [`Cache`].
     ///
-    /// To load a cache from a file, use [`Cache::from_file`]. Otherwise, you can create a new
-    /// cache config using [`CacheConfig::new`] and passing that to [`Cache::new`].
+    /// To load a cache configuration from a file, use [`Cache::from_file`]. Otherwise, you can
+    /// create a new cache config using [`CacheConfig::new`] and passing that to [`Cache::new`].
     ///
     /// If you want to disable the cache, you can call this method with `None`.
     ///