ls: Fix display of inodes and add allocation size feature

[kimono-koans]

Only reason the GNU test block size (block-size.sh) appears to be failing is because of issues related to dd not parsing 'conv' correctly (!).

• Add block size display ("-s", "--size") feature
• Add custom block size option ("--block-size")
• Add kibibytes ("-k", "--kibibytes") feature
• Correct ordering of preferences for block size options
• Add env var parsing for BLOCK_SIZE, LS_BLOCK_SIZE, POSIXLY_CORRECT
• Fix how both block size and inode are displayed. Instead of printing through either display_long_item or display_file_name have been moved into a display_more_info function, which could be useful to other "ls" features that print the same whether in Format::Long or not.
• Tests included to check proper alignment and block size values, and ordering of option preferences
• Seems like a lot but these are all related options.

[hbina]

I think there's some miscalculation somewhere, trying to figure out where,

output is sorted because we currently don't sort by default.

hbina@akarin ~/g/uutils (ls_2)> ls -l --block-size=512 | sort
drwxrwxr-x 2 hbina hbina   8 Dec 24 22:33 examples
drwxrwxr-x 2 hbina hbina   8 Feb  2 21:32 dir2
drwxrwxr-x 2 hbina hbina   8 Feb  2 23:18 dir1
drwxrwxr-x 2 hbina hbina   8 Jan 31 23:43 util
drwxrwxr-x 4 hbina hbina   8 Jan 31 23:20 target
drwxrwxr-x 4 hbina hbina   8 Jan 31 23:43 docs
drwxrwxr-x 6 hbina hbina   8 Dec 24 22:33 src
drwxrwxr-x 6 hbina hbina   8 Jan 31 23:43 tests
-rw-rw-r-- 1 hbina hbina  11 Dec 24 22:33 CODE_OF_CONDUCT.md <=== does not match
-rw-rw-r-- 1 hbina hbina  12 Jan 31 23:43 build.rs <=== does not match
-rw-rw-r-- 1 hbina hbina 131 Feb  2 21:31 Cargo.lock
-rw-rw-r-- 1 hbina hbina  14 Jan 31 23:43 GNUmakefile
-rw-rw-r-- 1 hbina hbina   1 Dec 24 22:33 Makefile
-rw-rw-r-- 1 hbina hbina  21 Dec 24 22:33 Makefile.toml
-rw-rw-r-- 1 hbina hbina  33 Feb  3 21:33 Cargo.toml
-rw-rw-r-- 1 hbina hbina  37 Feb  2 21:31 README.md
-rw-rw-r-- 1 hbina hbina   3 Dec 24 22:33 LICENSE
-rw-rw-r-- 1 hbina hbina   7 Jan 31 23:43 DEVELOPER_INSTRUCTIONS.md
-rw-rw-r-- 1 hbina hbina   8 Jan 31 23:43 CONTRIBUTING.md
total 384
hbina@akarin ~/g/uutils (ls_2)> cargo run --quiet -- ls -l --block-size=512 | sort
drwxrwxr-x 2 hbina hbina    8 Dec 24 22:33 examples
drwxrwxr-x 2 hbina hbina    8 Feb  2 21:32 dir2
drwxrwxr-x 2 hbina hbina    8 Feb  2 23:18 dir1
drwxrwxr-x 2 hbina hbina    8 Jan 31 23:43 util
drwxrwxr-x 4 hbina hbina    8 Jan 31 23:20 target
drwxrwxr-x 4 hbina hbina    8 Jan 31 23:43 docs
drwxrwxr-x 6 hbina hbina    8 Dec 24 22:33 src
drwxrwxr-x 6 hbina hbina    8 Jan 31 23:43 tests
-rw-rw-r-- 1 hbina hbina  136 Feb  2 21:31 Cargo.lock
-rw-rw-r-- 1 hbina hbina   16 Dec 24 22:33 CODE_OF_CONDUCT.md <=== does not match
-rw-rw-r-- 1 hbina hbina   16 Jan 31 23:43 build.rs <=== does not match
-rw-rw-r-- 1 hbina hbina   16 Jan 31 23:43 GNUmakefile
-rw-rw-r-- 1 hbina hbina   24 Dec 24 22:33 Makefile.toml
-rw-rw-r-- 1 hbina hbina   40 Feb  2 21:31 README.md
-rw-rw-r-- 1 hbina hbina   40 Feb  3 21:33 Cargo.toml
-rw-rw-r-- 1 hbina hbina    8 Dec 24 22:33 LICENSE
-rw-rw-r-- 1 hbina hbina    8 Dec 24 22:33 Makefile
-rw-rw-r-- 1 hbina hbina    8 Jan 31 23:43 CONTRIBUTING.md
-rw-rw-r-- 1 hbina hbina    8 Jan 31 23:43 DEVELOPER_INSTRUCTIONS.md
total 384

Edit: Fixed this with this change. Basically the size shown is not the number of blocks allocated, but the minimum number of blocks required for the file size.

diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs
index 48486865..967eecbe 100644
--- a/src/uu/ls/src/ls.rs
+++ b/src/uu/ls/src/ls.rs
@@ -1805,10 +1805,17 @@ fn pad_right(string: &str, count: usize) -> String {
 }
 
 fn customize_block_size(size: u64, config: &Config) -> u64 {
-    if config.block_size.is_some() {
-        size / config.block_size.unwrap() as u64
+    let block_size = config.block_size.unwrap_or(DEFAULT_BLOCK_SIZE) as u64;
+    size / block_size
+}
+
+fn customize_file_size(size: u64, config: &Config) -> u64 {
+    let block_size = config.block_size.unwrap_or(DEFAULT_BLOCK_SIZE) as u64;
+    let lower_bound = size / block_size;
+    if lower_bound * block_size < size {
+        lower_bound + 1
     } else {
-        size / DEFAULT_BLOCK_SIZE as u64
+        lower_bound
     }
 }
 
@@ -1930,7 +1937,7 @@ fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter<Stdout
             mut longest_size_len,
             mut longest_major_len,
             mut longest_minor_len,
-        ) = (1, 1, 1, 1, 1, 1, 1);
+        ) = (1, 1, 1, 1, 1, 0, 0);
 
         #[cfg(not(unix))]
         let (
@@ -2079,6 +2086,22 @@ fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter<Stdout
     }
 }
 
+fn get_file_size(md: &Metadata) -> u64 {
+    /* GNU ls will display sizes in terms of block size
+       md.len() will differ from this value when the file has some holes
+    */
+    #[cfg(unix)]
+    {
+        md.size()
+    }
+
+    #[cfg(not(unix))]
+    {
+        // no way to get block size for windows, fall-back to file size
+        md.len()
+    }
+}
+
 fn get_block_size(md: &Metadata) -> u64 {
     /* GNU ls will display sizes in terms of block size
        md.len() will differ from this value when the file has some holes
@@ -2515,7 +2538,7 @@ fn display_size_or_rdev(metadata: &Metadata, config: &Config) -> SizeOrDeviceId
     }
     if config.block_size.is_some() {
         SizeOrDeviceId::Size(display_size(
-            customize_block_size(get_block_size(metadata), config),
+            customize_file_size(get_file_size(metadata), config),
             config,
         ))
     } else {

[kimono-koans]

I think there's some miscalculation somewhere, trying to figure out where,

Okay, this is interesting, but I think it may be outside the scope of this PR, as it addresses the non-block size given in the LongFormat option. I've noticed similar behavior too.

Anyway, I think this might be better placed in a separate PR with some tests attached, but appreciate your work. Thank you.

[kimono-koans]

I think there's some miscalculation somewhere, trying to figure out where,

If you prepare a PR, let me know. I'd be pleased to review. One suggestion -- I would generic-ize the operation of rounding up to make clear what you are doing/not doing, with something like: https://doc.rust-lang.org/std/primitive.u64.html#method.unstable_div_ceil

[hbina]

I'll create the PR after yours are merged because its based on that.

[kimono-koans]

@sylvestre @tertsdiepraam Don't mean to rush a review but CICD won't tell me which test is failing. Perhaps I'm missing something. I see:

Warning: Congrats! The gnu test tests/cp/copy-FMR is now passing!
Warning: Congrats! The gnu test tests/misc/shuf-reservoir is now passing!
Warning: Congrats! The gnu test tests/misc/sort-stale-thread-mem is now passing!
Warning: Congrats! The gnu test tests/misc/tty-eof is now passing!

But no indication which test is newly failing per:

Warning: Changes from main: PASS -1 / FAIL -4 / ERROR +0 / SKIP +5

Any assistance you could provide would be appreciated. Thanks!

[kimono-koans]

@sylvestre @tertsdiepraam I'd like to request a review, when it is convenient. Thank you.

[tertsdiepraam]

I think it's correct, but it needs some cleanup or documentation in a few places. Nice work!

[kimono-koans]

I think it's correct, but it needs some cleanup or documentation in a few places. Nice work!

I think this may be ready to merge but I think it also needs your all clear, when it is convenient. Thank you for your review. It really needed some more TLC and your comments very much helped.

[kimono-koans]

@hbina FYI I added your suggestion with a test. Thanks!

[tertsdiepraam]

Thank you! It looks great now! I found one last thing to change and then it can get merged.