From 1dc463fd26c578290c2c6e88045516f511e96c82 Mon Sep 17 00:00:00 2001 From: Fuad Ismail Date: Thu, 12 Dec 2024 11:02:50 +0700 Subject: [PATCH 1/9] tests/csplit: add named pipe input file test. --- tests/by-util/test_csplit.rs | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/by-util/test_csplit.rs b/tests/by-util/test_csplit.rs index 9323b9851..991cd36ef 100644 --- a/tests/by-util/test_csplit.rs +++ b/tests/by-util/test_csplit.rs @@ -1417,3 +1417,34 @@ fn repeat_everything() { assert_eq!(at.read("xxz_004"), generate(37, 44 + 1)); assert_eq!(at.read("xxz_005"), generate(46, 50 + 1)); } + +#[cfg(unix)] +#[test] +fn test_named_pipe_input_file() { + let (at, mut ucmd) = at_and_ucmd!(); + + let mut fifo_writer = + create_named_pipe_with_writer(&at.plus_as_string("fifo"), &generate(1, 51)); + + let result = ucmd.args(&["fifo", "10"]).succeeds(); + fifo_writer.kill().unwrap(); + fifo_writer.wait().unwrap(); + result.stdout_only("18\n123\n"); + + let count = glob(&at.plus_as_string("xx*")) + .expect("there should be splits created") + .count(); + assert_eq!(count, 2); + assert_eq!(at.read("xx00"), generate(1, 10)); + assert_eq!(at.read("xx01"), generate(10, 51)); +} + +#[cfg(unix)] +fn create_named_pipe_with_writer(path: &str, data: &str) -> std::process::Child { + nix::unistd::mkfifo(path, nix::sys::stat::Mode::S_IRWXU).unwrap(); + std::process::Command::new("sh") + .arg("-c") + .arg(format!("echo -n '{}' > {path}", data)) + .spawn() + .unwrap() +} From 757c0b260e1ef7b886dfadd91530623054d2ddf3 Mon Sep 17 00:00:00 2001 From: Fuad Ismail Date: Thu, 12 Dec 2024 11:05:39 +0700 Subject: [PATCH 2/9] csplit: defer IO read error handling to iterator. --- src/uu/csplit/src/csplit.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/uu/csplit/src/csplit.rs b/src/uu/csplit/src/csplit.rs index d654c9271..a0bfc61cc 100644 --- a/src/uu/csplit/src/csplit.rs +++ b/src/uu/csplit/src/csplit.rs @@ -582,12 +582,6 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } else { let file = File::open(file_name) .map_err_context(|| format!("cannot access {}", file_name.quote()))?; - let file_metadata = file - .metadata() - .map_err_context(|| format!("cannot access {}", file_name.quote()))?; - if !file_metadata.is_file() { - return Err(CsplitError::NotRegularFile(file_name.to_string()).into()); - } Ok(csplit(&options, &patterns, BufReader::new(file))?) } } From 19f990f29ad1434b79a6704db21926f279ba436d Mon Sep 17 00:00:00 2001 From: Fuad Ismail Date: Thu, 12 Dec 2024 11:16:57 +0700 Subject: [PATCH 3/9] tests/csplit: add directory input file test. --- tests/by-util/test_csplit.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/by-util/test_csplit.rs b/tests/by-util/test_csplit.rs index 991cd36ef..697a8f47a 100644 --- a/tests/by-util/test_csplit.rs +++ b/tests/by-util/test_csplit.rs @@ -1448,3 +1448,14 @@ fn create_named_pipe_with_writer(path: &str, data: &str) -> std::process::Child .spawn() .unwrap() } + +#[test] +fn test_directory_input_file() { + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir("test_directory"); + + ucmd.args(&["test_directory", "1"]) + .fails() + .code_is(1) + .stderr_only("csplit: read error: Is a directory\n"); +} From 51dce9c5f8df412e94740411581d20101303645b Mon Sep 17 00:00:00 2001 From: Fuad Ismail Date: Thu, 12 Dec 2024 11:45:58 +0700 Subject: [PATCH 4/9] csplit: return UResult instead of io::Result from iterator to handle error message more uniformly. --- src/uu/csplit/src/csplit.rs | 18 +++++++++++------- src/uu/csplit/src/csplit_error.rs | 17 ++++++++++++++--- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/src/uu/csplit/src/csplit.rs b/src/uu/csplit/src/csplit.rs index a0bfc61cc..678d17094 100644 --- a/src/uu/csplit/src/csplit.rs +++ b/src/uu/csplit/src/csplit.rs @@ -87,7 +87,11 @@ pub fn csplit(options: &CsplitOptions, patterns: &[String], input: T) -> Resu where T: BufRead, { - let mut input_iter = InputSplitter::new(input.lines().enumerate()); + let enumerated_input_lines = input + .lines() + .map(|line| line.map_err_context(|| "read error".to_string())) + .enumerate(); + let mut input_iter = InputSplitter::new(enumerated_input_lines); let mut split_writer = SplitWriter::new(options); let patterns: Vec = patterns::get_patterns(patterns)?; let ret = do_csplit(&mut split_writer, patterns, &mut input_iter); @@ -117,7 +121,7 @@ fn do_csplit( input_iter: &mut InputSplitter, ) -> Result<(), CsplitError> where - I: Iterator)>, + I: Iterator)>, { // split the file based on patterns for pattern in patterns { @@ -305,7 +309,7 @@ impl SplitWriter<'_> { input_iter: &mut InputSplitter, ) -> Result<(), CsplitError> where - I: Iterator)>, + I: Iterator)>, { input_iter.rewind_buffer(); input_iter.set_size_of_buffer(1); @@ -358,7 +362,7 @@ impl SplitWriter<'_> { input_iter: &mut InputSplitter, ) -> Result<(), CsplitError> where - I: Iterator)>, + I: Iterator)>, { if offset >= 0 { // The offset is zero or positive, no need for a buffer on the lines read. @@ -470,7 +474,7 @@ impl SplitWriter<'_> { /// This is used to pass matching lines to the next split and to support patterns with a negative offset. struct InputSplitter where - I: Iterator)>, + I: Iterator)>, { iter: I, buffer: Vec<::Item>, @@ -483,7 +487,7 @@ where impl InputSplitter where - I: Iterator)>, + I: Iterator)>, { fn new(iter: I) -> Self { Self { @@ -547,7 +551,7 @@ where impl Iterator for InputSplitter where - I: Iterator)>, + I: Iterator)>, { type Item = ::Item; diff --git a/src/uu/csplit/src/csplit_error.rs b/src/uu/csplit/src/csplit_error.rs index 4a83b637b..ac1c8d01c 100644 --- a/src/uu/csplit/src/csplit_error.rs +++ b/src/uu/csplit/src/csplit_error.rs @@ -35,6 +35,8 @@ pub enum CsplitError { SuffixFormatTooManyPercents, #[error("{} is not a regular file", ._0.quote())] NotRegularFile(String), + #[error("{}", _0)] + UError(Box), } impl From for CsplitError { @@ -43,8 +45,17 @@ impl From for CsplitError { } } -impl UError for CsplitError { - fn code(&self) -> i32 { - 1 +impl From> for CsplitError { + fn from(error: Box) -> Self { + Self::UError(error) + } +} + +impl UError for CsplitError { + fn code(&self) -> i32 { + match self { + Self::UError(e) => e.code(), + _ => 1, + } } } From c8bc5d24550b3b7f7f19a0b66ec3e97356b62a85 Mon Sep 17 00:00:00 2001 From: Fuad Ismail Date: Tue, 7 Jan 2025 21:35:30 +0700 Subject: [PATCH 5/9] tests/csplit: handle directory input file test for Windows separately --- tests/by-util/test_csplit.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/by-util/test_csplit.rs b/tests/by-util/test_csplit.rs index 697a8f47a..7dcd79e4e 100644 --- a/tests/by-util/test_csplit.rs +++ b/tests/by-util/test_csplit.rs @@ -1454,8 +1454,14 @@ fn test_directory_input_file() { let (at, mut ucmd) = at_and_ucmd!(); at.mkdir("test_directory"); + #[cfg(unix)] ucmd.args(&["test_directory", "1"]) .fails() .code_is(1) .stderr_only("csplit: read error: Is a directory\n"); + #[cfg(windows)] + ucmd.args(&["test_directory", "1"]) + .fails() + .code_is(1) + .stderr_only("csplit: cannot open 'test_directory' for reading: Permission denied\n"); } From 96929734ea95d99c3ff9678a3cebaf667179921f Mon Sep 17 00:00:00 2001 From: Fuad Ismail Date: Tue, 7 Jan 2025 21:44:09 +0700 Subject: [PATCH 6/9] tests/csplit: modify no_such_file test expected error to conform better with original csplit error --- tests/by-util/test_csplit.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/by-util/test_csplit.rs b/tests/by-util/test_csplit.rs index 7dcd79e4e..bb714509e 100644 --- a/tests/by-util/test_csplit.rs +++ b/tests/by-util/test_csplit.rs @@ -1379,7 +1379,7 @@ fn no_such_file() { let (_, mut ucmd) = at_and_ucmd!(); ucmd.args(&["in", "0"]) .fails() - .stderr_contains("cannot access 'in': No such file or directory"); + .stderr_contains("cannot open 'in' for reading: No such file or directory"); } #[test] From 694298f0d1b1ee30deda46913820de73c311e2af Mon Sep 17 00:00:00 2001 From: Fuad Ismail Date: Tue, 7 Jan 2025 21:45:52 +0700 Subject: [PATCH 7/9] csplit: modify failure in opening file error message --- src/uu/csplit/src/csplit.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/csplit/src/csplit.rs b/src/uu/csplit/src/csplit.rs index 678d17094..501f97582 100644 --- a/src/uu/csplit/src/csplit.rs +++ b/src/uu/csplit/src/csplit.rs @@ -585,7 +585,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { Ok(csplit(&options, &patterns, stdin.lock())?) } else { let file = File::open(file_name) - .map_err_context(|| format!("cannot access {}", file_name.quote()))?; + .map_err_context(|| format!("cannot open {} for reading", file_name.quote()))?; Ok(csplit(&options, &patterns, BufReader::new(file))?) } } From 981019138da55cf10cc73ac9f18d09adb1d77116 Mon Sep 17 00:00:00 2001 From: Fuad Ismail Date: Tue, 7 Jan 2025 15:31:49 +0700 Subject: [PATCH 8/9] tests/csplit: ignore IRWXU from cspell check --- tests/by-util/test_csplit.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/by-util/test_csplit.rs b/tests/by-util/test_csplit.rs index bb714509e..0b2353a50 100644 --- a/tests/by-util/test_csplit.rs +++ b/tests/by-util/test_csplit.rs @@ -1441,6 +1441,7 @@ fn test_named_pipe_input_file() { #[cfg(unix)] fn create_named_pipe_with_writer(path: &str, data: &str) -> std::process::Child { + // cSpell:ignore IRWXU nix::unistd::mkfifo(path, nix::sys::stat::Mode::S_IRWXU).unwrap(); std::process::Command::new("sh") .arg("-c") From a4fdecbf48e33116fe91d993a43e47a074d18102 Mon Sep 17 00:00:00 2001 From: Fuad Ismail Date: Tue, 7 Jan 2025 23:13:47 +0700 Subject: [PATCH 9/9] tests/csplit: use printf instead of echo -n for maximum portability with all UNIX systems --- tests/by-util/test_csplit.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/by-util/test_csplit.rs b/tests/by-util/test_csplit.rs index 0b2353a50..38f5c97bf 100644 --- a/tests/by-util/test_csplit.rs +++ b/tests/by-util/test_csplit.rs @@ -1445,7 +1445,7 @@ fn create_named_pipe_with_writer(path: &str, data: &str) -> std::process::Child nix::unistd::mkfifo(path, nix::sys::stat::Mode::S_IRWXU).unwrap(); std::process::Command::new("sh") .arg("-c") - .arg(format!("echo -n '{}' > {path}", data)) + .arg(format!("printf '{}' > {path}", data)) .spawn() .unwrap() }