From 7f98d20e39bdbe78f657c86ad2ab240b17915f83 Mon Sep 17 00:00:00 2001 From: Socialpranker <273312799+Socialpranker@users.noreply.github.com> Date: Sat, 5 Sep 2026 21:17:06 +0200 Subject: [PATCH 1/2] df: preserve suffix-only block size units GNU echoes the unit back next to the number when --block-size (or DF_BLOCK_SIZE/BLOCK_SIZE/BLOCKSIZE) is given as a bare unit (--block-size=K -> "1K"), but not when a numeric multiplier is given (--block-size=1K or --block-size=1024 -> "1"). uutils printed a bare number in both cases. Adds suffix_from_parsed_block_size and block_size_from_env_with_suffix to uucore's parse_block_size; df's BlockSize now carries an optional display suffix, which also lets the header keep a full IEC spelling (--block-size=KiB -> "1KiB-blocks"). --- src/uu/df/src/blocks.rs | 93 ++++++++---- src/uu/df/src/df.rs | 2 +- src/uu/df/src/table.rs | 44 +++--- .../lib/features/parser/parse_block_size.rs | 136 ++++++++++++++++++ tests/by-util/test_df.rs | 124 ++++++++++++++++ 5 files changed, 349 insertions(+), 50 deletions(-) diff --git a/src/uu/df/src/blocks.rs b/src/uu/df/src/blocks.rs index 708709c6464..42af45fb3aa 100644 --- a/src/uu/df/src/blocks.rs +++ b/src/uu/df/src/blocks.rs @@ -119,31 +119,36 @@ pub(crate) enum HumanReadable { Binary, } -/// A block size to use in condensing the display of a large number of bytes. +/// A static block size, plus the unit to echo next to scaled values. /// -/// The [`BlockSize::Bytes`] variant represents a static block -/// size. -/// -/// The default variant is `Bytes(1024)`. +/// The unit is set only for a suffix-only spec: `--block-size=K` gives +/// `Bytes(1024, Some("K"))`, `--block-size=1K` gives `Bytes(1024, None)`. #[derive(Debug, PartialEq)] pub(crate) enum BlockSize { - /// A fixed number of bytes. - /// - /// The number must be positive. - Bytes(u64), + /// A positive number of bytes, with an optional display suffix. + Bytes(u64, Option), } impl BlockSize { /// Returns the associated value pub(crate) fn as_u64(&self) -> u64 { - match *self { - Self::Bytes(n) => n, + match self { + Self::Bytes(n, _) => *n, + } + } + + /// Returns the display suffix of a suffix-only spec. + pub(crate) fn suffix(&self) -> Option<&str> { + match self { + Self::Bytes(_, suffix) => suffix.as_deref(), } } pub(crate) fn to_header(&self) -> String { match self { - Self::Bytes(n) => { + // Full IEC form keeps its spelling: "KiB" -> "1KiB", not "1K". + Self::Bytes(_, Some(suffix)) if suffix.ends_with("iB") => format!("1{suffix}"), + Self::Bytes(n, _) => { if n % 1024 == 0 && n % 1000 != 0 { to_magnitude_and_suffix(*n as u128, SuffixType::Iec, false) } else { @@ -156,7 +161,7 @@ impl BlockSize { impl Default for BlockSize { fn default() -> Self { - Self::Bytes(parse_block_size::default_block_size()) + Self::Bytes(parse_block_size::default_block_size(), None) } } @@ -166,25 +171,28 @@ pub(crate) fn read_block_size(matches: &ArgMatches) -> Result 0 { - Ok(BlockSize::Bytes(bytes)) + let suffix = parse_block_size::suffix_from_parsed_block_size(s); + Ok(BlockSize::Bytes(bytes, suffix)) } else { Err(ParseSizeError::ParseFailure(format!("{}", s.quote()))) } } else if matches.get_flag(OPT_PORTABILITY) { Ok(BlockSize::default()) - } else if let Some(bytes) = - parse_block_size::block_size_from_env(&["DF_BLOCK_SIZE", "BLOCK_SIZE", "BLOCKSIZE"]).found() - { - Ok(BlockSize::Bytes(bytes)) } else { - Ok(BlockSize::default()) + let vars = ["DF_BLOCK_SIZE", "BLOCK_SIZE", "BLOCKSIZE"]; + match parse_block_size::block_size_from_env_with_suffix(&vars) { + (parse_block_size::BlockSizeEnv::Found(bytes), suffix) => { + Ok(BlockSize::Bytes(bytes, suffix)) + } + _ => Ok(BlockSize::default()), + } } } impl fmt::Display for BlockSize { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { - Self::Bytes(n) => { + Self::Bytes(n, _) => { let s = if n % 1024 == 0 && n % 1000 != 0 { to_magnitude_and_suffix(*n as u128, SuffixType::Iec, true) } else { @@ -356,23 +364,52 @@ mod tests { #[test] fn test_block_size_display() { - assert_eq!(format!("{}", BlockSize::Bytes(1024)), "1.0K"); - assert_eq!(format!("{}", BlockSize::Bytes(2 * 1024)), "2.0K"); - assert_eq!(format!("{}", BlockSize::Bytes(3 * 1024 * 1024)), "3.0M"); + assert_eq!(format!("{}", BlockSize::Bytes(1024, None)), "1.0K"); + assert_eq!(format!("{}", BlockSize::Bytes(2 * 1024, None)), "2.0K"); + assert_eq!( + format!("{}", BlockSize::Bytes(3 * 1024 * 1024, None)), + "3.0M" + ); } #[test] fn test_block_size_display_multiples_of_1000_and_1024() { - assert_eq!(format!("{}", BlockSize::Bytes(128_000)), "128kB"); - assert_eq!(format!("{}", BlockSize::Bytes(1000 * 1024)), "1.1MB"); - assert_eq!(format!("{}", BlockSize::Bytes(1_000_000_000_000)), "1.0TB"); + assert_eq!(format!("{}", BlockSize::Bytes(128_000, None)), "128kB"); + assert_eq!(format!("{}", BlockSize::Bytes(1000 * 1024, None)), "1.1MB"); + assert_eq!( + format!("{}", BlockSize::Bytes(1_000_000_000_000, None)), + "1.0TB" + ); + } + + #[test] + fn test_block_size_header_keeps_iec_spelling() { + assert_eq!( + BlockSize::Bytes(1024, Some("KiB".to_string())).to_header(), + "1KiB" + ); + assert_eq!( + BlockSize::Bytes(1024 * 1024, Some("MiB".to_string())).to_header(), + "1MiB" + ); + // A bare "K" and a numeric spec both stay in the short spelling. + assert_eq!( + BlockSize::Bytes(1024, Some("K".to_string())).to_header(), + "1K" + ); + assert_eq!(BlockSize::Bytes(2048, None).to_header(), "2K"); + // "KD" is 1000 bytes: decimal header, but values suffixed "K". + assert_eq!( + BlockSize::Bytes(1000, Some("K".to_string())).to_header(), + "1kB" + ); } #[test] fn test_default_block_size() { - assert_eq!(BlockSize::Bytes(1024), BlockSize::default()); + assert_eq!(BlockSize::Bytes(1024, None), BlockSize::default()); unsafe { env::set_var("POSIXLY_CORRECT", "1") }; - assert_eq!(BlockSize::Bytes(512), BlockSize::default()); + assert_eq!(BlockSize::Bytes(512, None), BlockSize::default()); unsafe { env::remove_var("POSIXLY_CORRECT") }; } } diff --git a/src/uu/df/src/df.rs b/src/uu/df/src/df.rs index 83da1496ddf..6f38efa95c5 100644 --- a/src/uu/df/src/df.rs +++ b/src/uu/df/src/df.rs @@ -694,7 +694,7 @@ mod tests { /// (`POSIXLY_CORRECT` halves it), so pin it for reproducible output. fn options() -> Options { Options { - block_size: BlockSize::Bytes(1024), + block_size: BlockSize::Bytes(1024, None), ..Options::default() } } diff --git a/src/uu/df/src/table.rs b/src/uu/df/src/table.rs index 71cc83ee57b..da265a04ed1 100644 --- a/src/uu/df/src/table.rs +++ b/src/uu/df/src/table.rs @@ -216,7 +216,7 @@ impl BytesCell { Self { bytes, scaled: { - let BlockSize::Bytes(d) = block_size; + let BlockSize::Bytes(d, _) = block_size; (bytes as f64 / *d as f64).ceil() as u64 }, } @@ -308,12 +308,14 @@ impl<'a> RowFormatter<'a> { let size = bytes_column.scaled; let s = if let Some(h) = self.options.human_readable { let size = if self.is_total_row { - let BlockSize::Bytes(d) = self.options.block_size; + let BlockSize::Bytes(d, _) = self.options.block_size; d * size } else { bytes_column.bytes }; to_magnitude_and_suffix(size.into(), SuffixType::HumanReadable(h), true) + } else if let Some(suffix) = self.options.block_size.suffix() { + format!("{size}{suffix}") } else { size.to_string() }; @@ -613,9 +615,9 @@ mod tests { fs_type: "my_type".to_string(), fs_mount: "my_mount".into(), - bytes: BytesCell::new(100, &BlockSize::Bytes(1)), - bytes_used: BytesCell::new(25, &BlockSize::Bytes(1)), - bytes_avail: BytesCell::new(75, &BlockSize::Bytes(1)), + bytes: BytesCell::new(100, &BlockSize::Bytes(1, None)), + bytes_used: BytesCell::new(25, &BlockSize::Bytes(1, None)), + bytes_avail: BytesCell::new(75, &BlockSize::Bytes(1, None)), bytes_usage: Some(0.25), #[cfg(target_vendor = "apple")] @@ -692,7 +694,7 @@ mod tests { fn test_header_with_block_size_1024() { init(); let options = Options { - block_size: BlockSize::Bytes(3 * 1024), + block_size: BlockSize::Bytes(3 * 1024, None), ..Default::default() }; assert_eq!( @@ -772,16 +774,16 @@ mod tests { fn test_row_formatter() { init(); let options = Options { - block_size: BlockSize::Bytes(1), + block_size: BlockSize::Bytes(1, None), ..Default::default() }; let row = Row { fs_device: "my_device".to_string(), fs_mount: "my_mount".into(), - bytes: BytesCell::new(100, &BlockSize::Bytes(1)), - bytes_used: BytesCell::new(25, &BlockSize::Bytes(1)), - bytes_avail: BytesCell::new(75, &BlockSize::Bytes(1)), + bytes: BytesCell::new(100, &BlockSize::Bytes(1, None)), + bytes_used: BytesCell::new(25, &BlockSize::Bytes(1, None)), + bytes_avail: BytesCell::new(75, &BlockSize::Bytes(1, None)), bytes_usage: Some(0.25), ..Default::default() @@ -798,7 +800,7 @@ mod tests { init(); let options = Options { columns: COLUMNS_WITH_FS_TYPE.to_vec(), - block_size: BlockSize::Bytes(1), + block_size: BlockSize::Bytes(1, None), ..Default::default() }; let row = Row { @@ -806,9 +808,9 @@ mod tests { fs_type: "my_type".to_string(), fs_mount: "my_mount".into(), - bytes: BytesCell::new(100, &BlockSize::Bytes(1)), - bytes_used: BytesCell::new(25, &BlockSize::Bytes(1)), - bytes_avail: BytesCell::new(75, &BlockSize::Bytes(1)), + bytes: BytesCell::new(100, &BlockSize::Bytes(1, None)), + bytes_used: BytesCell::new(25, &BlockSize::Bytes(1, None)), + bytes_avail: BytesCell::new(75, &BlockSize::Bytes(1, None)), bytes_usage: Some(0.25), ..Default::default() @@ -825,7 +827,7 @@ mod tests { init(); let options = Options { columns: COLUMNS_WITH_INODES.to_vec(), - block_size: BlockSize::Bytes(1), + block_size: BlockSize::Bytes(1, None), ..Default::default() }; let row = Row { @@ -851,11 +853,11 @@ mod tests { init(); let options = Options { columns: vec![Column::Size, Column::Itotal], - block_size: BlockSize::Bytes(100), + block_size: BlockSize::Bytes(100, None), ..Default::default() }; let row = Row { - bytes: BytesCell::new(100, &BlockSize::Bytes(100)), + bytes: BytesCell::new(100, &BlockSize::Bytes(100, None)), inodes: 10, ..Default::default() }; @@ -952,15 +954,15 @@ mod tests { fn test_row_formatter_with_round_up_byte_values() { fn get_formatted_values(bytes: u64, bytes_used: u64, bytes_avail: u64) -> Vec { let options = Options { - block_size: BlockSize::Bytes(1000), + block_size: BlockSize::Bytes(1000, None), columns: vec![Column::Size, Column::Used, Column::Avail], ..Default::default() }; let row = Row { - bytes: BytesCell::new(bytes, &BlockSize::Bytes(1000)), - bytes_used: BytesCell::new(bytes_used, &BlockSize::Bytes(1000)), - bytes_avail: BytesCell::new(bytes_avail, &BlockSize::Bytes(1000)), + bytes: BytesCell::new(bytes, &BlockSize::Bytes(1000, None)), + bytes_used: BytesCell::new(bytes_used, &BlockSize::Bytes(1000, None)), + bytes_avail: BytesCell::new(bytes_avail, &BlockSize::Bytes(1000, None)), ..Default::default() }; RowFormatter::new(&row, &options, false).get_cells() diff --git a/src/uucore/src/lib/features/parser/parse_block_size.rs b/src/uucore/src/lib/features/parser/parse_block_size.rs index f710a21d49b..ab2d7b8bc85 100644 --- a/src/uucore/src/lib/features/parser/parse_block_size.rs +++ b/src/uucore/src/lib/features/parser/parse_block_size.rs @@ -62,6 +62,42 @@ pub fn block_size_from_env(vars: &[&str]) -> BlockSizeEnv { BlockSizeEnv::NotSet } +/// Returns the GNU display unit of a suffix-only block-size spec, `None` if +/// the spec starts with a digit or is a plain `B`. +/// +/// ``` +/// # use uucore::parser::parse_block_size::suffix_from_parsed_block_size; +/// assert_eq!(suffix_from_parsed_block_size("k"), Some("K".into())); +/// assert_eq!(suffix_from_parsed_block_size("1K"), None); +/// ``` +pub fn suffix_from_parsed_block_size(s: &str) -> Option { + let mut chars = s.chars(); + let unit = chars.next()?.to_ascii_uppercase(); + if !unit.is_ascii_alphabetic() || unit == 'B' { + return None; + } + + Some(match chars.as_str() { + "" | "D" => unit.to_string(), + "B" if unit == 'K' => "kB".to_string(), + suffix => format!("{unit}{suffix}"), + }) +} + +/// Like [`block_size_from_env`], but also returns the +/// [`suffix_from_parsed_block_size`] of the variable that won. +pub fn block_size_from_env_with_suffix(vars: &[&str]) -> (BlockSizeEnv, Option) { + let result = block_size_from_env(vars); + let suffix = if matches!(result, BlockSizeEnv::Found(_)) { + vars.iter() + .find_map(|var| std::env::var(var).ok()) + .and_then(|value| suffix_from_parsed_block_size(&value)) + } else { + None + }; + (result, suffix) +} + /// Default block size when no env var or flag is set. /// /// Returns 512 if `POSIXLY_CORRECT` is set, 1024 otherwise. @@ -99,6 +135,106 @@ mod tests { } } + #[test] + fn test_suffix_from_parsed_block_size_bare_units() { + // A bare unit is echoed back, canonicalized to the form the unit is + // conventionally spelled with. + for (spec, expected) in [ + ("K", "K"), + ("k", "K"), + ("KB", "kB"), + ("kB", "kB"), + ("KiB", "KiB"), + ("kiB", "KiB"), + ("M", "M"), + ("m", "M"), + ("MB", "MB"), + ("MiB", "MiB"), + ("G", "G"), + ("T", "T"), + ("P", "P"), + ("E", "E"), + ] { + assert_eq!( + suffix_from_parsed_block_size(spec).as_deref(), + Some(expected), + "spec {spec:?}" + ); + } + } + + #[test] + fn test_suffix_from_parsed_block_size_decimal_marker() { + // `KD` selects a decimal multiplier but still displays as `K`. + assert_eq!( + suffix_from_parsed_block_size("KD").as_deref(), + Some("K"), + "the trailing D is a multiplier marker, not part of the unit" + ); + assert_eq!(suffix_from_parsed_block_size("MD").as_deref(), Some("M")); + } + + #[test] + fn test_suffix_from_parsed_block_size_numeric_specs_have_none() { + for spec in ["1K", "1024", "2K", "1MB", "0", ""] { + assert_eq!( + suffix_from_parsed_block_size(spec), + None, + "a spec with a leading number echoes no unit: {spec:?}" + ); + } + } + + #[test] + fn test_suffix_from_parsed_block_size_plain_bytes_have_none() { + assert_eq!(suffix_from_parsed_block_size("B"), None); + } + + #[test] + fn test_block_size_from_env_with_suffix_reports_unit() { + let _guard = ENV_LOCK.lock().unwrap(); + let vars = ["TEST_PROG_SFX", "BLOCK_SIZE", "BLOCKSIZE"]; + clear_env_vars(&vars); + + set_env_var("TEST_PROG_SFX", "K"); + assert_eq!( + block_size_from_env_with_suffix(&vars), + (BlockSizeEnv::Found(1024), Some("K".to_string())) + ); + + set_env_var("TEST_PROG_SFX", "1K"); + assert_eq!( + block_size_from_env_with_suffix(&vars), + (BlockSizeEnv::Found(1024), None), + "a numeric spec carries no unit to echo" + ); + + set_env_var("TEST_PROG_SFX", "bogus"); + assert_eq!( + block_size_from_env_with_suffix(&vars), + (BlockSizeEnv::SetButInvalid, None) + ); + + clear_env_vars(&vars); + } + + #[test] + fn test_block_size_from_env_with_suffix_uses_winning_var() { + let _guard = ENV_LOCK.lock().unwrap(); + let vars = ["TEST_PROG_SFX2", "BLOCK_SIZE", "BLOCKSIZE"]; + clear_env_vars(&vars); + + // The first *set* variable wins, and the unit comes from that one. + set_env_var("BLOCK_SIZE", "M"); + set_env_var("BLOCKSIZE", "K"); + assert_eq!( + block_size_from_env_with_suffix(&vars), + (BlockSizeEnv::Found(1024 * 1024), Some("M".to_string())) + ); + + clear_env_vars(&vars); + } + #[test] fn test_block_size_from_env_program_var_priority() { let _guard = ENV_LOCK.lock().unwrap(); diff --git a/tests/by-util/test_df.rs b/tests/by-util/test_df.rs index 98b2981ff1c..d9825f872e2 100644 --- a/tests/by-util/test_df.rs +++ b/tests/by-util/test_df.rs @@ -863,6 +863,130 @@ fn test_ignore_block_size_from_env_in_posix_portability_mode() { assert_eq!(header, default_block_size_header); } +/// The first data value of a `df --output=size` listing. +fn scaled_size(stdout: &str) -> String { + stdout.lines().nth(1).unwrap().trim().to_string() +} + +/// Assert that `value` is digits followed by exactly `suffix` (or by nothing, +/// when `suffix` is `None`). +fn assert_unit_suffix(value: &str, suffix: Option<&str>, context: &str) { + let digits = match suffix { + Some(suffix) => { + let stripped = value.strip_suffix(suffix).unwrap_or_else(|| { + panic!("expected {value:?} to end in {suffix:?} for {context}"); + }); + assert!( + !stripped.ends_with(|c: char| c.is_ascii_alphabetic()), + "expected exactly the unit {suffix:?} in {value:?} for {context}" + ); + stripped + } + None => value, + }; + assert!( + !digits.is_empty() && digits.chars().all(|c| c.is_ascii_digit()), + "expected a plain number in {value:?} for {context}" + ); +} + +#[test] +fn test_df_suffix_only_block_size() { + // --block-size=K echoes the unit ("123K"); =1K and =1024 do not. + for (arg, suffix) in [ + ("--block-size=K", Some("K")), + ("--block-size=k", Some("K")), + ("--block-size=M", Some("M")), + ("--block-size=KB", Some("kB")), + ("--block-size=KiB", Some("KiB")), + ("--block-size=MiB", Some("MiB")), + ("--block-size=G", Some("G")), + ("--block-size=1K", None), + ("--block-size=1024", None), + ("--block-size=2K", None), + ("--block-size=1MB", None), + ] { + let out = new_ucmd!() + .args(&[arg, "--output=size"]) + .succeeds() + .stdout_move_str(); + assert_unit_suffix(&scaled_size(&out), suffix, arg); + } +} + +#[test] +fn test_df_suffix_only_env_block_size() { + for (var, value, suffix) in [ + ("DF_BLOCK_SIZE", "M", Some("M")), + ("BLOCK_SIZE", "K", Some("K")), + ("BLOCKSIZE", "K", Some("K")), + ("DF_BLOCK_SIZE", "KB", Some("kB")), + ("DF_BLOCK_SIZE", "1M", None), + ("BLOCK_SIZE", "1024", None), + ] { + let out = new_ucmd!() + .env(var, value) + .arg("--output=size") + .succeeds() + .stdout_move_str(); + assert_unit_suffix(&scaled_size(&out), suffix, &format!("{var}={value}")); + } +} + +#[test] +fn test_df_block_size_header_keeps_iec_spelling() { + for (arg, header) in [ + ("--block-size=KiB", "1KiB-blocks"), + ("--block-size=MiB", "1MiB-blocks"), + ("--block-size=K", "1K-blocks"), + ("--block-size=KB", "1kB-blocks"), + ("--block-size=1K", "1K-blocks"), + ("--block-size=2K", "2K-blocks"), + ("--block-size=2KiB", "2K-blocks"), + ] { + let out = new_ucmd!() + .args(&[arg, "--output=size"]) + .succeeds() + .stdout_move_str(); + assert_eq!(out.lines().next().unwrap().trim(), header, "for {arg}"); + } +} + +#[test] +fn test_df_suffix_only_block_size_not_used_with_human_readable() { + // -h/--si print their own units and header; nothing is echoed. + for flag in ["-h", "-H"] { + let out = new_ucmd!() + .env("DF_BLOCK_SIZE", "K") + .args(&[flag, "--output=size"]) + .succeeds() + .stdout_move_str(); + assert_eq!( + out.lines().next().unwrap().trim(), + "Size", + "expected the human-readable header for {flag}, got {out:?}" + ); + } +} + +#[test] +fn test_df_suffix_only_block_size_ignored_in_posix_portability_mode() { + // -P pins the block size, so no unit is echoed next to the values. + let out = new_ucmd!() + .env("DF_BLOCK_SIZE", "M") + .arg("-P") + .succeeds() + .stdout_move_str(); + let size = out + .lines() + .nth(1) + .unwrap() + .split_whitespace() + .nth(1) + .unwrap(); + assert_unit_suffix(size, None, "-P with DF_BLOCK_SIZE=M"); +} + #[test] fn test_too_large_block_size() { fn run_command(size: &str) { From b2e2527f00865d511cd56875ba543cbf853c963b Mon Sep 17 00:00:00 2001 From: Socialpranker <273312799+Socialpranker@users.noreply.github.com> Date: Sat, 5 Sep 2026 21:17:06 +0200 Subject: [PATCH 2/2] ls: preserve suffix-only block size units Same rule as the previous commit, applied to ls: --block-size=K (and LS_BLOCK_SIZE/BLOCK_SIZE/BLOCKSIZE) echoes the unit next to the size, --block-size=1K does not. ls tracks the file-size and allocation suffixes separately, since BLOCKSIZE only affects the allocation column (-s and the total line) and must not leak into the -l size column. -k resets the allocation block size and its unit, leaving a file-size unit from the environment alone, matching GNU. --- src/uu/ls/src/config.rs | 116 ++++++++++++++++------ src/uu/ls/src/display.rs | 21 +++- tests/by-util/test_ls.rs | 208 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 314 insertions(+), 31 deletions(-) diff --git a/src/uu/ls/src/config.rs b/src/uu/ls/src/config.rs index d6a7fe83ced..9706a740ac9 100644 --- a/src/uu/ls/src/config.rs +++ b/src/uu/ls/src/config.rs @@ -124,42 +124,86 @@ const POSIXLY_CORRECT_BLOCK_SIZE: u64 = 512; const DEFAULT_BLOCK_SIZE: u64 = 1024; const DEFAULT_FILE_SIZE_BLOCK_SIZE: u64 = 1; -/// Resolve `(file_size_block_size, block_size)` from environment variables. -/// -/// `LS_BLOCK_SIZE` and `BLOCK_SIZE` affect both values. -/// `BLOCKSIZE` only affects `block_size` (allocation display with `-s`). -/// `POSIXLY_CORRECT` sets `block_size` to 512 as a last resort. -/// `-k` (`opt_kb`) forces `block_size` to `DEFAULT_BLOCK_SIZE`. -fn resolve_block_sizes_from_env(opt_kb: bool) -> (u64, u64) { - match parse_block_size::block_size_from_env(&["LS_BLOCK_SIZE", "BLOCK_SIZE"]) { - parse_block_size::BlockSizeEnv::Found(size) => { +/// The block sizes `ls` divides by, plus the units echoed next to them. +struct BlockSizes { + /// Divisor for file sizes (the size column of `-l`). + file_size: u64, + /// Divisor for allocated sizes (`-s`, and the `total` line). + alloc: u64, + /// Suffix echoed next to a file size, if any. + file_size_suffix: Option, + /// Suffix echoed next to an allocated size, if any. + alloc_suffix: Option, +} + +impl BlockSizes { + /// Both columns share one spec, as with `--block-size=SIZE`. + fn uniform(size: u64, suffix: Option) -> Self { + Self { + file_size: size, + alloc: size, + file_size_suffix: suffix.clone(), + alloc_suffix: suffix, + } + } + + /// Plain sizes with no suffix to echo. + fn plain(file_size: u64, alloc: u64) -> Self { + Self { + file_size, + alloc, + file_size_suffix: None, + alloc_suffix: None, + } + } +} + +/// Resolve the block sizes from the environment, in GNU's precedence order: +/// `LS_BLOCK_SIZE`/`BLOCK_SIZE` set both, `BLOCKSIZE` only the allocation +/// one, `POSIXLY_CORRECT` 512 as a last resort. `-k` (`opt_kb`) resets the +/// allocation block size and its unit, leaving the file-size ones alone. +fn resolve_block_sizes_from_env(opt_kb: bool) -> BlockSizes { + match parse_block_size::block_size_from_env_with_suffix(&["LS_BLOCK_SIZE", "BLOCK_SIZE"]) { + (parse_block_size::BlockSizeEnv::Found(size), suffix) => { if opt_kb { - (size, DEFAULT_BLOCK_SIZE) + BlockSizes { + file_size: size, + alloc: DEFAULT_BLOCK_SIZE, + file_size_suffix: suffix, + alloc_suffix: None, + } } else { - (size, size) + BlockSizes::uniform(size, suffix) } } - parse_block_size::BlockSizeEnv::SetButInvalid => (DEFAULT_BLOCK_SIZE, DEFAULT_BLOCK_SIZE), - parse_block_size::BlockSizeEnv::NotSet => { + (parse_block_size::BlockSizeEnv::SetButInvalid, _) => { + BlockSizes::plain(DEFAULT_BLOCK_SIZE, DEFAULT_BLOCK_SIZE) + } + (parse_block_size::BlockSizeEnv::NotSet, _) => { // Neither LS_BLOCK_SIZE nor BLOCK_SIZE was set; check BLOCKSIZE // which only affects allocation display, not file size. - match parse_block_size::block_size_from_env(&["BLOCKSIZE"]) { - parse_block_size::BlockSizeEnv::Found(size) => { + match parse_block_size::block_size_from_env_with_suffix(&["BLOCKSIZE"]) { + (parse_block_size::BlockSizeEnv::Found(size), suffix) => { if opt_kb { - (DEFAULT_FILE_SIZE_BLOCK_SIZE, DEFAULT_BLOCK_SIZE) + BlockSizes::plain(DEFAULT_FILE_SIZE_BLOCK_SIZE, DEFAULT_BLOCK_SIZE) } else { - (DEFAULT_FILE_SIZE_BLOCK_SIZE, size) + BlockSizes { + file_size: DEFAULT_FILE_SIZE_BLOCK_SIZE, + alloc: size, + file_size_suffix: None, + alloc_suffix: suffix, + } } } - parse_block_size::BlockSizeEnv::SetButInvalid => { + (parse_block_size::BlockSizeEnv::SetButInvalid, _) => { // BLOCKSIZE was set but invalid: stop lookup, use defaults. - (DEFAULT_FILE_SIZE_BLOCK_SIZE, DEFAULT_BLOCK_SIZE) + BlockSizes::plain(DEFAULT_FILE_SIZE_BLOCK_SIZE, DEFAULT_BLOCK_SIZE) } - parse_block_size::BlockSizeEnv::NotSet => { + (parse_block_size::BlockSizeEnv::NotSet, _) => { if std::env::var_os("POSIXLY_CORRECT").is_some() && !opt_kb { - (DEFAULT_FILE_SIZE_BLOCK_SIZE, POSIXLY_CORRECT_BLOCK_SIZE) + BlockSizes::plain(DEFAULT_FILE_SIZE_BLOCK_SIZE, POSIXLY_CORRECT_BLOCK_SIZE) } else { - (DEFAULT_FILE_SIZE_BLOCK_SIZE, DEFAULT_BLOCK_SIZE) + BlockSizes::plain(DEFAULT_FILE_SIZE_BLOCK_SIZE, DEFAULT_BLOCK_SIZE) } } } @@ -212,6 +256,13 @@ pub struct Config { pub(crate) file_size_block_size: u64, #[allow(dead_code)] pub(crate) block_size: u64, // is never read on Windows + /// Display suffix (e.g. `"K"`) echoed back next to a file size, set when + /// `--block-size`/`LS_BLOCK_SIZE`/`BLOCK_SIZE` held a suffix-only spec + /// rather than a numeric one. + pub(crate) file_size_block_size_suffix: Option, + /// Display suffix echoed back next to an allocated size (`-s`, `total`). + /// `BLOCKSIZE` feeds this one only; `-k` clears it. + pub(crate) block_size_suffix: Option, pub(crate) width: u16, // Dir and vdir needs access to this field pub quoting_style: QuotingStyle, @@ -754,13 +805,13 @@ impl Config { SizeFormat::Bytes }; - let (file_size_block_size, block_size) = if let Some(opt_block_size) = opt_block_size { + let block_sizes = if let Some(opt_block_size) = opt_block_size { // --block-size command-line argument: parse it, error on invalid // If --block-size=si or --block-size=human-readable, skip numeric parsing if opt_si { - (DEFAULT_FILE_SIZE_BLOCK_SIZE, 1000) + BlockSizes::plain(DEFAULT_FILE_SIZE_BLOCK_SIZE, 1000) } else if opt_hr { - (DEFAULT_FILE_SIZE_BLOCK_SIZE, DEFAULT_BLOCK_SIZE) + BlockSizes::plain(DEFAULT_FILE_SIZE_BLOCK_SIZE, DEFAULT_BLOCK_SIZE) } else { let size = parse_size_non_zero_u64(opt_block_size).map_err(|error| { let ls_error = LsError::BlockSizeParseError(opt_block_size.clone()); @@ -778,15 +829,22 @@ impl Config { ) })?; // --block-size overrides -k - (size, size) + let suffix = parse_block_size::suffix_from_parsed_block_size(opt_block_size); + BlockSizes::uniform(size, suffix) } } else if !opt_si && !opt_hr { resolve_block_sizes_from_env(opt_kb) } else if opt_si { - (DEFAULT_FILE_SIZE_BLOCK_SIZE, 1000) + BlockSizes::plain(DEFAULT_FILE_SIZE_BLOCK_SIZE, 1000) } else { - (DEFAULT_FILE_SIZE_BLOCK_SIZE, DEFAULT_BLOCK_SIZE) + BlockSizes::plain(DEFAULT_FILE_SIZE_BLOCK_SIZE, DEFAULT_BLOCK_SIZE) }; + let BlockSizes { + file_size: file_size_block_size, + alloc: block_size, + file_size_suffix: file_size_block_size_suffix, + alloc_suffix: block_size_suffix, + } = block_sizes; let long = { let author = options.get_flag(options::AUTHOR); @@ -1004,6 +1062,8 @@ impl Config { alloc_size: options.get_flag(options::size::ALLOCATION_SIZE), file_size_block_size, block_size, + file_size_block_size_suffix, + block_size_suffix, width, quoting_style, locale_quoting, diff --git a/src/uu/ls/src/display.rs b/src/uu/ls/src/display.rs index 7f1b4cf473c..591434d4106 100644 --- a/src/uu/ls/src/display.rs +++ b/src/uu/ls/src/display.rs @@ -39,7 +39,7 @@ use uucore::fsxattr::has_acl; use uucore::libc::{dev_t, major, minor}; use uucore::{ error::UResult, - format::human::human_readable, + format::human::{SizeFormat, human_readable}, fs::display_permissions, fsext::metadata_get_time, i18n::{UEncoding, get_ctype_encoding}, @@ -722,11 +722,26 @@ fn display_len_or_rdev(metadata: &Metadata, config: &Config) -> SizeOrDeviceId { let r = metadata.len() % config.file_size_block_size; if r == 0 { d } else { d + 1 } }; - SizeOrDeviceId::Size(display_size(len_adjusted, config)) + SizeOrDeviceId::Size(display_file_size(len_adjusted, config)) } +/// Render an allocated size (`-s`, and the `total` line). pub fn display_size(size: u64, config: &Config) -> String { - human_readable(size, config.size_format) + scaled_size(size, config, config.block_size_suffix.as_deref()) +} + +/// Render a file size (the size column of `-l`). +fn display_file_size(size: u64, config: &Config) -> String { + scaled_size(size, config, config.file_size_block_size_suffix.as_deref()) +} + +fn scaled_size(size: u64, config: &Config, suffix: Option<&str>) -> String { + let s = human_readable(size, config.size_format); + // -h/--si scale dynamically and print their own units. + match (&config.size_format, suffix) { + (SizeFormat::Bytes, Some(suffix)) => format!("{s}{suffix}"), + _ => s, + } } /// Takes a [`PathData`] struct and returns a cell with a name ready for displaying. diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 594e8e9a1e6..7414b000faa 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -6238,6 +6238,214 @@ fn test_ls_block_size_si_file_size() { .stdout_contains(" 2 "); } +#[test] +#[cfg(not(target_os = "openbsd"))] +fn test_ls_suffix_only_block_size() { + // --block-size=K echoes the unit ("1K"); =1K and =1024 do not ("1"). + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.write_bytes("file", &[0u8; 1024]); + + for (arg, expected) in [ + ("--block-size=K", "1K"), + ("--block-size=k", "1K"), + ("--block-size=KB", "2kB"), + ("--block-size=KiB", "1KiB"), + ("--block-size=M", "1M"), + ("--block-size=1K", "1"), + ("--block-size=1024", "1"), + ("--block-size=2K", "1"), + ("--block-size=1MB", "1"), + ] { + let out = scene + .ucmd() + .args(&["-l", arg, "file"]) + .succeeds() + .stdout_move_str(); + assert_eq!(long_size_column(&out), expected, "for {arg}"); + } +} + +#[test] +#[cfg(not(target_os = "openbsd"))] +fn test_ls_suffix_only_block_size_allocation() { + // The same holds for the allocation column of -s and for the total line. + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.write_bytes("file", &[0u8; 1024]); + + for (arg, suffix) in [ + ("--block-size=K", Some("K")), + ("--block-size=MiB", Some("MiB")), + ("--block-size=1K", None), + ("--block-size=1024", None), + ] { + let out = scene + .ucmd() + .args(&["-s", "-1", arg]) + .succeeds() + .stdout_move_str(); + let mut lines = out.lines(); + let total = lines.next().unwrap().split_whitespace().nth(1).unwrap(); + let alloc = lines.next().unwrap().split_whitespace().next().unwrap(); + for value in [total, alloc] { + assert_unit_suffix(value, suffix, arg); + } + } +} + +#[test] +#[cfg(not(target_os = "openbsd"))] +fn test_ls_suffix_only_env_block_size() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.write_bytes("file", &[0u8; 1024]); + + for (var, value, expected) in [ + ("LS_BLOCK_SIZE", "K", "1K"), + ("LS_BLOCK_SIZE", "M", "1M"), + ("BLOCK_SIZE", "M", "1M"), + ("LS_BLOCK_SIZE", "1K", "1"), + ("BLOCK_SIZE", "1024", "1"), + ] { + let out = scene + .ucmd() + .env(var, value) + .args(&["-l", "file"]) + .succeeds() + .stdout_move_str(); + assert_eq!(long_size_column(&out), expected, "for {var}={value}"); + } +} + +#[test] +#[cfg(not(target_os = "openbsd"))] +fn test_ls_suffix_only_blocksize_env_does_not_reach_file_size() { + // BLOCKSIZE drives only the allocation column, not the file size. + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.write_bytes("file", &[0u8; 1024]); + + let out = scene + .ucmd() + .env("BLOCKSIZE", "K") + .args(&["-l", "-s", "-1"]) + .succeeds() + .stdout_move_str(); + let line = out.lines().nth(1).unwrap(); + let mut fields = line.split_whitespace(); + let alloc = fields.next().unwrap(); + let size = fields.nth(4).unwrap(); + assert_unit_suffix(alloc, Some("K"), "BLOCKSIZE=K"); + assert_eq!( + size, "1024", + "BLOCKSIZE must not scale or annotate file sizes" + ); +} + +#[test] +#[cfg(not(target_os = "openbsd"))] +fn test_ls_suffix_only_block_size_with_kibibyte_flag() { + // -k resets the allocation block size and unit, not the file-size ones. + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.write_bytes("file", &[0u8; 1024]); + + let out = scene + .ucmd() + .env("LS_BLOCK_SIZE", "K") + .args(&["-l", "-s", "-1", "-k"]) + .succeeds() + .stdout_move_str(); + let line = out.lines().nth(1).unwrap(); + let mut fields = line.split_whitespace(); + let alloc = fields.next().unwrap(); + let size = fields.nth(4).unwrap(); + assert_unit_suffix(alloc, None, "-k with LS_BLOCK_SIZE=K"); + assert_eq!(size, "1K"); + + // -k alone strips the unit BLOCKSIZE would otherwise have contributed. + let out = scene + .ucmd() + .env("BLOCKSIZE", "K") + .args(&["-s", "-1", "-k"]) + .succeeds() + .stdout_move_str(); + let alloc = out + .lines() + .nth(1) + .unwrap() + .split_whitespace() + .next() + .unwrap(); + assert_unit_suffix(alloc, None, "-k with BLOCKSIZE=K"); +} + +#[test] +#[cfg(not(target_os = "openbsd"))] +fn test_ls_suffix_only_block_size_not_used_with_human_readable() { + // -h/--si print their own units, unaffected by the environment. + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.write_bytes("file", &[0u8; 1024]); + + for flag in ["-h", "--si"] { + let plain = scene + .ucmd() + .args(&["-l", flag, "file"]) + .succeeds() + .stdout_move_str(); + let with_env = scene + .ucmd() + .env("LS_BLOCK_SIZE", "K") + .args(&["-l", flag, "file"]) + .succeeds() + .stdout_move_str(); + assert_eq!( + long_size_column(&with_env), + long_size_column(&plain), + "LS_BLOCK_SIZE must not change what {flag} prints" + ); + } +} + +/// The size column of a single-entry `ls -l` listing. +#[cfg(not(target_os = "openbsd"))] +fn long_size_column(stdout: &str) -> String { + stdout + .lines() + .find(|line| line.starts_with('-')) + .unwrap_or_else(|| panic!("no file entry in {stdout:?}")) + .split_whitespace() + .nth(4) + .unwrap() + .to_string() +} + +/// Assert that `value` is digits followed by exactly `suffix` (or by nothing, +/// when `suffix` is `None`). +#[cfg(not(target_os = "openbsd"))] +fn assert_unit_suffix(value: &str, suffix: Option<&str>, context: &str) { + let digits = match suffix { + Some(suffix) => { + let stripped = value.strip_suffix(suffix).unwrap_or_else(|| { + panic!("expected {value:?} to end in {suffix:?} for {context}"); + }); + assert!( + !stripped.ends_with(|c: char| c.is_ascii_alphabetic()), + "expected exactly the unit {suffix:?} in {value:?} for {context}" + ); + stripped + } + None => value, + }; + assert!( + !digits.is_empty() && digits.chars().all(|c| c.is_ascii_digit()), + "expected a plain number{} in {value:?} for {context}", + suffix.map_or(String::new(), |s| format!(" before {s:?}")) + ); +} + #[test] fn test_ls_block_size_override_self() { new_ucmd!()