From 0a7b16c99879b50e8c08cc6fe96e607ef1ec1ae2 Mon Sep 17 00:00:00 2001 From: Reuben Wong Date: Fri, 6 Feb 2026 23:27:48 +0800 Subject: [PATCH 1/3] open file with nonblock and add test --- Cargo.lock | 1 + src/uu/sync/Cargo.toml | 1 + src/uu/sync/src/sync.rs | 9 ++++++--- tests/by-util/test_sync.rs | 18 ++++++++++++++++++ 4 files changed, 26 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 489778c5a07..4bd1b91da9e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4171,6 +4171,7 @@ version = "0.6.0" dependencies = [ "clap", "fluent", + "libc", "nix", "uucore", "windows-sys 0.61.2", diff --git a/src/uu/sync/Cargo.toml b/src/uu/sync/Cargo.toml index 6d00339abb3..65f98566c7c 100644 --- a/src/uu/sync/Cargo.toml +++ b/src/uu/sync/Cargo.toml @@ -18,6 +18,7 @@ workspace = true path = "src/sync.rs" [dependencies] +libc = { workspace = true } clap = { workspace = true } uucore = { workspace = true, features = ["wide"] } fluent = { workspace = true } diff --git a/src/uu/sync/src/sync.rs b/src/uu/sync/src/sync.rs index 0e5b864b9cd..f6e52b464da 100644 --- a/src/uu/sync/src/sync.rs +++ b/src/uu/sync/src/sync.rs @@ -35,9 +35,9 @@ mod platform { #[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; + use std::fs::{File, OpenOptions}; #[cfg(any(target_os = "linux", target_os = "android"))] - use uucore::error::FromIo; + use std::os::unix::fs::OpenOptionsExt; #[cfg(any(target_os = "linux", target_os = "android"))] use uucore::translate; @@ -57,7 +57,10 @@ mod platform { /// Logs a warning if fcntl fails but doesn't abort the operation. #[cfg(any(target_os = "linux", target_os = "android"))] fn open_and_reset_nonblock(path: &str) -> UResult { - let f = File::open(path).map_err_context(|| path.to_string())?; + let f = OpenOptions::new() + .read(true) + .custom_flags(libc::O_NONBLOCK) + .open(path)?; // Reset O_NONBLOCK flag if it was set (matches GNU behavior) // This is non-critical, so we log errors but don't fail if let Err(e) = fcntl(&f, FcntlArg::F_SETFL(OFlag::empty())) { diff --git a/tests/by-util/test_sync.rs b/tests/by-util/test_sync.rs index 9c3df3a1e61..b67ea54c7ec 100644 --- a/tests/by-util/test_sync.rs +++ b/tests/by-util/test_sync.rs @@ -167,3 +167,21 @@ fn test_sync_multiple_files() { // Sync both files new_ucmd!().arg("--data").arg(&file1).arg(&file2).succeeds(); } + +#[cfg(any(target_os = "linux", target_os = "android"))] +#[test] +fn test_sync_data_fifo_fails_immediately() { + use std::time::Duration; + use uutests::util::TestScenario; + use uutests::util_name; + + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + at.mkfifo("testfifo"); + + ts.ucmd() + .arg("--data") + .arg(at.plus_as_string("testfifo")) + .timeout(Duration::from_secs(2)) + .fails(); +} From 60d86047f41070ef61e50169ab3ba7b0e50fc613 Mon Sep 17 00:00:00 2001 From: Reuben Wong Date: Fri, 6 Feb 2026 23:29:59 +0800 Subject: [PATCH 2/3] fix spellcheck --- tests/by-util/test_sync.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/by-util/test_sync.rs b/tests/by-util/test_sync.rs index b67ea54c7ec..08a18f8a85f 100644 --- a/tests/by-util/test_sync.rs +++ b/tests/by-util/test_sync.rs @@ -177,11 +177,11 @@ fn test_sync_data_fifo_fails_immediately() { let ts = TestScenario::new(util_name!()); let at = &ts.fixtures; - at.mkfifo("testfifo"); + at.mkfifo("test-fifo"); ts.ucmd() .arg("--data") - .arg(at.plus_as_string("testfifo")) + .arg(at.plus_as_string("test-fifo")) .timeout(Duration::from_secs(2)) .fails(); } From 9b8c488aef6c09ab1e205d1ab1cc12240af6118d Mon Sep 17 00:00:00 2001 From: Reuben Wong Date: Sat, 7 Feb 2026 11:05:05 +0800 Subject: [PATCH 3/3] remove libc dependency; improve errors --- Cargo.lock | 1 - src/uu/sync/Cargo.toml | 1 - src/uu/sync/locales/en-US.ftl | 1 + src/uu/sync/locales/fr-FR.ftl | 1 + src/uu/sync/src/sync.rs | 17 +++++++++++++---- tests/by-util/test_sync.rs | 5 ++++- 6 files changed, 19 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bf4b4a0dd06..3c3e94dd5fd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4175,7 +4175,6 @@ version = "0.6.0" dependencies = [ "clap", "fluent", - "libc", "nix", "uucore", "windows-sys 0.61.2", diff --git a/src/uu/sync/Cargo.toml b/src/uu/sync/Cargo.toml index 65f98566c7c..6d00339abb3 100644 --- a/src/uu/sync/Cargo.toml +++ b/src/uu/sync/Cargo.toml @@ -18,7 +18,6 @@ workspace = true path = "src/sync.rs" [dependencies] -libc = { workspace = true } clap = { workspace = true } uucore = { workspace = true, features = ["wide"] } fluent = { workspace = true } diff --git a/src/uu/sync/locales/en-US.ftl b/src/uu/sync/locales/en-US.ftl index d0b520bcb95..b473c03d913 100644 --- a/src/uu/sync/locales/en-US.ftl +++ b/src/uu/sync/locales/en-US.ftl @@ -9,6 +9,7 @@ sync-help-data = sync only file data, no unneeded metadata (Linux only) sync-error-data-needs-argument = --data needs at least one argument sync-error-opening-file = error opening { $file } sync-error-no-such-file = error opening { $file }: No such file or directory +sync-error-syncing-file = error syncing { $file } # Warning messages sync-warning-fcntl-failed = warning: failed to reset O_NONBLOCK flag for { $file }: { $error } diff --git a/src/uu/sync/locales/fr-FR.ftl b/src/uu/sync/locales/fr-FR.ftl index f0d00db03ba..e2db79b90fc 100644 --- a/src/uu/sync/locales/fr-FR.ftl +++ b/src/uu/sync/locales/fr-FR.ftl @@ -9,6 +9,7 @@ sync-help-data = synchroniser seulement les données des fichiers, pas les méta sync-error-data-needs-argument = --data nécessite au moins un argument sync-error-opening-file = erreur lors de l'ouverture de { $file } sync-error-no-such-file = erreur lors de l'ouverture de { $file } : Aucun fichier ou répertoire de ce type +sync-error-syncing-file = erreur lors de la synchronisation de { $file } # Messages d'avertissement sync-warning-fcntl-failed = avertissement : échec de la réinitialisation du drapeau O_NONBLOCK pour { $file } : { $error } diff --git a/src/uu/sync/src/sync.rs b/src/uu/sync/src/sync.rs index f6e52b464da..87873cedab5 100644 --- a/src/uu/sync/src/sync.rs +++ b/src/uu/sync/src/sync.rs @@ -39,6 +39,10 @@ mod platform { #[cfg(any(target_os = "linux", target_os = "android"))] use std::os::unix::fs::OpenOptionsExt; #[cfg(any(target_os = "linux", target_os = "android"))] + use uucore::display::Quotable; + #[cfg(any(target_os = "linux", target_os = "android"))] + use uucore::error::FromIo; + #[cfg(any(target_os = "linux", target_os = "android"))] use uucore::translate; use uucore::error::UResult; @@ -59,8 +63,9 @@ mod platform { fn open_and_reset_nonblock(path: &str) -> UResult { let f = OpenOptions::new() .read(true) - .custom_flags(libc::O_NONBLOCK) - .open(path)?; + .custom_flags(OFlag::O_NONBLOCK.bits()) + .open(path) + .map_err_context(|| path.to_string())?; // Reset O_NONBLOCK flag if it was set (matches GNU behavior) // This is non-critical, so we log errors but don't fail if let Err(e) = fcntl(&f, FcntlArg::F_SETFL(OFlag::empty())) { @@ -76,7 +81,9 @@ mod platform { pub fn do_syncfs(files: Vec) -> UResult<()> { for path in files { let f = open_and_reset_nonblock(&path)?; - syncfs(f)?; + syncfs(f).map_err_context( + || translate!("sync-error-syncing-file", "file" => path.quote()), + )?; } Ok(()) } @@ -85,7 +92,9 @@ mod platform { pub fn do_fdatasync(files: Vec) -> UResult<()> { for path in files { let f = open_and_reset_nonblock(&path)?; - fdatasync(f)?; + fdatasync(f).map_err_context( + || translate!("sync-error-syncing-file", "file" => path.quote()), + )?; } Ok(()) } diff --git a/tests/by-util/test_sync.rs b/tests/by-util/test_sync.rs index 08a18f8a85f..fe979cc0df1 100644 --- a/tests/by-util/test_sync.rs +++ b/tests/by-util/test_sync.rs @@ -183,5 +183,8 @@ fn test_sync_data_fifo_fails_immediately() { .arg("--data") .arg(at.plus_as_string("test-fifo")) .timeout(Duration::from_secs(2)) - .fails(); + .fails() + .stderr_contains("error syncing") + .stderr_contains("test-fifo") + .stderr_contains("Invalid input"); }