feat(sync): remove unwrap and align with GNU behavior

Author: naoNao89

.vscode/cspell.dictionaries/jargon.wordlist.txt

@@ -128,6 +128,7 @@ semiprimes
 setcap
 setfacl
 setfattr
+SETFL
 shortcode
 shortcodes
 siginfo

src/uu/sync/src/sync.rs

@@ -29,11 +29,16 @@ static ARG_FILES: &str = "files";
 
 #[cfg(unix)]
 mod platform {
+    #[cfg(any(target_os = "linux", target_os = "android"))]
+    use nix::fcntl::{FcntlArg, OFlag, fcntl};
     use nix::unistd::sync;
     #[cfg(any(target_os = "linux", target_os = "android"))]
     use nix::unistd::{fdatasync, syncfs};
     #[cfg(any(target_os = "linux", target_os = "android"))]
     use std::fs::File;
+    #[cfg(any(target_os = "linux", target_os = "android"))]
+    use uucore::error::FromIo;
+
     use uucore::error::UResult;
 
     pub fn do_sync() -> UResult<()> {
@@ -44,7 +49,9 @@ mod platform {
     #[cfg(any(target_os = "linux", target_os = "android"))]
     pub fn do_syncfs(files: Vec<String>) -> UResult<()> {
         for path in files {
-            let f = File::open(path).unwrap();
+            let f = File::open(&path).map_err_context(|| path.clone())?;
+            // Reset O_NONBLOCK flag if it was set (matches GNU behavior)
+            let _ = fcntl(&f, FcntlArg::F_SETFL(OFlag::empty()));
             syncfs(f)?;
         }
         Ok(())
@@ -53,7 +60,9 @@ mod platform {
     #[cfg(any(target_os = "linux", target_os = "android"))]
     pub fn do_fdatasync(files: Vec<String>) -> UResult<()> {
         for path in files {
-            let f = File::open(path).unwrap();
+            let f = File::open(&path).map_err_context(|| path.clone())?;
+            // Reset O_NONBLOCK flag if it was set (matches GNU behavior)
+            let _ = fcntl(&f, FcntlArg::F_SETFL(OFlag::empty()));
             fdatasync(f)?;
         }
         Ok(())
@@ -157,15 +166,17 @@ mod platform {
 
     pub fn do_syncfs(files: Vec<String>) -> UResult<()> {
         for path in files {
-            flush_volume(
-                Path::new(&path)
-                    .components()
-                    .next()
-                    .unwrap()
-                    .as_os_str()
-                    .to_str()
-                    .unwrap(),
-            )?;
+            let maybe_first = Path::new(&path).components().next();
+            let vol_name = match maybe_first {
+                Some(c) => c.as_os_str().to_string_lossy().into_owned(),
+                None => {
+                    return Err(USimpleError::new(
+                        1,
+                        translate!("sync-error-no-such-file", "file" => path),
+                    ));
+                }
+            };
+            flush_volume(&vol_name)?;
         }
         Ok(())
     }

tests/by-util/test_sync.rs

@@ -90,3 +90,85 @@ fn test_sync_no_permission_file() {
     ts.ccmd("chmod").arg("0200").arg(f).succeeds();
     ts.ucmd().arg(f).succeeds();
 }
+
+#[cfg(any(target_os = "linux", target_os = "android"))]
+#[test]
+fn test_sync_data_nonblock_flag_reset() {
+    // Test that O_NONBLOCK flag is properly reset when syncing files
+    // This verifies the fix in commit 05612a4792ba2a5ad509cf995b91e868fc261229
+    use std::fs::File;
+    use std::io::Write;
+    use uutests::util::TestScenario;
+    use uutests::util_name;
+
+    let ts = TestScenario::new(util_name!());
+    let at = &ts.fixtures;
+    let test_file = "test_file.txt";
+
+    // Create a test file
+    at.write(test_file, "test content");
+
+    // Run sync --data with the file - should succeed
+    ts.ucmd().arg("--data").arg(test_file).succeeds();
+}
+
+#[cfg(any(target_os = "linux", target_os = "android"))]
+#[test]
+fn test_sync_fs_nonblock_flag_reset() {
+    // Test that O_NONBLOCK flag is properly reset when syncing filesystems
+    use std::fs;
+    use tempfile::tempdir;
+
+    let temporary_directory = tempdir().unwrap();
+    let temporary_path = fs::canonicalize(temporary_directory.path()).unwrap();
+
+    // Run sync --file-system with the path - should succeed
+    new_ucmd!()
+        .arg("--file-system")
+        .arg(&temporary_path)
+        .succeeds();
+}
+
+#[cfg(any(target_os = "linux", target_os = "android"))]
+#[test]
+fn test_sync_fdatasync_error_handling() {
+    // Test that fdatasync properly handles file opening errors
+    // This verifies the error handling fix in commit 05612a4792ba2a5ad509cf995b91e868fc261229
+    new_ucmd!()
+        .arg("--data")
+        .arg("/nonexistent/path/to/file")
+        .fails()
+        .stderr_contains("error opening");
+}
+
+#[cfg(target_os = "macos")]
+#[test]
+fn test_sync_syncfs_error_handling_macos() {
+    // Test that syncfs properly handles invalid paths on macOS
+    // This verifies the error handling fix in commit 05612a4792ba2a5ad509cf995b91e868fc261229
+    new_ucmd!()
+        .arg("--file-system")
+        .arg("/nonexistent/path/to/file")
+        .fails()
+        .stderr_contains("error opening");
+}
+
+#[test]
+fn test_sync_multiple_files() {
+    // Test syncing multiple files at once
+    use std::fs;
+    use tempfile::tempdir;
+
+    let temporary_directory = tempdir().unwrap();
+    let temp_path = temporary_directory.path();
+
+    // Create multiple test files
+    let file1 = temp_path.join("file1.txt");
+    let file2 = temp_path.join("file2.txt");
+
+    fs::write(&file1, "content1").unwrap();
+    fs::write(&file2, "content2").unwrap();
+
+    // Sync both files
+    new_ucmd!().arg("--data").arg(&file1).arg(&file2).succeeds();
+}