Merge pull request #6108 from BenWiederhake/dev-truncate-reference-create

truncate: correctly handle file (non-)creation

src/uu/truncate/src/truncate.rs

@@ -178,10 +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);
-    let f = OpenOptions::new().write(true).create(create).open(path)?;
+    match OpenOptions::new().write(true).create(create).open(path) {
-    f.set_len(size)
+        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.
@@ -233,19 +249,7 @@ fn truncate_reference_and_size(
     let fsize = metadata.len();
     let tsize = mode.to_size(fsize);
     for filename in filenames {
-        #[cfg(unix)]
+        file_truncate(filename, create, tsize)?;
-        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()))?;
     }
     Ok(())
 }
@@ -280,18 +284,7 @@ fn truncate_reference_file_only(
     })?;
     let tsize = metadata.len();
     for filename in filenames {
-        #[cfg(unix)]
+        file_truncate(filename, create, tsize)?;
-        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()))?;
     }
     Ok(())
 }
@@ -337,15 +330,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) {
+        // TODO: Fix duplicate call to stat
-            Ok(_) => continue,
+        file_truncate(filename, create, tsize)?;
-            Err(e) if e.kind() == ErrorKind::NotFound && !create => continue,
-            Err(e) => {
-                return Err(
-                    e.map_err_context(|| format!("cannot open {} for writing", filename.quote()))
-                )
-            }
-        }
     }
     Ok(())
 }

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!()
-    // new_ucmd!()
+        .args(&["--no-create", "--size", "K", "file"])
-    //     .args(&["--no-create", "--size", "K", "file"])
+        .succeeds();
-    //     .succeeds();
     new_ucmd!()
         .args(&["--size", "1024R", "file"])
         .fails()
@@ -270,9 +266,39 @@ 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() {
+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])
@@ -282,6 +308,34 @@ fn test_new_file_no_create() {
     assert!(!at.file_exists(filename));
 }
 
+/// Test for not creating a non-existent file.
+#[test]
+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]
+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!()