Better error messages when reflinking directories (#51)

src/lib.rs

@@ -40,10 +40,11 @@ use std::path::Path;
 /// Uses `ioctl_ficlone`. Supported file systems include btrfs and XFS (and maybe more in the future).
 /// NOTE that it generates a temporary file and is not atomic.
 ///
-/// ## OS X / iOS
+/// ## MacOS / OS X / iOS
 ///
 /// 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
 ///
@@ -58,7 +59,24 @@ use std::path::Path;
 pub fn reflink(from: impl AsRef<Path>, to: impl AsRef<Path>) -> io::Result<()> {
     #[cfg_attr(feature = "tracing", tracing_attributes::instrument(name = "reflink"))]
     fn inner(from: &Path, to: &Path) -> io::Result<()> {
-        sys::reflink(from, to).map_err(|err| check_is_file_and_error(from, err))
+        sys::reflink(from, to).map_err(|err| {
+            // Linux and Windows will return an inscrutable error when `from` is a directory or a
+            // symlink, so add the real problem to the error. We need to use `fs::symlink_metadata`
+            // here because `from.is_file()` traverses symlinks.
+            //
+            // According to https://www.manpagez.com/man/2/clonefile/, Macos otoh can reflink files,
+            // directories and symlinks, so the original error is fine.
+            if !cfg!(any(target_os = "macos", target_os = "ios"))
+                && !fs::symlink_metadata(from).map_or(false, |m| m.is_file())
+            {
+                io::Error::new(
+                    io::ErrorKind::InvalidInput,
+                    format!("the source path is not an existing regular file: {}", err),
+                )
+            } else {
+                err
+            }
+        })
     }
 
     inner(from.as_ref(), to.as_ref())
@@ -78,6 +96,15 @@ pub fn reflink(from: impl AsRef<Path>, to: impl AsRef<Path>) -> io::Result<()> {
 ///     Err(e) => println!("an error occured: {:?}", e)
 /// }
 /// ```
+///
+/// # Implementation details per platform
+///
+/// ## MacOS / OS X / iOS
+///
+/// If src names a directory, the directory hierarchy is cloned as if each item was cloned
+/// individually. This method does not provide a fallback for directories, so the fallback will also
+/// fail if reflinking failed. Macos supports reflinking symlinks, which is supported by the
+/// fallback.
 #[inline(always)]
 pub fn reflink_or_copy(from: impl AsRef<Path>, to: impl AsRef<Path>) -> io::Result<Option<u64>> {
     #[cfg_attr(
@@ -89,24 +116,22 @@ pub fn reflink_or_copy(from: impl AsRef<Path>, to: impl AsRef<Path>) -> io::Resu
             #[cfg(feature = "tracing")]
             tracing::warn!(?_err, "Failed to reflink, fallback to fs::copy");
 
-            fs::copy(from, to)
-                .map(Some)
-                .map_err(|err| check_is_file_and_error(from, err))
+            fs::copy(from, to).map(Some).map_err(|err| {
+                // Both regular files and symlinks to regular files can be copied, so unlike
+                // `reflink` we don't want to report invalid input on both files and and symlinks
+                if from.is_file() {
+                    err
+                } else {
+                    io::Error::new(
+                        io::ErrorKind::InvalidInput,
+                        format!("the source path is not an existing regular file: {}", err),
+                    )
+                }
+            })
         } else {
             Ok(None)
         }
     }
 
     inner(from.as_ref(), to.as_ref())
 }
-
-fn check_is_file_and_error(from: &Path, err: io::Error) -> io::Error {
-    if from.is_file() {
-        err
-    } else {
-        io::Error::new(
-            io::ErrorKind::InvalidInput,
-            "the source path is not an existing regular file",
-        )
-    }
-}

tests/reflink.rs

@@ -10,13 +10,9 @@ fn reflink_file_does_not_exist() {
     let from = Path::new("test/nonexistent-bogus-path");
     let to = Path::new("test/other-bogus-path");
 
-    match reflink(from, to) {
-        Ok(..) => panic!(),
-        Err(..) => {
-            assert!(!from.exists());
-            assert!(!to.exists());
-        }
-    }
+    reflink(from, to).unwrap_err();
+    assert!(!from.exists());
+    assert!(!to.exists());
 }
 
 #[test]
@@ -37,28 +33,57 @@ fn reflink_dest_is_dir() {
     let dir = tempdir().unwrap();
     let src_file_path = dir.path().join("src.txt");
     let _src_file = File::create(&src_file_path).unwrap();
-    match reflink(&src_file_path, dir.path()) {
-        Ok(()) => panic!(),
-        Err(e) => {
-            println!("{:?}", e);
-            if !cfg!(windows) {
-                assert_eq!(e.kind(), io::ErrorKind::AlreadyExists);
-            }
-        }
+    let err = reflink(&src_file_path, dir.path()).unwrap_err();
+    println!("{:?}", err);
+    if cfg!(windows) {
+        assert_eq!(err.kind(), io::ErrorKind::PermissionDenied);
+    } else {
+        assert_eq!(err.kind(), io::ErrorKind::AlreadyExists);
     }
 }
 
+// No reliable symlinking on windows, while macos can reflink symlinks.
+#[cfg(all(unix, not(any(target_os = "macos", target_os = "ios"))))]
+#[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");
+
+    let err = reflink(symlink, dest_file_path).unwrap_err();
+    println!("{:?}", err);
+    assert_eq!(err.kind(), io::ErrorKind::InvalidInput)
+}
+
+#[cfg(not(any(target_os = "macos", target_os = "ios")))]
 #[test]
 fn reflink_src_is_dir() {
     let dir = tempdir().unwrap();
     let dest_file_path = dir.path().join("dest.txt");
 
-    match reflink(dir.path(), dest_file_path) {
-        Ok(()) => panic!(),
-        Err(e) => {
-            println!("{:?}", e);
-            assert_eq!(e.kind(), io::ErrorKind::InvalidInput)
-        }
+    let err = reflink(dir.path(), dest_file_path).unwrap_err();
+    println!("{:?}", err);
+    assert_eq!(err.kind(), io::ErrorKind::InvalidInput);
+}
+
+#[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");
+
+    fs::create_dir(&src_file_path).unwrap();
+    fs::create_dir(&dest_file_path).unwrap();
+
+    let err = reflink(&src_file_path, &dest_file_path).unwrap_err();
+    println!("{:?}", err);
+    if cfg!(any(target_os = "macos", target_os = "ios")) {
+        assert_eq!(err.kind(), io::ErrorKind::AlreadyExists);
+    } else {
+        assert_eq!(err.kind(), io::ErrorKind::InvalidInput);
     }
 }
 
@@ -71,13 +96,9 @@ fn reflink_existing_dest_results_in_error() {
     let _src_file = File::create(&src_file_path).unwrap();
     let _dest_file = File::create(&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)
-        }
-    }
+    let err = reflink(&src_file_path, &dest_file_path).unwrap_err();
+    println!("{:?}", err);
+    assert_eq!(err.kind(), io::ErrorKind::AlreadyExists)
 }
 
 #[test]
@@ -88,15 +109,10 @@ fn reflink_ok() {
 
     fs::write(&src_file_path, b"this is a test").unwrap();
 
-    match reflink(&src_file_path, &dest_file_path) {
-        Ok(()) => {}
-        Err(e) => {
-            println!("{:?}", e);
-            // do not panic for now, CI envs are old and will probably error out
-            return;
-        }
-    }
-    assert_eq!(fs::read(&dest_file_path).unwrap(), b"this is a test");
+    let err = reflink(&src_file_path, &dest_file_path);
+    println!("{:?}", err);
+    // do not panic for now, CI envs are old and will probably error out
+    // assert_eq!(fs::read(&dest_file_path).unwrap(), b"this is a test");
 }
 
 #[test]