Skip to content

Commit

Permalink
Better error messages when reflinking directories.
Browse files Browse the repository at this point in the history
On macos, `clonefile` can handle directories (https://www.manpagez.com/man/2/clonefile/, https://opensource.apple.com/source/xnu/xnu-3789.21.4/bsd/man/man2/clonefile.2.auto.html - sorry, i couldn't find any better source online), which is a lot faster than manually walking the directory tree. This currently leads to a confusing error messages when the target directory exists, saying just `the source path is not an existing regular file` and not reporting the real cause, `File exists (os error 17)`.

This PR makes two changes: We return the original error for both files and directories, and for the error for irregular src paths, we still display the original error so the information isn't lost.
  • Loading branch information
konstin committed Nov 29, 2023
1 parent 3cf30da commit 39c8452
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 3 deletions.
5 changes: 3 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ use std::path::Path;
///
/// Uses `clonefile` library function. This is supported on OS X Version >=10.12 and iOS version >= 10.0
/// This will work on APFS partitions (which means most desktop systems are capable).
/// If src names a directory, the directory hierarchy is cloned as if each item was cloned individually.
///
/// ## Windows
///
Expand Down Expand Up @@ -101,12 +102,12 @@ pub fn reflink_or_copy(from: impl AsRef<Path>, to: impl AsRef<Path>) -> io::Resu
}

fn check_is_file_and_error(from: &Path, err: io::Error) -> io::Error {
if from.is_file() {
if fs::symlink_metadata(from).map_or(false, |m| m.is_file() || m.is_dir()) {
err
} else {
io::Error::new(
io::ErrorKind::InvalidInput,
"the source path is not an existing regular file",
format!("the source path is not an existing regular file: {}", err),
)
}
}
41 changes: 40 additions & 1 deletion tests/reflink.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,25 @@ fn reflink_dest_is_dir() {
}
}

#[cfg(unix)] // No reliable symlinking on windows
#[test]
fn reflink_src_is_symlink() {
let dir = tempdir().unwrap();
let target = dir.path().join("target.txt");
let symlink = dir.path().join("symlink.txt");
File::create(&target).unwrap();
std::os::unix::fs::symlink(&target, &symlink).unwrap();
let dest_file_path = dir.path().join("dest.txt");

match reflink(symlink, dest_file_path) {
Ok(()) => panic!(),
Err(e) => {
println!("{:?}", e);
assert_eq!(e.kind(), io::ErrorKind::InvalidInput)
}
}
}

#[test]
fn reflink_src_is_dir() {
let dir = tempdir().unwrap();
Expand All @@ -57,11 +76,31 @@ fn reflink_src_is_dir() {
Ok(()) => panic!(),
Err(e) => {
println!("{:?}", e);
assert_eq!(e.kind(), io::ErrorKind::InvalidInput)
// `io::ErrorKind::IsADirectory` is unstable
assert_eq!(e.kind().to_string(), "is a directory")
}
}
}

#[test]
fn reflink_existing_dest_dir_results_in_error() {
let dir = tempdir().unwrap();
let src_file_path = dir.path().join("src");
let dest_file_path = dir.path().join("dest");

let _src_file = fs::create_dir(&src_file_path).unwrap();
let _dest_file = fs::create_dir(&dest_file_path).unwrap();

match reflink(&src_file_path, &dest_file_path) {
Ok(()) => panic!(),
Err(e) => {
println!("{:?}", e);
assert_eq!(e.kind(), io::ErrorKind::AlreadyExists)
}
}
}


#[test]
fn reflink_existing_dest_results_in_error() {
let dir = tempdir().unwrap();
Expand Down

0 comments on commit 39c8452

Please sign in to comment.