Skip to content

Commit 5648194

Browse files
cerdelensylvestre
cerdelen
and
sylvestre
authored
Du size_format flag override (#10743)
* du: Extend size_format flag override tests * du: Extend size_override implementation for '-h' '--si' '-h' '--block-size' * du: Extend size_format flag override tests * du: Extend size_format override tests for correct error return with invalid block-size value regardless of override * du: Refactor size-format parsing code --------- Co-authored-by: Sylvestre Ledru <sylvestre@debian.org>
1 parent 3d56517 commit 5648194

File tree

2 files changed

+97
-36
lines changed

2 files changed

+97
-36
lines changed

‎src/uu/du/src/du.rs‎

Lines changed: 33 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -963,38 +963,61 @@ fn read_files_from(file_name: &OsStr) -> Result<Vec<PathBuf>, std::io::Error> {
963963
Ok(paths)
964964
}
965965

966-
fn get_block_size_arg_index_if_present(matches: &ArgMatches, flag: &str) -> Option<usize> {
967-
if matches.get_flag(flag) {
968-
// Indices of returns index even if flag is not present, thats why we need to if guard it
966+
fn get_size_format_flag_arg_index_if_present(matches: &ArgMatches, arg: &str) -> Option<usize> {
967+
if let Some(clap::parser::ValueSource::CommandLine) = matches.value_source(arg) {
969968
matches
970-
.indices_of(flag)
969+
.indices_of(arg)
971970
.and_then(|mut indices| indices.next_back())
972971
} else {
973972
None
974973
}
975974
}
976975

977-
fn handle_block_size_arg_override(matches: &ArgMatches) -> Option<SizeFormat> {
976+
fn parse_block_size_arg_or_default_fallback(matches: &ArgMatches) -> UResult<SizeFormat> {
977+
let block_size_str = matches.get_one::<String>(options::BLOCK_SIZE);
978+
let block_size = read_block_size(block_size_str.map(AsRef::as_ref))?;
979+
if block_size == 0 {
980+
return Err(std::io::Error::other(translate!("du-error-invalid-block-size-argument", "option" => options::BLOCK_SIZE, "value" => block_size_str.map_or("???BUG", |v| v).quote()))
981+
.into());
982+
}
983+
Ok(SizeFormat::BlockSize(block_size))
984+
}
985+
986+
fn parse_size_format(matches: &ArgMatches) -> UResult<SizeFormat> {
987+
let block_size_value_or_default_fallback = parse_block_size_arg_or_default_fallback(matches)?;
978988
let candidates = [
979989
(
980990
SizeFormat::BlockSize(1),
981-
get_block_size_arg_index_if_present(matches, options::BYTES),
991+
get_size_format_flag_arg_index_if_present(matches, options::BYTES),
982992
),
983993
(
984994
SizeFormat::BlockSize(1024),
985-
get_block_size_arg_index_if_present(matches, options::BLOCK_SIZE_1K),
995+
get_size_format_flag_arg_index_if_present(matches, options::BLOCK_SIZE_1K),
986996
),
987997
(
988998
SizeFormat::BlockSize(1024 * 1024),
989-
get_block_size_arg_index_if_present(matches, options::BLOCK_SIZE_1M),
999+
get_size_format_flag_arg_index_if_present(matches, options::BLOCK_SIZE_1M),
1000+
),
1001+
(
1002+
SizeFormat::HumanBinary,
1003+
get_size_format_flag_arg_index_if_present(matches, options::HUMAN_READABLE),
1004+
),
1005+
(
1006+
SizeFormat::HumanDecimal,
1007+
get_size_format_flag_arg_index_if_present(matches, options::SI),
1008+
),
1009+
(
1010+
block_size_value_or_default_fallback.clone(),
1011+
get_size_format_flag_arg_index_if_present(matches, options::BLOCK_SIZE),
9901012
),
9911013
];
9921014

993-
candidates
1015+
Ok(candidates
9941016
.into_iter()
9951017
.filter(|(_, idx)| idx.is_some())
9961018
.max_by_key(|&(_, idx)| idx.unwrap_or(0))
9971019
.map(|(size_format, _)| size_format)
1020+
.unwrap_or(block_size_value_or_default_fallback))
9981021
}
9991022

10001023
#[uucore::main]
@@ -1048,21 +1071,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
10481071
.map_or(MetadataTimeField::Modification, |s| s.as_str().into())
10491072
});
10501073

1051-
let size_format = if matches.get_flag(options::HUMAN_READABLE) {
1052-
SizeFormat::HumanBinary
1053-
} else if matches.get_flag(options::SI) {
1054-
SizeFormat::HumanDecimal
1055-
} else if let Some(size_format) = handle_block_size_arg_override(&matches) {
1056-
size_format
1057-
} else {
1058-
let block_size_str = matches.get_one::<String>(options::BLOCK_SIZE);
1059-
let block_size = read_block_size(block_size_str.map(AsRef::as_ref))?;
1060-
if block_size == 0 {
1061-
return Err(std::io::Error::other(translate!("du-error-invalid-block-size-argument", "option" => options::BLOCK_SIZE, "value" => block_size_str.map_or("???BUG", |v| v).quote()))
1062-
.into());
1063-
}
1064-
SizeFormat::BlockSize(block_size)
1065-
};
1074+
let size_format = parse_size_format(&matches)?;
10661075

10671076
let traversal_options = TraversalOptions {
10681077
all: matches.get_flag(options::ALL),

‎tests/by-util/test_du.rs‎

Lines changed: 64 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2064,24 +2064,45 @@ fn test_block_size_args_override() {
20642064
let ts = TestScenario::new(util_name!());
20652065
let at = &ts.fixtures;
20662066
let dir = "override_args_dir";
2067+
let nested_dir = "override_args_dir/nested_dir";
2068+
let nested_dir_2 = "override_args_dir/nested_dir_2";
20672069

2068-
at.mkdir(dir);
2069-
let fpath = at.plus(format!("{dir}/file"));
2070+
at.mkdir_all(nested_dir);
2071+
at.mkdir_all(nested_dir_2);
2072+
let fpath = at.plus(format!("{nested_dir}/file"));
20702073
std::fs::File::create(fpath)
20712074
.expect("cannot create test file")
20722075
.set_len(100_000_000)
20732076
.expect("cannot set file size");
20742077

2075-
let fpath2 = at.plus(format!("{dir}/file_2"));
2078+
let fpath2 = at.plus(format!("{nested_dir}/file_2"));
20762079
std::fs::File::create(fpath2)
20772080
.expect("cannot create test file")
20782081
.set_len(100_000_000)
20792082
.expect("cannot set file size");
20802083

2084+
let fpath = at.plus(format!("{nested_dir_2}/file_3"));
2085+
std::fs::File::create(fpath)
2086+
.expect("cannot create test file")
2087+
.set_len(100_000)
2088+
.expect("cannot set file size");
2089+
2090+
let fpath2 = at.plus(format!("{nested_dir_2}/file_4"));
2091+
std::fs::File::create(fpath2)
2092+
.expect("cannot create test file")
2093+
.set_len(100)
2094+
.expect("cannot set file size");
2095+
20812096
let test_cases = [
2082-
(["-sk", "-m"], "-sm"),
2083-
(["-sk", "-b"], "-sb"),
2084-
(["-sm", "-k"], "-sk"),
2097+
(["-sk", "-m"], vec!["-sm"]),
2098+
(["-sk", "-b"], vec!["-sb"]),
2099+
(["-sm", "-k"], vec!["-sk"]),
2100+
(["-sk", "--si"], vec!["-s", "--si"]),
2101+
(["-sk", "-h"], vec!["-s", "-h"]),
2102+
(["-sm", "--block-size=128"], vec!["-s", "--block-size=128"]),
2103+
(["--block-size=128", "-b"], vec!["-b"]),
2104+
(["--si", "-b"], vec!["-b"]),
2105+
(["-h", "-b"], vec!["-b"]),
20852106
];
20862107

20872108
for (idx, (overwriting_args, expected)) in test_cases.into_iter().enumerate() {
@@ -2095,7 +2116,7 @@ fn test_block_size_args_override() {
20952116
let single_args = ts
20962117
.ucmd()
20972118
.arg(dir)
2098-
.arg(expected)
2119+
.args(&expected)
20992120
.succeeds()
21002121
.stdout_move_str();
21012122

@@ -2111,23 +2132,30 @@ fn test_block_override_b_still_has_apparent_size() {
21112132
let ts = TestScenario::new(util_name!());
21122133
let at = &ts.fixtures;
21132134
let dir = "override_args_dir";
2135+
let nested_dir = "override_args_dir/nested_dir";
21142136

2115-
at.mkdir(dir);
2116-
let fpath = at.plus(format!("{dir}/file"));
2137+
at.mkdir_all(nested_dir);
2138+
let fpath = at.plus(format!("{nested_dir}/file"));
21172139
std::fs::File::create(fpath)
21182140
.expect("cannot create test file")
21192141
.set_len(100_000_000)
21202142
.expect("cannot set file size");
21212143

2122-
let fpath2 = at.plus(format!("{dir}/file_2"));
2144+
let fpath2 = at.plus(format!("{nested_dir}/file_2"));
21232145
std::fs::File::create(fpath2)
21242146
.expect("cannot create test file")
21252147
.set_len(100_000_000)
21262148
.expect("cannot set file size");
21272149

21282150
let test_cases = [
2129-
(["-sb", "-m"], ["-sm", "--apparent-size"]),
2130-
(["-sb", "-k"], ["-sk", "--apparent-size"]),
2151+
(["-b", "-m"], ["-m", "--apparent-size"]),
2152+
(["-b", "-k"], ["-k", "--apparent-size"]),
2153+
(["-b", "--si"], ["--si", "--apparent-size"]),
2154+
(["-b", "-h"], ["-h", "--apparent-size"]),
2155+
(
2156+
["-b", "--block-size=128"],
2157+
["--block-size=128", "--apparent-size"],
2158+
),
21312159
];
21322160

21332161
for (idx, (overwriting_args, expected)) in test_cases.into_iter().enumerate() {
@@ -2151,3 +2179,27 @@ fn test_block_override_b_still_has_apparent_size() {
21512179
);
21522180
}
21532181
}
2182+
2183+
#[test]
2184+
fn test_overriding_block_size_arg_with_invalid_value_still_errors() {
2185+
new_ucmd!()
2186+
.args(&["--block-size=abc", "-m"])
2187+
.fails_with_code(1)
2188+
.stderr_contains("invalid --block-size argument 'abc'");
2189+
new_ucmd!()
2190+
.args(&["--block-size=abc", "-k"])
2191+
.fails_with_code(1)
2192+
.stderr_contains("invalid --block-size argument 'abc'");
2193+
new_ucmd!()
2194+
.args(&["--block-size=abc", "-b"])
2195+
.fails_with_code(1)
2196+
.stderr_contains("invalid --block-size argument 'abc'");
2197+
new_ucmd!()
2198+
.args(&["--block-size=abc", "-h"])
2199+
.fails_with_code(1)
2200+
.stderr_contains("invalid --block-size argument 'abc'");
2201+
new_ucmd!()
2202+
.args(&["--block-size=abc", "--si"])
2203+
.fails_with_code(1)
2204+
.stderr_contains("invalid --block-size argument 'abc'");
2205+
}

0 commit comments

Comments
 (0)