From e253406026b64080a78335896caf28f06afa242d Mon Sep 17 00:00:00 2001 From: bootandy Date: Tue, 20 Mar 2018 12:10:05 -0400 Subject: [PATCH 1/5] du: Fix incorrect block size assumption. du and other tools like stat assume a 512 byte block. ls is the only tool to use 1024. Add Simple set of tests --- Cargo.lock | 3 ++ src/du/du.rs | 20 ++++++-- tests/fixtures/du/subdir/subwords.txt | Bin 0 -> 5120 bytes tests/fixtures/du/subdir/subwords2.txt | 1 + tests/fixtures/du/words.txt | 1 + tests/test_du.rs | 62 +++++++++++++++++++++++++ tests/tests.rs | 1 + 7 files changed, 84 insertions(+), 4 deletions(-) create mode 100644 tests/fixtures/du/subdir/subwords.txt create mode 100644 tests/fixtures/du/subdir/subwords2.txt create mode 100644 tests/fixtures/du/words.txt create mode 100644 tests/test_du.rs diff --git a/Cargo.lock b/Cargo.lock index 512e39651..e5d0b4285 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -791,6 +791,8 @@ version = "0.0.1" dependencies = [ "getopts 0.2.17 (registry+https://github.com/rust-lang/crates.io-index)", "nix 0.8.1 (registry+https://github.com/rust-lang/crates.io-index)", + "redox_syscall 0.1.37 (registry+https://github.com/rust-lang/crates.io-index)", + "redox_termios 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", "uucore 0.0.1", ] @@ -1430,6 +1432,7 @@ dependencies = [ "getopts 0.2.17 (registry+https://github.com/rust-lang/crates.io-index)", "kernel32-sys 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.39 (registry+https://github.com/rust-lang/crates.io-index)", + "redox_syscall 0.1.37 (registry+https://github.com/rust-lang/crates.io-index)", "uucore 0.0.1", "winapi 0.3.4 (registry+https://github.com/rust-lang/crates.io-index)", ] diff --git a/src/du/du.rs b/src/du/du.rs index e87308952..a78b8b56d 100644 --- a/src/du/du.rs +++ b/src/du/du.rs @@ -27,7 +27,7 @@ const LONG_HELP: &'static str = " Display values are in units of the first available SIZE from --block-size, and the DU_BLOCK_SIZE, BLOCK_SIZE and BLOCKSIZE environ‐ ment variables. Otherwise, units default to 1024 bytes (or 512 if - POSIXLY_CORRECT is set). + POSIXLY_CORRECT is set or if OS is Apple). SIZE is an integer and optional unit (example: 10M is 10*1024*1024). Units are K, M, G, T, P, E, Z, Y (powers of 1024) or KB, MB, ... (pow‐ @@ -69,6 +69,16 @@ impl Stat { } } +#[cfg(target_os = "macos")] +fn get_default_blocks() -> u64 { + 512 +} + +#[cfg(not(target_os = "macos"))] +fn get_default_blocks() -> u64 { + 1024 +} + // this takes `my_stat` to avoid having to stat files multiple times. // XXX: this should use the impl Trait return type when it is stabilized fn du(mut my_stat: Stat, options: &Options, depth: usize) -> Box> { @@ -285,7 +295,7 @@ pub fn uumain(args: Vec) -> i32 { }; number * multiple } - None => 1024, + None => get_default_blocks(), }; let convert_size = |size: u64| -> String { @@ -332,7 +342,7 @@ Try '{} --help' for more information.", let mut grand_total = 0; for path_str in strs.into_iter() { - let path = PathBuf::from(path_str); + let path = PathBuf::from(&path_str); match Stat::new(path) { Ok(stat) => { let iter = du(stat, &options, 0).into_iter(); @@ -398,7 +408,9 @@ Try '{} --help' for more information.", } } } - Err(error) => show_error!("{}", error), + Err(_) => { + show_info!("{}: {}", path_str, "No such file or directory"); + } } } diff --git a/tests/fixtures/du/subdir/subwords.txt b/tests/fixtures/du/subdir/subwords.txt new file mode 100644 index 0000000000000000000000000000000000000000..e7f3c2d40ecc31921643a456cda2de3a907b680a GIT binary patch literal 5120 zcmZP=1*0J_8UmvsFd71*Aut*OqaiRF0;3@?8UmvsFd71*Aut*OqaiRF0;3@?8Un*M F1OOBO00961 literal 0 HcmV?d00001 diff --git a/tests/fixtures/du/subdir/subwords2.txt b/tests/fixtures/du/subdir/subwords2.txt new file mode 100644 index 000000000..ce0136250 --- /dev/null +++ b/tests/fixtures/du/subdir/subwords2.txt @@ -0,0 +1 @@ +hello diff --git a/tests/fixtures/du/words.txt b/tests/fixtures/du/words.txt new file mode 100644 index 000000000..ce0136250 --- /dev/null +++ b/tests/fixtures/du/words.txt @@ -0,0 +1 @@ +hello diff --git a/tests/test_du.rs b/tests/test_du.rs new file mode 100644 index 000000000..f9eeb388b --- /dev/null +++ b/tests/test_du.rs @@ -0,0 +1,62 @@ +use common::util::*; +use std::fs::set_permissions; + +static SUB_DIR: &str = "subdir"; +static SUB_FILE: &str = "subdir/subwords.txt"; +static SUB_LINK: &str = "subdir/sublink.txt"; + +#[test] +fn test_du_basics() { + let (_at, mut ucmd) = at_and_ucmd!(); + + let result = ucmd.run(); + assert!(result.success); + assert_eq!(result.stderr, ""); + assert_eq!(result.stdout, "24\t./subdir\n32\t./\n"); +} + +#[test] +fn test_du_basics_subdir() { + let (_at, mut ucmd) = at_and_ucmd!(); + + let result = ucmd.arg(SUB_DIR).run(); + assert!(result.success); + assert_eq!(result.stderr, ""); + assert_eq!(result.stdout, "24\tsubdir\n"); +} + +#[test] +fn test_du_basics_bad_name() { + let (_at, mut ucmd) = at_and_ucmd!(); + + let result = ucmd.arg("bad_name").run(); + assert_eq!(result.stdout, ""); + assert_eq!(result.stderr, "du: bad_name: No such file or directory\n"); +} + +#[test] +fn test_du_soft_link() { + let ts = TestScenario::new("du"); + + let link = ts.cmd("ln").arg("-s").arg(SUB_FILE).arg(SUB_LINK).run(); + assert!(link.success); + + let result = ts.ucmd().arg(SUB_DIR).run(); + assert!(result.success); + assert_eq!(result.stderr, ""); + assert_eq!(result.stdout, "32\tsubdir\n"); +} + +// todo: +// du on file with no permissions +// du on soft link +// du on hard link +// du on multi dir with '-d' +// +/* + * let mut permissions = at.make_file(TEST_HELLO_WORLD_DEST) + * .metadata() + * .unwrap() + * .permissions(); + * permissions.set_readonly(true); + */ diff --git a/tests/tests.rs b/tests/tests.rs index cb164a91c..b1362dd0a 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -52,6 +52,7 @@ generic! { "cut", test_cut; "dircolors", test_dircolors; "dirname", test_dirname; + "du", test_du; "echo", test_echo; "env", test_env; "expr", test_expr; From b6c7771087fa9908b28b54bb72a66e50bbbb5869 Mon Sep 17 00:00:00 2001 From: bootandy Date: Tue, 20 Mar 2018 16:40:27 -0400 Subject: [PATCH 2/5] du: Fix double counting of hard links. hard linked files are no longer counted - this mimcs the behaviour of the original du. --- src/du/du.rs | 20 +++++++++++++++++--- tests/test_du.rs | 16 ++++++++++++++-- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/src/du/du.rs b/src/du/du.rs index a78b8b56d..35725ef60 100644 --- a/src/du/du.rs +++ b/src/du/du.rs @@ -19,6 +19,7 @@ use std::iter; use std::io::{stderr, Result, Write}; use std::os::unix::fs::MetadataExt; use std::path::PathBuf; +use std::collections::HashSet; use time::Timespec; const NAME: &'static str = "du"; @@ -48,6 +49,7 @@ struct Stat { size: u64, blocks: u64, nlink: u64, + inode: u64, created: u64, accessed: u64, modified: u64, @@ -62,6 +64,7 @@ impl Stat { size: metadata.len(), blocks: metadata.blocks() as u64, nlink: metadata.nlink() as u64, + inode: metadata.ino() as u64, created: metadata.mtime() as u64, accessed: metadata.atime() as u64, modified: metadata.mtime() as u64, @@ -81,7 +84,12 @@ fn get_default_blocks() -> u64 { // this takes `my_stat` to avoid having to stat files multiple times. // XXX: this should use the impl Trait return type when it is stabilized -fn du(mut my_stat: Stat, options: &Options, depth: usize) -> Box> { +fn du( + mut my_stat: Stat, + options: &Options, + depth: usize, + inodes: &mut HashSet, +) -> Box> { let mut stats = vec![]; let mut futures = vec![]; @@ -105,8 +113,12 @@ fn du(mut my_stat: Stat, options: &Options, depth: usize) -> Box match Stat::new(entry.path()) { Ok(this_stat) => { if this_stat.is_dir { - futures.push(du(this_stat, options, depth + 1)); + futures.push(du(this_stat, options, depth + 1, inodes)); } else { + if inodes.contains(&this_stat.inode) { + continue; + } + inodes.insert(this_stat.inode); my_stat.size += this_stat.size; my_stat.blocks += this_stat.blocks; if options.all { @@ -345,7 +357,9 @@ Try '{} --help' for more information.", let path = PathBuf::from(&path_str); match Stat::new(path) { Ok(stat) => { - let iter = du(stat, &options, 0).into_iter(); + let mut inodes: HashSet = HashSet::new(); + + let iter = du(stat, &options, 0, &mut inodes).into_iter(); let (_, len) = iter.size_hint(); let len = len.unwrap(); for (index, stat) in iter.enumerate() { diff --git a/tests/test_du.rs b/tests/test_du.rs index f9eeb388b..d71ad1453 100644 --- a/tests/test_du.rs +++ b/tests/test_du.rs @@ -47,10 +47,22 @@ fn test_du_soft_link() { assert_eq!(result.stdout, "32\tsubdir\n"); } +#[test] +fn test_du_hard_link() { + let ts = TestScenario::new("du"); + + let link = ts.cmd("ln").arg(SUB_FILE).arg(SUB_LINK).run(); + assert!(link.success); + + let result = ts.ucmd().arg(SUB_DIR).run(); + assert!(result.success); + assert_eq!(result.stderr, ""); + // We do not double count hard links as the inodes are identicle + assert_eq!(result.stdout, "24\tsubdir\n"); +} + // todo: // du on file with no permissions -// du on soft link -// du on hard link // du on multi dir with '-d' // /* From ea504bf0eceb3ae867449dbbb4d51b3f698d3968 Mon Sep 17 00:00:00 2001 From: bootandy Date: Tue, 20 Mar 2018 17:55:31 -0400 Subject: [PATCH 3/5] du: add test for -d flag --- .../{subwords2.txt => deeper/words.txt} | 0 .../du/subdir/{ => links}/subwords.txt | Bin tests/fixtures/du/subdir/links/subwords2.txt | 1 + tests/fixtures/du/words.txt | 2 +- tests/test_du.rs | 46 +++++++++--------- 5 files changed, 26 insertions(+), 23 deletions(-) rename tests/fixtures/du/subdir/{subwords2.txt => deeper/words.txt} (100%) rename tests/fixtures/du/subdir/{ => links}/subwords.txt (100%) create mode 100644 tests/fixtures/du/subdir/links/subwords2.txt diff --git a/tests/fixtures/du/subdir/subwords2.txt b/tests/fixtures/du/subdir/deeper/words.txt similarity index 100% rename from tests/fixtures/du/subdir/subwords2.txt rename to tests/fixtures/du/subdir/deeper/words.txt diff --git a/tests/fixtures/du/subdir/subwords.txt b/tests/fixtures/du/subdir/links/subwords.txt similarity index 100% rename from tests/fixtures/du/subdir/subwords.txt rename to tests/fixtures/du/subdir/links/subwords.txt diff --git a/tests/fixtures/du/subdir/links/subwords2.txt b/tests/fixtures/du/subdir/links/subwords2.txt new file mode 100644 index 000000000..ce0136250 --- /dev/null +++ b/tests/fixtures/du/subdir/links/subwords2.txt @@ -0,0 +1 @@ +hello diff --git a/tests/fixtures/du/words.txt b/tests/fixtures/du/words.txt index ce0136250..45b983be3 100644 --- a/tests/fixtures/du/words.txt +++ b/tests/fixtures/du/words.txt @@ -1 +1 @@ -hello +hi diff --git a/tests/test_du.rs b/tests/test_du.rs index d71ad1453..484bf937c 100644 --- a/tests/test_du.rs +++ b/tests/test_du.rs @@ -1,18 +1,22 @@ use common::util::*; -use std::fs::set_permissions; -static SUB_DIR: &str = "subdir"; -static SUB_FILE: &str = "subdir/subwords.txt"; -static SUB_LINK: &str = "subdir/sublink.txt"; +static SUB_DIR: &str = "subdir/deeper"; +static SUB_DIR_LINKS: &str = "subdir/links"; +static SUB_FILE: &str = "subdir/links/subwords.txt"; +static SUB_LINK: &str = "subdir/links/sublink.txt"; #[test] fn test_du_basics() { let (_at, mut ucmd) = at_and_ucmd!(); - + let answer = "32\t./subdir +8\t./subdir/deeper +24\t./subdir/links +40\t./ +"; let result = ucmd.run(); assert!(result.success); assert_eq!(result.stderr, ""); - assert_eq!(result.stdout, "24\t./subdir\n32\t./\n"); + assert_eq!(result.stdout, answer); } #[test] @@ -22,7 +26,7 @@ fn test_du_basics_subdir() { let result = ucmd.arg(SUB_DIR).run(); assert!(result.success); assert_eq!(result.stderr, ""); - assert_eq!(result.stdout, "24\tsubdir\n"); + assert_eq!(result.stdout, "8\tsubdir/deeper\n"); } #[test] @@ -41,10 +45,10 @@ fn test_du_soft_link() { let link = ts.cmd("ln").arg("-s").arg(SUB_FILE).arg(SUB_LINK).run(); assert!(link.success); - let result = ts.ucmd().arg(SUB_DIR).run(); + let result = ts.ucmd().arg(SUB_DIR_LINKS).run(); assert!(result.success); assert_eq!(result.stderr, ""); - assert_eq!(result.stdout, "32\tsubdir\n"); + assert_eq!(result.stdout, "32\tsubdir/links\n"); } #[test] @@ -54,21 +58,19 @@ fn test_du_hard_link() { let link = ts.cmd("ln").arg(SUB_FILE).arg(SUB_LINK).run(); assert!(link.success); - let result = ts.ucmd().arg(SUB_DIR).run(); + let result = ts.ucmd().arg(SUB_DIR_LINKS).run(); assert!(result.success); assert_eq!(result.stderr, ""); // We do not double count hard links as the inodes are identicle - assert_eq!(result.stdout, "24\tsubdir\n"); + assert_eq!(result.stdout, "24\tsubdir/links\n"); } -// todo: -// du on file with no permissions -// du on multi dir with '-d' -// -/* - * let mut permissions = at.make_file(TEST_HELLO_WORLD_DEST) - * .metadata() - * .unwrap() - * .permissions(); - * permissions.set_readonly(true); - */ +#[test] +fn test_du_d_flag() { + let ts = TestScenario::new("du"); + + let result = ts.ucmd().arg("-d").arg("1").run(); + assert!(result.success); + assert_eq!(result.stderr, ""); + assert_eq!(result.stdout, "32\t./subdir\n40\t./\n"); +} From 8530db90c48c2a84a508fa3075ed99d8721df649 Mon Sep 17 00:00:00 2001 From: bootandy Date: Wed, 21 Mar 2018 10:14:18 -0400 Subject: [PATCH 4/5] du: Fix tests for linux. Running du on mac gives different answers to linux. Hence our asserts must differ for each platform --- tests/test_du.rs | 64 ++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 56 insertions(+), 8 deletions(-) diff --git a/tests/test_du.rs b/tests/test_du.rs index 484bf937c..f9824b3fd 100644 --- a/tests/test_du.rs +++ b/tests/test_du.rs @@ -8,15 +8,27 @@ static SUB_LINK: &str = "subdir/links/sublink.txt"; #[test] fn test_du_basics() { let (_at, mut ucmd) = at_and_ucmd!(); + let result = ucmd.run(); + assert!(result.success); + assert_eq!(result.stderr, ""); +} +#[cfg(target_os = "macos")] +fn _du_basics(s: String) { let answer = "32\t./subdir 8\t./subdir/deeper 24\t./subdir/links 40\t./ "; - let result = ucmd.run(); - assert!(result.success); - assert_eq!(result.stderr, ""); - assert_eq!(result.stdout, answer); + assert_eq!(s, answer); +} +#[cfg(not(target_os = "macos"))] +fn _du_basics(s: String) { + let answer = "28\t./subdir +8\t./subdir/deeper +16\t./subdir/links +36\t./ +"; + assert_eq!(s, answer); } #[test] @@ -26,7 +38,16 @@ fn test_du_basics_subdir() { let result = ucmd.arg(SUB_DIR).run(); assert!(result.success); assert_eq!(result.stderr, ""); - assert_eq!(result.stdout, "8\tsubdir/deeper\n"); + _du_basics_subdir(result.stdout); +} + +#[cfg(target_os = "macos")] +fn _du_basics_subdir(s: String) { + assert_eq!(s, "8\tsubdir/deeper\n"); +} +#[cfg(not(target_os = "macos"))] +fn _du_basics_subdir(s: String) { + assert_eq!(s, "8\tsubdir/deeper\n"); } #[test] @@ -48,7 +69,16 @@ fn test_du_soft_link() { let result = ts.ucmd().arg(SUB_DIR_LINKS).run(); assert!(result.success); assert_eq!(result.stderr, ""); - assert_eq!(result.stdout, "32\tsubdir/links\n"); + _du_soft_link(result.stdout); +} + +#[cfg(target_os = "macos")] +fn _du_soft_link(s: String) { + assert_eq!(s, "32\tsubdir/links\n"); +} +#[cfg(not(target_os = "macos"))] +fn _du_soft_link(s: String) { + assert_eq!(s, "16\tsubdir/links\n"); } #[test] @@ -62,7 +92,16 @@ fn test_du_hard_link() { assert!(result.success); assert_eq!(result.stderr, ""); // We do not double count hard links as the inodes are identicle - assert_eq!(result.stdout, "24\tsubdir/links\n"); + _du_hard_link(result.stdout); +} + +#[cfg(target_os = "macos")] +fn _du_hard_link(s: String) { + assert_eq!(s, "24\tsubdir/links\n") +} +#[cfg(not(target_os = "macos"))] +fn _du_hard_link(s: String) { + assert_eq!(s, "16\tsubdir/links\n"); } #[test] @@ -72,5 +111,14 @@ fn test_du_d_flag() { let result = ts.ucmd().arg("-d").arg("1").run(); assert!(result.success); assert_eq!(result.stderr, ""); - assert_eq!(result.stdout, "32\t./subdir\n40\t./\n"); + _du_d_flag(result.stdout); +} + +#[cfg(target_os = "macos")] +fn _du_d_flag(s: String) { + assert_eq!(s, "32\t./subdir\n40\t./\n"); +} +#[cfg(not(target_os = "macos"))] +fn _du_d_flag(s: String) { + assert_eq!(s, "28\t./subdir\n36\t./\n"); } From 838ce7b3e35f0e5aef5c222c7ee864729b481c21 Mon Sep 17 00:00:00 2001 From: bootandy Date: Thu, 22 Mar 2018 12:05:33 -0400 Subject: [PATCH 5/5] Fix issues raised in review spelling use POSIXLY_CORRECT and BLOCKSIZE env variables to determine block size. move statics to const use show_error! not show_info! --- src/du/du.rs | 10 ++-------- tests/test_du.rs | 23 +++++++++++++---------- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/du/du.rs b/src/du/du.rs index 35725ef60..aa79d2663 100644 --- a/src/du/du.rs +++ b/src/du/du.rs @@ -28,7 +28,7 @@ const LONG_HELP: &'static str = " Display values are in units of the first available SIZE from --block-size, and the DU_BLOCK_SIZE, BLOCK_SIZE and BLOCKSIZE environ‐ ment variables. Otherwise, units default to 1024 bytes (or 512 if - POSIXLY_CORRECT is set or if OS is Apple). + POSIXLY_CORRECT is set). SIZE is an integer and optional unit (example: 10M is 10*1024*1024). Units are K, M, G, T, P, E, Z, Y (powers of 1024) or KB, MB, ... (pow‐ @@ -72,12 +72,6 @@ impl Stat { } } -#[cfg(target_os = "macos")] -fn get_default_blocks() -> u64 { - 512 -} - -#[cfg(not(target_os = "macos"))] fn get_default_blocks() -> u64 { 1024 } @@ -423,7 +417,7 @@ Try '{} --help' for more information.", } } Err(_) => { - show_info!("{}: {}", path_str, "No such file or directory"); + show_error!("{}: {}", path_str, "No such file or directory"); } } } diff --git a/tests/test_du.rs b/tests/test_du.rs index f9824b3fd..7ade82dd7 100644 --- a/tests/test_du.rs +++ b/tests/test_du.rs @@ -1,9 +1,9 @@ use common::util::*; -static SUB_DIR: &str = "subdir/deeper"; -static SUB_DIR_LINKS: &str = "subdir/links"; -static SUB_FILE: &str = "subdir/links/subwords.txt"; -static SUB_LINK: &str = "subdir/links/sublink.txt"; +const SUB_DIR: &str = "subdir/deeper"; +const SUB_DIR_LINKS: &str = "subdir/links"; +const SUB_FILE: &str = "subdir/links/subwords.txt"; +const SUB_LINK: &str = "subdir/links/sublink.txt"; #[test] fn test_du_basics() { @@ -43,7 +43,7 @@ fn test_du_basics_subdir() { #[cfg(target_os = "macos")] fn _du_basics_subdir(s: String) { - assert_eq!(s, "8\tsubdir/deeper\n"); + assert_eq!(s, "4\tsubdir/deeper\n"); } #[cfg(not(target_os = "macos"))] fn _du_basics_subdir(s: String) { @@ -56,7 +56,10 @@ fn test_du_basics_bad_name() { let result = ucmd.arg("bad_name").run(); assert_eq!(result.stdout, ""); - assert_eq!(result.stderr, "du: bad_name: No such file or directory\n"); + assert_eq!( + result.stderr, + "du: error: bad_name: No such file or directory\n" + ); } #[test] @@ -74,7 +77,7 @@ fn test_du_soft_link() { #[cfg(target_os = "macos")] fn _du_soft_link(s: String) { - assert_eq!(s, "32\tsubdir/links\n"); + assert_eq!(s, "16\tsubdir/links\n"); } #[cfg(not(target_os = "macos"))] fn _du_soft_link(s: String) { @@ -91,13 +94,13 @@ fn test_du_hard_link() { let result = ts.ucmd().arg(SUB_DIR_LINKS).run(); assert!(result.success); assert_eq!(result.stderr, ""); - // We do not double count hard links as the inodes are identicle + // We do not double count hard links as the inodes are identical _du_hard_link(result.stdout); } #[cfg(target_os = "macos")] fn _du_hard_link(s: String) { - assert_eq!(s, "24\tsubdir/links\n") + assert_eq!(s, "12\tsubdir/links\n") } #[cfg(not(target_os = "macos"))] fn _du_hard_link(s: String) { @@ -116,7 +119,7 @@ fn test_du_d_flag() { #[cfg(target_os = "macos")] fn _du_d_flag(s: String) { - assert_eq!(s, "32\t./subdir\n40\t./\n"); + assert_eq!(s, "16\t./subdir\n20\t./\n"); } #[cfg(not(target_os = "macos"))] fn _du_d_flag(s: String) {