From b28bfeb35eaa1ed1676bedd77a7c09734a787159 Mon Sep 17 00:00:00 2001 From: JP-Ellis Date: Tue, 21 Apr 2026 21:39:59 +1000 Subject: [PATCH 1/2] feat: add copy option Add a `Copy` option to force dotter to copy files to the target. Fixes: #115 Assisted-by: Claude Code:claude-sonnet-4-6 Signed-off-by: JP-Ellis --- src/actions.rs | 245 +++++++++++++++++++++++++++++++++++++++++++++- src/config.rs | 146 ++++++++++++++++++++++++++- src/deploy.rs | 170 +++++++++++++++++++++++++++++++- src/filesystem.rs | 95 +++++++++++++++++- src/init.rs | 1 + 5 files changed, 647 insertions(+), 10 deletions(-) diff --git a/src/actions.rs b/src/actions.rs index c2a6887..9011127 100644 --- a/src/actions.rs +++ b/src/actions.rs @@ -5,9 +5,9 @@ use anyhow::{Context, Result}; use crossterm::style::Stylize; use handlebars::Handlebars; -use crate::config::{SymbolicTarget, TemplateTarget, Variables}; +use crate::config::{CopyTarget, SymbolicTarget, TemplateTarget, Variables}; use crate::difference::{self, diff_nonempty, generate_template_diff, print_diff}; -use crate::filesystem::{Filesystem, SymlinkComparison, TemplateComparison}; +use crate::filesystem::{CopyComparison, Filesystem, SymlinkComparison, TemplateComparison}; #[cfg_attr(test, mockall::automock)] pub trait ActionRunner { @@ -27,6 +27,9 @@ pub trait ActionRunner { cache: &Path, target: &TemplateTarget, ) -> Result; + fn delete_copy(&mut self, source: &Path, target: &Path) -> Result; + fn create_copy(&mut self, source: &Path, target: &CopyTarget) -> Result; + fn update_copy(&mut self, source: &Path, target: &CopyTarget) -> Result; } pub struct RealActionRunner<'a> { @@ -101,6 +104,15 @@ impl ActionRunner for RealActionRunner<'_> { self.diff_context_lines, ) } + fn delete_copy(&mut self, source: &Path, target: &Path) -> Result { + delete_copy(source, target, self.fs, self.force) + } + fn create_copy(&mut self, source: &Path, target: &CopyTarget) -> Result { + create_copy(source, target, self.fs, self.force) + } + fn update_copy(&mut self, source: &Path, target: &CopyTarget) -> Result { + update_copy(source, target, self.fs, self.force) + } } // == DELETE == @@ -218,6 +230,55 @@ pub fn delete_template( } } +/// Returns true if the copy should be removed from cache. +pub fn delete_copy( + source: &Path, + target: &Path, + fs: &mut dyn Filesystem, + force: bool, +) -> Result { + info!("{} copy {:?} -> {:?}", "[-]".red(), source, target); + + let comparison = fs + .compare_copy(source, target) + .context("detect copy's current state")?; + debug!("Current state: {}", comparison); + + match comparison { + CopyComparison::Identical => { + debug!("Performing deletion"); + fs.remove_file(target).context("remove copy target")?; + fs.delete_parents(target, false) + .context("delete parents of copy target")?; + Ok(true) + } + CopyComparison::TargetNotPresent | CopyComparison::SourceNotPresent => { + warn!( + "Deleting copy {:?} -> {:?} but target doesn't exist. Removing from cache anyways.", + source, target + ); + Ok(true) + } + CopyComparison::Changed | CopyComparison::TargetNotRegularFile if force => { + warn!( + "Deleting copy {:?} -> {:?} but {}. Forcing.", + source, target, comparison + ); + fs.remove_file(target).context("remove copy target")?; + fs.delete_parents(target, false) + .context("delete parents of copy target")?; + Ok(true) + } + CopyComparison::Changed | CopyComparison::TargetNotRegularFile => { + error!( + "Deleting copy {:?} -> {:?} but {}. Skipping.", + source, target, comparison + ); + Ok(false) + } + } +} + fn perform_cache_deletion(fs: &mut dyn Filesystem, cache: &Path) -> Result<()> { fs.remove_file(cache).context("delete template cache")?; fs.delete_parents(cache, true) @@ -392,6 +453,82 @@ pub fn create_template( } } +/// Returns true if the copy should be added to cache. +pub fn create_copy( + source: &Path, + target: &CopyTarget, + fs: &mut dyn Filesystem, + force: bool, +) -> Result { + info!("{} copy {:?} -> {:?}", "[+]".green(), source, target.target); + + let comparison = fs + .compare_copy(source, &target.target) + .context("detect copy's current state")?; + debug!("Current state: {}", comparison); + + match comparison { + CopyComparison::TargetNotPresent => { + debug!("Performing creation"); + fs.create_dir_all( + target + .target + .parent() + .context("get parent of target file")?, + &target.owner, + ) + .context("create parent for target file")?; + fs.copy_file(source, &target.target, &target.owner) + .context("copy source to target")?; + fs.copy_permissions(source, &target.target, &target.owner) + .context("copy permissions from source to target")?; + Ok(true) + } + CopyComparison::Identical => { + warn!( + "Creating copy {:?} -> {:?} but target already exists and matches source. Adding to cache anyways.", + source, target.target + ); + Ok(true) + } + CopyComparison::SourceNotPresent => { + error!( + "Creating copy {:?} -> {:?} but source is missing. Skipping.", + source, target.target + ); + Ok(false) + } + CopyComparison::Changed | CopyComparison::TargetNotRegularFile if force => { + warn!( + "Creating copy {:?} -> {:?} but {}. Forcing.", + source, target.target, comparison + ); + fs.remove_file(&target.target) + .context("remove existing target while forcing")?; + fs.create_dir_all( + target + .target + .parent() + .context("get parent of target file")?, + &target.owner, + ) + .context("create parent for target file")?; + fs.copy_file(source, &target.target, &target.owner) + .context("copy source to target")?; + fs.copy_permissions(source, &target.target, &target.owner) + .context("copy permissions from source to target")?; + Ok(true) + } + CopyComparison::Changed | CopyComparison::TargetNotRegularFile => { + error!( + "Creating copy {:?} -> {:?} but {}. Skipping.", + source, target.target, comparison + ); + Ok(false) + } + } +} + // == UPDATE == /// Returns true if the symlink wasn't skipped @@ -567,6 +704,110 @@ pub fn update_template( } } +/// Returns true if the copy was not skipped. +pub fn update_copy( + source: &Path, + target: &CopyTarget, + fs: &mut dyn Filesystem, + force: bool, +) -> Result { + debug!("Updating copy {:?} -> {:?}...", source, target.target); + + let comparison = fs + .compare_copy(source, &target.target) + .context("detect copy's current state")?; + debug!("Current state: {}", comparison); + + match comparison { + CopyComparison::Identical => { + debug!("Already up to date"); + fs.set_owner(&target.target, &target.owner) + .context("set target file owner")?; + fs.copy_permissions(source, &target.target, &target.owner) + .context("copy permissions from source to target")?; + Ok(true) + } + CopyComparison::TargetNotPresent => { + warn!( + "Updating copy {:?} -> {:?} but target is missing. Creating it anyways.", + source, target.target + ); + fs.create_dir_all( + target + .target + .parent() + .context("get parent of target file")?, + &target.owner, + ) + .context("create parent for target file")?; + fs.copy_file(source, &target.target, &target.owner) + .context("copy source to target")?; + fs.copy_permissions(source, &target.target, &target.owner) + .context("copy permissions from source to target")?; + Ok(true) + } + CopyComparison::SourceNotPresent => { + error!( + "Updating copy {:?} -> {:?} but source is missing. Skipping.", + source, target.target + ); + Ok(false) + } + CopyComparison::Changed => { + // Source was updated; the target is stale. Re-copy to sync. + info!( + "{} copy {:?} -> {:?} (source changed)", + "[~]".yellow(), + source, + target.target + ); + fs.remove_file(&target.target) + .context("remove stale copy target")?; + fs.create_dir_all( + target + .target + .parent() + .context("get parent of target file")?, + &target.owner, + ) + .context("create parent for target file")?; + fs.copy_file(source, &target.target, &target.owner) + .context("copy source to target")?; + fs.copy_permissions(source, &target.target, &target.owner) + .context("copy permissions from source to target")?; + Ok(true) + } + CopyComparison::TargetNotRegularFile if force => { + warn!( + "Updating copy {:?} -> {:?} but {}. Forcing.", + source, target.target, comparison + ); + fs.remove_file(&target.target) + .context("remove target while forcing")?; + fs.create_dir_all( + target + .target + .parent() + .context("get parent of target file")?, + &target.owner, + ) + .context("create parent for target file")?; + fs.copy_file(source, &target.target, &target.owner) + .context("copy source to target")?; + fs.copy_permissions(source, &target.target, &target.owner) + .context("copy permissions from source to target")?; + Ok(true) + } + CopyComparison::TargetNotRegularFile => { + error!( + "Updating copy {:?} -> {:?} but {}. Skipping.", + source, target.target, comparison + ); + Ok(false) + } + } +} + pub(crate) fn perform_template_deploy( source: &Path, cache: &Path, diff --git a/src/config.rs b/src/config.rs index aef922b..dfdce42 100644 --- a/src/config.rs +++ b/src/config.rs @@ -45,6 +45,16 @@ pub struct TemplateTarget { pub condition: Option, } +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)] +#[serde(deny_unknown_fields)] +pub struct CopyTarget { + pub target: PathBuf, + pub owner: Option, + pub recurse: Option, + #[serde(rename = "if")] + pub condition: Option, +} + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)] #[serde(from = "FileTargetOuterRepr", into = "FileTargetOuterRepr")] pub enum FileTarget { @@ -52,6 +62,7 @@ pub enum FileTarget { Symbolic(SymbolicTarget), #[serde(rename = "template")] ComplexTemplate(TemplateTarget), + Copy(CopyTarget), } // Shims to allow Serde to represent FileTarget::Automatic as untagged while the @@ -69,6 +80,7 @@ enum FileTargetInnerRepr { Symbolic(SymbolicTarget), #[serde(rename = "template")] ComplexTemplate(TemplateTarget), + Copy(CopyTarget), } pub type Files = BTreeMap; @@ -81,6 +93,7 @@ pub type Helpers = BTreeMap; pub enum DefaultTargetType { Symbolic, Template, + Copy, #[default] Automatic, } @@ -210,6 +223,8 @@ pub fn load_configuration( pub struct Cache { pub symlinks: BTreeMap, pub templates: BTreeMap, + #[serde(default)] + pub copies: BTreeMap, } pub fn save_dummy_config( @@ -400,6 +415,7 @@ fn merge_configuration_files( DefaultTargetType::Template => { FileTarget::ComplexTemplate(TemplateTarget::from(target.clone())) } + DefaultTargetType::Copy => FileTarget::Copy(CopyTarget::from(target.clone())), _ => continue, }; } @@ -426,7 +442,8 @@ impl FileTarget { match self { FileTarget::Automatic(path) => path, FileTarget::Symbolic(SymbolicTarget { target, .. }) - | FileTarget::ComplexTemplate(TemplateTarget { target, .. }) => target, + | FileTarget::ComplexTemplate(TemplateTarget { target, .. }) + | FileTarget::Copy(CopyTarget { target, .. }) => target, } } @@ -434,7 +451,8 @@ impl FileTarget { match self { FileTarget::Automatic(path) => *path = new_path.into(), FileTarget::Symbolic(SymbolicTarget { target, .. }) - | FileTarget::ComplexTemplate(TemplateTarget { target, .. }) => { + | FileTarget::ComplexTemplate(TemplateTarget { target, .. }) + | FileTarget::Copy(CopyTarget { target, .. }) => { *target = new_path.into(); } } @@ -444,7 +462,8 @@ impl FileTarget { match self { FileTarget::Automatic(_) => None, FileTarget::Symbolic(SymbolicTarget { condition, .. }) - | FileTarget::ComplexTemplate(TemplateTarget { condition, .. }) => condition.as_ref(), + | FileTarget::ComplexTemplate(TemplateTarget { condition, .. }) + | FileTarget::Copy(CopyTarget { condition, .. }) => condition.as_ref(), } } } @@ -463,6 +482,7 @@ impl From for FileTarget { OR::Simple(x) => Self::Automatic(x), OR::Complex(IR::Symbolic(x)) => Self::Symbolic(x), OR::Complex(IR::ComplexTemplate(x)) => Self::ComplexTemplate(x), + OR::Complex(IR::Copy(x)) => Self::Copy(x), } } } @@ -474,6 +494,7 @@ impl From for FileTargetOuterRepr { FileTarget::Automatic(x) => Self::Simple(x), FileTarget::Symbolic(x) => Self::Complex(IR::Symbolic(x)), FileTarget::ComplexTemplate(x) => Self::Complex(IR::ComplexTemplate(x)), + FileTarget::Copy(x) => Self::Complex(IR::Copy(x)), } } } @@ -501,6 +522,17 @@ impl> From for TemplateTarget { } } +impl> From for CopyTarget { + fn from(input: T) -> Self { + CopyTarget { + target: input.into(), + owner: None, + condition: None, + recurse: None, + } + } +} + impl SymbolicTarget { pub fn into_template(self) -> TemplateTarget { TemplateTarget { @@ -552,6 +584,12 @@ fn expand_directory(source: &Path, target: &FileTarget, config: &Configuration) condition: _, recurse: Some(rec), }) => *rec, + FileTarget::Copy(CopyTarget { + target: _, + owner: _, + condition: _, + recurse: Some(rec), + }) => *rec, _ => config.recurse, }; @@ -665,6 +703,51 @@ mod test { .unwrap_err(); } + #[test] + fn deserialize_copy_file_target() { + #[derive(Debug, Deserialize)] + struct Helper { + file: FileTarget, + } + let parse = toml::from_str::; + + assert_eq!( + parse( + r#" + [file] + target = '~/.QuarticCat' + type = 'copy' + "#, + ) + .unwrap() + .file, + FileTarget::Copy(PathBuf::from("~/.QuarticCat").into()), + ); + assert_ne!( + parse( + r#" + [file] + target = '~/.QuarticCat' + type = 'copy' + if = 'bash' + "#, + ) + .unwrap() + .file, + FileTarget::Copy(PathBuf::from("~/.QuarticCat").into()), + ); + // copy type rejects template-only fields + parse( + r#" + [file] + target = '~/.QuarticCat' + type = 'copy' + append = 'whatever' + "#, + ) + .unwrap_err(); + } + #[test] fn settting_default_target_type_symbolic() { let global: GlobalConfig = toml::from_str( @@ -771,6 +854,63 @@ mod test { ); } + #[test] + fn setting_default_target_type_copy() { + let global: GlobalConfig = toml::from_str( + r#" + [settings] + default_target_type = "copy" + + [cat] + depends = [] + + [cat.files] + cat = '~/.QuarticCat' + + [derby] + depends = [] + + [derby.files] + derby = { target = '~/.DerbyLantern', type = 'template' } + + [sliver] + depends = [] + + [sliver.files] + sliver = { target = '~/.SliverBodacious', type = 'symbolic' } + "#, + ) + .unwrap(); + + let local: LocalConfig = toml::from_str( + r#" + packages = ['cat', 'derby', 'sliver'] + "#, + ) + .unwrap(); + + let config = merge_configuration_files(global, local, None).unwrap(); + + let cat = config.files.get(&PathBuf::from("cat")).unwrap(); + let derby = config.files.get(&PathBuf::from("derby")).unwrap(); + let sliver = config.files.get(&PathBuf::from("sliver")).unwrap(); + + // Automatic targets become Copy when default_target_type = "copy" + assert_eq!( + cat, + &FileTarget::Copy(PathBuf::from("~/.QuarticCat").into()) + ); + // Explicitly typed files are unaffected + assert_eq!( + derby, + &FileTarget::ComplexTemplate(PathBuf::from("~/.DerbyLantern").into()) + ); + assert_eq!( + sliver, + &FileTarget::Symbolic(PathBuf::from("~/.SliverBodacious").into()) + ); + } + #[test] fn inline_table_with_newlines() { // TOML 1.1 allows inline tables to span multiple lines (trailing commas diff --git a/src/deploy.rs b/src/deploy.rs index 5901e67..53f3043 100644 --- a/src/deploy.rs +++ b/src/deploy.rs @@ -7,7 +7,7 @@ use std::path::PathBuf; use crate::actions::{self, ActionRunner, RealActionRunner}; use crate::args::Options; -use crate::config::{self, Cache, FileTarget, SymbolicTarget, TemplateTarget}; +use crate::config::{self, Cache, CopyTarget, FileTarget, SymbolicTarget, TemplateTarget}; use crate::display_error; use crate::filesystem::{self, Filesystem, load_file}; use crate::handlebars_helpers::create_new_handlebars; @@ -79,6 +79,7 @@ Proceeding by copying instead of symlinking." let mut desired_symlinks = BTreeMap::::new(); let mut desired_templates = BTreeMap::::new(); + let mut desired_copies = BTreeMap::::new(); for (source, target) in config.files { if symlinks_enabled { @@ -98,6 +99,9 @@ Proceeding by copying instead of symlinking." FileTarget::ComplexTemplate(target) => { desired_templates.insert(source, target); } + FileTarget::Copy(target) => { + desired_copies.insert(source, target); + } } } else { match target { @@ -110,6 +114,9 @@ Proceeding by copying instead of symlinking." FileTarget::ComplexTemplate(target) => { desired_templates.insert(source, target); } + FileTarget::Copy(target) => { + desired_copies.insert(source, target); + } } } } @@ -128,6 +135,7 @@ Proceeding by copying instead of symlinking." &mut runner, &desired_symlinks, &desired_templates, + &desired_copies, &mut cache, opt, ); @@ -222,6 +230,16 @@ pub fn undeploy(opt: &Options) -> Result { ); } + for (deleted_copy, target) in cache.copies.clone() { + execute_action( + actions::delete_copy(&deleted_copy, &target, fs, opt.force), + || cache.copies.remove(&deleted_copy), + || format!("delete copy {deleted_copy:?} -> {target:?}"), + &mut suggest_force, + &mut error_occurred, + ); + } + // === Post-undeploy === if suggest_force { @@ -255,6 +273,7 @@ fn run_deploy( runner: &mut A, desired_symlinks: &BTreeMap, desired_templates: &BTreeMap, + desired_copies: &BTreeMap, cache: &mut Cache, opt: &Options, ) -> (bool, bool) { @@ -272,6 +291,11 @@ fn run_deploy( .iter() .map(|(k, v)| (k.clone(), v.clone())) .collect(); + let existing_copies: BTreeSet<(PathBuf, PathBuf)> = cache + .copies + .iter() + .map(|(k, v)| (k.clone(), v.clone())) + .collect(); let desired_symlinks: BTreeMap<(PathBuf, PathBuf), _> = desired_symlinks .iter() @@ -281,6 +305,10 @@ fn run_deploy( .iter() .map(|(k, v)| ((k.clone(), v.target.clone()), v)) .collect(); + let desired_copies_keyed: BTreeMap<(PathBuf, PathBuf), &CopyTarget> = desired_copies + .iter() + .map(|(k, v)| ((k.clone(), v.target.clone()), v)) + .collect(); // Avoid modifying cache while iterating over it let mut resulting_cache = cache.clone(); @@ -309,6 +337,18 @@ fn run_deploy( ); } + for (source, target) in + existing_copies.difference(&desired_copies_keyed.keys().cloned().collect()) + { + execute_action( + runner.delete_copy(source, target), + || resulting_cache.copies.remove(source), + || format!("delete copy {source:?} -> {target:?}"), + &mut suggest_force, + &mut error_occurred, + ); + } + for (source, target_path) in desired_symlinks .keys() .cloned() @@ -353,6 +393,28 @@ fn run_deploy( ); } + for (source, target_path) in desired_copies_keyed + .keys() + .cloned() + .collect::>() + .difference(&existing_copies) + { + let target = desired_copies_keyed + .get(&(source.clone(), target_path.clone())) + .unwrap(); + execute_action( + runner.create_copy(source, target), + || { + resulting_cache + .copies + .insert(source.clone(), target_path.clone()) + }, + || format!("create copy {source:?} -> {target_path:?}"), + &mut suggest_force, + &mut error_occurred, + ); + } + for (source, target_path) in existing_symlinks.intersection(&desired_symlinks.keys().cloned().collect()) { @@ -383,6 +445,21 @@ fn run_deploy( ); } + for (source, target_path) in + existing_copies.intersection(&desired_copies_keyed.keys().cloned().collect()) + { + let target = desired_copies_keyed + .get(&(source.clone(), target_path.clone())) + .unwrap(); + execute_action( + runner.update_copy(source, target), + || (), + || format!("update copy {source:?} -> {target_path:?}"), + &mut suggest_force, + &mut error_occurred, + ); + } + *cache = resulting_cache; (suggest_force, error_occurred) @@ -412,7 +489,7 @@ fn execute_action T, E: FnOnce() -> String>( #[cfg(test)] mod test { - use crate::filesystem::{SymlinkComparison, TemplateComparison}; + use crate::filesystem::{CopyComparison, SymlinkComparison, TemplateComparison}; use std::path::{Path, PathBuf}; @@ -464,6 +541,7 @@ mod test { &mut runner, &desired_symlinks, &desired_templates, + &BTreeMap::new(), &mut cache, &Options { cache_directory: "cache".into(), @@ -481,6 +559,47 @@ mod test { assert_eq!(cache.templates.len(), 1); } + #[test] + fn high_level_simple_copy() { + let c_out: config::CopyTarget = "c_out".into(); + + let desired_copies = maplit::btreemap! { + PathBuf::from("c_in") => c_out.clone() + }; + + let mut runner = actions::MockActionRunner::new(); + let mut seq = mockall::Sequence::new(); + let mut cache = Cache::default(); + + runner + .expect_create_copy() + .times(1) + .with( + mockall::predicate::function(|p: &Path| p == Path::new("c_in")), + mockall::predicate::eq(c_out), + ) + .in_sequence(&mut seq) + .returning(|_, _| Ok(true)); + + let (suggest_force, error_occurred) = run_deploy( + &mut runner, + &BTreeMap::new(), + &BTreeMap::new(), + &desired_copies, + &mut cache, + &Options { + cache_directory: "cache".into(), + force: false, + ..Options::default() + }, + ); + + assert!(!suggest_force); + assert!(!error_occurred); + assert!(cache.copies.contains_key(&PathBuf::from("c_in"))); + assert_eq!(cache.copies.len(), 1); + } + #[test] fn high_level_skip() { // Setup @@ -521,6 +640,7 @@ mod test { &mut runner, &desired_symlinks, &desired_templates, + &BTreeMap::new(), &mut cache, &Options { cache_directory: "cache".into(), @@ -552,6 +672,7 @@ mod test { PathBuf::from("a_in") => "a_out_old".into() }, templates: BTreeMap::new(), + copies: BTreeMap::new(), }; // Expectation @@ -573,6 +694,7 @@ mod test { &mut runner, &desired_symlinks, &BTreeMap::new(), + &BTreeMap::new(), &mut cache, &Options { cache_directory: "cache".into(), @@ -604,6 +726,7 @@ mod test { templates: maplit::btreemap! { PathBuf::from("a_in") => "a_out_old".into() }, + copies: BTreeMap::new(), }; // Expectation @@ -629,6 +752,7 @@ mod test { &mut runner, &desired_symlinks, &BTreeMap::new(), + &BTreeMap::new(), &mut cache, &Options { cache_directory: "cache".into(), @@ -659,6 +783,7 @@ mod test { templates: maplit::btreemap! { PathBuf::from("a_in") => "a_out_old".into() }, + copies: BTreeMap::new(), }; // Expectation @@ -678,6 +803,7 @@ mod test { &mut runner, &desired_symlinks, &BTreeMap::new(), + &BTreeMap::new(), &mut cache, &Options { cache_directory: "cache".into(), @@ -853,4 +979,44 @@ mod test { .unwrap() ); } + + #[test] + fn low_level_delete_copy() { + let mut fs = crate::filesystem::MockFilesystem::new(); + let mut seq = mockall::Sequence::new(); + + let opt = Options::default(); + let handlebars = handlebars::Handlebars::new(); + let variables = toml::map::Map::new(); + + // delete_copy: target exists and matches source → delete + fs.expect_compare_copy() + .times(1) + .with(function(path_eq("a_in")), function(path_eq("a_out"))) + .in_sequence(&mut seq) + .returning(|_, _| Ok(CopyComparison::Identical)); + fs.expect_remove_file() + .times(1) + .with(function(path_eq("a_out"))) + .in_sequence(&mut seq) + .returning(|_| Ok(())); + fs.expect_delete_parents() + .times(1) + .with(function(path_eq("a_out")), eq(false)) + .in_sequence(&mut seq) + .returning(|_, _| Ok(())); + + let mut runner = actions::RealActionRunner::new( + &mut fs, + &handlebars, + &variables, + opt.force, + opt.diff_context_lines, + ); + assert!( + runner + .delete_copy(&PathBuf::from("a_in"), &PathBuf::from("a_out")) + .unwrap() + ); + } } diff --git a/src/filesystem.rs b/src/filesystem.rs index 6a7c7be..a4357c3 100644 --- a/src/filesystem.rs +++ b/src/filesystem.rs @@ -49,6 +49,9 @@ pub trait Filesystem { /// Check state of expected symbolic link on disk fn compare_template(&mut self, target: &Path, cache: &Path) -> Result; + /// Check state of an expected copy on disk (source vs target, byte-for-byte). + fn compare_copy(&mut self, source: &Path, target: &Path) -> Result; + /// Removes a file or folder, elevating privileges if needed fn remove_file(&mut self, path: &Path) -> Result<()>; @@ -119,6 +122,10 @@ impl Filesystem for RealFilesystem { Ok(compare_template(target_state, cache_state)) } + fn compare_copy(&mut self, source: &Path, target: &Path) -> Result { + compare_copy(source, target) + } + fn remove_file(&mut self, path: &Path) -> Result<()> { let metadata = path.symlink_metadata().context("get metadata")?; if metadata.is_dir() { @@ -283,6 +290,10 @@ impl Filesystem for RealFilesystem { Ok(compare_template(target_state, cache_state)) } + fn compare_copy(&mut self, source: &Path, target: &Path) -> Result { + compare_copy(source, target) + } + fn remove_file(&mut self, path: &Path) -> Result<()> { let metadata = path.symlink_metadata().context("get metadata")?; let result = if metadata.is_dir() { @@ -422,8 +433,8 @@ impl Filesystem for RealFilesystem { use std::io::Write; if let Some(owner) = owner { - let contents = std::fs::read_to_string(source) - .context("read source file contents as current user")?; + let contents = + std::fs::read(source).context("read source file contents as current user")?; let mut child = self .sudo(format!( "Copying {source:?} -> {target:?} as user {owner:?}" @@ -443,7 +454,7 @@ impl Filesystem for RealFilesystem { .stdin .as_ref() .expect("has stdin") - .write_all(contents.as_bytes()) + .write_all(&contents) .context("give input to tee")?; let success = child.wait().context("wait for sudo tee")?.success(); @@ -597,6 +608,29 @@ impl Filesystem for DryRunFilesystem { Ok(compare_template(target_state, cache_state)) } + fn compare_copy(&mut self, source: &Path, target: &Path) -> Result { + let source_state = self.get_state(source).context("get source state")?; + let target_state = self.get_state(target).context("get target state")?; + + Ok(match (source_state, target_state) { + (FileState::Missing, _) => CopyComparison::SourceNotPresent, + (_, FileState::Missing) => CopyComparison::TargetNotPresent, + (_, FileState::SymbolicLink(_) | FileState::Directory) => { + CopyComparison::TargetNotRegularFile + } + (FileState::File(s), FileState::File(t)) => { + if s == t { + CopyComparison::Identical + } else { + CopyComparison::Changed + } + } + (FileState::SymbolicLink(_) | FileState::Directory, _) => { + CopyComparison::SourceNotPresent + } + }) + } + fn remove_file(&mut self, path: &Path) -> Result<()> { debug!("Removing file {:?}", path); self.file_states.insert(path.into(), FileState::Missing); @@ -779,6 +813,34 @@ impl std::fmt::Display for TemplateComparison { } } +#[derive(Debug, PartialEq, Eq)] +pub enum CopyComparison { + /// Target is a regular file and its content matches source byte-for-byte. + Identical, + /// Target is a regular file but its content differs from source. + Changed, + /// Target does not exist. + TargetNotPresent, + /// Source file does not exist. + SourceNotPresent, + /// Target exists but is not a regular file (e.g. a symlink or directory). + TargetNotRegularFile, +} + +impl std::fmt::Display for CopyComparison { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { + use self::CopyComparison::*; + match self { + Identical => "target matches source", + Changed => "target differs from source", + TargetNotPresent => "target missing", + SourceNotPresent => "source is missing", + TargetNotRegularFile => "target already exists and isn't a regular file", + } + .fmt(f) + } +} + fn compare_template(target_state: FileState, cache_state: FileState) -> TemplateComparison { match (target_state, cache_state) { (FileState::File(t), FileState::File(c)) => { @@ -795,6 +857,33 @@ fn compare_template(target_state: FileState, cache_state: FileState) -> Template } } +fn compare_copy(source: &Path, target: &Path) -> Result { + let source_state = get_file_state(source).context("get source state")?; + let target_state = get_file_state(target).context("get target state")?; + + Ok(match (source_state, target_state) { + (FileState::Missing, _) => CopyComparison::SourceNotPresent, + (_, FileState::Missing) => CopyComparison::TargetNotPresent, + (_, FileState::SymbolicLink(_) | FileState::Directory) => { + CopyComparison::TargetNotRegularFile + } + (FileState::File(_), FileState::File(_)) => { + // Re-read as bytes for a true binary comparison + let source_bytes = std::fs::read(source).context("read source bytes")?; + let target_bytes = std::fs::read(target).context("read target bytes")?; + if source_bytes == target_bytes { + CopyComparison::Identical + } else { + CopyComparison::Changed + } + } + (FileState::SymbolicLink(_) | FileState::Directory, _) => { + // Source is not a regular file — treat as source not present + CopyComparison::SourceNotPresent + } + }) +} + // === Utility functions === pub fn real_path(path: &Path) -> Result { let path = std::fs::canonicalize(path)?; diff --git a/src/init.rs b/src/init.rs index bacb0bf..b848c60 100644 --- a/src/init.rs +++ b/src/init.rs @@ -43,6 +43,7 @@ pub fn init(opt: Options) -> Result<()> { config::Cache { symlinks: BTreeMap::default(), templates: BTreeMap::default(), + copies: BTreeMap::default(), }, ) .context("save empty cache file")?; From 81693cb43d641c047bc941122bb4e7270dc90a81 Mon Sep 17 00:00:00 2001 From: JP-Ellis Date: Wed, 22 Apr 2026 10:17:53 +1000 Subject: [PATCH 2/2] feat: improve cache of copy targets For templates and symlinks, it is easy to see if the target has changed (e.g., symlink is destroyed, or the rendered template differs to the target). This is not as easy to do with copy target; if the source and target differ, it is unclear whether the source was modified and the target is stale, or the target was modified outside of dotter (or both). This changes the caching so that it stores a hash (using `xxhash`) of both source and target. On redeployment, the hashes can be compared and dotter can determine whether it is safe to override the target file. Assisted-by: Claude Code:claude-sonnet-4-6 Signed-off-by: JP-Ellis --- Cargo.lock | 7 + Cargo.toml | 1 + src/actions.rs | 330 ++++++++++++++----------- src/config.rs | 13 +- src/deploy.rs | 595 +++++++++++++++++++++++++++++++++++++++++++--- src/filesystem.rs | 112 ++++----- 6 files changed, 808 insertions(+), 250 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2046005..e267dfb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -518,6 +518,7 @@ dependencies = [ "watchexec", "watchexec-events", "watchexec-filterer-globset", + "xxhash-rust", ] [[package]] @@ -2805,6 +2806,12 @@ version = "0.51.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d7249219f66ced02969388cf2bb044a09756a083d0fab1e566056b04d9fbcaa5" +[[package]] +name = "xxhash-rust" +version = "0.8.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fdd20c5420375476fbd4394763288da7eb0cc0b8c11deed431a91562af7335d3" + [[package]] name = "zerocopy" version = "0.8.39" diff --git a/Cargo.toml b/Cargo.toml index 1f2d889..3463be0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,6 +22,7 @@ hostname = "0.3.*" log = "0.4.*" maplit = "1.*" evalexpr = "13" +xxhash-rust = { version = "0.8", features = ["xxh3"] } serde = { version = "1.*", features = ["derive"] } shellexpand = "2.*" simplelog = "0.12.*" diff --git a/src/actions.rs b/src/actions.rs index 9011127..0509c72 100644 --- a/src/actions.rs +++ b/src/actions.rs @@ -5,9 +5,9 @@ use anyhow::{Context, Result}; use crossterm::style::Stylize; use handlebars::Handlebars; -use crate::config::{CopyTarget, SymbolicTarget, TemplateTarget, Variables}; +use crate::config::{CopyEntry, CopyTarget, SymbolicTarget, TemplateTarget, Variables}; use crate::difference::{self, diff_nonempty, generate_template_diff, print_diff}; -use crate::filesystem::{CopyComparison, Filesystem, SymlinkComparison, TemplateComparison}; +use crate::filesystem::{FileHash, Filesystem, SymlinkComparison, TemplateComparison}; #[cfg_attr(test, mockall::automock)] pub trait ActionRunner { @@ -27,9 +27,14 @@ pub trait ActionRunner { cache: &Path, target: &TemplateTarget, ) -> Result; - fn delete_copy(&mut self, source: &Path, target: &Path) -> Result; - fn create_copy(&mut self, source: &Path, target: &CopyTarget) -> Result; - fn update_copy(&mut self, source: &Path, target: &CopyTarget) -> Result; + fn delete_copy(&mut self, source: &Path, cached: &CopyEntry) -> Result; + fn create_copy(&mut self, source: &Path, target: &CopyTarget) -> Result>; + fn update_copy( + &mut self, + source: &Path, + target: &CopyTarget, + cached: &CopyEntry, + ) -> Result>; } pub struct RealActionRunner<'a> { @@ -104,14 +109,19 @@ impl ActionRunner for RealActionRunner<'_> { self.diff_context_lines, ) } - fn delete_copy(&mut self, source: &Path, target: &Path) -> Result { - delete_copy(source, target, self.fs, self.force) + fn delete_copy(&mut self, source: &Path, cached: &CopyEntry) -> Result { + delete_copy(source, cached, self.fs, self.force) } - fn create_copy(&mut self, source: &Path, target: &CopyTarget) -> Result { + fn create_copy(&mut self, source: &Path, target: &CopyTarget) -> Result> { create_copy(source, target, self.fs, self.force) } - fn update_copy(&mut self, source: &Path, target: &CopyTarget) -> Result { - update_copy(source, target, self.fs, self.force) + fn update_copy( + &mut self, + source: &Path, + target: &CopyTarget, + cached: &CopyEntry, + ) -> Result> { + update_copy(source, target, cached, self.fs, self.force) } } @@ -230,49 +240,51 @@ pub fn delete_template( } } -/// Returns true if the copy should be removed from cache. +/// Returns true if the copy was deleted and should be removed from cache. pub fn delete_copy( source: &Path, - target: &Path, + cached: &CopyEntry, fs: &mut dyn Filesystem, force: bool, ) -> Result { - info!("{} copy {:?} -> {:?}", "[-]".red(), source, target); + info!("{} copy {:?} -> {:?}", "[-]".red(), source, cached.target); - let comparison = fs - .compare_copy(source, target) - .context("detect copy's current state")?; - debug!("Current state: {}", comparison); + let target_hash = fs + .checksum_file(&cached.target) + .context("checksum copy target")?; + debug!("Target hash: {}", target_hash); - match comparison { - CopyComparison::Identical => { - debug!("Performing deletion"); - fs.remove_file(target).context("remove copy target")?; - fs.delete_parents(target, false) - .context("delete parents of copy target")?; - Ok(true) - } - CopyComparison::TargetNotPresent | CopyComparison::SourceNotPresent => { + match target_hash { + FileHash::NotPresent => { warn!( "Deleting copy {:?} -> {:?} but target doesn't exist. Removing from cache anyways.", - source, target + source, cached.target ); Ok(true) } - CopyComparison::Changed | CopyComparison::TargetNotRegularFile if force => { + FileHash::Hash(h) if h == cached.target_checksum => { + debug!("Target unchanged, deleting"); + fs.remove_file(&cached.target) + .context("remove copy target")?; + fs.delete_parents(&cached.target, false) + .context("delete parents of copy target")?; + Ok(true) + } + FileHash::Hash(_) | FileHash::NotRegularFile if force => { warn!( - "Deleting copy {:?} -> {:?} but {}. Forcing.", - source, target, comparison + "Deleting copy {:?} -> {:?} but target was externally modified or is not a regular file. Forcing.", + source, cached.target ); - fs.remove_file(target).context("remove copy target")?; - fs.delete_parents(target, false) + fs.remove_file(&cached.target) + .context("remove copy target")?; + fs.delete_parents(&cached.target, false) .context("delete parents of copy target")?; Ok(true) } - CopyComparison::Changed | CopyComparison::TargetNotRegularFile => { + FileHash::Hash(_) | FileHash::NotRegularFile => { error!( - "Deleting copy {:?} -> {:?} but {}. Skipping.", - source, target, comparison + "Deleting copy {:?} -> {:?} but target was externally modified or is not a regular file. Skipping.", + source, cached.target ); Ok(false) } @@ -453,22 +465,34 @@ pub fn create_template( } } -/// Returns true if the copy should be added to cache. +/// Returns `Some(entry)` if the copy was deployed and should be added to cache, +/// or `None` if it was skipped. pub fn create_copy( source: &Path, target: &CopyTarget, fs: &mut dyn Filesystem, force: bool, -) -> Result { +) -> Result> { info!("{} copy {:?} -> {:?}", "[+]".green(), source, target.target); - let comparison = fs - .compare_copy(source, &target.target) - .context("detect copy's current state")?; - debug!("Current state: {}", comparison); + let source_hash = fs.checksum_file(source).context("checksum source")?; + let source_hash = match source_hash { + FileHash::Hash(h) => h, + _ => { + error!( + "Creating copy {:?} -> {:?} but source is missing or not a regular file. Skipping.", + source, target.target + ); + return Ok(None); + } + }; - match comparison { - CopyComparison::TargetNotPresent => { + let target_hash = fs + .checksum_file(&target.target) + .context("checksum target")?; + + match target_hash { + FileHash::NotPresent => { debug!("Performing creation"); fs.create_dir_all( target @@ -482,29 +506,31 @@ pub fn create_copy( .context("copy source to target")?; fs.copy_permissions(source, &target.target, &target.owner) .context("copy permissions from source to target")?; - Ok(true) - } - CopyComparison::Identical => { + // After copy, source and target are byte-identical + Ok(Some(CopyEntry { + target: target.target.clone(), + source_checksum: source_hash, + target_checksum: source_hash, + })) + } + FileHash::Hash(h) if h == source_hash => { warn!( - "Creating copy {:?} -> {:?} but target already exists and matches source. Adding to cache anyways.", + "Creating copy {:?} -> {:?} but target already matches source. Adding to cache.", source, target.target ); - Ok(true) + Ok(Some(CopyEntry { + target: target.target.clone(), + source_checksum: source_hash, + target_checksum: h, + })) } - CopyComparison::SourceNotPresent => { - error!( - "Creating copy {:?} -> {:?} but source is missing. Skipping.", - source, target.target - ); - Ok(false) - } - CopyComparison::Changed | CopyComparison::TargetNotRegularFile if force => { + FileHash::Hash(_) | FileHash::NotRegularFile if force => { warn!( - "Creating copy {:?} -> {:?} but {}. Forcing.", - source, target.target, comparison + "Creating copy {:?} -> {:?} but target exists with different content. Forcing.", + source, target.target ); fs.remove_file(&target.target) - .context("remove existing target while forcing")?; + .context("remove existing target")?; fs.create_dir_all( target .target @@ -517,14 +543,18 @@ pub fn create_copy( .context("copy source to target")?; fs.copy_permissions(source, &target.target, &target.owner) .context("copy permissions from source to target")?; - Ok(true) + Ok(Some(CopyEntry { + target: target.target.clone(), + source_checksum: source_hash, + target_checksum: source_hash, + })) } - CopyComparison::Changed | CopyComparison::TargetNotRegularFile => { + FileHash::Hash(_) | FileHash::NotRegularFile => { error!( - "Creating copy {:?} -> {:?} but {}. Skipping.", - source, target.target, comparison + "Creating copy {:?} -> {:?} but target exists with different content. Skipping.", + source, target.target ); - Ok(false) + Ok(None) } } } @@ -704,106 +734,122 @@ pub fn update_template( } } -/// Returns true if the copy was not skipped. +/// Copies `source` to `cached.target`, creating parent directories as needed. +/// Returns a fresh `CopyEntry` with both checksums set to `source_hash`. +fn perform_copy_update( + source: &Path, + cached: &CopyEntry, + target: &CopyTarget, + fs: &mut dyn Filesystem, + source_hash: u64, +) -> Result> { + debug_assert!( + cached.target == target.target, + "cached.target {:?} must match target.target {:?}", + cached.target, + target.target + ); + fs.create_dir_all( + cached + .target + .parent() + .context("get parent of target file")?, + &target.owner, + ) + .context("create parent for target file")?; + fs.copy_file(source, &cached.target, &target.owner) + .context("copy source to target")?; + fs.copy_permissions(source, &cached.target, &target.owner) + .context("copy permissions from source to target")?; + Ok(Some(CopyEntry { + target: cached.target.clone(), + source_checksum: source_hash, + target_checksum: source_hash, + })) +} + +/// Returns `Some(entry)` with updated checksums if the copy was processed successfully, +/// or `None` if it was skipped due to an external modification. pub fn update_copy( source: &Path, target: &CopyTarget, + cached: &CopyEntry, fs: &mut dyn Filesystem, force: bool, -) -> Result { - debug!("Updating copy {:?} -> {:?}...", source, target.target); +) -> Result> { + debug!("Updating copy {:?} -> {:?}...", source, cached.target); - let comparison = fs - .compare_copy(source, &target.target) - .context("detect copy's current state")?; - debug!("Current state: {}", comparison); - - match comparison { - CopyComparison::Identical => { - debug!("Already up to date"); - fs.set_owner(&target.target, &target.owner) - .context("set target file owner")?; - fs.copy_permissions(source, &target.target, &target.owner) - .context("copy permissions from source to target")?; - Ok(true) - } - CopyComparison::TargetNotPresent => { - warn!( - "Updating copy {:?} -> {:?} but target is missing. Creating it anyways.", - source, target.target - ); - fs.create_dir_all( - target - .target - .parent() - .context("get parent of target file")?, - &target.owner, - ) - .context("create parent for target file")?; - fs.copy_file(source, &target.target, &target.owner) - .context("copy source to target")?; - fs.copy_permissions(source, &target.target, &target.owner) - .context("copy permissions from source to target")?; - Ok(true) - } - CopyComparison::SourceNotPresent => { + let source_hash = fs.checksum_file(source).context("checksum source")?; + let source_hash = match source_hash { + FileHash::Hash(h) => h, + _ => { error!( "Updating copy {:?} -> {:?} but source is missing. Skipping.", - source, target.target + source, cached.target ); - Ok(false) + return Ok(None); } - CopyComparison::Changed => { - // Source was updated; the target is stale. Re-copy to sync. + }; + + let target_hash = fs + .checksum_file(&cached.target) + .context("checksum target")?; + debug!("Source hash: {source_hash:#018x}, target hash: {target_hash}"); + + let source_changed = source_hash != cached.source_checksum; + let target_changed = !matches!(&target_hash, FileHash::Hash(h) if *h == cached.target_checksum); + + match (source_changed, target_changed, &target_hash) { + (false, false, _) => { + // Nothing changed: ensure owner/perms are correct + debug!("Already up to date"); + fs.set_owner(&cached.target, &target.owner) + .context("set target file owner")?; + fs.copy_permissions(source, &cached.target, &target.owner) + .context("copy permissions from source to target")?; + Ok(Some(CopyEntry { + target: cached.target.clone(), + source_checksum: source_hash, + target_checksum: cached.target_checksum, + })) + } + (true, false, _) => { + // Source updated, target untouched: re-copy info!( "{} copy {:?} -> {:?} (source changed)", "[~]".yellow(), source, - target.target + cached.target ); - fs.remove_file(&target.target) + fs.remove_file(&cached.target) .context("remove stale copy target")?; - fs.create_dir_all( - target - .target - .parent() - .context("get parent of target file")?, - &target.owner, - ) - .context("create parent for target file")?; - fs.copy_file(source, &target.target, &target.owner) - .context("copy source to target")?; - fs.copy_permissions(source, &target.target, &target.owner) - .context("copy permissions from source to target")?; - Ok(true) + perform_copy_update(source, cached, target, fs, source_hash) } - CopyComparison::TargetNotRegularFile if force => { + (_, true, FileHash::NotPresent) => { + // Target missing: recreate (no remove_file needed) warn!( - "Updating copy {:?} -> {:?} but {}. Forcing.", - source, target.target, comparison + "Updating copy {:?} -> {:?} but target is missing. Recreating.", + source, cached.target ); - fs.remove_file(&target.target) - .context("remove target while forcing")?; - fs.create_dir_all( - target - .target - .parent() - .context("get parent of target file")?, - &target.owner, - ) - .context("create parent for target file")?; - fs.copy_file(source, &target.target, &target.owner) - .context("copy source to target")?; - fs.copy_permissions(source, &target.target, &target.owner) - .context("copy permissions from source to target")?; - Ok(true) + perform_copy_update(source, cached, target, fs, source_hash) } - CopyComparison::TargetNotRegularFile => { + (_, true, _) if force => { + // Target externally modified but --force + warn!( + "Updating copy {:?} -> {:?} but target was externally modified or is not a regular file. Forcing.", + source, cached.target + ); + fs.remove_file(&cached.target) + .context("remove externally modified target")?; + perform_copy_update(source, cached, target, fs, source_hash) + } + (_, true, _) => { + // Target externally modified: back off error!( - "Updating copy {:?} -> {:?} but {}. Skipping.", - source, target.target, comparison + "Updating copy {:?} -> {:?} but target was externally modified or is not a regular file. Skipping.", + source, cached.target ); - Ok(false) + Ok(None) } } } diff --git a/src/config.rs b/src/config.rs index dfdce42..cbb7e1f 100644 --- a/src/config.rs +++ b/src/config.rs @@ -55,6 +55,17 @@ pub struct CopyTarget { pub condition: Option, } +/// Cached state of a deployed copy: target path plus checksums recorded at deploy time. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)] +#[serde(deny_unknown_fields)] +pub struct CopyEntry { + pub target: PathBuf, + /// xxh3 hash of the source file at last deploy time. + pub source_checksum: u64, + /// xxh3 hash of the target file at last deploy time. + pub target_checksum: u64, +} + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)] #[serde(from = "FileTargetOuterRepr", into = "FileTargetOuterRepr")] pub enum FileTarget { @@ -224,7 +235,7 @@ pub struct Cache { pub symlinks: BTreeMap, pub templates: BTreeMap, #[serde(default)] - pub copies: BTreeMap, + pub copies: BTreeMap, } pub fn save_dummy_config( diff --git a/src/deploy.rs b/src/deploy.rs index 53f3043..48ba16a 100644 --- a/src/deploy.rs +++ b/src/deploy.rs @@ -230,11 +230,11 @@ pub fn undeploy(opt: &Options) -> Result { ); } - for (deleted_copy, target) in cache.copies.clone() { + for (deleted_copy, entry) in cache.copies.clone() { execute_action( - actions::delete_copy(&deleted_copy, &target, fs, opt.force), + actions::delete_copy(&deleted_copy, &entry, fs, opt.force), || cache.copies.remove(&deleted_copy), - || format!("delete copy {deleted_copy:?} -> {target:?}"), + || format!("delete copy {deleted_copy:?} -> {:?}", entry.target), &mut suggest_force, &mut error_occurred, ); @@ -294,7 +294,7 @@ fn run_deploy( let existing_copies: BTreeSet<(PathBuf, PathBuf)> = cache .copies .iter() - .map(|(k, v)| (k.clone(), v.clone())) + .map(|(k, v)| (k.clone(), v.target.clone())) .collect(); let desired_symlinks: BTreeMap<(PathBuf, PathBuf), _> = desired_symlinks @@ -340,8 +340,9 @@ fn run_deploy( for (source, target) in existing_copies.difference(&desired_copies_keyed.keys().cloned().collect()) { + let cached_entry = cache.copies.get(source).unwrap().clone(); execute_action( - runner.delete_copy(source, target), + runner.delete_copy(source, &cached_entry), || resulting_cache.copies.remove(source), || format!("delete copy {source:?} -> {target:?}"), &mut suggest_force, @@ -402,17 +403,18 @@ fn run_deploy( let target = desired_copies_keyed .get(&(source.clone(), target_path.clone())) .unwrap(); - execute_action( - runner.create_copy(source, target), - || { - resulting_cache - .copies - .insert(source.clone(), target_path.clone()) - }, - || format!("create copy {source:?} -> {target_path:?}"), - &mut suggest_force, - &mut error_occurred, - ); + match runner.create_copy(source, target) { + Ok(Some(entry)) => { + resulting_cache.copies.insert(source.clone(), entry); + } + Ok(None) => { + error_occurred = true; + } + Err(e) => { + error!("Error when trying to create copy {source:?} -> {target_path:?}: {e:?}"); + error_occurred = true; + } + } } for (source, target_path) in @@ -451,13 +453,20 @@ fn run_deploy( let target = desired_copies_keyed .get(&(source.clone(), target_path.clone())) .unwrap(); - execute_action( - runner.update_copy(source, target), - || (), - || format!("update copy {source:?} -> {target_path:?}"), - &mut suggest_force, - &mut error_occurred, - ); + let cached_entry = cache.copies.get(source).unwrap().clone(); + match runner.update_copy(source, target, &cached_entry) { + Ok(Some(entry)) => { + resulting_cache.copies.insert(source.clone(), entry); + } + Ok(None) => { + suggest_force = true; + error_occurred = true; + } + Err(e) => { + error!("Error when trying to update copy {source:?} -> {target_path:?}: {e:?}"); + error_occurred = true; + } + } } *cache = resulting_cache; @@ -489,7 +498,7 @@ fn execute_action T, E: FnOnce() -> String>( #[cfg(test)] mod test { - use crate::filesystem::{CopyComparison, SymlinkComparison, TemplateComparison}; + use crate::filesystem::{FileHash, SymlinkComparison, TemplateComparison}; use std::path::{Path, PathBuf}; @@ -562,11 +571,16 @@ mod test { #[test] fn high_level_simple_copy() { let c_out: config::CopyTarget = "c_out".into(); - let desired_copies = maplit::btreemap! { PathBuf::from("c_in") => c_out.clone() }; + let returned_entry = config::CopyEntry { + target: PathBuf::from("c_out"), + source_checksum: 12345, + target_checksum: 12345, + }; + let mut runner = actions::MockActionRunner::new(); let mut seq = mockall::Sequence::new(); let mut cache = Cache::default(); @@ -579,7 +593,7 @@ mod test { mockall::predicate::eq(c_out), ) .in_sequence(&mut seq) - .returning(|_, _| Ok(true)); + .returning(move |_, _| Ok(Some(returned_entry.clone()))); let (suggest_force, error_occurred) = run_deploy( &mut runner, @@ -596,8 +610,13 @@ mod test { assert!(!suggest_force); assert!(!error_occurred); - assert!(cache.copies.contains_key(&PathBuf::from("c_in"))); - assert_eq!(cache.copies.len(), 1); + let stored = cache + .copies + .get(&PathBuf::from("c_in")) + .expect("should be in cache"); + assert_eq!(stored.target, PathBuf::from("c_out")); + assert_eq!(stored.source_checksum, 12345); + assert_eq!(stored.target_checksum, 12345); } #[test] @@ -989,12 +1008,18 @@ mod test { let handlebars = handlebars::Handlebars::new(); let variables = toml::map::Map::new(); - // delete_copy: target exists and matches source → delete - fs.expect_compare_copy() + let cached = config::CopyEntry { + target: PathBuf::from("a_out"), + source_checksum: 42, + target_checksum: 42, + }; + + // Target hash matches cached → safe to delete + fs.expect_checksum_file() .times(1) - .with(function(path_eq("a_in")), function(path_eq("a_out"))) + .with(function(path_eq("a_out"))) .in_sequence(&mut seq) - .returning(|_, _| Ok(CopyComparison::Identical)); + .returning(|_| Ok(crate::filesystem::FileHash::Hash(42))); fs.expect_remove_file() .times(1) .with(function(path_eq("a_out"))) @@ -1013,10 +1038,508 @@ mod test { opt.force, opt.diff_context_lines, ); - assert!( - runner - .delete_copy(&PathBuf::from("a_in"), &PathBuf::from("a_out")) - .unwrap() + assert!(runner.delete_copy(&PathBuf::from("a_in"), &cached).unwrap()); + } + + #[test] + fn low_level_delete_copy_target_missing() { + let mut fs = crate::filesystem::MockFilesystem::new(); + + let opt = Options::default(); + let handlebars = handlebars::Handlebars::new(); + let variables = toml::map::Map::new(); + + let cached = config::CopyEntry { + target: PathBuf::from("a_out"), + source_checksum: 42, + target_checksum: 42, + }; + + // Target missing → warn + return true (remove from cache) + fs.expect_checksum_file() + .times(1) + .with(function(path_eq("a_out"))) + .returning(|_| Ok(FileHash::NotPresent)); + + let mut runner = actions::RealActionRunner::new( + &mut fs, + &handlebars, + &variables, + opt.force, + opt.diff_context_lines, + ); + assert!(runner.delete_copy(&PathBuf::from("a_in"), &cached).unwrap()); + } + + #[test] + fn low_level_delete_copy_force() { + let mut fs = crate::filesystem::MockFilesystem::new(); + let mut seq = mockall::Sequence::new(); + + let opt = Options { + force: true, + ..Options::default() + }; + let handlebars = handlebars::Handlebars::new(); + let variables = toml::map::Map::new(); + + let cached = config::CopyEntry { + target: PathBuf::from("a_out"), + source_checksum: 42, + target_checksum: 42, + }; + + // Target externally modified (hash differs) but --force → delete anyway + fs.expect_checksum_file() + .times(1) + .with(function(path_eq("a_out"))) + .in_sequence(&mut seq) + .returning(|_| Ok(FileHash::Hash(999))); + fs.expect_remove_file() + .times(1) + .with(function(path_eq("a_out"))) + .in_sequence(&mut seq) + .returning(|_| Ok(())); + fs.expect_delete_parents() + .times(1) + .with(function(path_eq("a_out")), eq(false)) + .in_sequence(&mut seq) + .returning(|_, _| Ok(())); + + let mut runner = actions::RealActionRunner::new( + &mut fs, + &handlebars, + &variables, + opt.force, + opt.diff_context_lines, + ); + assert!(runner.delete_copy(&PathBuf::from("a_in"), &cached).unwrap()); + } + + #[test] + fn low_level_create_copy() { + let mut fs = crate::filesystem::MockFilesystem::new(); + let mut seq = mockall::Sequence::new(); + + let opt = Options::default(); + let handlebars = handlebars::Handlebars::new(); + let variables = toml::map::Map::new(); + + let copy_target: config::CopyTarget = PathBuf::from("a_out").into(); + + // Source exists (hash 100) + fs.expect_checksum_file() + .times(1) + .with(function(path_eq("a_in"))) + .in_sequence(&mut seq) + .returning(|_| Ok(FileHash::Hash(100))); + // Target missing → create + fs.expect_checksum_file() + .times(1) + .with(function(path_eq("a_out"))) + .in_sequence(&mut seq) + .returning(|_| Ok(FileHash::NotPresent)); + fs.expect_create_dir_all().times(1).returning(|_, _| Ok(())); + fs.expect_copy_file() + .times(1) + .with( + function(path_eq("a_in")), + function(path_eq("a_out")), + eq(None), + ) + .in_sequence(&mut seq) + .returning(|_, _, _| Ok(())); + fs.expect_copy_permissions() + .times(1) + .in_sequence(&mut seq) + .returning(|_, _, _| Ok(())); + + let mut runner = actions::RealActionRunner::new( + &mut fs, + &handlebars, + &variables, + opt.force, + opt.diff_context_lines, + ); + let result = runner + .create_copy(&PathBuf::from("a_in"), ©_target) + .unwrap(); + let entry = result.expect("should have returned a CopyEntry"); + assert_eq!(entry.target, PathBuf::from("a_out")); + assert_eq!(entry.source_checksum, 100); + assert_eq!(entry.target_checksum, 100); // same as source since just copied + } + + #[test] + fn low_level_create_copy_target_matches() { + // Target already has same content as source → add to cache without copying + let mut fs = crate::filesystem::MockFilesystem::new(); + let mut seq = mockall::Sequence::new(); + + let opt = Options::default(); + let handlebars = handlebars::Handlebars::new(); + let variables = toml::map::Map::new(); + let copy_target: config::CopyTarget = PathBuf::from("a_out").into(); + + fs.expect_checksum_file() + .times(1) + .with(function(path_eq("a_in"))) + .in_sequence(&mut seq) + .returning(|_| Ok(FileHash::Hash(100))); + // Target hash matches source + fs.expect_checksum_file() + .times(1) + .with(function(path_eq("a_out"))) + .in_sequence(&mut seq) + .returning(|_| Ok(FileHash::Hash(100))); + + let mut runner = actions::RealActionRunner::new( + &mut fs, + &handlebars, + &variables, + opt.force, + opt.diff_context_lines, + ); + let result = runner + .create_copy(&PathBuf::from("a_in"), ©_target) + .unwrap(); + let entry = result.expect("should have returned a CopyEntry"); + assert_eq!(entry.source_checksum, 100); + assert_eq!(entry.target_checksum, 100); + } + + #[test] + fn low_level_create_copy_target_exists_different_no_force() { + // Target exists with different content, no --force → skip + let mut fs = crate::filesystem::MockFilesystem::new(); + let mut seq = mockall::Sequence::new(); + + let opt = Options::default(); + let handlebars = handlebars::Handlebars::new(); + let variables = toml::map::Map::new(); + let copy_target: config::CopyTarget = PathBuf::from("a_out").into(); + + fs.expect_checksum_file() + .times(1) + .with(function(path_eq("a_in"))) + .in_sequence(&mut seq) + .returning(|_| Ok(FileHash::Hash(100))); + // Target has different content + fs.expect_checksum_file() + .times(1) + .with(function(path_eq("a_out"))) + .in_sequence(&mut seq) + .returning(|_| Ok(FileHash::Hash(999))); + + let mut runner = actions::RealActionRunner::new( + &mut fs, + &handlebars, + &variables, + opt.force, + opt.diff_context_lines, + ); + let result = runner + .create_copy(&PathBuf::from("a_in"), ©_target) + .unwrap(); + assert!(result.is_none(), "should have been skipped without --force"); + } + + #[test] + fn low_level_create_copy_target_exists_different_force() { + // Target exists with different content, --force → overwrite + let mut fs = crate::filesystem::MockFilesystem::new(); + let mut seq = mockall::Sequence::new(); + + let opt = Options { + force: true, + ..Options::default() + }; + let handlebars = handlebars::Handlebars::new(); + let variables = toml::map::Map::new(); + let copy_target: config::CopyTarget = PathBuf::from("a_out").into(); + + fs.expect_checksum_file() + .times(1) + .with(function(path_eq("a_in"))) + .in_sequence(&mut seq) + .returning(|_| Ok(FileHash::Hash(100))); + // Target has different content + fs.expect_checksum_file() + .times(1) + .with(function(path_eq("a_out"))) + .in_sequence(&mut seq) + .returning(|_| Ok(FileHash::Hash(999))); + // --force: remove existing, re-create + fs.expect_remove_file() + .times(1) + .with(function(path_eq("a_out"))) + .in_sequence(&mut seq) + .returning(|_| Ok(())); + fs.expect_create_dir_all().times(1).returning(|_, _| Ok(())); + fs.expect_copy_file() + .times(1) + .in_sequence(&mut seq) + .returning(|_, _, _| Ok(())); + fs.expect_copy_permissions() + .times(1) + .in_sequence(&mut seq) + .returning(|_, _, _| Ok(())); + + let mut runner = actions::RealActionRunner::new( + &mut fs, + &handlebars, + &variables, + opt.force, + opt.diff_context_lines, + ); + let result = runner + .create_copy(&PathBuf::from("a_in"), ©_target) + .unwrap(); + let entry = result.expect("should have returned a CopyEntry"); + assert_eq!(entry.source_checksum, 100); + assert_eq!(entry.target_checksum, 100); + } + + #[test] + fn update_copy_source_changed() { + // Source changed (new hash), target untouched → re-copy + let mut fs = crate::filesystem::MockFilesystem::new(); + let mut seq = mockall::Sequence::new(); + + let opt = Options::default(); + let handlebars = handlebars::Handlebars::new(); + let variables = toml::map::Map::new(); + + let cached = config::CopyEntry { + target: PathBuf::from("a_out"), + source_checksum: 100, + target_checksum: 100, + }; + let copy_target: config::CopyTarget = PathBuf::from("a_out").into(); + + // Source has new hash (200) + fs.expect_checksum_file() + .times(1) + .with(function(path_eq("a_in"))) + .in_sequence(&mut seq) + .returning(|_| Ok(FileHash::Hash(200))); + // Target still has old hash (100 = cached.target_checksum → unchanged) + fs.expect_checksum_file() + .times(1) + .with(function(path_eq("a_out"))) + .in_sequence(&mut seq) + .returning(|_| Ok(FileHash::Hash(100))); + // → source changed, target unchanged → re-copy + fs.expect_remove_file() + .times(1) + .with(function(path_eq("a_out"))) + .in_sequence(&mut seq) + .returning(|_| Ok(())); + fs.expect_create_dir_all().times(1).returning(|_, _| Ok(())); + fs.expect_copy_file() + .times(1) + .in_sequence(&mut seq) + .returning(|_, _, _| Ok(())); + fs.expect_copy_permissions() + .times(1) + .in_sequence(&mut seq) + .returning(|_, _, _| Ok(())); + + let mut runner = actions::RealActionRunner::new( + &mut fs, + &handlebars, + &variables, + opt.force, + opt.diff_context_lines, + ); + let result = runner + .update_copy(&PathBuf::from("a_in"), ©_target, &cached) + .unwrap(); + let entry = result.expect("should have returned updated entry"); + assert_eq!(entry.source_checksum, 200); + assert_eq!(entry.target_checksum, 200); + } + + #[test] + fn update_copy_target_externally_modified() { + // Source unchanged, target externally edited → back off + let mut fs = crate::filesystem::MockFilesystem::new(); + let mut seq = mockall::Sequence::new(); + + let opt = Options::default(); + let handlebars = handlebars::Handlebars::new(); + let variables = toml::map::Map::new(); + + let cached = config::CopyEntry { + target: PathBuf::from("a_out"), + source_checksum: 100, + target_checksum: 100, + }; + let copy_target: config::CopyTarget = PathBuf::from("a_out").into(); + + // Source unchanged (still 100) + fs.expect_checksum_file() + .times(1) + .with(function(path_eq("a_in"))) + .in_sequence(&mut seq) + .returning(|_| Ok(FileHash::Hash(100))); + // Target externally modified (now 999) + fs.expect_checksum_file() + .times(1) + .with(function(path_eq("a_out"))) + .in_sequence(&mut seq) + .returning(|_| Ok(FileHash::Hash(999))); + // → source unchanged, target changed → skip + + let mut runner = actions::RealActionRunner::new( + &mut fs, + &handlebars, + &variables, + opt.force, + opt.diff_context_lines, + ); + let result = runner + .update_copy(&PathBuf::from("a_in"), ©_target, &cached) + .unwrap(); + assert!(result.is_none()); + } + + #[test] + fn update_copy_identical() { + // Nothing changed → set owner/perms, return cached entry + let mut fs = crate::filesystem::MockFilesystem::new(); + let mut seq = mockall::Sequence::new(); + + let opt = Options::default(); + let handlebars = handlebars::Handlebars::new(); + let variables = toml::map::Map::new(); + + let cached = config::CopyEntry { + target: PathBuf::from("a_out"), + source_checksum: 100, + target_checksum: 100, + }; + let copy_target: config::CopyTarget = PathBuf::from("a_out").into(); + + // Source unchanged + fs.expect_checksum_file() + .times(1) + .with(function(path_eq("a_in"))) + .in_sequence(&mut seq) + .returning(|_| Ok(FileHash::Hash(100))); + // Target unchanged + fs.expect_checksum_file() + .times(1) + .with(function(path_eq("a_out"))) + .in_sequence(&mut seq) + .returning(|_| Ok(FileHash::Hash(100))); + // → nothing changed → set_owner + copy_permissions only + fs.expect_set_owner() + .times(1) + .in_sequence(&mut seq) + .returning(|_, _| Ok(())); + fs.expect_copy_permissions() + .times(1) + .in_sequence(&mut seq) + .returning(|_, _, _| Ok(())); + + let mut runner = actions::RealActionRunner::new( + &mut fs, + &handlebars, + &variables, + opt.force, + opt.diff_context_lines, + ); + let result = runner + .update_copy(&PathBuf::from("a_in"), ©_target, &cached) + .unwrap(); + let entry = result.expect("should return entry even when unchanged"); + assert_eq!(entry.source_checksum, 100); + assert_eq!(entry.target_checksum, 100); + } + + #[test] + fn update_copy_target_missing() { + // Target was deleted externally → recreate without calling remove_file + let mut fs = crate::filesystem::MockFilesystem::new(); + let mut seq = mockall::Sequence::new(); + + let opt = Options::default(); + let handlebars = handlebars::Handlebars::new(); + let variables = toml::map::Map::new(); + + let cached = config::CopyEntry { + target: PathBuf::from("a_out"), + source_checksum: 100, + target_checksum: 100, + }; + let copy_target: config::CopyTarget = PathBuf::from("a_out").into(); + + // Source unchanged + fs.expect_checksum_file() + .times(1) + .with(function(path_eq("a_in"))) + .in_sequence(&mut seq) + .returning(|_| Ok(FileHash::Hash(100))); + // Target is missing + fs.expect_checksum_file() + .times(1) + .with(function(path_eq("a_out"))) + .in_sequence(&mut seq) + .returning(|_| Ok(FileHash::NotPresent)); + // No remove_file, target is already gone + fs.expect_create_dir_all().times(1).returning(|_, _| Ok(())); + fs.expect_copy_file() + .times(1) + .in_sequence(&mut seq) + .returning(|_, _, _| Ok(())); + fs.expect_copy_permissions() + .times(1) + .in_sequence(&mut seq) + .returning(|_, _, _| Ok(())); + + let mut runner = actions::RealActionRunner::new( + &mut fs, + &handlebars, + &variables, + opt.force, + opt.diff_context_lines, + ); + let result = runner + .update_copy(&PathBuf::from("a_in"), ©_target, &cached) + .unwrap(); + let entry = result.expect("should have returned a recreated CopyEntry"); + assert_eq!(entry.source_checksum, 100); + assert_eq!(entry.target_checksum, 100); + } + + #[test] + fn low_level_delete_copy_externally_modified() { + let mut fs = crate::filesystem::MockFilesystem::new(); + + let opt = Options::default(); + let handlebars = handlebars::Handlebars::new(); + let variables = toml::map::Map::new(); + + let cached = config::CopyEntry { + target: PathBuf::from("a_out"), + source_checksum: 42, + target_checksum: 42, + }; + + // Target hash does NOT match cached → externally modified, skip + fs.expect_checksum_file() + .times(1) + .with(function(path_eq("a_out"))) + .returning(|_| Ok(crate::filesystem::FileHash::Hash(999))); + + let mut runner = actions::RealActionRunner::new( + &mut fs, + &handlebars, + &variables, + opt.force, + opt.diff_context_lines, ); + assert!(!runner.delete_copy(&PathBuf::from("a_in"), &cached).unwrap()); } } diff --git a/src/filesystem.rs b/src/filesystem.rs index a4357c3..56d9c0a 100644 --- a/src/filesystem.rs +++ b/src/filesystem.rs @@ -49,8 +49,9 @@ pub trait Filesystem { /// Check state of expected symbolic link on disk fn compare_template(&mut self, target: &Path, cache: &Path) -> Result; - /// Check state of an expected copy on disk (source vs target, byte-for-byte). - fn compare_copy(&mut self, source: &Path, target: &Path) -> Result; + /// Compute the xxh3 hash of a file. Returns `NotPresent` if missing, `NotRegularFile` + /// if the path is a symlink or directory. + fn checksum_file(&mut self, path: &Path) -> Result; /// Removes a file or folder, elevating privileges if needed fn remove_file(&mut self, path: &Path) -> Result<()>; @@ -122,8 +123,8 @@ impl Filesystem for RealFilesystem { Ok(compare_template(target_state, cache_state)) } - fn compare_copy(&mut self, source: &Path, target: &Path) -> Result { - compare_copy(source, target) + fn checksum_file(&mut self, path: &Path) -> Result { + checksum_file(path) } fn remove_file(&mut self, path: &Path) -> Result<()> { @@ -290,8 +291,8 @@ impl Filesystem for RealFilesystem { Ok(compare_template(target_state, cache_state)) } - fn compare_copy(&mut self, source: &Path, target: &Path) -> Result { - compare_copy(source, target) + fn checksum_file(&mut self, path: &Path) -> Result { + checksum_file(path) } fn remove_file(&mut self, path: &Path) -> Result<()> { @@ -608,27 +609,18 @@ impl Filesystem for DryRunFilesystem { Ok(compare_template(target_state, cache_state)) } - fn compare_copy(&mut self, source: &Path, target: &Path) -> Result { - let source_state = self.get_state(source).context("get source state")?; - let target_state = self.get_state(target).context("get target state")?; - - Ok(match (source_state, target_state) { - (FileState::Missing, _) => CopyComparison::SourceNotPresent, - (_, FileState::Missing) => CopyComparison::TargetNotPresent, - (_, FileState::SymbolicLink(_) | FileState::Directory) => { - CopyComparison::TargetNotRegularFile - } - (FileState::File(s), FileState::File(t)) => { - if s == t { - CopyComparison::Identical - } else { - CopyComparison::Changed - } - } - (FileState::SymbolicLink(_) | FileState::Directory, _) => { - CopyComparison::SourceNotPresent - } - }) + fn checksum_file(&mut self, path: &Path) -> Result { + use xxhash_rust::xxh3::xxh3_64; + match self.file_states.get(path) { + Some(FileState::Missing) => Ok(FileHash::NotPresent), + Some(FileState::SymbolicLink(_) | FileState::Directory) => Ok(FileHash::NotRegularFile), + Some(FileState::File(Some(content))) => Ok(FileHash::Hash(xxh3_64(content.as_bytes()))), + // Binary content not tracked in dry-run; fall back to real disk. + // NOTE: hash reflects pre-deploy on-disk state, not simulated state. + Some(FileState::File(None)) => checksum_file(path), + // Path not in simulated state; fall back to real filesystem. + None => checksum_file(path), + } } fn remove_file(&mut self, path: &Path) -> Result<()> { @@ -813,31 +805,24 @@ impl std::fmt::Display for TemplateComparison { } } +/// Result of hashing a file on disk. #[derive(Debug, PartialEq, Eq)] -pub enum CopyComparison { - /// Target is a regular file and its content matches source byte-for-byte. - Identical, - /// Target is a regular file but its content differs from source. - Changed, - /// Target does not exist. - TargetNotPresent, - /// Source file does not exist. - SourceNotPresent, - /// Target exists but is not a regular file (e.g. a symlink or directory). - TargetNotRegularFile, +pub enum FileHash { + /// File does not exist. + NotPresent, + /// Path exists but is not a regular file (symlink, directory, etc.). + NotRegularFile, + /// xxh3 hash of the file's contents. + Hash(u64), } -impl std::fmt::Display for CopyComparison { +impl std::fmt::Display for FileHash { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { - use self::CopyComparison::*; match self { - Identical => "target matches source", - Changed => "target differs from source", - TargetNotPresent => "target missing", - SourceNotPresent => "source is missing", - TargetNotRegularFile => "target already exists and isn't a regular file", + FileHash::NotPresent => "not present".fmt(f), + FileHash::NotRegularFile => "not a regular file".fmt(f), + FileHash::Hash(h) => write!(f, "hash({h:#018x})"), } - .fmt(f) } } @@ -857,31 +842,16 @@ fn compare_template(target_state: FileState, cache_state: FileState) -> Template } } -fn compare_copy(source: &Path, target: &Path) -> Result { - let source_state = get_file_state(source).context("get source state")?; - let target_state = get_file_state(target).context("get target state")?; - - Ok(match (source_state, target_state) { - (FileState::Missing, _) => CopyComparison::SourceNotPresent, - (_, FileState::Missing) => CopyComparison::TargetNotPresent, - (_, FileState::SymbolicLink(_) | FileState::Directory) => { - CopyComparison::TargetNotRegularFile - } - (FileState::File(_), FileState::File(_)) => { - // Re-read as bytes for a true binary comparison - let source_bytes = std::fs::read(source).context("read source bytes")?; - let target_bytes = std::fs::read(target).context("read target bytes")?; - if source_bytes == target_bytes { - CopyComparison::Identical - } else { - CopyComparison::Changed - } - } - (FileState::SymbolicLink(_) | FileState::Directory, _) => { - // Source is not a regular file — treat as source not present - CopyComparison::SourceNotPresent - } - }) +pub(crate) fn checksum_file(path: &Path) -> Result { + use xxhash_rust::xxh3::xxh3_64; + match path.symlink_metadata() { + Err(e) if e.kind() == ErrorKind::NotFound => return Ok(FileHash::NotPresent), + Err(e) => return Err(e).context("stat file for checksum"), + Ok(m) if !m.is_file() => return Ok(FileHash::NotRegularFile), + Ok(_) => {} + } + let bytes = std::fs::read(path).context("read file for checksum")?; + Ok(FileHash::Hash(xxh3_64(&bytes))) } // === Utility functions ===