From 4fcf912c85e7b519dc54118044d5fb405f353d18 Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Sun, 3 Mar 2024 13:06:14 +0100 Subject: [PATCH 1/3] truncate: remove two-year-old file-creation todos In commit 129cfe12b826377a5eeb24577c492bcb0da02033 --- tests/by-util/test_truncate.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/tests/by-util/test_truncate.rs b/tests/by-util/test_truncate.rs index 81b87ed2e..f7122902e 100644 --- a/tests/by-util/test_truncate.rs +++ b/tests/by-util/test_truncate.rs @@ -43,9 +43,6 @@ fn test_reference() { let mut file = at.make_file(FILE2); // manpage: "A FILE argument that does not exist is created." - // TODO: 'truncate' does not create the file in this case, - // but should because '--no-create' wasn't specified. - at.touch(FILE1); // TODO: remove this when 'no-create' is fixed scene.ucmd().arg("-s").arg("+5KB").arg(FILE1).succeeds(); scene @@ -240,10 +237,9 @@ fn test_reference_with_size_file_not_found() { #[test] fn test_truncate_bytes_size() { - // TODO: this should succeed without error, uncomment when '--no-create' is fixed - // new_ucmd!() - // .args(&["--no-create", "--size", "K", "file"]) - // .succeeds(); + new_ucmd!() + .args(&["--no-create", "--size", "K", "file"]) + .succeeds(); new_ucmd!() .args(&["--size", "1024R", "file"]) .fails() From a3ab064f352f7b6c632b15f627806ae5ca287f5b Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Sun, 3 Mar 2024 20:09:07 +0100 Subject: [PATCH 2/3] truncate: don't error in --no-create with reference case --- src/uu/truncate/src/truncate.rs | 18 +++++++----------- tests/by-util/test_truncate.rs | 32 +++++++++++++++++++++++++++++++- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/src/uu/truncate/src/truncate.rs b/src/uu/truncate/src/truncate.rs index 9368ce9b1..c0e38f59b 100644 --- a/src/uu/truncate/src/truncate.rs +++ b/src/uu/truncate/src/truncate.rs @@ -180,8 +180,11 @@ pub fn uu_app() -> Command { /// size of the file. fn file_truncate(filename: &str, create: bool, size: u64) -> std::io::Result<()> { let path = Path::new(filename); - let f = OpenOptions::new().write(true).create(create).open(path)?; - f.set_len(size) + match OpenOptions::new().write(true).create(create).open(path) { + Ok(file) => file.set_len(size), + Err(e) if e.kind() == ErrorKind::NotFound && !create => Ok(()), + Err(e) => Err(e), + } } /// Truncate files to a size relative to a given file. @@ -337,15 +340,8 @@ fn truncate_size_only(size_string: &str, filenames: &[String], create: bool) -> Err(_) => 0, }; let tsize = mode.to_size(fsize); - match file_truncate(filename, create, tsize) { - Ok(_) => continue, - Err(e) if e.kind() == ErrorKind::NotFound && !create => continue, - Err(e) => { - return Err( - e.map_err_context(|| format!("cannot open {} for writing", filename.quote())) - ) - } - } + file_truncate(filename, create, tsize) + .map_err_context(|| format!("cannot open {} for writing", filename.quote()))?; } Ok(()) } diff --git a/tests/by-util/test_truncate.rs b/tests/by-util/test_truncate.rs index f7122902e..41a28736d 100644 --- a/tests/by-util/test_truncate.rs +++ b/tests/by-util/test_truncate.rs @@ -268,7 +268,7 @@ fn test_new_file() { /// Test for not creating a non-existent file. #[test] -fn test_new_file_no_create() { +fn test_new_file_no_create_size_only() { let (at, mut ucmd) = at_and_ucmd!(); let filename = "new_file_that_does_not_exist_yet"; ucmd.args(&["-s", "8", "-c", filename]) @@ -278,6 +278,36 @@ fn test_new_file_no_create() { assert!(!at.file_exists(filename)); } +/// Test for not creating a non-existent file. +#[test] +#[ignore = "other bug"] +fn test_new_file_no_create_reference_only() { + let (at, mut ucmd) = at_and_ucmd!(); + let mut old_file = at.make_file(FILE1); + old_file.write_all(b"1234567890").unwrap(); + let filename = "new_file_that_does_not_exist_yet"; + ucmd.args(&["-r", FILE1, "-c", filename]) + .succeeds() + .no_stdout() + .no_stderr(); + assert!(!at.file_exists(filename)); +} + +/// Test for not creating a non-existent file. +#[test] +#[ignore = "other bug"] +fn test_new_file_no_create_size_and_reference() { + let (at, mut ucmd) = at_and_ucmd!(); + let mut old_file = at.make_file(FILE1); + old_file.write_all(b"1234567890").unwrap(); + let filename = "new_file_that_does_not_exist_yet"; + ucmd.args(&["-r", FILE1, "-s", "+8", "-c", filename]) + .succeeds() + .no_stdout() + .no_stderr(); + assert!(!at.file_exists(filename)); +} + #[test] fn test_division_by_zero_size_only() { new_ucmd!() From a1ad751aa9738e0285491dd36ce92d6ce1718456 Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Sun, 3 Mar 2024 20:14:05 +0100 Subject: [PATCH 3/3] truncate: deduplicate fifo check, fix handling of missing files The fifo check used to include 'metadata(filename)?', which would error if the file does not exist. In our case however, this is not an error. --- src/uu/truncate/src/truncate.rs | 46 +++++++++++++-------------------- tests/by-util/test_truncate.rs | 32 +++++++++++++++++++++-- 2 files changed, 48 insertions(+), 30 deletions(-) diff --git a/src/uu/truncate/src/truncate.rs b/src/uu/truncate/src/truncate.rs index c0e38f59b..7af25085f 100644 --- a/src/uu/truncate/src/truncate.rs +++ b/src/uu/truncate/src/truncate.rs @@ -178,13 +178,26 @@ pub fn uu_app() -> Command { /// /// If the file could not be opened, or there was a problem setting the /// size of the file. -fn file_truncate(filename: &str, create: bool, size: u64) -> std::io::Result<()> { +fn file_truncate(filename: &str, create: bool, size: u64) -> UResult<()> { + #[cfg(unix)] + if let Ok(metadata) = std::fs::metadata(filename) { + if metadata.file_type().is_fifo() { + return Err(USimpleError::new( + 1, + format!( + "cannot open {} for writing: No such device or address", + filename.quote() + ), + )); + } + } let path = Path::new(filename); match OpenOptions::new().write(true).create(create).open(path) { Ok(file) => file.set_len(size), Err(e) if e.kind() == ErrorKind::NotFound && !create => Ok(()), Err(e) => Err(e), } + .map_err_context(|| format!("cannot open {} for writing", filename.quote())) } /// Truncate files to a size relative to a given file. @@ -236,19 +249,7 @@ fn truncate_reference_and_size( let fsize = metadata.len(); let tsize = mode.to_size(fsize); for filename in filenames { - #[cfg(unix)] - if std::fs::metadata(filename)?.file_type().is_fifo() { - return Err(USimpleError::new( - 1, - format!( - "cannot open {} for writing: No such device or address", - filename.quote() - ), - )); - } - - file_truncate(filename, create, tsize) - .map_err_context(|| format!("cannot open {} for writing", filename.quote()))?; + file_truncate(filename, create, tsize)?; } Ok(()) } @@ -283,18 +284,7 @@ fn truncate_reference_file_only( })?; let tsize = metadata.len(); for filename in filenames { - #[cfg(unix)] - if std::fs::metadata(filename)?.file_type().is_fifo() { - return Err(USimpleError::new( - 1, - format!( - "cannot open {} for writing: No such device or address", - filename.quote() - ), - )); - } - file_truncate(filename, create, tsize) - .map_err_context(|| format!("cannot open {} for writing", filename.quote()))?; + file_truncate(filename, create, tsize)?; } Ok(()) } @@ -340,8 +330,8 @@ fn truncate_size_only(size_string: &str, filenames: &[String], create: bool) -> Err(_) => 0, }; let tsize = mode.to_size(fsize); - file_truncate(filename, create, tsize) - .map_err_context(|| format!("cannot open {} for writing", filename.quote()))?; + // TODO: Fix duplicate call to stat + file_truncate(filename, create, tsize)?; } Ok(()) } diff --git a/tests/by-util/test_truncate.rs b/tests/by-util/test_truncate.rs index 41a28736d..e6a128186 100644 --- a/tests/by-util/test_truncate.rs +++ b/tests/by-util/test_truncate.rs @@ -266,6 +266,36 @@ fn test_new_file() { assert_eq!(at.read_bytes(filename), vec![b'\0'; 8]); } +/// Test that truncating a non-existent file creates that file, even in reference-mode. +#[test] +fn test_new_file_reference() { + let (at, mut ucmd) = at_and_ucmd!(); + let mut old_file = at.make_file(FILE1); + old_file.write_all(b"1234567890").unwrap(); + let filename = "new_file_that_does_not_exist_yet"; + ucmd.args(&["-r", FILE1, filename]) + .succeeds() + .no_stdout() + .no_stderr(); + assert!(at.file_exists(filename)); + assert_eq!(at.read_bytes(filename), vec![b'\0'; 10]); +} + +/// Test that truncating a non-existent file creates that file, even in size-and-reference-mode. +#[test] +fn test_new_file_size_and_reference() { + let (at, mut ucmd) = at_and_ucmd!(); + let mut old_file = at.make_file(FILE1); + old_file.write_all(b"1234567890").unwrap(); + let filename = "new_file_that_does_not_exist_yet"; + ucmd.args(&["-s", "+3", "-r", FILE1, filename]) + .succeeds() + .no_stdout() + .no_stderr(); + assert!(at.file_exists(filename)); + assert_eq!(at.read_bytes(filename), vec![b'\0'; 13]); +} + /// Test for not creating a non-existent file. #[test] fn test_new_file_no_create_size_only() { @@ -280,7 +310,6 @@ fn test_new_file_no_create_size_only() { /// Test for not creating a non-existent file. #[test] -#[ignore = "other bug"] fn test_new_file_no_create_reference_only() { let (at, mut ucmd) = at_and_ucmd!(); let mut old_file = at.make_file(FILE1); @@ -295,7 +324,6 @@ fn test_new_file_no_create_reference_only() { /// Test for not creating a non-existent file. #[test] -#[ignore = "other bug"] fn test_new_file_no_create_size_and_reference() { let (at, mut ucmd) = at_and_ucmd!(); let mut old_file = at.make_file(FILE1);