diff --git a/src/lib.rs b/src/lib.rs index 36eb22b..0d504ef 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -80,9 +80,9 @@ pub struct CopyBuilder { overwrite_if_newer: bool, /// Overwrite target files if they differ in size overwrite_if_size_differs: bool, - /// A list of include filters - exclude_filters: Vec, /// A list of exclude filters + exclude_filters: Vec, + /// A list of include filters include_filters: Vec, /// An optional progress function. Has a performance penalty as the total number of files need to be calculated. progress_callback: Option, @@ -167,27 +167,26 @@ fn copy_file(source: &Path, options: CopyBuilder) -> Result<(), std::io::Error> return Ok(()); } - // File newer? - if options.overwrite_if_newer { - if is_file_newer(source, &dest_entry) { + // Conditional overwrite checks (OR semantics: copy if any enabled condition matches) + if options.overwrite_if_newer || options.overwrite_if_size_differs { + let newer = options.overwrite_if_newer && is_file_newer(source, &dest_entry); + let size_differs = + options.overwrite_if_size_differs && is_filesize_different(source, &dest_entry); + if newer { debug!( "Source newer: CP {} DST {}", source.display(), dest_entry.display() ); - copy(source, &dest_entry)?; } - return Ok(()); - } - - // Different size? - if options.overwrite_if_size_differs { - if is_filesize_different(source, &dest_entry) { + if size_differs { debug!( "Source differs: CP {} DST {}", source.display(), dest_entry.display() ); + } + if newer || size_differs { copy(source, &dest_entry)?; } return Ok(()); @@ -353,28 +352,26 @@ impl CopyBuilder { ); } - // File newer? - if dest_exists && self.overwrite_if_newer { - if is_file_newer(entry.path(), &dest_entry) { + // Conditional overwrite checks (OR semantics: copy if any enabled condition matches) + if dest_exists && (self.overwrite_if_newer || self.overwrite_if_size_differs) { + let newer = self.overwrite_if_newer && is_file_newer(entry.path(), &dest_entry); + let size_differs = self.overwrite_if_size_differs + && is_filesize_different(entry.path(), &dest_entry); + if newer { debug!( "Source newer: CP {} DST {}", entry.path().display(), dest_entry.display() ); - } else { - continue; } - } - - // Different size? - if dest_exists && self.overwrite_if_size_differs { - if is_filesize_different(entry.path(), &dest_entry) { + if size_differs { debug!( "Source differs: CP {} DST {}", entry.path().display(), dest_entry.display() ); - } else { + } + if !newer && !size_differs { continue; } } diff --git a/src/tests.rs b/src/tests.rs index 57dff82..ac81c52 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -143,6 +143,184 @@ fn copy_overwrite() { std::fs::remove_dir_all(dest_dir).unwrap(); } +// Bug 2: when both overwrite_if_newer and overwrite_if_size_differs are set, +// the copy should happen if EITHER condition is true (OR semantics), not both (AND). + +#[test] +fn overwrite_or_newer_same_size() { + // Source IS newer, same size → should copy even though size is unchanged. + use std::fs::File; + use std::io::Write; + + let source_dir = "or_newer_same_size_src"; + let dest_dir = "or_newer_same_size_dst"; + + std::env::set_var("RUST_LOG", "debug"); + let _ = env_logger::try_init(); + create_dir_all(source_dir).unwrap(); + create_dir_all(dest_dir).unwrap(); + + // Write dest first so it is older. + let mut f = File::create(format!("{dest_dir}/file.txt")).unwrap(); + write!(f, "hello123").unwrap(); // 8 bytes + drop(f); + + // Small delay so the source mtime is strictly newer. + std::thread::sleep(std::time::Duration::from_millis(50)); + + // Write source after with the same length (same size, but newer). + let source_content = "world456"; // 8 bytes + let mut f = File::create(format!("{source_dir}/file.txt")).unwrap(); + write!(f, "{source_content}").unwrap(); + drop(f); + + CopyBuilder::new(source_dir, dest_dir) + .overwrite_if_newer(true) + .overwrite_if_size_differs(true) + .run() + .unwrap(); + + let result = std::fs::read_to_string(format!("{dest_dir}/file.txt")).unwrap(); + assert_eq!( + result, source_content, + "File should be overwritten because source is newer (even though size is the same)" + ); + + std::fs::remove_dir_all(source_dir).unwrap(); + std::fs::remove_dir_all(dest_dir).unwrap(); +} + +#[test] +fn overwrite_or_size_differs_not_newer() { + // Source is NOT newer, but sizes differ → should copy even though source is older. + use std::fs::File; + use std::io::Write; + + let source_dir = "or_size_diff_not_newer_src"; + let dest_dir = "or_size_diff_not_newer_dst"; + + std::env::set_var("RUST_LOG", "debug"); + let _ = env_logger::try_init(); + create_dir_all(source_dir).unwrap(); + create_dir_all(dest_dir).unwrap(); + + // Write source first so it is older. + let source_content = "short"; + let mut f = File::create(format!("{source_dir}/file.txt")).unwrap(); + write!(f, "{source_content}").unwrap(); + drop(f); + + // Small delay so the dest mtime is strictly newer. + std::thread::sleep(std::time::Duration::from_millis(50)); + + // Write dest after with different (larger) content, making it newer AND bigger. + let mut f = File::create(format!("{dest_dir}/file.txt")).unwrap(); + write!(f, "much longer destination content").unwrap(); + drop(f); + + // Source is older but smaller; sizes differ, so should be copied. + CopyBuilder::new(source_dir, dest_dir) + .overwrite_if_newer(true) + .overwrite_if_size_differs(true) + .run() + .unwrap(); + + let result = std::fs::read_to_string(format!("{dest_dir}/file.txt")).unwrap(); + assert_eq!( + result, source_content, + "File should be overwritten because sizes differ (even though source is not newer)" + ); + + std::fs::remove_dir_all(source_dir).unwrap(); + std::fs::remove_dir_all(dest_dir).unwrap(); +} + +// Bug 3: the same OR-semantics bug existed in copy_file() used by run_par(). + +#[cfg(feature = "jwalk")] +#[test] +fn overwrite_or_newer_same_size_par() { + // Source IS newer, same size → run_par() should copy. + use std::fs::File; + use std::io::Write; + + let source_dir = "or_newer_same_size_par_src"; + let dest_dir = "or_newer_same_size_par_dst"; + + std::env::set_var("RUST_LOG", "debug"); + let _ = env_logger::try_init(); + create_dir_all(source_dir).unwrap(); + create_dir_all(dest_dir).unwrap(); + + let mut f = File::create(format!("{dest_dir}/file.txt")).unwrap(); + write!(f, "hello123").unwrap(); + drop(f); + + std::thread::sleep(std::time::Duration::from_millis(50)); + + let source_content = "world456"; + let mut f = File::create(format!("{source_dir}/file.txt")).unwrap(); + write!(f, "{source_content}").unwrap(); + drop(f); + + CopyBuilder::new(source_dir, dest_dir) + .overwrite_if_newer(true) + .overwrite_if_size_differs(true) + .run_par() + .unwrap(); + + let result = std::fs::read_to_string(format!("{dest_dir}/file.txt")).unwrap(); + assert_eq!( + result, source_content, + "run_par: file should be overwritten because source is newer (even though size is the same)" + ); + + std::fs::remove_dir_all(source_dir).unwrap(); + std::fs::remove_dir_all(dest_dir).unwrap(); +} + +#[cfg(feature = "jwalk")] +#[test] +fn overwrite_or_size_differs_not_newer_par() { + // Source is NOT newer, but sizes differ → run_par() should copy. + use std::fs::File; + use std::io::Write; + + let source_dir = "or_size_diff_not_newer_par_src"; + let dest_dir = "or_size_diff_not_newer_par_dst"; + + std::env::set_var("RUST_LOG", "debug"); + let _ = env_logger::try_init(); + create_dir_all(source_dir).unwrap(); + create_dir_all(dest_dir).unwrap(); + + let source_content = "short"; + let mut f = File::create(format!("{source_dir}/file.txt")).unwrap(); + write!(f, "{source_content}").unwrap(); + drop(f); + + std::thread::sleep(std::time::Duration::from_millis(50)); + + let mut f = File::create(format!("{dest_dir}/file.txt")).unwrap(); + write!(f, "much longer destination content").unwrap(); + drop(f); + + CopyBuilder::new(source_dir, dest_dir) + .overwrite_if_newer(true) + .overwrite_if_size_differs(true) + .run_par() + .unwrap(); + + let result = std::fs::read_to_string(format!("{dest_dir}/file.txt")).unwrap(); + assert_eq!( + result, source_content, + "run_par: file should be overwritten because sizes differ (even though source is not newer)" + ); + + std::fs::remove_dir_all(source_dir).unwrap(); + std::fs::remove_dir_all(dest_dir).unwrap(); +} + #[test] fn copy_exclude() { std::env::set_var("RUST_LOG", "DEBUG");