From 98abe7d9fdf2e99c84724da93acfa737c6dd3a29 Mon Sep 17 00:00:00 2001 From: Johann Woelper Date: Wed, 15 Apr 2026 21:51:54 +0200 Subject: [PATCH] fix: bug where differing sizes and creation date combinations would not copy file. Deprecate jwalk. More tests. --- src/lib.rs | 126 ++++++--------------------------------------------- src/tests.rs | 85 ++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 112 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 36eb22b..937d3ea 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -122,88 +122,6 @@ fn is_filesize_different(file_a: &Path, file_b: &Path) -> bool { } } -#[cfg(feature = "jwalk")] -fn copy_file(source: &Path, options: CopyBuilder) -> Result<(), std::io::Error> { - let abs_source = options.source.canonicalize()?; - let abs_dest = options.destination.canonicalize()?; - - let rel_dest = source - .strip_prefix(&abs_source) - .map_err(|e| Error::new(ErrorKind::Other, format!("Could not strip prefix: {:?}", e)))?; - let dest_entry = abs_dest.join(rel_dest); - - if source.is_file() { - // the source exists - - // Early out if target is present and overwrite is off - if !options.overwrite_all - && dest_entry.is_file() - && !options.overwrite_if_newer - && !options.overwrite_if_size_differs - { - return Ok(()); - } - - for f in &options.exclude_filters { - if source.to_string_lossy().contains(f) { - return Ok(()); - } - } - - for f in &options.include_filters { - if !source.to_string_lossy().contains(f) { - return Ok(()); - } - } - - // File is not present: copy it - if !dest_entry.is_file() { - debug!( - "Dest not present: CP {} DST {}", - source.display(), - dest_entry.display() - ); - copy(source, dest_entry)?; - return Ok(()); - } - - // File newer? - if options.overwrite_if_newer { - if is_file_newer(source, &dest_entry) { - 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) { - debug!( - "Source differs: CP {} DST {}", - source.display(), - dest_entry.display() - ); - copy(source, &dest_entry)?; - } - return Ok(()); - } - - // The regular copy operation - debug!("CP {} DST {}", source.display(), dest_entry.display()); - copy(source, dest_entry)?; - } else if source.is_dir() && !dest_entry.is_dir() { - debug!("MKDIR {}", source.display()); - std::fs::create_dir_all(dest_entry)?; - } - - Ok(()) -} - impl CopyBuilder { /// Construct a new CopyBuilder with `source` and `dest`. pub fn new, Q: AsRef>(source: P, dest: Q) -> CopyBuilder { @@ -353,28 +271,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; } } @@ -411,26 +327,12 @@ impl CopyBuilder { /// Execute the copy operation in parallel. The usage of this function is discouraged /// until proven to work faster. #[cfg(feature = "jwalk")] + #[deprecated( + since = "0.3.21", + note = "please use `run` instead. This is nowjust a wrapper around `run`." + )] pub fn run_par(&self) -> Result<(), std::io::Error> { - if !self.destination.is_dir() { - debug!("MKDIR {:?}", &self.destination); - std::fs::create_dir_all(&self.destination)?; - } - let abs_source = self.source.canonicalize()?; - let abs_dest = self.destination.canonicalize()?; - debug!( - "Building copy operation: SRC {} DST {}", - abs_source.display(), - abs_dest.display() - ); - for entry in JWalkDir::new(&abs_source) - .into_iter() - .filter_map(|e| e.ok()) - { - let _ = copy_file(&entry.path(), self.clone()); - } - - Ok(()) + self.run() } } diff --git a/src/tests.rs b/src/tests.rs index 57dff82..058da85 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -316,3 +316,88 @@ fn copy_cargo_progress() { std::fs::remove_dir_all(output_dir).unwrap(); std::fs::remove_file(archive).unwrap(); } + +#[test] +/// Source is NOT newer, but sizes differ → should copy even though source is older. +fn overwrite_or_size_differs_not_newer() { + let sample_dir = "overwrite_size_diff"; + use std::fs::File; + use std::io::Write; + std::env::set_var("RUST_LOG", "DEBUG"); + let _ = env_logger::builder().try_init(); + + let source_dir = format!("{sample_dir}_src"); + let dest_dir = format!("{sample_dir}_dest"); + + create_dir_all(&source_dir).unwrap(); + create_dir_all(&dest_dir).unwrap(); + + // Write source first so it is older. + File::create(format!("{source_dir}/file.txt")).unwrap(); + + // 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, "", "File should be identical to source"); + + std::fs::remove_dir_all(source_dir).unwrap(); + std::fs::remove_dir_all(dest_dir).unwrap(); +} + +#[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 sample_dir = "overwrite_or_newer_same_size"; + + let source_dir = format!("{sample_dir}_src"); + let dest_dir = format!("{sample_dir}_dest"); + + 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(); +}