From fd5140fa476ca690cade17148d2e0765da1424c4 Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Mon, 25 Mar 2024 23:26:20 +0530 Subject: [PATCH 01/33] Fix clone doing standard copy on sparse=auto --- src/uu/cp/src/platform/linux.rs | 54 +++++++++++++++++++++++++++++++-- 1 file changed, 52 insertions(+), 2 deletions(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index 674e66ea575..fa721432c90 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -32,6 +32,9 @@ enum CloneFallback { /// Use [`std::fs::copy`]. FSCopy, + + /// Use sparse_copy + SparseCopy, } /// Use the Linux `ioctl_ficlone` API to do a copy-on-write clone. @@ -53,6 +56,7 @@ where match fallback { CloneFallback::Error => Err(std::io::Error::last_os_error()), CloneFallback::FSCopy => std::fs::copy(source, dest).map(|_| ()), + CloneFallback::SparseCopy => sparse_copy(source, dest), } } @@ -132,6 +136,18 @@ where Ok(num_bytes_copied) } +fn check_sparse_detection(source: &Path, sparse_flag: &mut bool) -> std::io::Result<()> { + let src_file = File::open(source)?; + + use std::os::unix::prelude::MetadataExt; + + let size: usize = src_file.metadata()?.size().try_into().unwrap(); + let blocks: usize = src_file.metadata()?.blocks().try_into().unwrap(); + if blocks < size / 512 { + *sparse_flag = true; + } + Ok(()) +} /// Copies `source` to `dest` using copy-on-write if possible. /// /// The `source_is_fifo` flag must be set to `true` if and only if @@ -159,11 +175,26 @@ pub(crate) fn copy_on_write( copy_debug.reflink = OffloadReflinkDebug::No; sparse_copy(source, dest) } - (ReflinkMode::Never, _) => { + (ReflinkMode::Never, SparseMode::Never) => { copy_debug.sparse_detection = SparseDebug::No; copy_debug.reflink = OffloadReflinkDebug::No; std::fs::copy(source, dest).map(|_| ()) } + (ReflinkMode::Never, SparseMode::Auto) => { + copy_debug.sparse_detection = SparseDebug::No; + copy_debug.reflink = OffloadReflinkDebug::No; + let mut sparse_flag = false; + let res = check_sparse_detection(source, &mut sparse_flag); + if res.is_ok() { + if sparse_flag { + sparse_copy(source, dest) + } else { + std::fs::copy(source, dest).map(|_| ()) + } + } else { + res + } + } (ReflinkMode::Auto, SparseMode::Always) => { copy_debug.offload = OffloadReflinkDebug::Avoided; copy_debug.sparse_detection = SparseDebug::Zeros; @@ -171,7 +202,26 @@ pub(crate) fn copy_on_write( sparse_copy(source, dest) } - (ReflinkMode::Auto, _) => { + (ReflinkMode::Auto, SparseMode::Auto) => { + copy_debug.sparse_detection = SparseDebug::No; + copy_debug.reflink = OffloadReflinkDebug::Unsupported; + if source_is_fifo { + copy_fifo_contents(source, dest).map(|_| ()) + } else { + let mut sparse_flag = false; + let res = check_sparse_detection(source, &mut sparse_flag); + if res.is_ok() { + if sparse_flag { + clone(source, dest, CloneFallback::SparseCopy) + } else { + std::fs::copy(source, dest).map(|_| ()) + } + } else { + res + } + } + } + (ReflinkMode::Auto, SparseMode::Never) => { copy_debug.sparse_detection = SparseDebug::No; copy_debug.reflink = OffloadReflinkDebug::Unsupported; if source_is_fifo { From 2f2957056506c3f0a90d87d8f4109acad3186468 Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Tue, 26 Mar 2024 00:07:15 +0530 Subject: [PATCH 02/33] bug fix --- src/uu/cp/src/platform/linux.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index fa721432c90..57c60a14851 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -214,7 +214,7 @@ pub(crate) fn copy_on_write( if sparse_flag { clone(source, dest, CloneFallback::SparseCopy) } else { - std::fs::copy(source, dest).map(|_| ()) + clone(source, dest, CloneFallback::FSCopy) } } else { res From 08a44bb21a00e4019704e91234269a01cbf76533 Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Tue, 26 Mar 2024 01:23:09 +0530 Subject: [PATCH 03/33] Making code more idiomatic --- src/uu/cp/src/platform/linux.rs | 34 +++++++++++---------------------- 1 file changed, 11 insertions(+), 23 deletions(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index 57c60a14851..801550b88a0 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -136,7 +136,7 @@ where Ok(num_bytes_copied) } -fn check_sparse_detection(source: &Path, sparse_flag: &mut bool) -> std::io::Result<()> { +fn check_sparse_detection(source: &Path) -> Result { let src_file = File::open(source)?; use std::os::unix::prelude::MetadataExt; @@ -144,9 +144,9 @@ fn check_sparse_detection(source: &Path, sparse_flag: &mut bool) -> std::io::Res let size: usize = src_file.metadata()?.size().try_into().unwrap(); let blocks: usize = src_file.metadata()?.blocks().try_into().unwrap(); if blocks < size / 512 { - *sparse_flag = true; + return Ok(true); } - Ok(()) + Ok(false) } /// Copies `source` to `dest` using copy-on-write if possible. /// @@ -183,16 +183,10 @@ pub(crate) fn copy_on_write( (ReflinkMode::Never, SparseMode::Auto) => { copy_debug.sparse_detection = SparseDebug::No; copy_debug.reflink = OffloadReflinkDebug::No; - let mut sparse_flag = false; - let res = check_sparse_detection(source, &mut sparse_flag); - if res.is_ok() { - if sparse_flag { - sparse_copy(source, dest) - } else { - std::fs::copy(source, dest).map(|_| ()) - } - } else { - res + match check_sparse_detection(source) { + Ok(true) => sparse_copy(source, dest), + Ok(false) => std::fs::copy(source, dest).map(|_| ()), + Err(e) => Err(e), } } (ReflinkMode::Auto, SparseMode::Always) => { @@ -208,16 +202,10 @@ pub(crate) fn copy_on_write( if source_is_fifo { copy_fifo_contents(source, dest).map(|_| ()) } else { - let mut sparse_flag = false; - let res = check_sparse_detection(source, &mut sparse_flag); - if res.is_ok() { - if sparse_flag { - clone(source, dest, CloneFallback::SparseCopy) - } else { - clone(source, dest, CloneFallback::FSCopy) - } - } else { - res + match check_sparse_detection(source) { + Ok(true) => clone(source, dest, CloneFallback::SparseCopy), + Ok(false) => clone(source, dest, CloneFallback::FSCopy), + Err(e) => Err(e), } } } From 15686cd15ec5071a7bf14cf81f1cb531bb0ae438 Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Tue, 26 Mar 2024 14:03:42 +0530 Subject: [PATCH 04/33] Added handling for destination fifo files --- src/uu/cp/src/platform/linux.rs | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index 801550b88a0..3e2e36101e6 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -3,9 +3,9 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. // spell-checker:ignore ficlone reflink ftruncate pwrite fiemap -use std::fs::{File, OpenOptions}; +use std::fs::{self, File, OpenOptions}; use std::io::Read; -use std::os::unix::fs::OpenOptionsExt; +use std::os::unix::fs::{FileTypeExt, OpenOptionsExt}; use std::os::unix::io::AsRawFd; use std::path::Path; @@ -136,6 +136,14 @@ where Ok(num_bytes_copied) } +fn check_dest_is_fifo(dest: &Path) -> bool { + let file_type = fs::metadata(dest); + match file_type { + Ok(f) => f.file_type().is_fifo(), + + _ => false, + } +} fn check_sparse_detection(source: &Path) -> Result { let src_file = File::open(source)?; @@ -148,6 +156,7 @@ fn check_sparse_detection(source: &Path) -> Result { } Ok(false) } + /// Copies `source` to `dest` using copy-on-write if possible. /// /// The `source_is_fifo` flag must be set to `true` if and only if @@ -202,10 +211,11 @@ pub(crate) fn copy_on_write( if source_is_fifo { copy_fifo_contents(source, dest).map(|_| ()) } else { - match check_sparse_detection(source) { - Ok(true) => clone(source, dest, CloneFallback::SparseCopy), - Ok(false) => clone(source, dest, CloneFallback::FSCopy), - Err(e) => Err(e), + match (check_sparse_detection(source), check_dest_is_fifo(dest)) { + (Ok(true), false) => clone(source, dest, CloneFallback::SparseCopy), + (Ok(true), true) => clone(source, dest, CloneFallback::FSCopy), + (Ok(false), _) => clone(source, dest, CloneFallback::FSCopy), + (Err(e), _) => Err(e), } } } From 1c9ed4ebd36c80c64c8834787528af1aa815472a Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Tue, 26 Mar 2024 17:53:37 +0530 Subject: [PATCH 05/33] Trying a fix --- src/uu/cp/src/platform/linux.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index 3e2e36101e6..9f0b6943f58 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -151,7 +151,7 @@ fn check_sparse_detection(source: &Path) -> Result { let size: usize = src_file.metadata()?.size().try_into().unwrap(); let blocks: usize = src_file.metadata()?.blocks().try_into().unwrap(); - if blocks < size / 512 { + if blocks < size / 512 && blocks != 0 { return Ok(true); } Ok(false) From 9b00609d5b6c0c9bff54e6526b5c84fe5430e5e6 Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Tue, 26 Mar 2024 18:27:04 +0530 Subject: [PATCH 06/33] Trying fix --- src/uu/cp/src/platform/linux.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index 9f0b6943f58..03d9568c469 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -151,7 +151,9 @@ fn check_sparse_detection(source: &Path) -> Result { let size: usize = src_file.metadata()?.size().try_into().unwrap(); let blocks: usize = src_file.metadata()?.blocks().try_into().unwrap(); - if blocks < size / 512 && blocks != 0 { + let block_size: usize = src_file.metadata()?.blocks().try_into().unwrap(); + + if size > block_size && blocks < size / 512 { return Ok(true); } Ok(false) From 2c5207098ece91a0c705eb6da678edefe96af651 Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Tue, 26 Mar 2024 19:06:34 +0530 Subject: [PATCH 07/33] Empty --- src/uu/cp/src/platform/linux.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index 03d9568c469..7dd48ce96c0 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -153,7 +153,7 @@ fn check_sparse_detection(source: &Path) -> Result { let blocks: usize = src_file.metadata()?.blocks().try_into().unwrap(); let block_size: usize = src_file.metadata()?.blocks().try_into().unwrap(); - if size > block_size && blocks < size / 512 { + if size > block_size + 512 && blocks < size / 512 && blocks != 0 { return Ok(true); } Ok(false) From 1b9135c51bb3e450d01b8bd1cb380e7515f0e84a Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Tue, 26 Mar 2024 19:42:04 +0530 Subject: [PATCH 08/33] Switching to old behaviour for non-sparse_files Reverting to old behaviour for non sparse files --- src/uu/cp/src/platform/linux.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index 7dd48ce96c0..11b44777bd6 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -151,9 +151,8 @@ fn check_sparse_detection(source: &Path) -> Result { let size: usize = src_file.metadata()?.size().try_into().unwrap(); let blocks: usize = src_file.metadata()?.blocks().try_into().unwrap(); - let block_size: usize = src_file.metadata()?.blocks().try_into().unwrap(); - if size > block_size + 512 && blocks < size / 512 && blocks != 0 { + if blocks < size / 512 && blocks != 0 { return Ok(true); } Ok(false) @@ -216,7 +215,7 @@ pub(crate) fn copy_on_write( match (check_sparse_detection(source), check_dest_is_fifo(dest)) { (Ok(true), false) => clone(source, dest, CloneFallback::SparseCopy), (Ok(true), true) => clone(source, dest, CloneFallback::FSCopy), - (Ok(false), _) => clone(source, dest, CloneFallback::FSCopy), + (Ok(false), _) => std::fs::copy(source, dest).map(|_| ()), (Err(e), _) => Err(e), } } From aef3dcc549876cc8b4c1589624005474a66f9e7e Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Tue, 26 Mar 2024 22:57:29 +0530 Subject: [PATCH 09/33] Removed a bug --- src/uu/cp/src/platform/linux.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index 11b44777bd6..799e98aa6a9 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -215,7 +215,7 @@ pub(crate) fn copy_on_write( match (check_sparse_detection(source), check_dest_is_fifo(dest)) { (Ok(true), false) => clone(source, dest, CloneFallback::SparseCopy), (Ok(true), true) => clone(source, dest, CloneFallback::FSCopy), - (Ok(false), _) => std::fs::copy(source, dest).map(|_| ()), + (Ok(false), _) => clone(source, dest, CloneFallback::FSCopy), (Err(e), _) => Err(e), } } From ad9a90708d331505bcceab5ddd6ae10283e4aa71 Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Wed, 27 Mar 2024 13:10:18 +0530 Subject: [PATCH 10/33] Removed uncessary unwrap and added comments --- src/uu/cp/src/platform/linux.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index 799e98aa6a9..b3f6087b2e7 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -7,6 +7,7 @@ use std::fs::{self, File, OpenOptions}; use std::io::Read; use std::os::unix::fs::{FileTypeExt, OpenOptionsExt}; use std::os::unix::io::AsRawFd; +use std::os::unix::prelude::MetadataExt; use std::path::Path; use quick_error::ResultExt; @@ -66,8 +67,6 @@ fn sparse_copy

(source: P, dest: P) -> std::io::Result<()> where P: AsRef, { - use std::os::unix::prelude::MetadataExt; - let mut src_file = File::open(source)?; let dst_file = File::create(dest)?; let dst_fd = dst_file.as_raw_fd(); @@ -137,6 +136,7 @@ where } fn check_dest_is_fifo(dest: &Path) -> bool { + //If our destination file exists and its a fifo , we do a standard copy . let file_type = fs::metadata(dest); match file_type { Ok(f) => f.file_type().is_fifo(), @@ -144,14 +144,16 @@ fn check_dest_is_fifo(dest: &Path) -> bool { _ => false, } } + fn check_sparse_detection(source: &Path) -> Result { let src_file = File::open(source)?; - use std::os::unix::prelude::MetadataExt; - - let size: usize = src_file.metadata()?.size().try_into().unwrap(); - let blocks: usize = src_file.metadata()?.blocks().try_into().unwrap(); + let size = src_file.metadata()?.size(); + let blocks = src_file.metadata()?.blocks(); + //cp uses a crude heuristic to detect sparse_files , an estimated formula which seems to accurately + //replicate that , might need to change if we observe any edge cases that i haven't thought of but this should work for majority of the files. + //Reference:https://doc.rust-lang.org/std/os/unix/fs/trait.MetadataExt.html#tymethod.blocks if blocks < size / 512 && blocks != 0 { return Ok(true); } From 2e80b71e8102f00c55fd5a7c819c550fee0f4c56 Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Wed, 27 Mar 2024 14:23:18 +0530 Subject: [PATCH 11/33] Added os flags --- src/uu/cp/src/platform/linux.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index b3f6087b2e7..24d27bd35c7 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -135,6 +135,7 @@ where Ok(num_bytes_copied) } +#[cfg(any(target_os = "linux", target_os = "android"))] fn check_dest_is_fifo(dest: &Path) -> bool { //If our destination file exists and its a fifo , we do a standard copy . let file_type = fs::metadata(dest); @@ -145,6 +146,7 @@ fn check_dest_is_fifo(dest: &Path) -> bool { } } +#[cfg(any(target_os = "linux", target_os = "android"))] fn check_sparse_detection(source: &Path) -> Result { let src_file = File::open(source)?; From cee93a4fa365d122f2a982b19215ec13bb4871ad Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Wed, 27 Mar 2024 22:32:08 +0530 Subject: [PATCH 12/33] Added tests --- tests/by-util/test_cp.rs | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index e3b373da19d..deae2b62f2d 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -3218,6 +3218,39 @@ fn test_copy_contents_fifo() { assert_eq!(at.read("outfile"), "foo"); } +#[cfg(target_os = "linux")] +#[test] +fn test_sparse_auto_for_sparse_files() { + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + at.touch("a"); + at.write("a", "hello"); + at.truncate("a", "-s10000"); + ts.ucmd().arg("--sparse=auto").arg("a").arg("b").succeeds(); + let result = ts.ccmd("du").arg("-h").arg("b").succeeds(); + let stdout_str = result.stdout_str(); + if !stdout_str.contains("4.0K b") { + panic!("Failure: stdout was \n{stdout_str}"); + } +} + +#[cfg(target_os = "linux")] +#[test] +fn test_sparse_auto_for_non_sparse_files() { + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + let a_bytes = [1 as u8; 10000]; + at.touch("a"); + at.write("a", "hello"); + at.append_bytes("a", &a_bytes); + ts.ucmd().arg("--sparse=auto").arg("a").arg("b").succeeds(); + let result = ts.ccmd("du").arg("-h").arg("b").succeeds(); + let stdout_str = result.stdout_str(); + if !stdout_str.contains("12.0K b") { + panic!("Failure: stdout was \n{stdout_str}"); + } +} + #[cfg(target_os = "linux")] #[test] fn test_reflink_never_sparse_always() { From 3c869cad3dfdfbaa36d87384f30e2c100ee43206 Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Thu, 28 Mar 2024 12:50:04 +0530 Subject: [PATCH 13/33] Changed tests so they don't use du --- tests/by-util/test_cp.rs | 46 +++++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index deae2b62f2d..8bae2c75e80 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -3227,11 +3227,24 @@ fn test_sparse_auto_for_sparse_files() { at.write("a", "hello"); at.truncate("a", "-s10000"); ts.ucmd().arg("--sparse=auto").arg("a").arg("b").succeeds(); - let result = ts.ccmd("du").arg("-h").arg("b").succeeds(); - let stdout_str = result.stdout_str(); - if !stdout_str.contains("4.0K b") { - panic!("Failure: stdout was \n{stdout_str}"); - } + + let src_size = std::fs::metadata(at.plus("a")) + .expect("Metadata of source file cannot be read") + .size(); + + let src_blocks = std::fs::metadata(at.plus("a")) + .expect("Metadata of source file cannot be read") + .blocks(); + + let dest_size = std::fs::metadata(at.plus("b")) + .expect("Metadata of copied file cannot be read") + .size(); + let dest_blocks = std::fs::metadata(at.plus("b")) + .expect("Metadata of copied file cannot be read") + .blocks(); + + assert_eq!(src_size, dest_size); + assert_eq!(src_blocks, dest_blocks); } #[cfg(target_os = "linux")] @@ -3244,11 +3257,24 @@ fn test_sparse_auto_for_non_sparse_files() { at.write("a", "hello"); at.append_bytes("a", &a_bytes); ts.ucmd().arg("--sparse=auto").arg("a").arg("b").succeeds(); - let result = ts.ccmd("du").arg("-h").arg("b").succeeds(); - let stdout_str = result.stdout_str(); - if !stdout_str.contains("12.0K b") { - panic!("Failure: stdout was \n{stdout_str}"); - } + + let src_size = std::fs::metadata(at.plus("a")) + .expect("Metadata of source file cannot be read") + .size(); + + let src_blocks = std::fs::metadata(at.plus("a")) + .expect("Metadata of source file cannot be read") + .blocks(); + + let dest_size = std::fs::metadata(at.plus("b")) + .expect("Metadata of copied file cannot be read") + .size(); + let dest_blocks = std::fs::metadata(at.plus("b")) + .expect("Metadata of copied file cannot be read") + .blocks(); + + assert_eq!(src_size, dest_size); + assert_eq!(src_blocks, dest_blocks); } #[cfg(target_os = "linux")] From 4137db0f58fe7988d6d935e2d4c20028c7ae724c Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Mon, 25 Mar 2024 23:26:20 +0530 Subject: [PATCH 14/33] Fix clone doing standard copy on sparse=auto --- src/uu/cp/src/platform/linux.rs | 54 +++++++++++++++++++++++++++++++-- 1 file changed, 52 insertions(+), 2 deletions(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index 674e66ea575..fa721432c90 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -32,6 +32,9 @@ enum CloneFallback { /// Use [`std::fs::copy`]. FSCopy, + + /// Use sparse_copy + SparseCopy, } /// Use the Linux `ioctl_ficlone` API to do a copy-on-write clone. @@ -53,6 +56,7 @@ where match fallback { CloneFallback::Error => Err(std::io::Error::last_os_error()), CloneFallback::FSCopy => std::fs::copy(source, dest).map(|_| ()), + CloneFallback::SparseCopy => sparse_copy(source, dest), } } @@ -132,6 +136,18 @@ where Ok(num_bytes_copied) } +fn check_sparse_detection(source: &Path, sparse_flag: &mut bool) -> std::io::Result<()> { + let src_file = File::open(source)?; + + use std::os::unix::prelude::MetadataExt; + + let size: usize = src_file.metadata()?.size().try_into().unwrap(); + let blocks: usize = src_file.metadata()?.blocks().try_into().unwrap(); + if blocks < size / 512 { + *sparse_flag = true; + } + Ok(()) +} /// Copies `source` to `dest` using copy-on-write if possible. /// /// The `source_is_fifo` flag must be set to `true` if and only if @@ -159,11 +175,26 @@ pub(crate) fn copy_on_write( copy_debug.reflink = OffloadReflinkDebug::No; sparse_copy(source, dest) } - (ReflinkMode::Never, _) => { + (ReflinkMode::Never, SparseMode::Never) => { copy_debug.sparse_detection = SparseDebug::No; copy_debug.reflink = OffloadReflinkDebug::No; std::fs::copy(source, dest).map(|_| ()) } + (ReflinkMode::Never, SparseMode::Auto) => { + copy_debug.sparse_detection = SparseDebug::No; + copy_debug.reflink = OffloadReflinkDebug::No; + let mut sparse_flag = false; + let res = check_sparse_detection(source, &mut sparse_flag); + if res.is_ok() { + if sparse_flag { + sparse_copy(source, dest) + } else { + std::fs::copy(source, dest).map(|_| ()) + } + } else { + res + } + } (ReflinkMode::Auto, SparseMode::Always) => { copy_debug.offload = OffloadReflinkDebug::Avoided; copy_debug.sparse_detection = SparseDebug::Zeros; @@ -171,7 +202,26 @@ pub(crate) fn copy_on_write( sparse_copy(source, dest) } - (ReflinkMode::Auto, _) => { + (ReflinkMode::Auto, SparseMode::Auto) => { + copy_debug.sparse_detection = SparseDebug::No; + copy_debug.reflink = OffloadReflinkDebug::Unsupported; + if source_is_fifo { + copy_fifo_contents(source, dest).map(|_| ()) + } else { + let mut sparse_flag = false; + let res = check_sparse_detection(source, &mut sparse_flag); + if res.is_ok() { + if sparse_flag { + clone(source, dest, CloneFallback::SparseCopy) + } else { + std::fs::copy(source, dest).map(|_| ()) + } + } else { + res + } + } + } + (ReflinkMode::Auto, SparseMode::Never) => { copy_debug.sparse_detection = SparseDebug::No; copy_debug.reflink = OffloadReflinkDebug::Unsupported; if source_is_fifo { From bc205430d6dfb0f7907135a71c7507ed33f113a0 Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Tue, 26 Mar 2024 00:07:15 +0530 Subject: [PATCH 15/33] bug fix --- src/uu/cp/src/platform/linux.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index fa721432c90..57c60a14851 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -214,7 +214,7 @@ pub(crate) fn copy_on_write( if sparse_flag { clone(source, dest, CloneFallback::SparseCopy) } else { - std::fs::copy(source, dest).map(|_| ()) + clone(source, dest, CloneFallback::FSCopy) } } else { res From c1b24795feaca5c1cd6ab2d38a3e2cdb5db62a5c Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Tue, 26 Mar 2024 01:23:09 +0530 Subject: [PATCH 16/33] Making code more idiomatic --- src/uu/cp/src/platform/linux.rs | 34 +++++++++++---------------------- 1 file changed, 11 insertions(+), 23 deletions(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index 57c60a14851..801550b88a0 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -136,7 +136,7 @@ where Ok(num_bytes_copied) } -fn check_sparse_detection(source: &Path, sparse_flag: &mut bool) -> std::io::Result<()> { +fn check_sparse_detection(source: &Path) -> Result { let src_file = File::open(source)?; use std::os::unix::prelude::MetadataExt; @@ -144,9 +144,9 @@ fn check_sparse_detection(source: &Path, sparse_flag: &mut bool) -> std::io::Res let size: usize = src_file.metadata()?.size().try_into().unwrap(); let blocks: usize = src_file.metadata()?.blocks().try_into().unwrap(); if blocks < size / 512 { - *sparse_flag = true; + return Ok(true); } - Ok(()) + Ok(false) } /// Copies `source` to `dest` using copy-on-write if possible. /// @@ -183,16 +183,10 @@ pub(crate) fn copy_on_write( (ReflinkMode::Never, SparseMode::Auto) => { copy_debug.sparse_detection = SparseDebug::No; copy_debug.reflink = OffloadReflinkDebug::No; - let mut sparse_flag = false; - let res = check_sparse_detection(source, &mut sparse_flag); - if res.is_ok() { - if sparse_flag { - sparse_copy(source, dest) - } else { - std::fs::copy(source, dest).map(|_| ()) - } - } else { - res + match check_sparse_detection(source) { + Ok(true) => sparse_copy(source, dest), + Ok(false) => std::fs::copy(source, dest).map(|_| ()), + Err(e) => Err(e), } } (ReflinkMode::Auto, SparseMode::Always) => { @@ -208,16 +202,10 @@ pub(crate) fn copy_on_write( if source_is_fifo { copy_fifo_contents(source, dest).map(|_| ()) } else { - let mut sparse_flag = false; - let res = check_sparse_detection(source, &mut sparse_flag); - if res.is_ok() { - if sparse_flag { - clone(source, dest, CloneFallback::SparseCopy) - } else { - clone(source, dest, CloneFallback::FSCopy) - } - } else { - res + match check_sparse_detection(source) { + Ok(true) => clone(source, dest, CloneFallback::SparseCopy), + Ok(false) => clone(source, dest, CloneFallback::FSCopy), + Err(e) => Err(e), } } } From 6b7f79fadf3fb074f4058210051a2f01ebeb8c90 Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Tue, 26 Mar 2024 14:03:42 +0530 Subject: [PATCH 17/33] Added handling for destination fifo files --- src/uu/cp/src/platform/linux.rs | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index 801550b88a0..3e2e36101e6 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -3,9 +3,9 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. // spell-checker:ignore ficlone reflink ftruncate pwrite fiemap -use std::fs::{File, OpenOptions}; +use std::fs::{self, File, OpenOptions}; use std::io::Read; -use std::os::unix::fs::OpenOptionsExt; +use std::os::unix::fs::{FileTypeExt, OpenOptionsExt}; use std::os::unix::io::AsRawFd; use std::path::Path; @@ -136,6 +136,14 @@ where Ok(num_bytes_copied) } +fn check_dest_is_fifo(dest: &Path) -> bool { + let file_type = fs::metadata(dest); + match file_type { + Ok(f) => f.file_type().is_fifo(), + + _ => false, + } +} fn check_sparse_detection(source: &Path) -> Result { let src_file = File::open(source)?; @@ -148,6 +156,7 @@ fn check_sparse_detection(source: &Path) -> Result { } Ok(false) } + /// Copies `source` to `dest` using copy-on-write if possible. /// /// The `source_is_fifo` flag must be set to `true` if and only if @@ -202,10 +211,11 @@ pub(crate) fn copy_on_write( if source_is_fifo { copy_fifo_contents(source, dest).map(|_| ()) } else { - match check_sparse_detection(source) { - Ok(true) => clone(source, dest, CloneFallback::SparseCopy), - Ok(false) => clone(source, dest, CloneFallback::FSCopy), - Err(e) => Err(e), + match (check_sparse_detection(source), check_dest_is_fifo(dest)) { + (Ok(true), false) => clone(source, dest, CloneFallback::SparseCopy), + (Ok(true), true) => clone(source, dest, CloneFallback::FSCopy), + (Ok(false), _) => clone(source, dest, CloneFallback::FSCopy), + (Err(e), _) => Err(e), } } } From dddd35ffa71ff2dccb3ac3ba2f18ee5e4c9b6f29 Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Tue, 26 Mar 2024 17:53:37 +0530 Subject: [PATCH 18/33] Trying a fix --- src/uu/cp/src/platform/linux.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index 3e2e36101e6..9f0b6943f58 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -151,7 +151,7 @@ fn check_sparse_detection(source: &Path) -> Result { let size: usize = src_file.metadata()?.size().try_into().unwrap(); let blocks: usize = src_file.metadata()?.blocks().try_into().unwrap(); - if blocks < size / 512 { + if blocks < size / 512 && blocks != 0 { return Ok(true); } Ok(false) From 0dc57727d712f40625c830bc70a1be2175ee24ba Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Tue, 26 Mar 2024 18:27:04 +0530 Subject: [PATCH 19/33] Trying fix --- src/uu/cp/src/platform/linux.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index 9f0b6943f58..03d9568c469 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -151,7 +151,9 @@ fn check_sparse_detection(source: &Path) -> Result { let size: usize = src_file.metadata()?.size().try_into().unwrap(); let blocks: usize = src_file.metadata()?.blocks().try_into().unwrap(); - if blocks < size / 512 && blocks != 0 { + let block_size: usize = src_file.metadata()?.blocks().try_into().unwrap(); + + if size > block_size && blocks < size / 512 { return Ok(true); } Ok(false) From 66de26cd6af9a04eb2274150ebc656e4bcdb2013 Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Tue, 26 Mar 2024 19:06:34 +0530 Subject: [PATCH 20/33] Empty --- src/uu/cp/src/platform/linux.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index 03d9568c469..7dd48ce96c0 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -153,7 +153,7 @@ fn check_sparse_detection(source: &Path) -> Result { let blocks: usize = src_file.metadata()?.blocks().try_into().unwrap(); let block_size: usize = src_file.metadata()?.blocks().try_into().unwrap(); - if size > block_size && blocks < size / 512 { + if size > block_size + 512 && blocks < size / 512 && blocks != 0 { return Ok(true); } Ok(false) From c8762be1da6f2d11b2ad8280e3e9ac0d090d9f21 Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Tue, 26 Mar 2024 19:42:04 +0530 Subject: [PATCH 21/33] Switching to old behaviour for non-sparse_files Reverting to old behaviour for non sparse files --- src/uu/cp/src/platform/linux.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index 7dd48ce96c0..11b44777bd6 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -151,9 +151,8 @@ fn check_sparse_detection(source: &Path) -> Result { let size: usize = src_file.metadata()?.size().try_into().unwrap(); let blocks: usize = src_file.metadata()?.blocks().try_into().unwrap(); - let block_size: usize = src_file.metadata()?.blocks().try_into().unwrap(); - if size > block_size + 512 && blocks < size / 512 && blocks != 0 { + if blocks < size / 512 && blocks != 0 { return Ok(true); } Ok(false) @@ -216,7 +215,7 @@ pub(crate) fn copy_on_write( match (check_sparse_detection(source), check_dest_is_fifo(dest)) { (Ok(true), false) => clone(source, dest, CloneFallback::SparseCopy), (Ok(true), true) => clone(source, dest, CloneFallback::FSCopy), - (Ok(false), _) => clone(source, dest, CloneFallback::FSCopy), + (Ok(false), _) => std::fs::copy(source, dest).map(|_| ()), (Err(e), _) => Err(e), } } From c5a2df361bc2e20cffdacb54a8c9e8f43d358149 Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Tue, 26 Mar 2024 22:57:29 +0530 Subject: [PATCH 22/33] Removed a bug --- src/uu/cp/src/platform/linux.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index 11b44777bd6..799e98aa6a9 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -215,7 +215,7 @@ pub(crate) fn copy_on_write( match (check_sparse_detection(source), check_dest_is_fifo(dest)) { (Ok(true), false) => clone(source, dest, CloneFallback::SparseCopy), (Ok(true), true) => clone(source, dest, CloneFallback::FSCopy), - (Ok(false), _) => std::fs::copy(source, dest).map(|_| ()), + (Ok(false), _) => clone(source, dest, CloneFallback::FSCopy), (Err(e), _) => Err(e), } } From 7fc74fada5a4a2d0671ebab0ef3d36eb802c7a09 Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Wed, 27 Mar 2024 13:10:18 +0530 Subject: [PATCH 23/33] Removed uncessary unwrap and added comments --- src/uu/cp/src/platform/linux.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index 799e98aa6a9..b3f6087b2e7 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -7,6 +7,7 @@ use std::fs::{self, File, OpenOptions}; use std::io::Read; use std::os::unix::fs::{FileTypeExt, OpenOptionsExt}; use std::os::unix::io::AsRawFd; +use std::os::unix::prelude::MetadataExt; use std::path::Path; use quick_error::ResultExt; @@ -66,8 +67,6 @@ fn sparse_copy

(source: P, dest: P) -> std::io::Result<()> where P: AsRef, { - use std::os::unix::prelude::MetadataExt; - let mut src_file = File::open(source)?; let dst_file = File::create(dest)?; let dst_fd = dst_file.as_raw_fd(); @@ -137,6 +136,7 @@ where } fn check_dest_is_fifo(dest: &Path) -> bool { + //If our destination file exists and its a fifo , we do a standard copy . let file_type = fs::metadata(dest); match file_type { Ok(f) => f.file_type().is_fifo(), @@ -144,14 +144,16 @@ fn check_dest_is_fifo(dest: &Path) -> bool { _ => false, } } + fn check_sparse_detection(source: &Path) -> Result { let src_file = File::open(source)?; - use std::os::unix::prelude::MetadataExt; - - let size: usize = src_file.metadata()?.size().try_into().unwrap(); - let blocks: usize = src_file.metadata()?.blocks().try_into().unwrap(); + let size = src_file.metadata()?.size(); + let blocks = src_file.metadata()?.blocks(); + //cp uses a crude heuristic to detect sparse_files , an estimated formula which seems to accurately + //replicate that , might need to change if we observe any edge cases that i haven't thought of but this should work for majority of the files. + //Reference:https://doc.rust-lang.org/std/os/unix/fs/trait.MetadataExt.html#tymethod.blocks if blocks < size / 512 && blocks != 0 { return Ok(true); } From 2e03c4c4d0b4857441e13dd292a3669e79e6d12d Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Wed, 27 Mar 2024 14:23:18 +0530 Subject: [PATCH 24/33] Added os flags --- src/uu/cp/src/platform/linux.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index b3f6087b2e7..24d27bd35c7 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -135,6 +135,7 @@ where Ok(num_bytes_copied) } +#[cfg(any(target_os = "linux", target_os = "android"))] fn check_dest_is_fifo(dest: &Path) -> bool { //If our destination file exists and its a fifo , we do a standard copy . let file_type = fs::metadata(dest); @@ -145,6 +146,7 @@ fn check_dest_is_fifo(dest: &Path) -> bool { } } +#[cfg(any(target_os = "linux", target_os = "android"))] fn check_sparse_detection(source: &Path) -> Result { let src_file = File::open(source)?; From 34e3a2d54ee2119cda7af034efb42a1ae4574edb Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Wed, 27 Mar 2024 22:32:08 +0530 Subject: [PATCH 25/33] Added tests --- tests/by-util/test_cp.rs | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index e3b373da19d..deae2b62f2d 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -3218,6 +3218,39 @@ fn test_copy_contents_fifo() { assert_eq!(at.read("outfile"), "foo"); } +#[cfg(target_os = "linux")] +#[test] +fn test_sparse_auto_for_sparse_files() { + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + at.touch("a"); + at.write("a", "hello"); + at.truncate("a", "-s10000"); + ts.ucmd().arg("--sparse=auto").arg("a").arg("b").succeeds(); + let result = ts.ccmd("du").arg("-h").arg("b").succeeds(); + let stdout_str = result.stdout_str(); + if !stdout_str.contains("4.0K b") { + panic!("Failure: stdout was \n{stdout_str}"); + } +} + +#[cfg(target_os = "linux")] +#[test] +fn test_sparse_auto_for_non_sparse_files() { + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + let a_bytes = [1 as u8; 10000]; + at.touch("a"); + at.write("a", "hello"); + at.append_bytes("a", &a_bytes); + ts.ucmd().arg("--sparse=auto").arg("a").arg("b").succeeds(); + let result = ts.ccmd("du").arg("-h").arg("b").succeeds(); + let stdout_str = result.stdout_str(); + if !stdout_str.contains("12.0K b") { + panic!("Failure: stdout was \n{stdout_str}"); + } +} + #[cfg(target_os = "linux")] #[test] fn test_reflink_never_sparse_always() { From a82de20dfbd3127347301c239ac7e0eeba1101fa Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Thu, 28 Mar 2024 12:50:04 +0530 Subject: [PATCH 26/33] Changed tests so they don't use du --- tests/by-util/test_cp.rs | 46 +++++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index deae2b62f2d..8bae2c75e80 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -3227,11 +3227,24 @@ fn test_sparse_auto_for_sparse_files() { at.write("a", "hello"); at.truncate("a", "-s10000"); ts.ucmd().arg("--sparse=auto").arg("a").arg("b").succeeds(); - let result = ts.ccmd("du").arg("-h").arg("b").succeeds(); - let stdout_str = result.stdout_str(); - if !stdout_str.contains("4.0K b") { - panic!("Failure: stdout was \n{stdout_str}"); - } + + let src_size = std::fs::metadata(at.plus("a")) + .expect("Metadata of source file cannot be read") + .size(); + + let src_blocks = std::fs::metadata(at.plus("a")) + .expect("Metadata of source file cannot be read") + .blocks(); + + let dest_size = std::fs::metadata(at.plus("b")) + .expect("Metadata of copied file cannot be read") + .size(); + let dest_blocks = std::fs::metadata(at.plus("b")) + .expect("Metadata of copied file cannot be read") + .blocks(); + + assert_eq!(src_size, dest_size); + assert_eq!(src_blocks, dest_blocks); } #[cfg(target_os = "linux")] @@ -3244,11 +3257,24 @@ fn test_sparse_auto_for_non_sparse_files() { at.write("a", "hello"); at.append_bytes("a", &a_bytes); ts.ucmd().arg("--sparse=auto").arg("a").arg("b").succeeds(); - let result = ts.ccmd("du").arg("-h").arg("b").succeeds(); - let stdout_str = result.stdout_str(); - if !stdout_str.contains("12.0K b") { - panic!("Failure: stdout was \n{stdout_str}"); - } + + let src_size = std::fs::metadata(at.plus("a")) + .expect("Metadata of source file cannot be read") + .size(); + + let src_blocks = std::fs::metadata(at.plus("a")) + .expect("Metadata of source file cannot be read") + .blocks(); + + let dest_size = std::fs::metadata(at.plus("b")) + .expect("Metadata of copied file cannot be read") + .size(); + let dest_blocks = std::fs::metadata(at.plus("b")) + .expect("Metadata of copied file cannot be read") + .blocks(); + + assert_eq!(src_size, dest_size); + assert_eq!(src_blocks, dest_blocks); } #[cfg(target_os = "linux")] From 66552df4ad40668e83d2e69d92d4b74a9c66a5dc Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Sun, 31 Mar 2024 13:06:48 +0530 Subject: [PATCH 27/33] Fixed a bug where files without any data and disk allocation wouldn't get sparse copied --- src/uu/cp/src/platform/linux.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index 24d27bd35c7..9e3aef6a435 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -148,17 +148,28 @@ fn check_dest_is_fifo(dest: &Path) -> bool { #[cfg(any(target_os = "linux", target_os = "android"))] fn check_sparse_detection(source: &Path) -> Result { - let src_file = File::open(source)?; + let mut src_file = File::open(source)?; let size = src_file.metadata()?.size(); let blocks = src_file.metadata()?.blocks(); + let blksize = src_file.metadata()?.blksize(); + + let mut data_flag = false; + let mut buf = vec![0; blksize as usize]; + let read = src_file.read(&mut buf)?; + if buf.iter().any(|&x| x != 0x0) { + data_flag = true; + } //cp uses a crude heuristic to detect sparse_files , an estimated formula which seems to accurately //replicate that , might need to change if we observe any edge cases that i haven't thought of but this should work for majority of the files. //Reference:https://doc.rust-lang.org/std/os/unix/fs/trait.MetadataExt.html#tymethod.blocks if blocks < size / 512 && blocks != 0 { return Ok(true); + } else if blocks == 0 && data_flag == false { + return Ok(true); } + Ok(false) } From 211002e678d01a664ecf26eb6dda42aa1a4e47dd Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Sun, 31 Mar 2024 20:42:58 +0530 Subject: [PATCH 28/33] simplified logic and added suggestions --- src/uu/cp/src/platform/linux.rs | 34 ++++++++++++++++++--------------- tests/by-util/test_cp.rs | 4 ++-- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index bcf436fcd08..72f92602bb4 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -150,25 +150,29 @@ fn check_dest_is_fifo(dest: &Path) -> bool { fn check_sparse_detection(source: &Path) -> Result { let mut src_file = File::open(source)?; - let size = src_file.metadata()?.size(); - let blocks = src_file.metadata()?.blocks(); - let blksize = src_file.metadata()?.blksize(); + let metadata = src_file.metadata()?; + let size = metadata.size(); + let blocks = metadata.blocks(); + let blksize = metadata.blksize(); + // cp uses a crude heuristic to detect sparse_files, an estimated formula which seems to accurately + //replicate that, is used. Modifications might be required if we observe any edges cases but this + //should work for majority of the files. + //Reference:https://doc.rust-lang.org/std/os/unix/fs/trait.MetadataExt.html#tymethod.blocks - let mut data_flag = false; let mut buf = vec![0; blksize as usize]; - let read = src_file.read(&mut buf)?; - if buf.iter().any(|&x| x != 0x0) { - data_flag = true; - } - //cp uses a crude heuristic to detect sparse_files , an estimated formula which seems to accurately - //replicate that , might need to change if we observe any edge cases that i haven't thought of but this should work for majority of the files. - //Reference:https://doc.rust-lang.org/std/os/unix/fs/trait.MetadataExt.html#tymethod.blocks - if blocks < size / 512 && blocks != 0 { - return Ok(true); - } else if blocks == 0 && data_flag == false { + + // Reading the file to account for virtual files which have 0 blocks allocated in disk + //and it is unknown if it contains data or not, virtual files need to be standard copied and + //categorised as not sparse. + + if blocks == 0 { + src_file.read(&mut buf)?; + if buf.iter().any(|&x| x != 0x0) { + return Ok(false); + } + } else if blocks < size / 512 { return Ok(true); } - Ok(false) } diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 8bae2c75e80..a22d1fc66df 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -3218,7 +3218,7 @@ fn test_copy_contents_fifo() { assert_eq!(at.read("outfile"), "foo"); } -#[cfg(target_os = "linux")] +#[cfg(any(target_os = "linux", target_os = "android"))] #[test] fn test_sparse_auto_for_sparse_files() { let ts = TestScenario::new(util_name!()); @@ -3247,7 +3247,7 @@ fn test_sparse_auto_for_sparse_files() { assert_eq!(src_blocks, dest_blocks); } -#[cfg(target_os = "linux")] +#[cfg(any(target_os = "linux", target_os = "android"))] #[test] fn test_sparse_auto_for_non_sparse_files() { let ts = TestScenario::new(util_name!()); From 65213d463abdc23794772d646f15922607154b78 Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Sun, 31 Mar 2024 21:51:29 +0530 Subject: [PATCH 29/33] Lint change --- src/uu/cp/src/platform/linux.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index 72f92602bb4..c94b5b8a038 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -163,10 +163,11 @@ fn check_sparse_detection(source: &Path) -> Result { // Reading the file to account for virtual files which have 0 blocks allocated in disk //and it is unknown if it contains data or not, virtual files need to be standard copied and - //categorised as not sparse. + //categorized as not sparse. if blocks == 0 { - src_file.read(&mut buf)?; + let _ = src_file.read(&mut buf)?; + if buf.iter().any(|&x| x != 0x0) { return Ok(false); } From 7a14d33d5c80b133660ccee0f03ca2fcd5c90e53 Mon Sep 17 00:00:00 2001 From: Anirban Halder <92542059+AnirbanHalder654322@users.noreply.github.com> Date: Mon, 1 Apr 2024 19:25:57 +0530 Subject: [PATCH 30/33] Apply suggestions from code review Co-authored-by: Sylvestre Ledru --- src/uu/cp/src/platform/linux.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index c94b5b8a038..ffca39f32fb 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -137,7 +137,7 @@ where #[cfg(any(target_os = "linux", target_os = "android"))] fn check_dest_is_fifo(dest: &Path) -> bool { - //If our destination file exists and its a fifo , we do a standard copy . + // If our destination file exists and it is a FIFO, we do a standard copy . let file_type = fs::metadata(dest); match file_type { Ok(f) => f.file_type().is_fifo(), @@ -155,15 +155,15 @@ fn check_sparse_detection(source: &Path) -> Result { let blocks = metadata.blocks(); let blksize = metadata.blksize(); // cp uses a crude heuristic to detect sparse_files, an estimated formula which seems to accurately - //replicate that, is used. Modifications might be required if we observe any edges cases but this - //should work for majority of the files. - //Reference:https://doc.rust-lang.org/std/os/unix/fs/trait.MetadataExt.html#tymethod.blocks + // replicate that, is used. Modifications might be required if we observe any edges cases but this + // should work for majority of the files. + // Reference:https://doc.rust-lang.org/std/os/unix/fs/trait.MetadataExt.html#tymethod.blocks let mut buf = vec![0; blksize as usize]; // Reading the file to account for virtual files which have 0 blocks allocated in disk - //and it is unknown if it contains data or not, virtual files need to be standard copied and - //categorized as not sparse. + // and it is unknown if it contains data or not, virtual files need to be standard copied and + // categorized as not sparse. if blocks == 0 { let _ = src_file.read(&mut buf)?; From d48b98f13be95c1ff12fb83ad45dd96fcd7b232d Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Mon, 1 Apr 2024 23:10:31 +0530 Subject: [PATCH 31/33] Added testing for virtual files --- tests/by-util/test_cp.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index a22d1fc66df..5f3b4cc039a 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -2330,6 +2330,26 @@ fn test_cp_sparse_always_non_empty() { assert_eq!(at.metadata("dst_file_sparse").blocks(), touched_block_count); } +#[test] +#[cfg(target_os = "linux")] +fn test_cp_for_virtual_files() { + use std::os::unix::prelude::MetadataExt; + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + ts.ucmd() + .arg("--sparse=auto") + .arg("/sys/kernel/address_bits") + .arg("b") + .succeeds(); + + let dest_size = std::fs::metadata(at.plus("b")) + .expect("Metadata of copied file cannot be read") + .size(); + if dest_size == 0 { + panic!("Copy unsuccesful"); + } +} + #[cfg(any(target_os = "linux", target_os = "android"))] #[test] fn test_cp_sparse_invalid_option() { From d2192431da1d943e383ee23ce2ee5bdd77d7c517 Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Sun, 31 Mar 2024 20:42:58 +0530 Subject: [PATCH 32/33] simplified logic and added suggestions --- src/uu/cp/src/platform/linux.rs | 35 +++++++++++++++++++-------------- tests/by-util/test_cp.rs | 4 ++-- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index bcf436fcd08..c94b5b8a038 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -150,25 +150,30 @@ fn check_dest_is_fifo(dest: &Path) -> bool { fn check_sparse_detection(source: &Path) -> Result { let mut src_file = File::open(source)?; - let size = src_file.metadata()?.size(); - let blocks = src_file.metadata()?.blocks(); - let blksize = src_file.metadata()?.blksize(); + let metadata = src_file.metadata()?; + let size = metadata.size(); + let blocks = metadata.blocks(); + let blksize = metadata.blksize(); + // cp uses a crude heuristic to detect sparse_files, an estimated formula which seems to accurately + //replicate that, is used. Modifications might be required if we observe any edges cases but this + //should work for majority of the files. + //Reference:https://doc.rust-lang.org/std/os/unix/fs/trait.MetadataExt.html#tymethod.blocks - let mut data_flag = false; let mut buf = vec![0; blksize as usize]; - let read = src_file.read(&mut buf)?; - if buf.iter().any(|&x| x != 0x0) { - data_flag = true; - } - //cp uses a crude heuristic to detect sparse_files , an estimated formula which seems to accurately - //replicate that , might need to change if we observe any edge cases that i haven't thought of but this should work for majority of the files. - //Reference:https://doc.rust-lang.org/std/os/unix/fs/trait.MetadataExt.html#tymethod.blocks - if blocks < size / 512 && blocks != 0 { - return Ok(true); - } else if blocks == 0 && data_flag == false { + + // Reading the file to account for virtual files which have 0 blocks allocated in disk + //and it is unknown if it contains data or not, virtual files need to be standard copied and + //categorized as not sparse. + + if blocks == 0 { + let _ = src_file.read(&mut buf)?; + + if buf.iter().any(|&x| x != 0x0) { + return Ok(false); + } + } else if blocks < size / 512 { return Ok(true); } - Ok(false) } diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 8bae2c75e80..a22d1fc66df 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -3218,7 +3218,7 @@ fn test_copy_contents_fifo() { assert_eq!(at.read("outfile"), "foo"); } -#[cfg(target_os = "linux")] +#[cfg(any(target_os = "linux", target_os = "android"))] #[test] fn test_sparse_auto_for_sparse_files() { let ts = TestScenario::new(util_name!()); @@ -3247,7 +3247,7 @@ fn test_sparse_auto_for_sparse_files() { assert_eq!(src_blocks, dest_blocks); } -#[cfg(target_os = "linux")] +#[cfg(any(target_os = "linux", target_os = "android"))] #[test] fn test_sparse_auto_for_non_sparse_files() { let ts = TestScenario::new(util_name!()); From 9503a26efd9267431afa5b05c00351d65e9e25bb Mon Sep 17 00:00:00 2001 From: AnirbanHalder654322 Date: Mon, 1 Apr 2024 23:10:31 +0530 Subject: [PATCH 33/33] Added testing for virtual files Co-authored-by: Sylvestre Ledru --- src/uu/cp/src/platform/linux.rs | 12 ++++++------ tests/by-util/test_cp.rs | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index c94b5b8a038..ffca39f32fb 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -137,7 +137,7 @@ where #[cfg(any(target_os = "linux", target_os = "android"))] fn check_dest_is_fifo(dest: &Path) -> bool { - //If our destination file exists and its a fifo , we do a standard copy . + // If our destination file exists and it is a FIFO, we do a standard copy . let file_type = fs::metadata(dest); match file_type { Ok(f) => f.file_type().is_fifo(), @@ -155,15 +155,15 @@ fn check_sparse_detection(source: &Path) -> Result { let blocks = metadata.blocks(); let blksize = metadata.blksize(); // cp uses a crude heuristic to detect sparse_files, an estimated formula which seems to accurately - //replicate that, is used. Modifications might be required if we observe any edges cases but this - //should work for majority of the files. - //Reference:https://doc.rust-lang.org/std/os/unix/fs/trait.MetadataExt.html#tymethod.blocks + // replicate that, is used. Modifications might be required if we observe any edges cases but this + // should work for majority of the files. + // Reference:https://doc.rust-lang.org/std/os/unix/fs/trait.MetadataExt.html#tymethod.blocks let mut buf = vec![0; blksize as usize]; // Reading the file to account for virtual files which have 0 blocks allocated in disk - //and it is unknown if it contains data or not, virtual files need to be standard copied and - //categorized as not sparse. + // and it is unknown if it contains data or not, virtual files need to be standard copied and + // categorized as not sparse. if blocks == 0 { let _ = src_file.read(&mut buf)?; diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index a22d1fc66df..afe7a01c36f 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -2330,6 +2330,26 @@ fn test_cp_sparse_always_non_empty() { assert_eq!(at.metadata("dst_file_sparse").blocks(), touched_block_count); } +#[test] +#[cfg(target_os = "linux")] +fn test_cp_for_virtual_files() { + use std::os::unix::prelude::MetadataExt; + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + ts.ucmd() + .arg("--sparse=auto") + .arg("/sys/kernel/address_bits") + .arg("b") + .succeeds(); + + let dest_size = std::fs::metadata(at.plus("b")) + .expect("Metadata of copied file cannot be read") + .size(); + if dest_size == 0 { + panic!("Copy unsuccessful"); + } +} + #[cfg(any(target_os = "linux", target_os = "android"))] #[test] fn test_cp_sparse_invalid_option() {