RGBCube/uutils-coreutils
1
Fork 0
mirror of https://github.com/RGBCube/uutils-coreutils synced 2025-07-28 11:37:44 +00:00
Code Issues Activity

Merge pull request #5693 from cakebaker/ls_second_blocksize

Browse source 
ls: introduce 2nd blocksize & fix todos in tests
This commit is contained in:
Sylvestre Ledru 2023-12-23 09:57:08 +01:00 • committed by GitHub
commit 6b1f51385f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 74 additions and 43 deletions
Download patch file Download diff file Expand all files Collapse all files

70
src/uu/ls/src/ls.rs
View file

@ -161,8 +161,8 @@ pub mod options {
const DEFAULT_TERM_WIDTH: u16 = 80; const DEFAULT_TERM_WIDTH: u16 = 80;
const POSIXLY_CORRECT_BLOCK_SIZE: u64 = 512; const POSIXLY_CORRECT_BLOCK_SIZE: u64 = 512;
#[cfg(unix)]
const DEFAULT_BLOCK_SIZE: u64 = 1024; const DEFAULT_BLOCK_SIZE: u64 = 1024;
const DEFAULT_FILE_SIZE_BLOCK_SIZE: u64 = 1;
#[derive(Debug)] #[derive(Debug)]
enum LsError { enum LsError {
@ -409,7 +409,9 @@ pub struct Config {
color: Option<LsColors>, color: Option<LsColors>,
long: LongFormat, long: LongFormat,
alloc_size: bool, alloc_size: bool,
block_size: Option<u64>, file_size_block_size: u64,
#[allow(dead_code)]
block_size: u64, // is never read on Windows
width: u16, width: u16,
// Dir and vdir needs access to this field // Dir and vdir needs access to this field
pub quoting_style: QuotingStyle, pub quoting_style: QuotingStyle,
@ -822,6 +824,7 @@ impl Config {
let env_var_block_size = std::env::var_os("BLOCK_SIZE"); let env_var_block_size = std::env::var_os("BLOCK_SIZE");
let env_var_ls_block_size = std::env::var_os("LS_BLOCK_SIZE"); let env_var_ls_block_size = std::env::var_os("LS_BLOCK_SIZE");
let env_var_posixly_correct = std::env::var_os("POSIXLY_CORRECT"); let env_var_posixly_correct = std::env::var_os("POSIXLY_CORRECT");
let mut is_env_var_blocksize = false;
let raw_block_size = if let Some(opt_block_size) = opt_block_size { let raw_block_size = if let Some(opt_block_size) = opt_block_size {
OsString::from(opt_block_size) OsString::from(opt_block_size)
@ -831,6 +834,7 @@ impl Config {
} else if let Some(env_var_block_size) = env_var_block_size { } else if let Some(env_var_block_size) = env_var_block_size {
env_var_block_size env_var_block_size
} else if let Some(env_var_blocksize) = env_var_blocksize { } else if let Some(env_var_blocksize) = env_var_blocksize {
is_env_var_blocksize = true;
env_var_blocksize env_var_blocksize
} else { } else {
OsString::from("") OsString::from("")
@ -839,9 +843,16 @@ impl Config {
OsString::from("") OsString::from("")
}; };
let block_size: Option<u64> = if !opt_si && !opt_hr && !raw_block_size.is_empty() { let (file_size_block_size, block_size) = if !opt_si && !opt_hr && !raw_block_size.is_empty()
{
match parse_size_u64(&raw_block_size.to_string_lossy()) { match parse_size_u64(&raw_block_size.to_string_lossy()) {
Ok(size) => Some(size), Ok(size) => {
if is_env_var_blocksize {
(DEFAULT_FILE_SIZE_BLOCK_SIZE, size)
} else {
(size, size)
}
}
Err(_) => { Err(_) => {
// only fail if invalid block size was specified with --block-size, // only fail if invalid block size was specified with --block-size,
// ignore invalid block size from env vars // ignore invalid block size from env vars
@ -850,13 +861,19 @@ impl Config {
invalid_block_size.clone(), invalid_block_size.clone(),
))); )));
} }
None if is_env_var_blocksize {
(DEFAULT_FILE_SIZE_BLOCK_SIZE, DEFAULT_BLOCK_SIZE)
} else {
(DEFAULT_BLOCK_SIZE, DEFAULT_BLOCK_SIZE)
}
} }
} }
} else if env_var_posixly_correct.is_some() { } else if env_var_posixly_correct.is_some() {
Some(POSIXLY_CORRECT_BLOCK_SIZE) (DEFAULT_FILE_SIZE_BLOCK_SIZE, POSIXLY_CORRECT_BLOCK_SIZE)
} else if opt_si {
(DEFAULT_FILE_SIZE_BLOCK_SIZE, 1000)
} else { } else {
None (DEFAULT_FILE_SIZE_BLOCK_SIZE, DEFAULT_BLOCK_SIZE)
}; };
let long = { let long = {
@ -1062,6 +1079,7 @@ impl Config {
inode: options.get_flag(options::INODE), inode: options.get_flag(options::INODE),
long, long,
alloc_size: options.get_flag(options::size::ALLOCATION_SIZE), alloc_size: options.get_flag(options::size::ALLOCATION_SIZE),
file_size_block_size,
block_size, block_size,
width, width,
quoting_style, quoting_style,
@ -2479,13 +2497,7 @@ fn get_block_size(md: &Metadata, config: &Config) -> u64 {
}; };
match config.size_format { match config.size_format {
SizeFormat::Binary | SizeFormat::Decimal => raw_blocks, SizeFormat::Binary | SizeFormat::Decimal => raw_blocks,
SizeFormat::Bytes => { SizeFormat::Bytes => raw_blocks / config.block_size,
if let Some(user_block_size) = config.block_size {
raw_blocks / user_block_size
} else {
raw_blocks / DEFAULT_BLOCK_SIZE
}
}
} }
} }
#[cfg(not(unix))] #[cfg(not(unix))]
@ -2955,26 +2967,16 @@ fn display_len_or_rdev(metadata: &Metadata, config: &Config) -> SizeOrDeviceId {
return SizeOrDeviceId::Device(major.to_string(), minor.to_string()); return SizeOrDeviceId::Device(major.to_string(), minor.to_string());
} }
} }
// Reported file len only adjusted for block_size when block_size is set let len_adjusted = {
if let Some(user_block_size) = config.block_size { let d = metadata.len() / config.file_size_block_size;
// ordinary division of unsigned integers rounds down, let r = metadata.len() % config.file_size_block_size;
// this is similar to the Rust API for division that rounds up, if r == 0 {
// currently in nightly only, however once d
// https://github.com/rust-lang/rust/pull/88582 : "div_ceil" } else {
// is stable we should use that instead d + 1
let len_adjusted = { }
let d = metadata.len() / user_block_size; };
let r = metadata.len() % user_block_size; SizeOrDeviceId::Size(display_size(len_adjusted, config))
if r == 0 {
d
} else {
d + 1
}
};
SizeOrDeviceId::Size(display_size(len_adjusted, config))
} else {
SizeOrDeviceId::Size(display_size(metadata.len(), config))
}
} }
fn display_size(size: u64, config: &Config) -> String { fn display_size(size: u64, config: &Config) -> String {

47
tests/by-util/test_ls.rs
View file

@ -3862,8 +3862,8 @@ fn test_posixly_correct_and_block_size_env_vars() {
.arg("-l") .arg("-l")
.env("POSIXLY_CORRECT", "some_value") .env("POSIXLY_CORRECT", "some_value")
.succeeds() .succeeds()
.stdout_contains_line("total 8"); .stdout_contains_line("total 8")
//.stdout_contains(" 1024 "); // TODO needs second internal blocksize .stdout_contains(" 1024 ");
scene scene
.ucmd() .ucmd()
@ -3886,8 +3886,8 @@ fn test_posixly_correct_and_block_size_env_vars() {
.arg("-l") .arg("-l")
.env("BLOCKSIZE", "512") .env("BLOCKSIZE", "512")
.succeeds() .succeeds()
.stdout_contains_line("total 8"); .stdout_contains_line("total 8")
//.stdout_contains(" 1024 "); // TODO needs second internal blocksize .stdout_contains(" 1024 ");
} }
#[test] #[test]
@ -3900,13 +3900,42 @@ fn test_ls_invalid_block_size() {
.stderr_is("ls: invalid --block-size argument 'invalid'\n"); .stderr_is("ls: invalid --block-size argument 'invalid'\n");
} }
// TODO ensure the correct block size is used when using -l because #[cfg(all(unix, feature = "dd"))]
// the output of "ls -l" and "BLOCK_SIZE=invalid ls -l" is different
#[test] #[test]
fn test_ls_invalid_block_size_in_env_var() { fn test_ls_invalid_block_size_in_env_var() {
new_ucmd!().env("LS_BLOCK_SIZE", "invalid").succeeds(); let scene = TestScenario::new(util_name!());
new_ucmd!().env("BLOCK_SIZE", "invalid").succeeds();
new_ucmd!().env("BLOCKSIZE", "invalid").succeeds(); scene
.ccmd("dd")
.arg("if=/dev/zero")
.arg("of=file")
.arg("bs=1024")
.arg("count=1")
.succeeds();
scene
.ucmd()
.arg("-og")
.env("LS_BLOCK_SIZE", "invalid")
.succeeds()
.stdout_contains_line("total 4")
.stdout_contains(" 1 1 "); // hardlink count + file size
scene
.ucmd()
.arg("-og")
.env("BLOCK_SIZE", "invalid")
.succeeds()
.stdout_contains_line("total 4")
.stdout_contains(" 1 1 "); // hardlink count + file size
scene
.ucmd()
.arg("-og")
.env("BLOCKSIZE", "invalid")
.succeeds()
.stdout_contains_line("total 4")
.stdout_contains(" 1024 ");
} }
#[cfg(all(unix, feature = "dd"))] #[cfg(all(unix, feature = "dd"))]