Fix mtime not persisting in the cache

I previously introduced a commit that had a test intended to prove that we were persisting the mtime of files copied into the cache to close out this comment #302 (comment). The test passed locally, but failed on CI.

It appears the behavior of fs-extra with regards to mtime for copying/moving directories is system dependent. To compensate, I switched to `cp_r` which explicitly is designed to preserve mtime https://crates.io/crates/cp_r. It has ~27,000 lifetime downloads and has activity as recently as a 9 days ago. https://docs.rs/cp_r/latest/cp_r/#release-history

Copying and preserving mtime is the primary reason this library exists. From the docs.rs:

> Copy a directory tree, including mtimes and permissions.

It does not have the ability to skip-overwriting existing files which I need, however it has a generic `filter` closure which allows me to assemble this behavior myself.

buildpacks/ruby/Cargo.toml

@@ -33,4 +33,3 @@ toml = "0.8"
 
 [dev-dependencies]
 libcnb-test = "=0.23.0"
-filetime = "0.2.25"

buildpacks/ruby/src/main.rs

@@ -29,9 +29,6 @@ mod user_errors;
 #[cfg(test)]
 use libcnb_test as _;
 
-#[cfg(test)]
-use filetime as _;
-
 use clap as _;
 
 struct RubyBuildpack;

commons/Cargo.toml

@@ -16,7 +16,6 @@ byte-unit = "5"
 const_format = "0.2"
 # TODO: Consolidate on either the regex crate or the fancy-regex crate, since this repo currently uses both.
 fancy-regex = "0.13"
-fs_extra = "1"
 fs-err = "2"
 fun_run = "0.2"
 glob = "0.3"
@@ -32,6 +31,7 @@ sha2 = "0.10"
 tempfile = "3"
 thiserror = "1"
 walkdir = "2"
+cp_r = "0.5.2"
 
 [dev-dependencies]
 filetime = "0.2"

commons/src/cache/app_cache.rs

@@ -2,7 +2,6 @@ use crate::cache::clean::{lru_clean, FilesWithSize};
 use crate::cache::in_app_dir_cache_layer::InAppDirCacheLayer;
 use crate::cache::{CacheConfig, CacheError, KeepPath};
 use byte_unit::{AdjustedByte, Byte, UnitType};
-use fs_extra::dir::CopyOptions;
 use libcnb::build::BuildContext;
 use libcnb::data::layer::LayerName;
 use std::path::Path;
@@ -148,22 +147,20 @@ impl AppCache {
  fs_err::create_dir_all(&self.path).map_err(CacheError::IoError)?;
  fs_err::create_dir_all(&self.cache).map_err(CacheError::IoError)?;
 
- fs_extra::dir::move_dir(
- &self.cache,
- &self.path,
- &CopyOptions {
- overwrite: false,
- skip_exist: true,
- copy_inside: true,
- content_only: true,
- ..CopyOptions::default()
- },
- )
- .map_err(|error| CacheError::CopyCacheToAppError {
- path: self.path.clone(),
- cache: self.cache.clone(),
- error,
- })?;
+ cp_r::CopyOptions::new()
+ .create_destination(true)
+ // Do not overwrite
+ .filter(|to_file, _| {
+ let destination = self.path.join(to_file);
+ let exists = destination.try_exists().unwrap_or(false);
+ Ok(!exists)
+ })
+ .copy_tree(&self.cache, &self.path)
+ .map_err(|error| CacheError::CopyCacheToAppError {
+ path: self.path.clone(),
+ cache: self.cache.clone(),
+ error,
+ })?;
 
  Ok(self)
  }
@@ -276,21 +273,14 @@ pub fn build<B: libcnb::Buildpack>(
 ///
 /// - If the copy command fails an `IoExtraError` will be raised.
 fn preserve_path_save(store: &AppCache) -> Result<&AppCache, CacheError> {
- fs_extra::dir::copy(
- &store.path,
- &store.cache,
- &CopyOptions {
- overwrite: true,
- copy_inside: true, // Recursive
- content_only: true, // Don't copy top level directory name
- ..CopyOptions::default()
- },
- )
- .map_err(|error| CacheError::CopyAppToCacheError {
- path: store.path.clone(),
- cache: store.cache.clone(),
- error,
- })?;
+ cp_r::CopyOptions::new()
+ .create_destination(true)
+ .copy_tree(&store.path, &store.cache)
+ .map_err(|error| CacheError::CopyAppToCacheError {
+ path: store.path.clone(),
+ cache: store.cache.clone(),
+ error,
+ })?;
 
  Ok(store)
 }
@@ -306,21 +296,16 @@ fn preserve_path_save(store: &AppCache) -> Result<&AppCache, CacheError> {
 ///
 /// - If the move command fails an `IoExtraError` will be raised.
 fn remove_path_save(store: &AppCache) -> Result<&AppCache, CacheError> {
- fs_extra::dir::move_dir(
- &store.path,
- &store.cache,
- &CopyOptions {
- overwrite: true,
- copy_inside: true, // Recursive
- content_only: true, // Don't copy top level directory name
- ..CopyOptions::default()
- },
- )
- .map_err(|error| CacheError::DestructiveMoveAppToCacheError {
- path: store.path.clone(),
- cache: store.cache.clone(),
- error,
- })?;
+ cp_r::CopyOptions::new()
+ .create_destination(true)
+ .copy_tree(&store.path, &store.cache)
+ .map_err(|error| CacheError::DestructiveMoveAppToCacheError {
+ path: store.path.clone(),
+ cache: store.cache.clone(),
+ error,
+ })?;
+
+ fs_err::remove_dir_all(&store.path).map_err(CacheError::IoError)?;
 
  Ok(store)
 }
@@ -475,7 +460,7 @@ mod tests {
  }
 
  #[test]
- fn mtime_preserved() {
+ fn mtime_preserved_keep_path_build_only() {
  let tmpdir = tempfile::tempdir().unwrap();
  let cache_path = tmpdir.path().join("cache");
  let app_path = tmpdir.path().join("app");
@@ -501,7 +486,53 @@ mod tests {
 
  let metadata = fs_err::metadata(store.cache.join("lol.txt")).unwrap();
  let actual = filetime::FileTime::from_last_modification_time(&metadata);
+ assert_eq!(mtime, actual);
+
+ assert!(!store.path.join("lol.txt").exists());
+
+ store.load().unwrap();
+
+ let metadata = fs_err::metadata(store.cache.join("lol.txt")).unwrap();
+ let actual = filetime::FileTime::from_last_modification_time(&metadata);
+ assert_eq!(mtime, actual);
+ }
 
+ #[test]
+ fn mtime_preserved_keep_path_runtime() {
+ let tmpdir = tempfile::tempdir().unwrap();
+ let cache_path = tmpdir.path().join("cache");
+ let app_path = tmpdir.path().join("app");
+
+ fs_err::create_dir_all(&cache_path).unwrap();
+ fs_err::create_dir_all(&app_path).unwrap();
+
+ let store = AppCache {
+ path: app_path.clone(),
+ cache: cache_path,
+ limit: Byte::from_u64(512),
+ keep_path: KeepPath::Runtime,
+ cache_state: CacheState::NewEmpty,
+ };
+
+ let mtime = filetime::FileTime::from_unix_time(1000, 0);
+
+ let path = app_path.join("lol.txt");
+ fs_err::write(&path, "hahaha").unwrap();
+ filetime::set_file_mtime(&path, mtime).unwrap();
+
+ store.save().unwrap();
+
+ let metadata = fs_err::metadata(store.cache.join("lol.txt")).unwrap();
+ let actual = filetime::FileTime::from_last_modification_time(&metadata);
+ assert_eq!(mtime, actual);
+
+ fs_err::remove_dir_all(&app_path).unwrap();
+ assert!(!store.path.join("lol.txt").exists());
+
+ store.load().unwrap();
+
+ let metadata = fs_err::metadata(store.cache.join("lol.txt")).unwrap();
+ let actual = filetime::FileTime::from_last_modification_time(&metadata);
  assert_eq!(mtime, actual);
  }
 }

commons/src/cache/error.rs

@@ -14,21 +14,21 @@ pub enum CacheError {
  CopyCacheToAppError {
  path: PathBuf,
  cache: PathBuf,
- error: fs_extra::error::Error,
+ error: cp_r::Error,
  },
 
  #[error("Could not copy files from the application to cache.\nFrom: {path} To: {cache}\nError: {error}")]
  CopyAppToCacheError {
  path: PathBuf,
  cache: PathBuf,
- error: fs_extra::error::Error,
+ error: cp_r::Error,
  },
 
  #[error("Could not move files out of the application to the cache.\nFrom: {path} To: {cache}\nError: {error}")]
  DestructiveMoveAppToCacheError {
  path: PathBuf,
  cache: PathBuf,
- error: fs_extra::error::Error,
+ error: cp_r::Error,
  },
 
  #[error("IO error: {0}")]