From 753f8b0e907ad8cc8469bf7833746965a94d9fe1 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Tue, 23 Sep 2025 19:26:09 -0500 Subject: [PATCH 01/76] Initial commit --- src/uu/ls/src/ls.rs | 63 ++++++++++++++++++++++++++++----------------- 1 file changed, 39 insertions(+), 24 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index ba7f26f1aad..c317f299a9a 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2063,7 +2063,7 @@ pub fn list(locs: Vec<&Path>, config: &Config) -> UResult<()> { &path_data.p_buf, path_data.must_dereference, )?); - enter_directory( + recursive_loop( path_data, read_dir, config, @@ -2183,15 +2183,13 @@ fn should_display(entry: &DirEntry, config: &Config) -> bool { .any(|p| p.matches_with(&file_name, options)) } -#[allow(clippy::cognitive_complexity)] fn enter_directory( path_data: &PathData, read_dir: ReadDir, config: &Config, state: &mut ListState, - listed_ancestors: &mut HashSet, dired: &mut DiredOutput, -) -> UResult<()> { +) -> UResult> { // Create vec of entries with initial dot files let mut entries: Vec = if config.files == Files::All { vec![ @@ -2245,28 +2243,42 @@ fn enter_directory( display_items(&entries, config, state, dired)?; + Ok(entries) +} + +fn recursive_loop( + path_data: &PathData, + read_dir: ReadDir, + config: &Config, + state: &mut ListState, + listed_ancestors: &mut HashSet, + dired: &mut DiredOutput, +) -> UResult<()> { + let mut queue: Vec = enter_directory(path_data, read_dir, config, state, dired)? + .into_iter() + .skip(if config.files == Files::All { 2 } else { 0 }) + .filter(|p| { + p.ft.get() + .is_some_and(|o_ft| o_ft.is_some_and(|ft| ft.is_dir())) + }) + .collect(); + if config.recursive { - for e in entries - .iter() - .skip(if config.files == Files::All { 2 } else { 0 }) - .filter(|p| { - p.ft.get() - .is_some_and(|o_ft| o_ft.is_some_and(|ft| ft.is_dir())) - }) - { - match fs::read_dir(&e.p_buf) { + while let Some(item) = queue.pop() { + match fs::read_dir(&item.p_buf) { Err(err) => { state.out.flush()?; show!(LsError::IOErrorContext( - e.p_buf.clone(), + item.p_buf.clone(), err, - e.command_line + item.command_line )); } Ok(rd) => { - if listed_ancestors - .insert(FileInformation::from_path(&e.p_buf, e.must_dereference)?) - { + if listed_ancestors.insert(FileInformation::from_path( + &item.p_buf, + item.must_dereference, + )?) { // when listing several directories in recursive mode, we show // "dirname:" at the beginning of the file list writeln!(state.out)?; @@ -2276,20 +2288,23 @@ fn enter_directory( // 2 = \n + \n dired.padding = 2; dired::indent(&mut state.out)?; - let dir_name_size = e.p_buf.to_string_lossy().len(); + let dir_name_size = item.p_buf.to_string_lossy().len(); dired::calculate_subdired(dired, dir_name_size); // inject dir name dired::add_dir_name(dired, dir_name_size); } - show_dir_name(e, &mut state.out, config)?; + show_dir_name(&item, &mut state.out, config)?; writeln!(state.out)?; - enter_directory(e, rd, config, state, listed_ancestors, dired)?; - listed_ancestors - .remove(&FileInformation::from_path(&e.p_buf, e.must_dereference)?); + let mut res = enter_directory(&item, rd, config, state, dired)?; + queue.append(&mut res); + listed_ancestors.remove(&FileInformation::from_path( + &item.p_buf, + item.must_dereference, + )?); } else { state.out.flush()?; - show!(LsError::AlreadyListedError(e.p_buf.clone())); + show!(LsError::AlreadyListedError(item.p_buf.clone())); } } } From 3d3d9dee90349d57483152094e59e1e81baf1806 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Tue, 23 Sep 2025 19:28:30 -0500 Subject: [PATCH 02/76] Closer --- src/uu/ls/src/ls.rs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index c317f299a9a..57977798f0a 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2243,7 +2243,16 @@ fn enter_directory( display_items(&entries, config, state, dired)?; - Ok(entries) + let res = entries + .into_iter() + .skip(if config.files == Files::All { 2 } else { 0 }) + .filter(|p| { + p.ft.get() + .is_some_and(|o_ft| o_ft.is_some_and(|ft| ft.is_dir())) + }) + .collect(); + + Ok(res) } fn recursive_loop( @@ -2254,14 +2263,7 @@ fn recursive_loop( listed_ancestors: &mut HashSet, dired: &mut DiredOutput, ) -> UResult<()> { - let mut queue: Vec = enter_directory(path_data, read_dir, config, state, dired)? - .into_iter() - .skip(if config.files == Files::All { 2 } else { 0 }) - .filter(|p| { - p.ft.get() - .is_some_and(|o_ft| o_ft.is_some_and(|ft| ft.is_dir())) - }) - .collect(); + let mut queue: Vec = enter_directory(path_data, read_dir, config, state, dired)?; if config.recursive { while let Some(item) = queue.pop() { From 8393d5fcfc65e633c12d3f3bd180df6567a52168 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Tue, 23 Sep 2025 19:40:41 -0500 Subject: [PATCH 03/76] Get queue order correct --- src/uu/ls/src/ls.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 57977798f0a..71ccca3f258 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2250,6 +2250,7 @@ fn enter_directory( p.ft.get() .is_some_and(|o_ft| o_ft.is_some_and(|ft| ft.is_dir())) }) + .rev() .collect(); Ok(res) From b8bee00807b48a1cd25681d3fd4c194588eaf9a9 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Tue, 23 Sep 2025 20:18:40 -0500 Subject: [PATCH 04/76] Use one contains, instead of insert and remove --- src/uu/ls/src/ls.rs | 55 +++++++++++++++++++++------------------------ 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 71ccca3f258..7785dd606f3 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2277,38 +2277,35 @@ fn recursive_loop( item.command_line )); } - Ok(rd) => { - if listed_ancestors.insert(FileInformation::from_path( + Ok(_rd) + if listed_ancestors.contains(&FileInformation::from_path( &item.p_buf, item.must_dereference, - )?) { - // when listing several directories in recursive mode, we show - // "dirname:" at the beginning of the file list - writeln!(state.out)?; - if config.dired { - // We already injected the first dir - // Continue with the others - // 2 = \n + \n - dired.padding = 2; - dired::indent(&mut state.out)?; - let dir_name_size = item.p_buf.to_string_lossy().len(); - dired::calculate_subdired(dired, dir_name_size); - // inject dir name - dired::add_dir_name(dired, dir_name_size); - } - - show_dir_name(&item, &mut state.out, config)?; - writeln!(state.out)?; - let mut res = enter_directory(&item, rd, config, state, dired)?; - queue.append(&mut res); - listed_ancestors.remove(&FileInformation::from_path( - &item.p_buf, - item.must_dereference, - )?); - } else { - state.out.flush()?; - show!(LsError::AlreadyListedError(item.p_buf.clone())); + )?) => + { + state.out.flush()?; + show!(LsError::AlreadyListedError(item.p_buf.clone())); + } + Ok(rd) => { + // when listing several directories in recursive mode, we show + // "dirname:" at the beginning of the file list + writeln!(state.out)?; + if config.dired { + // We already injected the first dir + // Continue with the others + // 2 = \n + \n + dired.padding = 2; + dired::indent(&mut state.out)?; + let dir_name_size = item.p_buf.to_string_lossy().len(); + dired::calculate_subdired(dired, dir_name_size); + // inject dir name + dired::add_dir_name(dired, dir_name_size); } + + show_dir_name(&item, &mut state.out, config)?; + writeln!(state.out)?; + let mut res = enter_directory(&item, rd, config, state, dired)?; + queue.append(&mut res); } } } From fe259c41d91789723aa8dff7a0fd4555ff68ec32 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Tue, 23 Sep 2025 21:02:59 -0500 Subject: [PATCH 05/76] No need for HashSet --- src/uu/ls/src/ls.rs | 36 ++++++++++++------------------------ 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 7785dd606f3..13ceda8b47c 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -14,7 +14,6 @@ use std::os::windows::fs::MetadataExt; use std::{ cell::{LazyCell, OnceCell}, cmp::Reverse, - collections::HashSet, ffi::{OsStr, OsString}, fmt::Write as FmtWrite, fs::{self, DirEntry, FileType, Metadata, ReadDir}, @@ -2058,19 +2057,8 @@ pub fn list(locs: Vec<&Path>, config: &Config) -> UResult<()> { writeln!(state.out)?; } } - let mut listed_ancestors = HashSet::new(); - listed_ancestors.insert(FileInformation::from_path( - &path_data.p_buf, - path_data.must_dereference, - )?); - recursive_loop( - path_data, - read_dir, - config, - &mut state, - &mut listed_ancestors, - &mut dired, - )?; + + recursive_loop(path_data, read_dir, config, &mut state, &mut dired)?; } if config.dired && !config.hyperlink { dired::print_dired_output(config, &dired, &mut state.out)?; @@ -2261,11 +2249,12 @@ fn recursive_loop( read_dir: ReadDir, config: &Config, state: &mut ListState, - listed_ancestors: &mut HashSet, dired: &mut DiredOutput, ) -> UResult<()> { let mut queue: Vec = enter_directory(path_data, read_dir, config, state, dired)?; + let listed_ancestor = FileInformation::from_path(&path_data.p_buf, path_data.must_dereference)?; + if config.recursive { while let Some(item) = queue.pop() { match fs::read_dir(&item.p_buf) { @@ -2277,16 +2266,15 @@ fn recursive_loop( item.command_line )); } - Ok(_rd) - if listed_ancestors.contains(&FileInformation::from_path( - &item.p_buf, - item.must_dereference, - )?) => - { - state.out.flush()?; - show!(LsError::AlreadyListedError(item.p_buf.clone())); - } Ok(rd) => { + let item_info = FileInformation::from_path(&item.p_buf, item.must_dereference)?; + + if item_info == listed_ancestor { + state.out.flush()?; + show!(LsError::AlreadyListedError(item.p_buf.clone())); + continue; + } + // when listing several directories in recursive mode, we show // "dirname:" at the beginning of the file list writeln!(state.out)?; From 7c2c27b2ce12e9b88433ddc50c097ee88d244918 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Tue, 23 Sep 2025 21:10:37 -0500 Subject: [PATCH 06/76] Cleanup --- src/uu/ls/src/ls.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 13ceda8b47c..05e72a69494 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2234,10 +2234,7 @@ fn enter_directory( let res = entries .into_iter() .skip(if config.files == Files::All { 2 } else { 0 }) - .filter(|p| { - p.ft.get() - .is_some_and(|o_ft| o_ft.is_some_and(|ft| ft.is_dir())) - }) + .filter(|p| p.file_type(&mut state.out).is_some_and(|ft| ft.is_dir())) .rev() .collect(); @@ -2304,10 +2301,10 @@ fn recursive_loop( fn get_metadata_with_deref_opt(p_buf: &Path, dereference: bool) -> std::io::Result { if dereference { - p_buf.metadata() - } else { - p_buf.symlink_metadata() + return p_buf.metadata(); } + + p_buf.symlink_metadata() } fn display_dir_entry_size( From da4e85d4befad84f66ea6c26f86fff47b230433d Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Tue, 23 Sep 2025 22:51:42 -0500 Subject: [PATCH 07/76] Reduce syscalls via retained metadata --- src/uu/ls/src/ls.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 05e72a69494..4a9934cf5ab 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -59,7 +59,6 @@ use uucore::{ error::{UError, UResult, set_exit_code}, format::human::{SizeFormat, human_readable}, format_usage, - fs::FileInformation, fs::display_permissions, fsext::{MetadataTimeField, metadata_get_time}, line_ending::LineEnding, @@ -2250,7 +2249,10 @@ fn recursive_loop( ) -> UResult<()> { let mut queue: Vec = enter_directory(path_data, read_dir, config, state, dired)?; - let listed_ancestor = FileInformation::from_path(&path_data.p_buf, path_data.must_dereference)?; + let listed_ancestor_md = match path_data.get_metadata(&mut state.out) { + Some(md) => md, + None => &get_metadata_with_deref_opt(&path_data.p_buf, path_data.must_dereference)?, + }; if config.recursive { while let Some(item) = queue.pop() { @@ -2264,9 +2266,14 @@ fn recursive_loop( )); } Ok(rd) => { - let item_info = FileInformation::from_path(&item.p_buf, item.must_dereference)?; + let item_info_md = match item.get_metadata(&mut state.out) { + Some(md) => md, + None => &get_metadata_with_deref_opt(&item.p_buf, item.must_dereference)?, + }; - if item_info == listed_ancestor { + if item_info_md.dev() == listed_ancestor_md.dev() + && item_info_md.ino() == listed_ancestor_md.ino() + { state.out.flush()?; show!(LsError::AlreadyListedError(item.p_buf.clone())); continue; From aba399728d4be98f74788b8d976bc1cc8f93c88b Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Tue, 23 Sep 2025 23:29:44 -0500 Subject: [PATCH 08/76] Fix lints --- src/uu/ls/src/ls.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 4a9934cf5ab..51520f41116 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2271,6 +2271,17 @@ fn recursive_loop( None => &get_metadata_with_deref_opt(&item.p_buf, item.must_dereference)?, }; + #[cfg(target_os = "windows")] + if item_info_md.volume_serial_number() + == listed_ancestor_md.volume_serial_number() + && item_info_md.file_index() == listed_ancestor_md.file_index() + { + state.out.flush()?; + show!(LsError::AlreadyListedError(item.p_buf.clone())); + continue; + } + + #[cfg(not(target_os = "windows"))] if item_info_md.dev() == listed_ancestor_md.dev() && item_info_md.ino() == listed_ancestor_md.ino() { From 09cbb15d734030ea0c55a39b8760a8b2ab067f95 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Tue, 23 Sep 2025 23:38:54 -0500 Subject: [PATCH 09/76] Fix lints --- src/uu/ls/src/ls.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 51520f41116..c2fa0459867 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -37,6 +37,8 @@ use thiserror::Error; #[cfg(unix)] use uucore::entries; +#[cfg(target_os = "windows")] +use uucore::fs::FileInformation; #[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))] use uucore::fsxattr::has_acl; #[cfg(unix)] @@ -2249,6 +2251,11 @@ fn recursive_loop( ) -> UResult<()> { let mut queue: Vec = enter_directory(path_data, read_dir, config, state, dired)?; + #[cfg(target_os = "windows")] + let listed_ancestor_md = + FileInformation::from_path(path_data.p_buf, path_data.must_dereference)?; + + #[cfg(not(target_os = "windows"))] let listed_ancestor_md = match path_data.get_metadata(&mut state.out) { Some(md) => md, None => &get_metadata_with_deref_opt(&path_data.p_buf, path_data.must_dereference)?, @@ -2266,16 +2273,18 @@ fn recursive_loop( )); } Ok(rd) => { + #[cfg(target_os = "windows")] + let item_info_md = + FileInformation::from_path(item.p_buf, item.must_dereference)?; + + #[cfg(not(target_os = "windows"))] let item_info_md = match item.get_metadata(&mut state.out) { Some(md) => md, None => &get_metadata_with_deref_opt(&item.p_buf, item.must_dereference)?, }; #[cfg(target_os = "windows")] - if item_info_md.volume_serial_number() - == listed_ancestor_md.volume_serial_number() - && item_info_md.file_index() == listed_ancestor_md.file_index() - { + if item_info_md == listed_ancestor_md { state.out.flush()?; show!(LsError::AlreadyListedError(item.p_buf.clone())); continue; From c1304de0a2614a9f0a2cb07fe06fa876d466e85b Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Tue, 23 Sep 2025 23:39:35 -0500 Subject: [PATCH 10/76] Cleanup --- src/uu/ls/src/ls.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index c2fa0459867..4e369fa3db6 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2274,25 +2274,24 @@ fn recursive_loop( } Ok(rd) => { #[cfg(target_os = "windows")] - let item_info_md = - FileInformation::from_path(item.p_buf, item.must_dereference)?; + let item_md = FileInformation::from_path(item.p_buf, item.must_dereference)?; #[cfg(not(target_os = "windows"))] - let item_info_md = match item.get_metadata(&mut state.out) { + let item_md = match item.get_metadata(&mut state.out) { Some(md) => md, None => &get_metadata_with_deref_opt(&item.p_buf, item.must_dereference)?, }; #[cfg(target_os = "windows")] - if item_info_md == listed_ancestor_md { + if item_md == listed_ancestor_md { state.out.flush()?; show!(LsError::AlreadyListedError(item.p_buf.clone())); continue; } #[cfg(not(target_os = "windows"))] - if item_info_md.dev() == listed_ancestor_md.dev() - && item_info_md.ino() == listed_ancestor_md.ino() + if item_md.dev() == listed_ancestor_md.dev() + && item_md.ino() == listed_ancestor_md.ino() { state.out.flush()?; show!(LsError::AlreadyListedError(item.p_buf.clone())); From a90d73c99088bc81c5fd2d5e08aa0ba94fc60025 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Tue, 23 Sep 2025 23:48:23 -0500 Subject: [PATCH 11/76] Cleanup --- src/uu/ls/src/ls.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 4e369fa3db6..30acbb90414 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2253,7 +2253,7 @@ fn recursive_loop( #[cfg(target_os = "windows")] let listed_ancestor_md = - FileInformation::from_path(path_data.p_buf, path_data.must_dereference)?; + FileInformation::from_path(&path_data.p_buf, path_data.must_dereference)?; #[cfg(not(target_os = "windows"))] let listed_ancestor_md = match path_data.get_metadata(&mut state.out) { @@ -2274,7 +2274,7 @@ fn recursive_loop( } Ok(rd) => { #[cfg(target_os = "windows")] - let item_md = FileInformation::from_path(item.p_buf, item.must_dereference)?; + let item_md = FileInformation::from_path(&item.p_buf, item.must_dereference)?; #[cfg(not(target_os = "windows"))] let item_md = match item.get_metadata(&mut state.out) { From 11f1739c119b218aacbdbf8de0c84e8b0695c378 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 24 Sep 2025 11:12:21 -0500 Subject: [PATCH 12/76] Use "stack" instead of queue --- src/uu/ls/src/ls.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 30acbb90414..8ab423efab5 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2249,7 +2249,7 @@ fn recursive_loop( state: &mut ListState, dired: &mut DiredOutput, ) -> UResult<()> { - let mut queue: Vec = enter_directory(path_data, read_dir, config, state, dired)?; + let mut stack: Vec = enter_directory(path_data, read_dir, config, state, dired)?; #[cfg(target_os = "windows")] let listed_ancestor_md = @@ -2262,7 +2262,7 @@ fn recursive_loop( }; if config.recursive { - while let Some(item) = queue.pop() { + while let Some(item) = stack.pop() { match fs::read_dir(&item.p_buf) { Err(err) => { state.out.flush()?; @@ -2316,7 +2316,7 @@ fn recursive_loop( show_dir_name(&item, &mut state.out, config)?; writeln!(state.out)?; let mut res = enter_directory(&item, rd, config, state, dired)?; - queue.append(&mut res); + stack.append(&mut res); } } } From 08e2e27865b5be4e72b6de4e713190f008e6619c Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 24 Sep 2025 12:19:24 -0500 Subject: [PATCH 13/76] More likely inode is different than device, allows early bailout --- src/uu/ls/src/ls.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 8ab423efab5..76d52ab968b 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2290,8 +2290,8 @@ fn recursive_loop( } #[cfg(not(target_os = "windows"))] - if item_md.dev() == listed_ancestor_md.dev() - && item_md.ino() == listed_ancestor_md.ino() + if item_md.ino() == listed_ancestor_md.ino() + && item_md.dev() == listed_ancestor_md.dev() { state.out.flush()?; show!(LsError::AlreadyListedError(item.p_buf.clone())); From 451b4a563c6c6f8d9ce339c15322d47a3a753a91 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 24 Sep 2025 12:24:40 -0500 Subject: [PATCH 14/76] Use single mut vector instead of appending new vectors to stack --- src/uu/ls/src/ls.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 76d52ab968b..48151e36e5c 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2178,7 +2178,8 @@ fn enter_directory( config: &Config, state: &mut ListState, dired: &mut DiredOutput, -) -> UResult> { + stack: &mut Vec, +) -> UResult<()> { // Create vec of entries with initial dot files let mut entries: Vec = if config.files == Files::All { vec![ @@ -2232,14 +2233,15 @@ fn enter_directory( display_items(&entries, config, state, dired)?; - let res = entries + let iter = entries .into_iter() .skip(if config.files == Files::All { 2 } else { 0 }) .filter(|p| p.file_type(&mut state.out).is_some_and(|ft| ft.is_dir())) - .rev() - .collect(); + .rev(); + + stack.extend(iter); - Ok(res) + Ok(()) } fn recursive_loop( @@ -2249,7 +2251,9 @@ fn recursive_loop( state: &mut ListState, dired: &mut DiredOutput, ) -> UResult<()> { - let mut stack: Vec = enter_directory(path_data, read_dir, config, state, dired)?; + let mut stack: Vec = Vec::new(); + + enter_directory(path_data, read_dir, config, state, dired, &mut stack)?; #[cfg(target_os = "windows")] let listed_ancestor_md = @@ -2315,8 +2319,7 @@ fn recursive_loop( show_dir_name(&item, &mut state.out, config)?; writeln!(state.out)?; - let mut res = enter_directory(&item, rd, config, state, dired)?; - stack.append(&mut res); + enter_directory(&item, rd, config, state, dired, &mut stack)?; } } } From fc70e391cfe90f20e8ce52a35486baae45c3acaa Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 24 Sep 2025 12:45:18 -0500 Subject: [PATCH 15/76] Fix bug re: recursive symlinks --- src/uu/ls/src/ls.rs | 42 +++++++++--------------------------------- 1 file changed, 9 insertions(+), 33 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 48151e36e5c..a4fea026f8d 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -7,6 +7,7 @@ #[cfg(unix)] use std::collections::HashMap; +use std::collections::HashSet; #[cfg(unix)] use std::os::unix::fs::{FileTypeExt, MetadataExt}; #[cfg(windows)] @@ -61,13 +62,11 @@ use uucore::{ error::{UError, UResult, set_exit_code}, format::human::{SizeFormat, human_readable}, format_usage, - fs::display_permissions, + fs::{FileInformation, display_permissions}, fsext::{MetadataTimeField, metadata_get_time}, line_ending::LineEnding, os_str_as_bytes_lossy, - parser::parse_glob, - parser::parse_size::parse_size_u64, - parser::shortcut_value_parser::ShortcutValueParser, + parser::{parse_glob, parse_size::parse_size_u64, shortcut_value_parser::ShortcutValueParser}, quoting_style::{QuotingStyle, locale_aware_escape_dir_name, locale_aware_escape_name}, show, show_error, show_warning, time::{FormatSystemTimeFallback, format, format_system_time}, @@ -2251,20 +2250,13 @@ fn recursive_loop( state: &mut ListState, dired: &mut DiredOutput, ) -> UResult<()> { + let mut listed_ancestors = HashSet::new(); + listed_ancestors.insert(FileInformation::from_path(&path_data.p_buf, true)?); + let mut stack: Vec = Vec::new(); enter_directory(path_data, read_dir, config, state, dired, &mut stack)?; - #[cfg(target_os = "windows")] - let listed_ancestor_md = - FileInformation::from_path(&path_data.p_buf, path_data.must_dereference)?; - - #[cfg(not(target_os = "windows"))] - let listed_ancestor_md = match path_data.get_metadata(&mut state.out) { - Some(md) => md, - None => &get_metadata_with_deref_opt(&path_data.p_buf, path_data.must_dereference)?, - }; - if config.recursive { while let Some(item) = stack.pop() { match fs::read_dir(&item.p_buf) { @@ -2277,26 +2269,9 @@ fn recursive_loop( )); } Ok(rd) => { - #[cfg(target_os = "windows")] - let item_md = FileInformation::from_path(&item.p_buf, item.must_dereference)?; - - #[cfg(not(target_os = "windows"))] - let item_md = match item.get_metadata(&mut state.out) { - Some(md) => md, - None => &get_metadata_with_deref_opt(&item.p_buf, item.must_dereference)?, - }; + let file_info = FileInformation::from_path(&item.p_buf, true)?; - #[cfg(target_os = "windows")] - if item_md == listed_ancestor_md { - state.out.flush()?; - show!(LsError::AlreadyListedError(item.p_buf.clone())); - continue; - } - - #[cfg(not(target_os = "windows"))] - if item_md.ino() == listed_ancestor_md.ino() - && item_md.dev() == listed_ancestor_md.dev() - { + if listed_ancestors.contains(&file_info) { state.out.flush()?; show!(LsError::AlreadyListedError(item.p_buf.clone())); continue; @@ -2320,6 +2295,7 @@ fn recursive_loop( show_dir_name(&item, &mut state.out, config)?; writeln!(state.out)?; enter_directory(&item, rd, config, state, dired, &mut stack)?; + listed_ancestors.insert(file_info); } } } From 78a80c244922e93ccad4f69a541cd726dd70e5dd Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 24 Sep 2025 12:53:19 -0500 Subject: [PATCH 16/76] Cleanup --- src/uu/ls/src/ls.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index a4fea026f8d..ade9d76228d 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2251,7 +2251,10 @@ fn recursive_loop( dired: &mut DiredOutput, ) -> UResult<()> { let mut listed_ancestors = HashSet::new(); - listed_ancestors.insert(FileInformation::from_path(&path_data.p_buf, true)?); + listed_ancestors.insert(FileInformation::from_path( + &path_data.p_buf, + path_data.must_dereference, + )?); let mut stack: Vec = Vec::new(); @@ -2269,7 +2272,7 @@ fn recursive_loop( )); } Ok(rd) => { - let file_info = FileInformation::from_path(&item.p_buf, true)?; + let file_info = FileInformation::from_path(&item.p_buf, item.must_dereference)?; if listed_ancestors.contains(&file_info) { state.out.flush()?; From bc1877f4823825934c2cd71d68f973b6b96be6ad Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 24 Sep 2025 13:17:57 -0500 Subject: [PATCH 17/76] Fix lints --- src/uu/ls/src/ls.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index ade9d76228d..d9429c4c9a2 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -38,8 +38,6 @@ use thiserror::Error; #[cfg(unix)] use uucore::entries; -#[cfg(target_os = "windows")] -use uucore::fs::FileInformation; #[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))] use uucore::fsxattr::has_acl; #[cfg(unix)] From f344169a881af4bfbb9f79f52824f206899af64e Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 24 Sep 2025 15:07:37 -0500 Subject: [PATCH 18/76] Better names --- src/uu/ls/src/ls.rs | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index d9429c4c9a2..119086a4657 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2056,7 +2056,7 @@ pub fn list(locs: Vec<&Path>, config: &Config) -> UResult<()> { } } - recursive_loop(path_data, read_dir, config, &mut state, &mut dired)?; + recurse_directories(path_data, read_dir, config, &mut state, &mut dired)?; } if config.dired && !config.hyperlink { dired::print_dired_output(config, &dired, &mut state.out)?; @@ -2175,7 +2175,7 @@ fn enter_directory( config: &Config, state: &mut ListState, dired: &mut DiredOutput, - stack: &mut Vec, + entries_stack: &mut Vec, ) -> UResult<()> { // Create vec of entries with initial dot files let mut entries: Vec = if config.files == Files::All { @@ -2230,18 +2230,18 @@ fn enter_directory( display_items(&entries, config, state, dired)?; - let iter = entries + let new_dirs = entries .into_iter() .skip(if config.files == Files::All { 2 } else { 0 }) .filter(|p| p.file_type(&mut state.out).is_some_and(|ft| ft.is_dir())) .rev(); - stack.extend(iter); + entries_stack.extend(new_dirs); Ok(()) } -fn recursive_loop( +fn recurse_directories( path_data: &PathData, read_dir: ReadDir, config: &Config, @@ -2254,12 +2254,19 @@ fn recursive_loop( path_data.must_dereference, )?); - let mut stack: Vec = Vec::new(); + let mut entries_stack: Vec = Vec::new(); - enter_directory(path_data, read_dir, config, state, dired, &mut stack)?; + enter_directory( + path_data, + read_dir, + config, + state, + dired, + &mut entries_stack, + )?; if config.recursive { - while let Some(item) = stack.pop() { + while let Some(item) = entries_stack.pop() { match fs::read_dir(&item.p_buf) { Err(err) => { state.out.flush()?; @@ -2295,7 +2302,7 @@ fn recursive_loop( show_dir_name(&item, &mut state.out, config)?; writeln!(state.out)?; - enter_directory(&item, rd, config, state, dired, &mut stack)?; + enter_directory(&item, rd, config, state, dired, &mut entries_stack)?; listed_ancestors.insert(file_info); } } From c6f2a7bb3c3299fbe1eb5463006deeacf920cee8 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 24 Sep 2025 16:00:43 -0500 Subject: [PATCH 19/76] GNU implementation does not error out upon a already listed directory --- src/uu/ls/src/ls.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 119086a4657..480d4f33f25 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -200,6 +200,7 @@ enum LsError { #[error("{}", translate!("ls-error-dired-and-zero-incompatible"))] DiredAndZeroAreIncompatible, + #[allow(dead_code)] #[error("{}", translate!("ls-error-not-listing-already-listed", "path" => .0.to_string_lossy()))] AlreadyListedError(PathBuf), @@ -2275,13 +2276,13 @@ fn recurse_directories( err, item.command_line )); + continue; } Ok(rd) => { let file_info = FileInformation::from_path(&item.p_buf, item.must_dereference)?; if listed_ancestors.contains(&file_info) { - state.out.flush()?; - show!(LsError::AlreadyListedError(item.p_buf.clone())); + // GNU does not error out here continue; } From 5839aa44613838607b998fd2416485382310aa66 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 24 Sep 2025 17:08:59 -0500 Subject: [PATCH 20/76] Revert "GNU implementation does not error out upon a already listed directory" This reverts commit c6f2a7bb3c3299fbe1eb5463006deeacf920cee8. --- src/uu/ls/src/ls.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 480d4f33f25..119086a4657 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -200,7 +200,6 @@ enum LsError { #[error("{}", translate!("ls-error-dired-and-zero-incompatible"))] DiredAndZeroAreIncompatible, - #[allow(dead_code)] #[error("{}", translate!("ls-error-not-listing-already-listed", "path" => .0.to_string_lossy()))] AlreadyListedError(PathBuf), @@ -2276,13 +2275,13 @@ fn recurse_directories( err, item.command_line )); - continue; } Ok(rd) => { let file_info = FileInformation::from_path(&item.p_buf, item.must_dereference)?; if listed_ancestors.contains(&file_info) { - // GNU does not error out here + state.out.flush()?; + show!(LsError::AlreadyListedError(item.p_buf.clone())); continue; } From 01dedb210802b50f49d0d4de8200ba0942165d0e Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 24 Sep 2025 21:03:59 -0500 Subject: [PATCH 21/76] Cleanup --- src/uu/ls/src/ls.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 119086a4657..5e4d658562d 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2279,7 +2279,7 @@ fn recurse_directories( Ok(rd) => { let file_info = FileInformation::from_path(&item.p_buf, item.must_dereference)?; - if listed_ancestors.contains(&file_info) { + if listed_ancestors.insert(file_info) { state.out.flush()?; show!(LsError::AlreadyListedError(item.p_buf.clone())); continue; @@ -2303,7 +2303,6 @@ fn recurse_directories( show_dir_name(&item, &mut state.out, config)?; writeln!(state.out)?; enter_directory(&item, rd, config, state, dired, &mut entries_stack)?; - listed_ancestors.insert(file_info); } } } From d10ae3c152382e913ba04fca5ae035fa5d0875a1 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 24 Sep 2025 21:08:22 -0500 Subject: [PATCH 22/76] Cleanup --- src/uu/ls/src/ls.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 5e4d658562d..ec7c6315e91 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2279,7 +2279,7 @@ fn recurse_directories( Ok(rd) => { let file_info = FileInformation::from_path(&item.p_buf, item.must_dereference)?; - if listed_ancestors.insert(file_info) { + if !listed_ancestors.insert(file_info) { state.out.flush()?; show!(LsError::AlreadyListedError(item.p_buf.clone())); continue; From 3d29b5d32cdbbf2b8ac07ef2330674606ddbb99a Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 24 Sep 2025 23:01:24 -0500 Subject: [PATCH 23/76] Refactor --- src/uu/ls/src/ls.rs | 223 +++++++++++++++++++++++--------------------- 1 file changed, 117 insertions(+), 106 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index ec7c6315e91..f7a7712cc79 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2437,127 +2437,93 @@ fn display_items( config: &Config, state: &mut ListState, dired: &mut DiredOutput, -) -> UResult<()> { +) -> Result<(), Box> { // `-Z`, `--context`: // Display the SELinux security context or '?' if none is found. When used with the `-l` // option, print the security context to the left of the size column. - let quoted = items.iter().any(|item| { let name = locale_aware_escape_name(&item.display_name, config.quoting_style); os_str_starts_with(&name, b"'") }); - if config.format == Format::Long { - let padding_collection = calculate_padding_collection(items, config, state); + let padding_collection = calculate_padding_collection(items, config, state); - for item in items { - #[cfg(unix)] - let should_display_leading_info = config.inode || config.alloc_size; - #[cfg(not(unix))] - let should_display_leading_info = config.alloc_size; - - if should_display_leading_info { - let more_info = display_additional_leading_info( - item, - &padding_collection, - config, - &mut state.out, - )?; - - write!(state.out, "{more_info}")?; - } - - display_item_long(item, &padding_collection, config, state, dired, quoted)?; - } - } else { - let mut longest_context_len = 1; - let prefix_context = if config.context { + match config.format { + Format::Long => { for item in items { - let context_len = item.security_context.len(); - longest_context_len = context_len.max(longest_context_len); - } - Some(longest_context_len) - } else { - None - }; - - let padding = calculate_padding_collection(items, config, state); - - // we need to apply normal color to non filename output - if let Some(style_manager) = &mut state.style_manager { - write!(state.out, "{}", style_manager.apply_normal())?; - } - - let mut names_vec = Vec::new(); - for i in items { - let more_info = display_additional_leading_info(i, &padding, config, &mut state.out)?; - // it's okay to set current column to zero which is used to decide - // whether text will wrap or not, because when format is grid or - // column ls will try to place the item name in a new line if it - // wraps. - let cell = display_item_name( - i, - config, - prefix_context, - more_info, - state, - LazyCell::new(Box::new(|| 0)), - ); - - names_vec.push(cell); - } - - let mut names = names_vec.into_iter(); + #[cfg(unix)] + let should_display_leading_info = config.inode || config.alloc_size; + #[cfg(not(unix))] + let should_display_leading_info = config.alloc_size; + + if should_display_leading_info { + let more_info = display_additional_leading_info( + item, + &padding_collection, + config, + &mut state.out, + )?; + + write!(state.out, "{more_info}")?; + } - match config.format { - Format::Columns => { - display_grid( - names, - config.width, - Direction::TopToBottom, - &mut state.out, - quoted, - config.tab_size, - )?; - } - Format::Across => { - display_grid( - names, - config.width, - Direction::LeftToRight, - &mut state.out, - quoted, - config.tab_size, - )?; + display_item_long(item, &padding_collection, config, state, dired, quoted)?; } - Format::Commas => { - let mut current_col = 0; - if let Some(name) = names.next() { - write_os_str(&mut state.out, &name)?; - current_col = ansi_width(&name.to_string_lossy()) as u16 + 2; + } + _ => { + let mut names = + display_short_common(items, config, state, &padding_collection)?.into_iter(); + + match config.format { + Format::Columns => { + display_grid( + names, + config.width, + Direction::TopToBottom, + &mut state.out, + quoted, + config.tab_size, + )?; } - for name in names { - let name_width = ansi_width(&name.to_string_lossy()) as u16; - // If the width is 0 we print one single line - if config.width != 0 && current_col + name_width + 1 > config.width { - current_col = name_width + 2; - writeln!(state.out, ",")?; - } else { - current_col += name_width + 2; - write!(state.out, ", ")?; - } - write_os_str(&mut state.out, &name)?; + Format::Across => { + display_grid( + names, + config.width, + Direction::LeftToRight, + &mut state.out, + quoted, + config.tab_size, + )?; } - // Current col is never zero again if names have been printed. - // So we print a newline. - if current_col > 0 { - write!(state.out, "{}", config.line_ending)?; + Format::Commas => { + let mut current_col = 0; + if let Some(name) = names.next() { + write_os_str(&mut state.out, &name)?; + current_col = ansi_width(&name.to_string_lossy()) as u16 + 2; + } + for name in names { + let name_width = ansi_width(&name.to_string_lossy()) as u16; + // If the width is 0 we print one single line + if config.width != 0 && current_col + name_width + 1 > config.width { + current_col = name_width + 2; + writeln!(state.out, ",")?; + } else { + current_col += name_width + 2; + write!(state.out, ", ")?; + } + write_os_str(&mut state.out, &name)?; + } + // Current col is never zero again if names have been printed. + // So we print a newline. + if current_col > 0 { + write!(state.out, "{}", config.line_ending)?; + } } - } - _ => { - for name in names { - write_os_str(&mut state.out, &name)?; - write!(state.out, "{}", config.line_ending)?; + _ => { + for name in names { + write_os_str(&mut state.out, &name)?; + write!(state.out, "{}", config.line_ending)?; + } } } } @@ -2566,6 +2532,51 @@ fn display_items( Ok(()) } +fn display_short_common( + items: &[PathData], + config: &Config, + state: &mut ListState, + padding_collection: &PaddingCollection, +) -> Result, Box> { + let mut longest_context_len = 1; + let prefix_context = if config.context { + for item in items { + let context_len = item.security_context.len(); + longest_context_len = context_len.max(longest_context_len); + } + Some(longest_context_len) + } else { + None + }; + + // we need to apply normal color to non filename output + if let Some(style_manager) = &mut state.style_manager { + write!(state.out, "{}", style_manager.apply_normal())?; + } + + let mut names_vec = Vec::new(); + for i in items { + let more_info = + display_additional_leading_info(i, &padding_collection, config, &mut state.out)?; + // it's okay to set current column to zero which is used to decide + // whether text will wrap or not, because when format is grid or + // column ls will try to place the item name in a new line if it + // wraps. + let cell = display_item_name( + i, + config, + prefix_context, + more_info, + state, + LazyCell::new(Box::new(|| 0)), + ); + + names_vec.push(cell); + } + + Ok(names_vec) +} + #[allow(unused_variables)] fn get_block_size(md: &Metadata, config: &Config) -> u64 { /* GNU ls will display sizes in terms of block size From 3f8055fa4cac2d6482bcf23050c8ef3ee4c61a47 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 24 Sep 2025 23:07:36 -0500 Subject: [PATCH 24/76] Fix lints --- src/uu/ls/src/ls.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index f7a7712cc79..618f08069a4 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2557,7 +2557,7 @@ fn display_short_common( let mut names_vec = Vec::new(); for i in items { let more_info = - display_additional_leading_info(i, &padding_collection, config, &mut state.out)?; + display_additional_leading_info(i, padding_collection, config, &mut state.out)?; // it's okay to set current column to zero which is used to decide // whether text will wrap or not, because when format is grid or // column ls will try to place the item name in a new line if it From dcaff6fee5b75d5b705bd32cc60c8eea0490cb03 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Tue, 23 Sep 2025 19:26:09 -0500 Subject: [PATCH 25/76] Initial commit --- src/uu/ls/src/ls.rs | 63 ++++++++++++++++++++++++++++----------------- 1 file changed, 39 insertions(+), 24 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index ba7f26f1aad..c317f299a9a 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2063,7 +2063,7 @@ pub fn list(locs: Vec<&Path>, config: &Config) -> UResult<()> { &path_data.p_buf, path_data.must_dereference, )?); - enter_directory( + recursive_loop( path_data, read_dir, config, @@ -2183,15 +2183,13 @@ fn should_display(entry: &DirEntry, config: &Config) -> bool { .any(|p| p.matches_with(&file_name, options)) } -#[allow(clippy::cognitive_complexity)] fn enter_directory( path_data: &PathData, read_dir: ReadDir, config: &Config, state: &mut ListState, - listed_ancestors: &mut HashSet, dired: &mut DiredOutput, -) -> UResult<()> { +) -> UResult> { // Create vec of entries with initial dot files let mut entries: Vec = if config.files == Files::All { vec![ @@ -2245,28 +2243,42 @@ fn enter_directory( display_items(&entries, config, state, dired)?; + Ok(entries) +} + +fn recursive_loop( + path_data: &PathData, + read_dir: ReadDir, + config: &Config, + state: &mut ListState, + listed_ancestors: &mut HashSet, + dired: &mut DiredOutput, +) -> UResult<()> { + let mut queue: Vec = enter_directory(path_data, read_dir, config, state, dired)? + .into_iter() + .skip(if config.files == Files::All { 2 } else { 0 }) + .filter(|p| { + p.ft.get() + .is_some_and(|o_ft| o_ft.is_some_and(|ft| ft.is_dir())) + }) + .collect(); + if config.recursive { - for e in entries - .iter() - .skip(if config.files == Files::All { 2 } else { 0 }) - .filter(|p| { - p.ft.get() - .is_some_and(|o_ft| o_ft.is_some_and(|ft| ft.is_dir())) - }) - { - match fs::read_dir(&e.p_buf) { + while let Some(item) = queue.pop() { + match fs::read_dir(&item.p_buf) { Err(err) => { state.out.flush()?; show!(LsError::IOErrorContext( - e.p_buf.clone(), + item.p_buf.clone(), err, - e.command_line + item.command_line )); } Ok(rd) => { - if listed_ancestors - .insert(FileInformation::from_path(&e.p_buf, e.must_dereference)?) - { + if listed_ancestors.insert(FileInformation::from_path( + &item.p_buf, + item.must_dereference, + )?) { // when listing several directories in recursive mode, we show // "dirname:" at the beginning of the file list writeln!(state.out)?; @@ -2276,20 +2288,23 @@ fn enter_directory( // 2 = \n + \n dired.padding = 2; dired::indent(&mut state.out)?; - let dir_name_size = e.p_buf.to_string_lossy().len(); + let dir_name_size = item.p_buf.to_string_lossy().len(); dired::calculate_subdired(dired, dir_name_size); // inject dir name dired::add_dir_name(dired, dir_name_size); } - show_dir_name(e, &mut state.out, config)?; + show_dir_name(&item, &mut state.out, config)?; writeln!(state.out)?; - enter_directory(e, rd, config, state, listed_ancestors, dired)?; - listed_ancestors - .remove(&FileInformation::from_path(&e.p_buf, e.must_dereference)?); + let mut res = enter_directory(&item, rd, config, state, dired)?; + queue.append(&mut res); + listed_ancestors.remove(&FileInformation::from_path( + &item.p_buf, + item.must_dereference, + )?); } else { state.out.flush()?; - show!(LsError::AlreadyListedError(e.p_buf.clone())); + show!(LsError::AlreadyListedError(item.p_buf.clone())); } } } From 95cf0ad5f24c2cbc72b8b3c4bebb8e32cb0c52cf Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Tue, 23 Sep 2025 19:28:30 -0500 Subject: [PATCH 26/76] Closer --- src/uu/ls/src/ls.rs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index c317f299a9a..57977798f0a 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2243,7 +2243,16 @@ fn enter_directory( display_items(&entries, config, state, dired)?; - Ok(entries) + let res = entries + .into_iter() + .skip(if config.files == Files::All { 2 } else { 0 }) + .filter(|p| { + p.ft.get() + .is_some_and(|o_ft| o_ft.is_some_and(|ft| ft.is_dir())) + }) + .collect(); + + Ok(res) } fn recursive_loop( @@ -2254,14 +2263,7 @@ fn recursive_loop( listed_ancestors: &mut HashSet, dired: &mut DiredOutput, ) -> UResult<()> { - let mut queue: Vec = enter_directory(path_data, read_dir, config, state, dired)? - .into_iter() - .skip(if config.files == Files::All { 2 } else { 0 }) - .filter(|p| { - p.ft.get() - .is_some_and(|o_ft| o_ft.is_some_and(|ft| ft.is_dir())) - }) - .collect(); + let mut queue: Vec = enter_directory(path_data, read_dir, config, state, dired)?; if config.recursive { while let Some(item) = queue.pop() { From 27eebd610c698f3d183adfb3ffa37b44af0e6178 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Tue, 23 Sep 2025 19:40:41 -0500 Subject: [PATCH 27/76] Get queue order correct --- src/uu/ls/src/ls.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 57977798f0a..71ccca3f258 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2250,6 +2250,7 @@ fn enter_directory( p.ft.get() .is_some_and(|o_ft| o_ft.is_some_and(|ft| ft.is_dir())) }) + .rev() .collect(); Ok(res) From 18bbcb9b699fc713b1e5b5a087909e7431a2d63a Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Tue, 23 Sep 2025 20:18:40 -0500 Subject: [PATCH 28/76] Use one contains, instead of insert and remove --- src/uu/ls/src/ls.rs | 55 +++++++++++++++++++++------------------------ 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 71ccca3f258..7785dd606f3 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2277,38 +2277,35 @@ fn recursive_loop( item.command_line )); } - Ok(rd) => { - if listed_ancestors.insert(FileInformation::from_path( + Ok(_rd) + if listed_ancestors.contains(&FileInformation::from_path( &item.p_buf, item.must_dereference, - )?) { - // when listing several directories in recursive mode, we show - // "dirname:" at the beginning of the file list - writeln!(state.out)?; - if config.dired { - // We already injected the first dir - // Continue with the others - // 2 = \n + \n - dired.padding = 2; - dired::indent(&mut state.out)?; - let dir_name_size = item.p_buf.to_string_lossy().len(); - dired::calculate_subdired(dired, dir_name_size); - // inject dir name - dired::add_dir_name(dired, dir_name_size); - } - - show_dir_name(&item, &mut state.out, config)?; - writeln!(state.out)?; - let mut res = enter_directory(&item, rd, config, state, dired)?; - queue.append(&mut res); - listed_ancestors.remove(&FileInformation::from_path( - &item.p_buf, - item.must_dereference, - )?); - } else { - state.out.flush()?; - show!(LsError::AlreadyListedError(item.p_buf.clone())); + )?) => + { + state.out.flush()?; + show!(LsError::AlreadyListedError(item.p_buf.clone())); + } + Ok(rd) => { + // when listing several directories in recursive mode, we show + // "dirname:" at the beginning of the file list + writeln!(state.out)?; + if config.dired { + // We already injected the first dir + // Continue with the others + // 2 = \n + \n + dired.padding = 2; + dired::indent(&mut state.out)?; + let dir_name_size = item.p_buf.to_string_lossy().len(); + dired::calculate_subdired(dired, dir_name_size); + // inject dir name + dired::add_dir_name(dired, dir_name_size); } + + show_dir_name(&item, &mut state.out, config)?; + writeln!(state.out)?; + let mut res = enter_directory(&item, rd, config, state, dired)?; + queue.append(&mut res); } } } From 1b6b1b5d38bb23a6836e0e27179a4a724bd82af6 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Tue, 23 Sep 2025 21:02:59 -0500 Subject: [PATCH 29/76] No need for HashSet --- src/uu/ls/src/ls.rs | 36 ++++++++++++------------------------ 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 7785dd606f3..13ceda8b47c 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -14,7 +14,6 @@ use std::os::windows::fs::MetadataExt; use std::{ cell::{LazyCell, OnceCell}, cmp::Reverse, - collections::HashSet, ffi::{OsStr, OsString}, fmt::Write as FmtWrite, fs::{self, DirEntry, FileType, Metadata, ReadDir}, @@ -2058,19 +2057,8 @@ pub fn list(locs: Vec<&Path>, config: &Config) -> UResult<()> { writeln!(state.out)?; } } - let mut listed_ancestors = HashSet::new(); - listed_ancestors.insert(FileInformation::from_path( - &path_data.p_buf, - path_data.must_dereference, - )?); - recursive_loop( - path_data, - read_dir, - config, - &mut state, - &mut listed_ancestors, - &mut dired, - )?; + + recursive_loop(path_data, read_dir, config, &mut state, &mut dired)?; } if config.dired && !config.hyperlink { dired::print_dired_output(config, &dired, &mut state.out)?; @@ -2261,11 +2249,12 @@ fn recursive_loop( read_dir: ReadDir, config: &Config, state: &mut ListState, - listed_ancestors: &mut HashSet, dired: &mut DiredOutput, ) -> UResult<()> { let mut queue: Vec = enter_directory(path_data, read_dir, config, state, dired)?; + let listed_ancestor = FileInformation::from_path(&path_data.p_buf, path_data.must_dereference)?; + if config.recursive { while let Some(item) = queue.pop() { match fs::read_dir(&item.p_buf) { @@ -2277,16 +2266,15 @@ fn recursive_loop( item.command_line )); } - Ok(_rd) - if listed_ancestors.contains(&FileInformation::from_path( - &item.p_buf, - item.must_dereference, - )?) => - { - state.out.flush()?; - show!(LsError::AlreadyListedError(item.p_buf.clone())); - } Ok(rd) => { + let item_info = FileInformation::from_path(&item.p_buf, item.must_dereference)?; + + if item_info == listed_ancestor { + state.out.flush()?; + show!(LsError::AlreadyListedError(item.p_buf.clone())); + continue; + } + // when listing several directories in recursive mode, we show // "dirname:" at the beginning of the file list writeln!(state.out)?; From 7d56a8c0de984cb5a8066ee2a12cc43acdb97af7 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Tue, 23 Sep 2025 21:10:37 -0500 Subject: [PATCH 30/76] Cleanup --- src/uu/ls/src/ls.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 13ceda8b47c..05e72a69494 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2234,10 +2234,7 @@ fn enter_directory( let res = entries .into_iter() .skip(if config.files == Files::All { 2 } else { 0 }) - .filter(|p| { - p.ft.get() - .is_some_and(|o_ft| o_ft.is_some_and(|ft| ft.is_dir())) - }) + .filter(|p| p.file_type(&mut state.out).is_some_and(|ft| ft.is_dir())) .rev() .collect(); @@ -2304,10 +2301,10 @@ fn recursive_loop( fn get_metadata_with_deref_opt(p_buf: &Path, dereference: bool) -> std::io::Result { if dereference { - p_buf.metadata() - } else { - p_buf.symlink_metadata() + return p_buf.metadata(); } + + p_buf.symlink_metadata() } fn display_dir_entry_size( From 37b2916b27acdbd08b2f93940cdf0c6e32ab924f Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Tue, 23 Sep 2025 22:51:42 -0500 Subject: [PATCH 31/76] Reduce syscalls via retained metadata --- src/uu/ls/src/ls.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 05e72a69494..4a9934cf5ab 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -59,7 +59,6 @@ use uucore::{ error::{UError, UResult, set_exit_code}, format::human::{SizeFormat, human_readable}, format_usage, - fs::FileInformation, fs::display_permissions, fsext::{MetadataTimeField, metadata_get_time}, line_ending::LineEnding, @@ -2250,7 +2249,10 @@ fn recursive_loop( ) -> UResult<()> { let mut queue: Vec = enter_directory(path_data, read_dir, config, state, dired)?; - let listed_ancestor = FileInformation::from_path(&path_data.p_buf, path_data.must_dereference)?; + let listed_ancestor_md = match path_data.get_metadata(&mut state.out) { + Some(md) => md, + None => &get_metadata_with_deref_opt(&path_data.p_buf, path_data.must_dereference)?, + }; if config.recursive { while let Some(item) = queue.pop() { @@ -2264,9 +2266,14 @@ fn recursive_loop( )); } Ok(rd) => { - let item_info = FileInformation::from_path(&item.p_buf, item.must_dereference)?; + let item_info_md = match item.get_metadata(&mut state.out) { + Some(md) => md, + None => &get_metadata_with_deref_opt(&item.p_buf, item.must_dereference)?, + }; - if item_info == listed_ancestor { + if item_info_md.dev() == listed_ancestor_md.dev() + && item_info_md.ino() == listed_ancestor_md.ino() + { state.out.flush()?; show!(LsError::AlreadyListedError(item.p_buf.clone())); continue; From 5b4aa52a7d0ceea67656a41de7f26173f356b59a Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Tue, 23 Sep 2025 23:29:44 -0500 Subject: [PATCH 32/76] Fix lints --- src/uu/ls/src/ls.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 4a9934cf5ab..51520f41116 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2271,6 +2271,17 @@ fn recursive_loop( None => &get_metadata_with_deref_opt(&item.p_buf, item.must_dereference)?, }; + #[cfg(target_os = "windows")] + if item_info_md.volume_serial_number() + == listed_ancestor_md.volume_serial_number() + && item_info_md.file_index() == listed_ancestor_md.file_index() + { + state.out.flush()?; + show!(LsError::AlreadyListedError(item.p_buf.clone())); + continue; + } + + #[cfg(not(target_os = "windows"))] if item_info_md.dev() == listed_ancestor_md.dev() && item_info_md.ino() == listed_ancestor_md.ino() { From f2a413ec1e35b411de61fd2d4ac8d0ddb0ec67a7 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Tue, 23 Sep 2025 23:38:54 -0500 Subject: [PATCH 33/76] Fix lints --- src/uu/ls/src/ls.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 51520f41116..c2fa0459867 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -37,6 +37,8 @@ use thiserror::Error; #[cfg(unix)] use uucore::entries; +#[cfg(target_os = "windows")] +use uucore::fs::FileInformation; #[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))] use uucore::fsxattr::has_acl; #[cfg(unix)] @@ -2249,6 +2251,11 @@ fn recursive_loop( ) -> UResult<()> { let mut queue: Vec = enter_directory(path_data, read_dir, config, state, dired)?; + #[cfg(target_os = "windows")] + let listed_ancestor_md = + FileInformation::from_path(path_data.p_buf, path_data.must_dereference)?; + + #[cfg(not(target_os = "windows"))] let listed_ancestor_md = match path_data.get_metadata(&mut state.out) { Some(md) => md, None => &get_metadata_with_deref_opt(&path_data.p_buf, path_data.must_dereference)?, @@ -2266,16 +2273,18 @@ fn recursive_loop( )); } Ok(rd) => { + #[cfg(target_os = "windows")] + let item_info_md = + FileInformation::from_path(item.p_buf, item.must_dereference)?; + + #[cfg(not(target_os = "windows"))] let item_info_md = match item.get_metadata(&mut state.out) { Some(md) => md, None => &get_metadata_with_deref_opt(&item.p_buf, item.must_dereference)?, }; #[cfg(target_os = "windows")] - if item_info_md.volume_serial_number() - == listed_ancestor_md.volume_serial_number() - && item_info_md.file_index() == listed_ancestor_md.file_index() - { + if item_info_md == listed_ancestor_md { state.out.flush()?; show!(LsError::AlreadyListedError(item.p_buf.clone())); continue; From 663fa39e64e5c5d30ddfcb442588b0eb085712c5 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Tue, 23 Sep 2025 23:39:35 -0500 Subject: [PATCH 34/76] Cleanup --- src/uu/ls/src/ls.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index c2fa0459867..4e369fa3db6 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2274,25 +2274,24 @@ fn recursive_loop( } Ok(rd) => { #[cfg(target_os = "windows")] - let item_info_md = - FileInformation::from_path(item.p_buf, item.must_dereference)?; + let item_md = FileInformation::from_path(item.p_buf, item.must_dereference)?; #[cfg(not(target_os = "windows"))] - let item_info_md = match item.get_metadata(&mut state.out) { + let item_md = match item.get_metadata(&mut state.out) { Some(md) => md, None => &get_metadata_with_deref_opt(&item.p_buf, item.must_dereference)?, }; #[cfg(target_os = "windows")] - if item_info_md == listed_ancestor_md { + if item_md == listed_ancestor_md { state.out.flush()?; show!(LsError::AlreadyListedError(item.p_buf.clone())); continue; } #[cfg(not(target_os = "windows"))] - if item_info_md.dev() == listed_ancestor_md.dev() - && item_info_md.ino() == listed_ancestor_md.ino() + if item_md.dev() == listed_ancestor_md.dev() + && item_md.ino() == listed_ancestor_md.ino() { state.out.flush()?; show!(LsError::AlreadyListedError(item.p_buf.clone())); From 95eadae98db0eb1cca5cc5e7e0f13c3f75cf4b0d Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Tue, 23 Sep 2025 23:48:23 -0500 Subject: [PATCH 35/76] Cleanup --- src/uu/ls/src/ls.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 4e369fa3db6..30acbb90414 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2253,7 +2253,7 @@ fn recursive_loop( #[cfg(target_os = "windows")] let listed_ancestor_md = - FileInformation::from_path(path_data.p_buf, path_data.must_dereference)?; + FileInformation::from_path(&path_data.p_buf, path_data.must_dereference)?; #[cfg(not(target_os = "windows"))] let listed_ancestor_md = match path_data.get_metadata(&mut state.out) { @@ -2274,7 +2274,7 @@ fn recursive_loop( } Ok(rd) => { #[cfg(target_os = "windows")] - let item_md = FileInformation::from_path(item.p_buf, item.must_dereference)?; + let item_md = FileInformation::from_path(&item.p_buf, item.must_dereference)?; #[cfg(not(target_os = "windows"))] let item_md = match item.get_metadata(&mut state.out) { From 7f18433049da098d01d7336c9f4526f71843b2a2 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 24 Sep 2025 11:12:21 -0500 Subject: [PATCH 36/76] Use "stack" instead of queue --- src/uu/ls/src/ls.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 30acbb90414..8ab423efab5 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2249,7 +2249,7 @@ fn recursive_loop( state: &mut ListState, dired: &mut DiredOutput, ) -> UResult<()> { - let mut queue: Vec = enter_directory(path_data, read_dir, config, state, dired)?; + let mut stack: Vec = enter_directory(path_data, read_dir, config, state, dired)?; #[cfg(target_os = "windows")] let listed_ancestor_md = @@ -2262,7 +2262,7 @@ fn recursive_loop( }; if config.recursive { - while let Some(item) = queue.pop() { + while let Some(item) = stack.pop() { match fs::read_dir(&item.p_buf) { Err(err) => { state.out.flush()?; @@ -2316,7 +2316,7 @@ fn recursive_loop( show_dir_name(&item, &mut state.out, config)?; writeln!(state.out)?; let mut res = enter_directory(&item, rd, config, state, dired)?; - queue.append(&mut res); + stack.append(&mut res); } } } From 3ab7e24aa54252fb2f4f435217e4c4e2cde7c87e Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 24 Sep 2025 12:19:24 -0500 Subject: [PATCH 37/76] More likely inode is different than device, allows early bailout --- src/uu/ls/src/ls.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 8ab423efab5..76d52ab968b 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2290,8 +2290,8 @@ fn recursive_loop( } #[cfg(not(target_os = "windows"))] - if item_md.dev() == listed_ancestor_md.dev() - && item_md.ino() == listed_ancestor_md.ino() + if item_md.ino() == listed_ancestor_md.ino() + && item_md.dev() == listed_ancestor_md.dev() { state.out.flush()?; show!(LsError::AlreadyListedError(item.p_buf.clone())); From 7e5d2fbca2c1d8f2893971eb55987929482a1392 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 24 Sep 2025 12:24:40 -0500 Subject: [PATCH 38/76] Use single mut vector instead of appending new vectors to stack --- src/uu/ls/src/ls.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 76d52ab968b..48151e36e5c 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2178,7 +2178,8 @@ fn enter_directory( config: &Config, state: &mut ListState, dired: &mut DiredOutput, -) -> UResult> { + stack: &mut Vec, +) -> UResult<()> { // Create vec of entries with initial dot files let mut entries: Vec = if config.files == Files::All { vec![ @@ -2232,14 +2233,15 @@ fn enter_directory( display_items(&entries, config, state, dired)?; - let res = entries + let iter = entries .into_iter() .skip(if config.files == Files::All { 2 } else { 0 }) .filter(|p| p.file_type(&mut state.out).is_some_and(|ft| ft.is_dir())) - .rev() - .collect(); + .rev(); + + stack.extend(iter); - Ok(res) + Ok(()) } fn recursive_loop( @@ -2249,7 +2251,9 @@ fn recursive_loop( state: &mut ListState, dired: &mut DiredOutput, ) -> UResult<()> { - let mut stack: Vec = enter_directory(path_data, read_dir, config, state, dired)?; + let mut stack: Vec = Vec::new(); + + enter_directory(path_data, read_dir, config, state, dired, &mut stack)?; #[cfg(target_os = "windows")] let listed_ancestor_md = @@ -2315,8 +2319,7 @@ fn recursive_loop( show_dir_name(&item, &mut state.out, config)?; writeln!(state.out)?; - let mut res = enter_directory(&item, rd, config, state, dired)?; - stack.append(&mut res); + enter_directory(&item, rd, config, state, dired, &mut stack)?; } } } From e17f198ab4decb4a9b17ba9b80659b96907fb247 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 24 Sep 2025 12:45:18 -0500 Subject: [PATCH 39/76] Fix bug re: recursive symlinks --- src/uu/ls/src/ls.rs | 42 +++++++++--------------------------------- 1 file changed, 9 insertions(+), 33 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 48151e36e5c..a4fea026f8d 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -7,6 +7,7 @@ #[cfg(unix)] use std::collections::HashMap; +use std::collections::HashSet; #[cfg(unix)] use std::os::unix::fs::{FileTypeExt, MetadataExt}; #[cfg(windows)] @@ -61,13 +62,11 @@ use uucore::{ error::{UError, UResult, set_exit_code}, format::human::{SizeFormat, human_readable}, format_usage, - fs::display_permissions, + fs::{FileInformation, display_permissions}, fsext::{MetadataTimeField, metadata_get_time}, line_ending::LineEnding, os_str_as_bytes_lossy, - parser::parse_glob, - parser::parse_size::parse_size_u64, - parser::shortcut_value_parser::ShortcutValueParser, + parser::{parse_glob, parse_size::parse_size_u64, shortcut_value_parser::ShortcutValueParser}, quoting_style::{QuotingStyle, locale_aware_escape_dir_name, locale_aware_escape_name}, show, show_error, show_warning, time::{FormatSystemTimeFallback, format, format_system_time}, @@ -2251,20 +2250,13 @@ fn recursive_loop( state: &mut ListState, dired: &mut DiredOutput, ) -> UResult<()> { + let mut listed_ancestors = HashSet::new(); + listed_ancestors.insert(FileInformation::from_path(&path_data.p_buf, true)?); + let mut stack: Vec = Vec::new(); enter_directory(path_data, read_dir, config, state, dired, &mut stack)?; - #[cfg(target_os = "windows")] - let listed_ancestor_md = - FileInformation::from_path(&path_data.p_buf, path_data.must_dereference)?; - - #[cfg(not(target_os = "windows"))] - let listed_ancestor_md = match path_data.get_metadata(&mut state.out) { - Some(md) => md, - None => &get_metadata_with_deref_opt(&path_data.p_buf, path_data.must_dereference)?, - }; - if config.recursive { while let Some(item) = stack.pop() { match fs::read_dir(&item.p_buf) { @@ -2277,26 +2269,9 @@ fn recursive_loop( )); } Ok(rd) => { - #[cfg(target_os = "windows")] - let item_md = FileInformation::from_path(&item.p_buf, item.must_dereference)?; - - #[cfg(not(target_os = "windows"))] - let item_md = match item.get_metadata(&mut state.out) { - Some(md) => md, - None => &get_metadata_with_deref_opt(&item.p_buf, item.must_dereference)?, - }; + let file_info = FileInformation::from_path(&item.p_buf, true)?; - #[cfg(target_os = "windows")] - if item_md == listed_ancestor_md { - state.out.flush()?; - show!(LsError::AlreadyListedError(item.p_buf.clone())); - continue; - } - - #[cfg(not(target_os = "windows"))] - if item_md.ino() == listed_ancestor_md.ino() - && item_md.dev() == listed_ancestor_md.dev() - { + if listed_ancestors.contains(&file_info) { state.out.flush()?; show!(LsError::AlreadyListedError(item.p_buf.clone())); continue; @@ -2320,6 +2295,7 @@ fn recursive_loop( show_dir_name(&item, &mut state.out, config)?; writeln!(state.out)?; enter_directory(&item, rd, config, state, dired, &mut stack)?; + listed_ancestors.insert(file_info); } } } From b4a0dde42b2d9149001766e0054e3e28bff93e13 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 24 Sep 2025 12:53:19 -0500 Subject: [PATCH 40/76] Cleanup --- src/uu/ls/src/ls.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index a4fea026f8d..ade9d76228d 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2251,7 +2251,10 @@ fn recursive_loop( dired: &mut DiredOutput, ) -> UResult<()> { let mut listed_ancestors = HashSet::new(); - listed_ancestors.insert(FileInformation::from_path(&path_data.p_buf, true)?); + listed_ancestors.insert(FileInformation::from_path( + &path_data.p_buf, + path_data.must_dereference, + )?); let mut stack: Vec = Vec::new(); @@ -2269,7 +2272,7 @@ fn recursive_loop( )); } Ok(rd) => { - let file_info = FileInformation::from_path(&item.p_buf, true)?; + let file_info = FileInformation::from_path(&item.p_buf, item.must_dereference)?; if listed_ancestors.contains(&file_info) { state.out.flush()?; From aad56e0cdefbc40edd361dbf06ecb87ac91f4e92 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 24 Sep 2025 13:17:57 -0500 Subject: [PATCH 41/76] Fix lints --- src/uu/ls/src/ls.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index ade9d76228d..d9429c4c9a2 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -38,8 +38,6 @@ use thiserror::Error; #[cfg(unix)] use uucore::entries; -#[cfg(target_os = "windows")] -use uucore::fs::FileInformation; #[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))] use uucore::fsxattr::has_acl; #[cfg(unix)] From 97e1292799fb1d1d2b922f343a64f052a3ca9372 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 24 Sep 2025 15:07:37 -0500 Subject: [PATCH 42/76] Better names --- src/uu/ls/src/ls.rs | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index d9429c4c9a2..119086a4657 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2056,7 +2056,7 @@ pub fn list(locs: Vec<&Path>, config: &Config) -> UResult<()> { } } - recursive_loop(path_data, read_dir, config, &mut state, &mut dired)?; + recurse_directories(path_data, read_dir, config, &mut state, &mut dired)?; } if config.dired && !config.hyperlink { dired::print_dired_output(config, &dired, &mut state.out)?; @@ -2175,7 +2175,7 @@ fn enter_directory( config: &Config, state: &mut ListState, dired: &mut DiredOutput, - stack: &mut Vec, + entries_stack: &mut Vec, ) -> UResult<()> { // Create vec of entries with initial dot files let mut entries: Vec = if config.files == Files::All { @@ -2230,18 +2230,18 @@ fn enter_directory( display_items(&entries, config, state, dired)?; - let iter = entries + let new_dirs = entries .into_iter() .skip(if config.files == Files::All { 2 } else { 0 }) .filter(|p| p.file_type(&mut state.out).is_some_and(|ft| ft.is_dir())) .rev(); - stack.extend(iter); + entries_stack.extend(new_dirs); Ok(()) } -fn recursive_loop( +fn recurse_directories( path_data: &PathData, read_dir: ReadDir, config: &Config, @@ -2254,12 +2254,19 @@ fn recursive_loop( path_data.must_dereference, )?); - let mut stack: Vec = Vec::new(); + let mut entries_stack: Vec = Vec::new(); - enter_directory(path_data, read_dir, config, state, dired, &mut stack)?; + enter_directory( + path_data, + read_dir, + config, + state, + dired, + &mut entries_stack, + )?; if config.recursive { - while let Some(item) = stack.pop() { + while let Some(item) = entries_stack.pop() { match fs::read_dir(&item.p_buf) { Err(err) => { state.out.flush()?; @@ -2295,7 +2302,7 @@ fn recursive_loop( show_dir_name(&item, &mut state.out, config)?; writeln!(state.out)?; - enter_directory(&item, rd, config, state, dired, &mut stack)?; + enter_directory(&item, rd, config, state, dired, &mut entries_stack)?; listed_ancestors.insert(file_info); } } From a9c22c340cd53b8ccef9dd6fc1ed3e48f5cd0dd3 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 24 Sep 2025 16:00:43 -0500 Subject: [PATCH 43/76] GNU implementation does not error out upon a already listed directory --- src/uu/ls/src/ls.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 119086a4657..480d4f33f25 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -200,6 +200,7 @@ enum LsError { #[error("{}", translate!("ls-error-dired-and-zero-incompatible"))] DiredAndZeroAreIncompatible, + #[allow(dead_code)] #[error("{}", translate!("ls-error-not-listing-already-listed", "path" => .0.to_string_lossy()))] AlreadyListedError(PathBuf), @@ -2275,13 +2276,13 @@ fn recurse_directories( err, item.command_line )); + continue; } Ok(rd) => { let file_info = FileInformation::from_path(&item.p_buf, item.must_dereference)?; if listed_ancestors.contains(&file_info) { - state.out.flush()?; - show!(LsError::AlreadyListedError(item.p_buf.clone())); + // GNU does not error out here continue; } From 3e7e35fe22c0f180257194008bb4950ff3d741fb Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 24 Sep 2025 17:08:59 -0500 Subject: [PATCH 44/76] Revert "GNU implementation does not error out upon a already listed directory" This reverts commit c6f2a7bb3c3299fbe1eb5463006deeacf920cee8. --- src/uu/ls/src/ls.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 480d4f33f25..119086a4657 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -200,7 +200,6 @@ enum LsError { #[error("{}", translate!("ls-error-dired-and-zero-incompatible"))] DiredAndZeroAreIncompatible, - #[allow(dead_code)] #[error("{}", translate!("ls-error-not-listing-already-listed", "path" => .0.to_string_lossy()))] AlreadyListedError(PathBuf), @@ -2276,13 +2275,13 @@ fn recurse_directories( err, item.command_line )); - continue; } Ok(rd) => { let file_info = FileInformation::from_path(&item.p_buf, item.must_dereference)?; if listed_ancestors.contains(&file_info) { - // GNU does not error out here + state.out.flush()?; + show!(LsError::AlreadyListedError(item.p_buf.clone())); continue; } From 08b45c77a42a620a8f932b90a1508a318e4518c6 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 24 Sep 2025 21:03:59 -0500 Subject: [PATCH 45/76] Cleanup --- src/uu/ls/src/ls.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 119086a4657..5e4d658562d 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2279,7 +2279,7 @@ fn recurse_directories( Ok(rd) => { let file_info = FileInformation::from_path(&item.p_buf, item.must_dereference)?; - if listed_ancestors.contains(&file_info) { + if listed_ancestors.insert(file_info) { state.out.flush()?; show!(LsError::AlreadyListedError(item.p_buf.clone())); continue; @@ -2303,7 +2303,6 @@ fn recurse_directories( show_dir_name(&item, &mut state.out, config)?; writeln!(state.out)?; enter_directory(&item, rd, config, state, dired, &mut entries_stack)?; - listed_ancestors.insert(file_info); } } } From 0191fc39b5009d5b7461683f107ef397045f665e Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 24 Sep 2025 21:08:22 -0500 Subject: [PATCH 46/76] Cleanup --- src/uu/ls/src/ls.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 5e4d658562d..ec7c6315e91 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2279,7 +2279,7 @@ fn recurse_directories( Ok(rd) => { let file_info = FileInformation::from_path(&item.p_buf, item.must_dereference)?; - if listed_ancestors.insert(file_info) { + if !listed_ancestors.insert(file_info) { state.out.flush()?; show!(LsError::AlreadyListedError(item.p_buf.clone())); continue; From 68443ea4ad3fa11a281e178e3bab662e76df7ab6 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 24 Sep 2025 23:01:24 -0500 Subject: [PATCH 47/76] Refactor --- src/uu/ls/src/ls.rs | 223 +++++++++++++++++++++++--------------------- 1 file changed, 117 insertions(+), 106 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index ec7c6315e91..f7a7712cc79 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2437,127 +2437,93 @@ fn display_items( config: &Config, state: &mut ListState, dired: &mut DiredOutput, -) -> UResult<()> { +) -> Result<(), Box> { // `-Z`, `--context`: // Display the SELinux security context or '?' if none is found. When used with the `-l` // option, print the security context to the left of the size column. - let quoted = items.iter().any(|item| { let name = locale_aware_escape_name(&item.display_name, config.quoting_style); os_str_starts_with(&name, b"'") }); - if config.format == Format::Long { - let padding_collection = calculate_padding_collection(items, config, state); + let padding_collection = calculate_padding_collection(items, config, state); - for item in items { - #[cfg(unix)] - let should_display_leading_info = config.inode || config.alloc_size; - #[cfg(not(unix))] - let should_display_leading_info = config.alloc_size; - - if should_display_leading_info { - let more_info = display_additional_leading_info( - item, - &padding_collection, - config, - &mut state.out, - )?; - - write!(state.out, "{more_info}")?; - } - - display_item_long(item, &padding_collection, config, state, dired, quoted)?; - } - } else { - let mut longest_context_len = 1; - let prefix_context = if config.context { + match config.format { + Format::Long => { for item in items { - let context_len = item.security_context.len(); - longest_context_len = context_len.max(longest_context_len); - } - Some(longest_context_len) - } else { - None - }; - - let padding = calculate_padding_collection(items, config, state); - - // we need to apply normal color to non filename output - if let Some(style_manager) = &mut state.style_manager { - write!(state.out, "{}", style_manager.apply_normal())?; - } - - let mut names_vec = Vec::new(); - for i in items { - let more_info = display_additional_leading_info(i, &padding, config, &mut state.out)?; - // it's okay to set current column to zero which is used to decide - // whether text will wrap or not, because when format is grid or - // column ls will try to place the item name in a new line if it - // wraps. - let cell = display_item_name( - i, - config, - prefix_context, - more_info, - state, - LazyCell::new(Box::new(|| 0)), - ); - - names_vec.push(cell); - } - - let mut names = names_vec.into_iter(); + #[cfg(unix)] + let should_display_leading_info = config.inode || config.alloc_size; + #[cfg(not(unix))] + let should_display_leading_info = config.alloc_size; + + if should_display_leading_info { + let more_info = display_additional_leading_info( + item, + &padding_collection, + config, + &mut state.out, + )?; + + write!(state.out, "{more_info}")?; + } - match config.format { - Format::Columns => { - display_grid( - names, - config.width, - Direction::TopToBottom, - &mut state.out, - quoted, - config.tab_size, - )?; - } - Format::Across => { - display_grid( - names, - config.width, - Direction::LeftToRight, - &mut state.out, - quoted, - config.tab_size, - )?; + display_item_long(item, &padding_collection, config, state, dired, quoted)?; } - Format::Commas => { - let mut current_col = 0; - if let Some(name) = names.next() { - write_os_str(&mut state.out, &name)?; - current_col = ansi_width(&name.to_string_lossy()) as u16 + 2; + } + _ => { + let mut names = + display_short_common(items, config, state, &padding_collection)?.into_iter(); + + match config.format { + Format::Columns => { + display_grid( + names, + config.width, + Direction::TopToBottom, + &mut state.out, + quoted, + config.tab_size, + )?; } - for name in names { - let name_width = ansi_width(&name.to_string_lossy()) as u16; - // If the width is 0 we print one single line - if config.width != 0 && current_col + name_width + 1 > config.width { - current_col = name_width + 2; - writeln!(state.out, ",")?; - } else { - current_col += name_width + 2; - write!(state.out, ", ")?; - } - write_os_str(&mut state.out, &name)?; + Format::Across => { + display_grid( + names, + config.width, + Direction::LeftToRight, + &mut state.out, + quoted, + config.tab_size, + )?; } - // Current col is never zero again if names have been printed. - // So we print a newline. - if current_col > 0 { - write!(state.out, "{}", config.line_ending)?; + Format::Commas => { + let mut current_col = 0; + if let Some(name) = names.next() { + write_os_str(&mut state.out, &name)?; + current_col = ansi_width(&name.to_string_lossy()) as u16 + 2; + } + for name in names { + let name_width = ansi_width(&name.to_string_lossy()) as u16; + // If the width is 0 we print one single line + if config.width != 0 && current_col + name_width + 1 > config.width { + current_col = name_width + 2; + writeln!(state.out, ",")?; + } else { + current_col += name_width + 2; + write!(state.out, ", ")?; + } + write_os_str(&mut state.out, &name)?; + } + // Current col is never zero again if names have been printed. + // So we print a newline. + if current_col > 0 { + write!(state.out, "{}", config.line_ending)?; + } } - } - _ => { - for name in names { - write_os_str(&mut state.out, &name)?; - write!(state.out, "{}", config.line_ending)?; + _ => { + for name in names { + write_os_str(&mut state.out, &name)?; + write!(state.out, "{}", config.line_ending)?; + } } } } @@ -2566,6 +2532,51 @@ fn display_items( Ok(()) } +fn display_short_common( + items: &[PathData], + config: &Config, + state: &mut ListState, + padding_collection: &PaddingCollection, +) -> Result, Box> { + let mut longest_context_len = 1; + let prefix_context = if config.context { + for item in items { + let context_len = item.security_context.len(); + longest_context_len = context_len.max(longest_context_len); + } + Some(longest_context_len) + } else { + None + }; + + // we need to apply normal color to non filename output + if let Some(style_manager) = &mut state.style_manager { + write!(state.out, "{}", style_manager.apply_normal())?; + } + + let mut names_vec = Vec::new(); + for i in items { + let more_info = + display_additional_leading_info(i, &padding_collection, config, &mut state.out)?; + // it's okay to set current column to zero which is used to decide + // whether text will wrap or not, because when format is grid or + // column ls will try to place the item name in a new line if it + // wraps. + let cell = display_item_name( + i, + config, + prefix_context, + more_info, + state, + LazyCell::new(Box::new(|| 0)), + ); + + names_vec.push(cell); + } + + Ok(names_vec) +} + #[allow(unused_variables)] fn get_block_size(md: &Metadata, config: &Config) -> u64 { /* GNU ls will display sizes in terms of block size From d3ba40d879034ddc24bf4af3c69849491b7d037f Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 24 Sep 2025 23:07:36 -0500 Subject: [PATCH 48/76] Fix lints --- src/uu/ls/src/ls.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index f7a7712cc79..618f08069a4 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2557,7 +2557,7 @@ fn display_short_common( let mut names_vec = Vec::new(); for i in items { let more_info = - display_additional_leading_info(i, &padding_collection, config, &mut state.out)?; + display_additional_leading_info(i, padding_collection, config, &mut state.out)?; // it's okay to set current column to zero which is used to decide // whether text will wrap or not, because when format is grid or // column ls will try to place the item name in a new line if it From 1dc2ad9e0aa04b15a46d408bc258f09885bf1d9a Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Sun, 28 Sep 2025 01:18:42 -0500 Subject: [PATCH 49/76] Reduce stat calls for additional info in modes which don't need such info, clone struct to avoid additional syscalls --- src/uu/ls/src/ls.rs | 89 +++++++++++++++++++++++++++------------------ 1 file changed, 53 insertions(+), 36 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 618f08069a4..59bf5fe3867 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1906,6 +1906,19 @@ impl PathData { .get_or_init(|| self.get_metadata(out).map(|md| md.file_type())) .as_ref() } + + fn with_new_display_name(&self, display_name: &str) -> PathData { + PathData { + md: self.md.clone(), + ft: self.ft.clone(), + de: None, + display_name: display_name.into(), + p_buf: self.p_buf.clone(), + security_context: self.security_context.clone(), + must_dereference: self.must_dereference, + command_line: self.command_line, + } + } } /// Show the directory name in the case where several arguments are given to ls @@ -2058,6 +2071,7 @@ pub fn list(locs: Vec<&Path>, config: &Config) -> UResult<()> { recurse_directories(path_data, read_dir, config, &mut state, &mut dired)?; } + if config.dired && !config.hyperlink { dired::print_dired_output(config, &dired, &mut state.out)?; } @@ -2178,26 +2192,20 @@ fn enter_directory( entries_stack: &mut Vec, ) -> UResult<()> { // Create vec of entries with initial dot files - let mut entries: Vec = if config.files == Files::All { - vec![ - PathData::new( - path_data.p_buf.clone(), - None, - Some(".".into()), - config, - false, - ), - PathData::new( - path_data.p_buf.join(".."), - None, - Some("..".into()), - config, - false, - ), - ] - } else { - vec![] - }; + let mut entries: Vec = Vec::new(); + + if config.files == Files::All { + let current_dir = path_data.with_new_display_name("."); + + entries.push(current_dir); + + if let Some(parent) = path_data.p_buf.parent() { + let current_parent = + PathData::new(parent.to_path_buf(), None, Some("..".into()), config, false); + + entries.push(current_parent); + } + } // Convert those entries to the PathData struct for raw_entry in read_dir { @@ -2248,12 +2256,6 @@ fn recurse_directories( state: &mut ListState, dired: &mut DiredOutput, ) -> UResult<()> { - let mut listed_ancestors = HashSet::new(); - listed_ancestors.insert(FileInformation::from_path( - &path_data.p_buf, - path_data.must_dereference, - )?); - let mut entries_stack: Vec = Vec::new(); enter_directory( @@ -2266,6 +2268,12 @@ fn recurse_directories( )?; if config.recursive { + let mut listed_ancestors = HashSet::new(); + + let file_info = FileInformation::from_path(&path_data.p_buf, path_data.must_dereference)?; + + listed_ancestors.insert(file_info); + while let Some(item) = entries_stack.pop() { match fs::read_dir(&item.p_buf) { Err(err) => { @@ -2532,6 +2540,7 @@ fn display_items( Ok(()) } +#[inline] fn display_short_common( items: &[PathData], config: &Config, @@ -2555,9 +2564,15 @@ fn display_short_common( } let mut names_vec = Vec::new(); + let should_display_leading_info = config.inode || config.alloc_size; + for i in items { - let more_info = - display_additional_leading_info(i, padding_collection, config, &mut state.out)?; + let more_info: Option = if should_display_leading_info { + display_additional_leading_info(i, padding_collection, config, &mut state.out).ok() + } else { + None + }; + // it's okay to set current column to zero which is used to decide // whether text will wrap or not, because when format is grid or // column ls will try to place the item name in a new line if it @@ -2801,7 +2816,7 @@ fn display_item_long( item, config, None, - String::new(), + None, state, LazyCell::new(Box::new(|| { ansi_width(&String::from_utf8_lossy(&output_display)) @@ -2896,7 +2911,7 @@ fn display_item_long( item, config, None, - String::new(), + None, state, LazyCell::new(Box::new(|| { ansi_width(&String::from_utf8_lossy(&output_display)) @@ -3088,7 +3103,7 @@ fn display_item_name( path: &PathData, config: &Config, prefix_context: Option, - more_info: String, + more_info: Option, state: &mut ListState, current_column: LazyCell usize + '_>>, ) -> OsString { @@ -3114,10 +3129,12 @@ fn display_item_name( ); } - if config.format != Format::Long && !more_info.is_empty() { - let old_name = name; - name = more_info.into(); - name.push(&old_name); + if config.format != Format::Long { + if let Some(info) = more_info { + let old_name = name; + name = info.into(); + name.push(&old_name); + } } if config.indicator_style != IndicatorStyle::None { @@ -3182,7 +3199,7 @@ fn display_item_name( ) .is_err() { - name.push(path.p_buf.read_link().unwrap()); + name.push(target); } else { name.push(color_name( locale_aware_escape_name(target.as_os_str(), config.quoting_style), From 36fa836564caef2dab3a5c4e9b108c147efc8643 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Sun, 28 Sep 2025 01:37:19 -0500 Subject: [PATCH 50/76] Fix lints --- src/uu/ls/src/ls.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 59bf5fe3867..f884ad5d5d5 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2564,7 +2564,10 @@ fn display_short_common( } let mut names_vec = Vec::new(); + #[cfg(unix)] let should_display_leading_info = config.inode || config.alloc_size; + #[cfg(not(unix))] + let should_display_leading_info = config.alloc_size; for i in items { let more_info: Option = if should_display_leading_info { From e89886e17d4490c6c1359066d719ca241a152772 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Sun, 28 Sep 2025 01:18:42 -0500 Subject: [PATCH 51/76] Reduce stat calls for additional info in modes which don't need such info, clone struct to avoid additional syscalls Fix lints --- src/uu/ls/src/ls.rs | 92 +++++++++++++++++++++++++++------------------ 1 file changed, 56 insertions(+), 36 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 618f08069a4..f884ad5d5d5 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1906,6 +1906,19 @@ impl PathData { .get_or_init(|| self.get_metadata(out).map(|md| md.file_type())) .as_ref() } + + fn with_new_display_name(&self, display_name: &str) -> PathData { + PathData { + md: self.md.clone(), + ft: self.ft.clone(), + de: None, + display_name: display_name.into(), + p_buf: self.p_buf.clone(), + security_context: self.security_context.clone(), + must_dereference: self.must_dereference, + command_line: self.command_line, + } + } } /// Show the directory name in the case where several arguments are given to ls @@ -2058,6 +2071,7 @@ pub fn list(locs: Vec<&Path>, config: &Config) -> UResult<()> { recurse_directories(path_data, read_dir, config, &mut state, &mut dired)?; } + if config.dired && !config.hyperlink { dired::print_dired_output(config, &dired, &mut state.out)?; } @@ -2178,26 +2192,20 @@ fn enter_directory( entries_stack: &mut Vec, ) -> UResult<()> { // Create vec of entries with initial dot files - let mut entries: Vec = if config.files == Files::All { - vec![ - PathData::new( - path_data.p_buf.clone(), - None, - Some(".".into()), - config, - false, - ), - PathData::new( - path_data.p_buf.join(".."), - None, - Some("..".into()), - config, - false, - ), - ] - } else { - vec![] - }; + let mut entries: Vec = Vec::new(); + + if config.files == Files::All { + let current_dir = path_data.with_new_display_name("."); + + entries.push(current_dir); + + if let Some(parent) = path_data.p_buf.parent() { + let current_parent = + PathData::new(parent.to_path_buf(), None, Some("..".into()), config, false); + + entries.push(current_parent); + } + } // Convert those entries to the PathData struct for raw_entry in read_dir { @@ -2248,12 +2256,6 @@ fn recurse_directories( state: &mut ListState, dired: &mut DiredOutput, ) -> UResult<()> { - let mut listed_ancestors = HashSet::new(); - listed_ancestors.insert(FileInformation::from_path( - &path_data.p_buf, - path_data.must_dereference, - )?); - let mut entries_stack: Vec = Vec::new(); enter_directory( @@ -2266,6 +2268,12 @@ fn recurse_directories( )?; if config.recursive { + let mut listed_ancestors = HashSet::new(); + + let file_info = FileInformation::from_path(&path_data.p_buf, path_data.must_dereference)?; + + listed_ancestors.insert(file_info); + while let Some(item) = entries_stack.pop() { match fs::read_dir(&item.p_buf) { Err(err) => { @@ -2532,6 +2540,7 @@ fn display_items( Ok(()) } +#[inline] fn display_short_common( items: &[PathData], config: &Config, @@ -2555,9 +2564,18 @@ fn display_short_common( } let mut names_vec = Vec::new(); + #[cfg(unix)] + let should_display_leading_info = config.inode || config.alloc_size; + #[cfg(not(unix))] + let should_display_leading_info = config.alloc_size; + for i in items { - let more_info = - display_additional_leading_info(i, padding_collection, config, &mut state.out)?; + let more_info: Option = if should_display_leading_info { + display_additional_leading_info(i, padding_collection, config, &mut state.out).ok() + } else { + None + }; + // it's okay to set current column to zero which is used to decide // whether text will wrap or not, because when format is grid or // column ls will try to place the item name in a new line if it @@ -2801,7 +2819,7 @@ fn display_item_long( item, config, None, - String::new(), + None, state, LazyCell::new(Box::new(|| { ansi_width(&String::from_utf8_lossy(&output_display)) @@ -2896,7 +2914,7 @@ fn display_item_long( item, config, None, - String::new(), + None, state, LazyCell::new(Box::new(|| { ansi_width(&String::from_utf8_lossy(&output_display)) @@ -3088,7 +3106,7 @@ fn display_item_name( path: &PathData, config: &Config, prefix_context: Option, - more_info: String, + more_info: Option, state: &mut ListState, current_column: LazyCell usize + '_>>, ) -> OsString { @@ -3114,10 +3132,12 @@ fn display_item_name( ); } - if config.format != Format::Long && !more_info.is_empty() { - let old_name = name; - name = more_info.into(); - name.push(&old_name); + if config.format != Format::Long { + if let Some(info) = more_info { + let old_name = name; + name = info.into(); + name.push(&old_name); + } } if config.indicator_style != IndicatorStyle::None { @@ -3182,7 +3202,7 @@ fn display_item_name( ) .is_err() { - name.push(path.p_buf.read_link().unwrap()); + name.push(target); } else { name.push(color_name( locale_aware_escape_name(target.as_os_str(), config.quoting_style), From d0c7b82e96e9dab1199bac17a4d7fe1e28983e75 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Sun, 28 Sep 2025 01:46:58 -0500 Subject: [PATCH 52/76] Revert "Reduce stat calls for additional info in modes which don't need such info, clone struct to avoid additional syscalls" This reverts commit e89886e17d4490c6c1359066d719ca241a152772. --- src/uu/ls/src/ls.rs | 92 ++++++++++++++++++--------------------------- 1 file changed, 36 insertions(+), 56 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index f884ad5d5d5..618f08069a4 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1906,19 +1906,6 @@ impl PathData { .get_or_init(|| self.get_metadata(out).map(|md| md.file_type())) .as_ref() } - - fn with_new_display_name(&self, display_name: &str) -> PathData { - PathData { - md: self.md.clone(), - ft: self.ft.clone(), - de: None, - display_name: display_name.into(), - p_buf: self.p_buf.clone(), - security_context: self.security_context.clone(), - must_dereference: self.must_dereference, - command_line: self.command_line, - } - } } /// Show the directory name in the case where several arguments are given to ls @@ -2071,7 +2058,6 @@ pub fn list(locs: Vec<&Path>, config: &Config) -> UResult<()> { recurse_directories(path_data, read_dir, config, &mut state, &mut dired)?; } - if config.dired && !config.hyperlink { dired::print_dired_output(config, &dired, &mut state.out)?; } @@ -2192,20 +2178,26 @@ fn enter_directory( entries_stack: &mut Vec, ) -> UResult<()> { // Create vec of entries with initial dot files - let mut entries: Vec = Vec::new(); - - if config.files == Files::All { - let current_dir = path_data.with_new_display_name("."); - - entries.push(current_dir); - - if let Some(parent) = path_data.p_buf.parent() { - let current_parent = - PathData::new(parent.to_path_buf(), None, Some("..".into()), config, false); - - entries.push(current_parent); - } - } + let mut entries: Vec = if config.files == Files::All { + vec![ + PathData::new( + path_data.p_buf.clone(), + None, + Some(".".into()), + config, + false, + ), + PathData::new( + path_data.p_buf.join(".."), + None, + Some("..".into()), + config, + false, + ), + ] + } else { + vec![] + }; // Convert those entries to the PathData struct for raw_entry in read_dir { @@ -2256,6 +2248,12 @@ fn recurse_directories( state: &mut ListState, dired: &mut DiredOutput, ) -> UResult<()> { + let mut listed_ancestors = HashSet::new(); + listed_ancestors.insert(FileInformation::from_path( + &path_data.p_buf, + path_data.must_dereference, + )?); + let mut entries_stack: Vec = Vec::new(); enter_directory( @@ -2268,12 +2266,6 @@ fn recurse_directories( )?; if config.recursive { - let mut listed_ancestors = HashSet::new(); - - let file_info = FileInformation::from_path(&path_data.p_buf, path_data.must_dereference)?; - - listed_ancestors.insert(file_info); - while let Some(item) = entries_stack.pop() { match fs::read_dir(&item.p_buf) { Err(err) => { @@ -2540,7 +2532,6 @@ fn display_items( Ok(()) } -#[inline] fn display_short_common( items: &[PathData], config: &Config, @@ -2564,18 +2555,9 @@ fn display_short_common( } let mut names_vec = Vec::new(); - #[cfg(unix)] - let should_display_leading_info = config.inode || config.alloc_size; - #[cfg(not(unix))] - let should_display_leading_info = config.alloc_size; - for i in items { - let more_info: Option = if should_display_leading_info { - display_additional_leading_info(i, padding_collection, config, &mut state.out).ok() - } else { - None - }; - + let more_info = + display_additional_leading_info(i, padding_collection, config, &mut state.out)?; // it's okay to set current column to zero which is used to decide // whether text will wrap or not, because when format is grid or // column ls will try to place the item name in a new line if it @@ -2819,7 +2801,7 @@ fn display_item_long( item, config, None, - None, + String::new(), state, LazyCell::new(Box::new(|| { ansi_width(&String::from_utf8_lossy(&output_display)) @@ -2914,7 +2896,7 @@ fn display_item_long( item, config, None, - None, + String::new(), state, LazyCell::new(Box::new(|| { ansi_width(&String::from_utf8_lossy(&output_display)) @@ -3106,7 +3088,7 @@ fn display_item_name( path: &PathData, config: &Config, prefix_context: Option, - more_info: Option, + more_info: String, state: &mut ListState, current_column: LazyCell usize + '_>>, ) -> OsString { @@ -3132,12 +3114,10 @@ fn display_item_name( ); } - if config.format != Format::Long { - if let Some(info) = more_info { - let old_name = name; - name = info.into(); - name.push(&old_name); - } + if config.format != Format::Long && !more_info.is_empty() { + let old_name = name; + name = more_info.into(); + name.push(&old_name); } if config.indicator_style != IndicatorStyle::None { @@ -3202,7 +3182,7 @@ fn display_item_name( ) .is_err() { - name.push(target); + name.push(path.p_buf.read_link().unwrap()); } else { name.push(color_name( locale_aware_escape_name(target.as_os_str(), config.quoting_style), From f01b14b54629b3e00c813d8382711f2c7e776015 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Sun, 28 Sep 2025 01:48:04 -0500 Subject: [PATCH 53/76] Revert "Merge branch 'recursive_loop' of https://github.com/kimono-koans/coreutils into recursive_loop" This reverts commit de1fd8d8318e37df35aadc13cd81c7a70c6dff7b, reversing changes made to d0c7b82e96e9dab1199bac17a4d7fe1e28983e75. --- src/uu/ls/src/ls.rs | 92 ++++++++++++++++++--------------------------- 1 file changed, 36 insertions(+), 56 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index f884ad5d5d5..618f08069a4 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1906,19 +1906,6 @@ impl PathData { .get_or_init(|| self.get_metadata(out).map(|md| md.file_type())) .as_ref() } - - fn with_new_display_name(&self, display_name: &str) -> PathData { - PathData { - md: self.md.clone(), - ft: self.ft.clone(), - de: None, - display_name: display_name.into(), - p_buf: self.p_buf.clone(), - security_context: self.security_context.clone(), - must_dereference: self.must_dereference, - command_line: self.command_line, - } - } } /// Show the directory name in the case where several arguments are given to ls @@ -2071,7 +2058,6 @@ pub fn list(locs: Vec<&Path>, config: &Config) -> UResult<()> { recurse_directories(path_data, read_dir, config, &mut state, &mut dired)?; } - if config.dired && !config.hyperlink { dired::print_dired_output(config, &dired, &mut state.out)?; } @@ -2192,20 +2178,26 @@ fn enter_directory( entries_stack: &mut Vec, ) -> UResult<()> { // Create vec of entries with initial dot files - let mut entries: Vec = Vec::new(); - - if config.files == Files::All { - let current_dir = path_data.with_new_display_name("."); - - entries.push(current_dir); - - if let Some(parent) = path_data.p_buf.parent() { - let current_parent = - PathData::new(parent.to_path_buf(), None, Some("..".into()), config, false); - - entries.push(current_parent); - } - } + let mut entries: Vec = if config.files == Files::All { + vec![ + PathData::new( + path_data.p_buf.clone(), + None, + Some(".".into()), + config, + false, + ), + PathData::new( + path_data.p_buf.join(".."), + None, + Some("..".into()), + config, + false, + ), + ] + } else { + vec![] + }; // Convert those entries to the PathData struct for raw_entry in read_dir { @@ -2256,6 +2248,12 @@ fn recurse_directories( state: &mut ListState, dired: &mut DiredOutput, ) -> UResult<()> { + let mut listed_ancestors = HashSet::new(); + listed_ancestors.insert(FileInformation::from_path( + &path_data.p_buf, + path_data.must_dereference, + )?); + let mut entries_stack: Vec = Vec::new(); enter_directory( @@ -2268,12 +2266,6 @@ fn recurse_directories( )?; if config.recursive { - let mut listed_ancestors = HashSet::new(); - - let file_info = FileInformation::from_path(&path_data.p_buf, path_data.must_dereference)?; - - listed_ancestors.insert(file_info); - while let Some(item) = entries_stack.pop() { match fs::read_dir(&item.p_buf) { Err(err) => { @@ -2540,7 +2532,6 @@ fn display_items( Ok(()) } -#[inline] fn display_short_common( items: &[PathData], config: &Config, @@ -2564,18 +2555,9 @@ fn display_short_common( } let mut names_vec = Vec::new(); - #[cfg(unix)] - let should_display_leading_info = config.inode || config.alloc_size; - #[cfg(not(unix))] - let should_display_leading_info = config.alloc_size; - for i in items { - let more_info: Option = if should_display_leading_info { - display_additional_leading_info(i, padding_collection, config, &mut state.out).ok() - } else { - None - }; - + let more_info = + display_additional_leading_info(i, padding_collection, config, &mut state.out)?; // it's okay to set current column to zero which is used to decide // whether text will wrap or not, because when format is grid or // column ls will try to place the item name in a new line if it @@ -2819,7 +2801,7 @@ fn display_item_long( item, config, None, - None, + String::new(), state, LazyCell::new(Box::new(|| { ansi_width(&String::from_utf8_lossy(&output_display)) @@ -2914,7 +2896,7 @@ fn display_item_long( item, config, None, - None, + String::new(), state, LazyCell::new(Box::new(|| { ansi_width(&String::from_utf8_lossy(&output_display)) @@ -3106,7 +3088,7 @@ fn display_item_name( path: &PathData, config: &Config, prefix_context: Option, - more_info: Option, + more_info: String, state: &mut ListState, current_column: LazyCell usize + '_>>, ) -> OsString { @@ -3132,12 +3114,10 @@ fn display_item_name( ); } - if config.format != Format::Long { - if let Some(info) = more_info { - let old_name = name; - name = info.into(); - name.push(&old_name); - } + if config.format != Format::Long && !more_info.is_empty() { + let old_name = name; + name = more_info.into(); + name.push(&old_name); } if config.indicator_style != IndicatorStyle::None { @@ -3202,7 +3182,7 @@ fn display_item_name( ) .is_err() { - name.push(target); + name.push(path.p_buf.read_link().unwrap()); } else { name.push(color_name( locale_aware_escape_name(target.as_os_str(), config.quoting_style), From 04e02662004a6d58e713b6d629b03cdf1391c7d6 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Sun, 28 Sep 2025 02:18:29 -0500 Subject: [PATCH 54/76] Avoid additional info generation --- src/uu/ls/src/ls.rs | 44 +++++++++++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 618f08069a4..4822635ac3b 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -16,7 +16,6 @@ use std::{ cell::{LazyCell, OnceCell}, cmp::Reverse, ffi::{OsStr, OsString}, - fmt::Write as FmtWrite, fs::{self, DirEntry, FileType, Metadata, ReadDir}, io::{BufWriter, ErrorKind, IsTerminal, Stdout, Write, stdout}, iter, @@ -2411,7 +2410,8 @@ fn display_additional_leading_info( } else { "?".to_owned() }; - write!(result, "{} ", pad_left(&i, padding.inode)).unwrap(); + result.push_str(&pad_left(&i, padding.inode)); + result.push(' '); } } @@ -2423,9 +2423,11 @@ fn display_additional_leading_info( }; // extra space is insert to align the sizes, as needed for all formats, except for the comma format. if config.format == Format::Commas { - write!(result, "{s} ").unwrap(); + result.push_str(&s); + result.push(' '); } else { - write!(result, "{} ", pad_left(&s, padding.block_size)).unwrap(); + result.push_str(&pad_left(&s, padding.block_size)); + result.push(' '); } } Ok(result) @@ -2556,8 +2558,22 @@ fn display_short_common( let mut names_vec = Vec::new(); for i in items { - let more_info = - display_additional_leading_info(i, padding_collection, config, &mut state.out)?; + #[cfg(unix)] + let should_display_leading_info = config.inode || config.alloc_size; + #[cfg(not(unix))] + let should_display_leading_info = config.alloc_size; + + let more_info = if should_display_leading_info { + Some(display_additional_leading_info( + i, + padding_collection, + config, + &mut state.out, + )?) + } else { + None + }; + // it's okay to set current column to zero which is used to decide // whether text will wrap or not, because when format is grid or // column ls will try to place the item name in a new line if it @@ -2801,7 +2817,7 @@ fn display_item_long( item, config, None, - String::new(), + None, state, LazyCell::new(Box::new(|| { ansi_width(&String::from_utf8_lossy(&output_display)) @@ -2896,7 +2912,7 @@ fn display_item_long( item, config, None, - String::new(), + None, state, LazyCell::new(Box::new(|| { ansi_width(&String::from_utf8_lossy(&output_display)) @@ -3088,7 +3104,7 @@ fn display_item_name( path: &PathData, config: &Config, prefix_context: Option, - more_info: String, + more_info: Option, state: &mut ListState, current_column: LazyCell usize + '_>>, ) -> OsString { @@ -3114,10 +3130,12 @@ fn display_item_name( ); } - if config.format != Format::Long && !more_info.is_empty() { - let old_name = name; - name = more_info.into(); - name.push(&old_name); + if config.format != Format::Long { + if let Some(info) = more_info { + let old_name = name; + name = info.into(); + name.push(old_name); + } } if config.indicator_style != IndicatorStyle::None { From 46a8e773e7023bea10097c5ae9b95213960d88bf Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Mon, 29 Sep 2025 03:13:53 -0500 Subject: [PATCH 55/76] Use PathData for Listed Ancestors when possible --- src/uu/ls/src/colors.rs | 4 +- src/uu/ls/src/ls.rs | 139 +++++++++++++++++++++++++--------------- 2 files changed, 90 insertions(+), 53 deletions(-) diff --git a/src/uu/ls/src/colors.rs b/src/uu/ls/src/colors.rs index 4affbbf8ca2..8a22a8d105d 100644 --- a/src/uu/ls/src/colors.rs +++ b/src/uu/ls/src/colors.rs @@ -7,7 +7,6 @@ use super::get_metadata_with_deref_opt; use lscolors::{Indicator, LsColors, Style}; use std::ffi::OsString; use std::fs::{DirEntry, Metadata}; -use std::io::{BufWriter, Stdout}; /// We need this struct to be able to store the previous style. /// This because we need to check the previous value in case we don't need @@ -153,7 +152,6 @@ pub(crate) fn color_name( name: OsString, path: &PathData, style_manager: &mut StyleManager, - out: &mut BufWriter, target_symlink: Option<&PathData>, wrap: bool, ) -> OsString { @@ -193,7 +191,7 @@ pub(crate) fn color_name( let md = md_res.or_else(|_| path.p_buf.symlink_metadata()); style_manager.apply_style_based_on_metadata(path, md.ok().as_ref(), name, wrap) } else { - let md_option = path.get_metadata(out); + let md_option = path.get_metadata(); let symlink_metadata = path.p_buf.symlink_metadata().ok(); let md = md_option.or(symlink_metadata.as_ref()); style_manager.apply_style_based_on_metadata(path, md, name, wrap) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 4822635ac3b..791ae20d316 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -17,6 +17,7 @@ use std::{ cmp::Reverse, ffi::{OsStr, OsString}, fs::{self, DirEntry, FileType, Metadata, ReadDir}, + hash::Hash, io::{BufWriter, ErrorKind, IsTerminal, Stdout, Write, stdout}, iter, num::IntErrorKind, @@ -24,6 +25,8 @@ use std::{ path::{Path, PathBuf}, time::{Duration, SystemTime, UNIX_EPOCH}, }; +#[cfg(windows)] +use uucore::fs::FileInformation; use ansi_width::ansi_width; use clap::{ @@ -59,7 +62,7 @@ use uucore::{ error::{UError, UResult, set_exit_code}, format::human::{SizeFormat, human_readable}, format_usage, - fs::{FileInformation, display_permissions}, + fs::display_permissions, fsext::{MetadataTimeField, metadata_get_time}, line_ending::LineEnding, os_str_as_bytes_lossy, @@ -1861,7 +1864,7 @@ impl PathData { } } - fn get_metadata(&self, out: &mut BufWriter) -> Option<&Metadata> { + fn get_metadata(&self) -> Option<&Metadata> { self.md .get_or_init(|| { // check if we can use DirEntry metadata @@ -1876,6 +1879,7 @@ impl PathData { match get_metadata_with_deref_opt(self.p_buf.as_path(), self.must_dereference) { Err(err) => { // FIXME: A bit tricky to propagate the result here + let mut out = stdout(); out.flush().unwrap(); let errno = err.raw_os_error().unwrap_or(1i32); // a bad fd will throw an error when dereferenced, @@ -1900,13 +1904,46 @@ impl PathData { .as_ref() } - fn file_type(&self, out: &mut BufWriter) -> Option<&FileType> { + fn file_type(&self) -> Option<&FileType> { self.ft - .get_or_init(|| self.get_metadata(out).map(|md| md.file_type())) + .get_or_init(|| self.get_metadata().map(|md| md.file_type())) .as_ref() } } +impl PartialEq for PathData { + fn eq(&self, other: &Self) -> bool { + self.get_metadata().map(|md| md.ino()) == other.get_metadata().map(|md| md.ino()) + && self.get_metadata().map(|md| md.dev()) == other.get_metadata().map(|md| md.dev()) + } +} + +impl Eq for PathData {} + +impl Hash for PathData { + fn hash(&self, state: &mut H) { + if let Some(md) = self.get_metadata() { + md.ino().hash(state); + md.dev().hash(state); + } + } +} + +impl Clone for PathData { + fn clone(&self) -> Self { + Self { + md: self.md.clone(), + ft: self.ft.clone(), + de: None, + display_name: self.display_name.clone(), + p_buf: self.p_buf.clone(), + must_dereference: self.must_dereference, + security_context: self.security_context.clone(), + command_line: self.command_line, + } + } +} + /// Show the directory name in the case where several arguments are given to ls /// or the recursive flag is passed. /// @@ -1982,11 +2019,11 @@ pub fn list(locs: Vec<&Path>, config: &Config) -> UResult<()> { // Proper GNU handling is don't show if dereferenced symlink DNE // but only for the base dir, for a child dir show, and print ?s // in long format - if path_data.get_metadata(&mut state.out).is_none() { + if path_data.get_metadata().is_none() { continue; } - let show_dir_contents = match path_data.file_type(&mut state.out) { + let show_dir_contents = match path_data.file_type() { Some(ft) => !config.directory && ft.is_dir(), None => { set_exit_code(1); @@ -2001,8 +2038,8 @@ pub fn list(locs: Vec<&Path>, config: &Config) -> UResult<()> { } } - sort_entries(&mut files, config, &mut state.out); - sort_entries(&mut dirs, config, &mut state.out); + sort_entries(&mut files, config); + sort_entries(&mut dirs, config); if let Some(style_manager) = state.style_manager.as_mut() { // ls will try to write a reset before anything is written if normal @@ -2063,17 +2100,17 @@ pub fn list(locs: Vec<&Path>, config: &Config) -> UResult<()> { Ok(()) } -fn sort_entries(entries: &mut [PathData], config: &Config, out: &mut BufWriter) { +fn sort_entries(entries: &mut [PathData], config: &Config) { match config.sort { Sort::Time => entries.sort_by_key(|k| { Reverse( - k.get_metadata(out) + k.get_metadata() .and_then(|md| metadata_get_time(md, config.time)) .unwrap_or(UNIX_EPOCH), ) }), Sort::Size => { - entries.sort_by_key(|k| Reverse(k.get_metadata(out).map_or(0, |md| md.len()))); + entries.sort_by_key(|k| Reverse(k.get_metadata().map_or(0, |md| md.len()))); } // The default sort in GNU ls is case insensitive Sort::Name => entries.sort_by(|a, b| a.display_name.cmp(&b.display_name)), @@ -2216,7 +2253,7 @@ fn enter_directory( } } - sort_entries(&mut entries, config, &mut state.out); + sort_entries(&mut entries, config); // Print total after any error display if config.format == Format::Long || config.alloc_size { @@ -2232,7 +2269,7 @@ fn enter_directory( let new_dirs = entries .into_iter() .skip(if config.files == Files::All { 2 } else { 0 }) - .filter(|p| p.file_type(&mut state.out).is_some_and(|ft| ft.is_dir())) + .filter(|p| p.file_type().is_some_and(|ft| ft.is_dir())) .rev(); entries_stack.extend(new_dirs); @@ -2240,6 +2277,7 @@ fn enter_directory( Ok(()) } +#[allow(clippy::mutable_key_type)] fn recurse_directories( path_data: &PathData, read_dir: ReadDir, @@ -2247,11 +2285,18 @@ fn recurse_directories( state: &mut ListState, dired: &mut DiredOutput, ) -> UResult<()> { + #[cfg(target_os = "windows")] let mut listed_ancestors = HashSet::new(); + #[cfg(target_os = "windows")] listed_ancestors.insert(FileInformation::from_path( - &path_data.p_buf, + path_data.p_buf, path_data.must_dereference, - )?); + )); + + #[cfg(not(target_os = "windows"))] + let mut listed_ancestors: HashSet = HashSet::new(); + #[cfg(not(target_os = "windows"))] + listed_ancestors.insert(path_data.clone()); let mut entries_stack: Vec = Vec::new(); @@ -2276,9 +2321,17 @@ fn recurse_directories( )); } Ok(rd) => { - let file_info = FileInformation::from_path(&item.p_buf, item.must_dereference)?; - - if !listed_ancestors.insert(file_info) { + #[cfg(target_os = "windows")] + if !listed_ancestors.insert(FileInformation::from_path( + path_data.p_buf, + path_data.must_dereference, + )) { + state.out.flush()?; + show!(LsError::AlreadyListedError(item.p_buf.clone())); + continue; + } + #[cfg(not(target_os = "windows"))] + if !listed_ancestors.insert(item.clone()) { state.out.flush()?; show!(LsError::AlreadyListedError(item.p_buf.clone())); continue; @@ -2324,7 +2377,7 @@ fn display_dir_entry_size( state: &mut ListState, ) -> (usize, usize, usize, usize, usize, usize) { // TODO: Cache/memorize the display_* results so we don't have to recalculate them. - if let Some(md) = entry.get_metadata(&mut state.out) { + if let Some(md) = entry.get_metadata() { let (size_len, major_len, minor_len) = match display_len_or_rdev(md, config) { SizeOrDeviceId::Device(major, minor) => { (major.len() + minor.len() + 2usize, major.len(), minor.len()) @@ -2381,7 +2434,7 @@ fn return_total( let mut total_size = 0; for item in items { total_size += item - .get_metadata(out) + .get_metadata() .as_ref() .map_or(0, |md| get_block_size(md, config)); } @@ -2399,13 +2452,12 @@ fn display_additional_leading_info( item: &PathData, padding: &PaddingCollection, config: &Config, - out: &mut BufWriter, ) -> UResult { let mut result = String::new(); #[cfg(unix)] { if config.inode { - let i = if let Some(md) = item.get_metadata(out) { + let i = if let Some(md) = item.get_metadata() { get_inode(md) } else { "?".to_owned() @@ -2416,7 +2468,7 @@ fn display_additional_leading_info( } if config.alloc_size { - let s = if let Some(md) = item.get_metadata(out) { + let s = if let Some(md) = item.get_metadata() { display_size(get_block_size(md, config), config) } else { "?".to_owned() @@ -2459,12 +2511,8 @@ fn display_items( let should_display_leading_info = config.alloc_size; if should_display_leading_info { - let more_info = display_additional_leading_info( - item, - &padding_collection, - config, - &mut state.out, - )?; + let more_info = + display_additional_leading_info(item, &padding_collection, config)?; write!(state.out, "{more_info}")?; } @@ -2568,7 +2616,6 @@ fn display_short_common( i, padding_collection, config, - &mut state.out, )?) } else { None @@ -2741,7 +2788,7 @@ fn display_item_long( if config.dired { output_display.extend(b" "); } - if let Some(md) = item.get_metadata(&mut state.out) { + if let Some(md) = item.get_metadata() { #[cfg(any(not(unix), target_os = "android", target_os = "macos"))] // TODO: See how Mac should work here let is_acl_set = false; @@ -3056,8 +3103,8 @@ fn file_is_executable(md: &Metadata) -> bool { return md.mode() & ((S_IXUSR | S_IXGRP | S_IXOTH) as u32) != 0; } -fn classify_file(path: &PathData, out: &mut BufWriter) -> Option { - let file_type = path.file_type(out)?; +fn classify_file(path: &PathData) -> Option { + let file_type = path.file_type()?; if file_type.is_dir() { Some('/') @@ -3073,7 +3120,7 @@ fn classify_file(path: &PathData, out: &mut BufWriter) -> Option { } else if file_type.is_file() // Safe unwrapping if the file was removed between listing and display // See https://github.com/uutils/coreutils/issues/5371 - && path.get_metadata(out).is_some_and(file_is_executable) + && path.get_metadata().is_some_and(file_is_executable) { Some('*') } else { @@ -3120,14 +3167,7 @@ fn display_item_name( if let Some(style_manager) = &mut state.style_manager { let len = name.len(); - name = color_name( - name, - path, - style_manager, - &mut state.out, - None, - is_wrap(len), - ); + name = color_name(name, path, style_manager, None, is_wrap(len)); } if config.format != Format::Long { @@ -3139,7 +3179,7 @@ fn display_item_name( } if config.indicator_style != IndicatorStyle::None { - let sym = classify_file(path, &mut state.out); + let sym = classify_file(path); let char_opt = match config.indicator_style { IndicatorStyle::Classify => sym, @@ -3166,8 +3206,8 @@ fn display_item_name( } if config.format == Format::Long - && path.file_type(&mut state.out).is_some() - && path.file_type(&mut state.out).unwrap().is_symlink() + && path.file_type().is_some() + && path.file_type().unwrap().is_symlink() && !path.must_dereference { match path.p_buf.read_link() { @@ -3193,7 +3233,7 @@ fn display_item_name( // Because we use an absolute path, we can assume this is guaranteed to exist. // Otherwise, we use path.md(), which will guarantee we color to the same // color of non-existent symlinks according to style_for_path_with_metadata. - if path.get_metadata(&mut state.out).is_none() + if path.get_metadata().is_none() && get_metadata_with_deref_opt( target_data.p_buf.as_path(), target_data.must_dereference, @@ -3206,7 +3246,6 @@ fn display_item_name( locale_aware_escape_name(target.as_os_str(), config.quoting_style), path, style_manager, - &mut state.out, Some(&target_data), is_wrap(name.len()), )); @@ -3368,7 +3407,7 @@ fn calculate_padding_collection( for item in items { #[cfg(unix)] if config.inode { - let inode_len = if let Some(md) = item.get_metadata(&mut state.out) { + let inode_len = if let Some(md) = item.get_metadata() { display_inode(md).len() } else { continue; @@ -3377,7 +3416,7 @@ fn calculate_padding_collection( } if config.alloc_size { - if let Some(md) = item.get_metadata(&mut state.out) { + if let Some(md) = item.get_metadata() { let block_size_len = display_size(get_block_size(md, config), config).len(); padding_collections.block_size = block_size_len.max(padding_collections.block_size); } @@ -3427,7 +3466,7 @@ fn calculate_padding_collection( for item in items { if config.alloc_size { - if let Some(md) = item.get_metadata(&mut state.out) { + if let Some(md) = item.get_metadata() { let block_size_len = display_size(get_block_size(md, config), config).len(); padding_collections.block_size = block_size_len.max(padding_collections.block_size); } From 2f075ba89589ce958fae75a99466cb9612e8c698 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Mon, 29 Sep 2025 03:37:52 -0500 Subject: [PATCH 56/76] Fix lints --- src/uu/ls/src/ls.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 791ae20d316..fa2056d5e5f 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1911,6 +1911,7 @@ impl PathData { } } +#[cfg(not(windows))] impl PartialEq for PathData { fn eq(&self, other: &Self) -> bool { self.get_metadata().map(|md| md.ino()) == other.get_metadata().map(|md| md.ino()) @@ -1918,8 +1919,10 @@ impl PartialEq for PathData { } } +#[cfg(not(windows))] impl Eq for PathData {} +#[cfg(not(windows))] impl Hash for PathData { fn hash(&self, state: &mut H) { if let Some(md) = self.get_metadata() { From d504e97c6c9f7734d1f0407d5e359080c226c6c7 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Mon, 29 Sep 2025 03:55:13 -0500 Subject: [PATCH 57/76] Fix further merge conflicts Cleanup Fix lints Fix lints Fix lints Fix lints Fix lints --- src/uu/ls/src/colors.rs | 2 +- src/uu/ls/src/ls.rs | 108 ++++++++++++++++++++++------------------ 2 files changed, 60 insertions(+), 50 deletions(-) diff --git a/src/uu/ls/src/colors.rs b/src/uu/ls/src/colors.rs index a7f58d0fd58..f7acb2f5a30 100644 --- a/src/uu/ls/src/colors.rs +++ b/src/uu/ls/src/colors.rs @@ -180,7 +180,7 @@ pub(crate) fn color_name( if let Some(target) = target_symlink { // use the optional target_symlink - // Use fn symlink_metadata directly instead of get_metadata() here because ls + // Use fn symlink_metadata directly instead of metadata here because ls // should not exit with an err, if we are unable to obtain the target_metadata style_manager.apply_style_based_on_colorable(target, name, wrap) } else { diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 3eafaa9b068..fe7e6aafc1c 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -10,6 +10,8 @@ use fnv::FnvHashMap as HashMap; use fnv::FnvHashSet as HashSet; use std::borrow::Cow; use std::cell::RefCell; +#[cfg(not(windows))] +use std::hash::Hash; #[cfg(unix)] use std::os::unix::fs::{FileTypeExt, MetadataExt}; #[cfg(windows)] @@ -19,7 +21,6 @@ use std::{ cmp::Reverse, ffi::{OsStr, OsString}, fs::{self, DirEntry, FileType, Metadata, ReadDir}, - hash::Hash, io::{BufWriter, ErrorKind, IsTerminal, Stdout, Write, stdout}, iter, num::IntErrorKind, @@ -1944,8 +1945,8 @@ impl Colorable for PathData { #[cfg(not(windows))] impl PartialEq for PathData { fn eq(&self, other: &Self) -> bool { - self.get_metadata().map(|md| md.ino()) == other.get_metadata().map(|md| md.ino()) - && self.get_metadata().map(|md| md.dev()) == other.get_metadata().map(|md| md.dev()) + self.metadata().map(|md| md.ino()) == other.metadata().map(|md| md.ino()) + && self.metadata().map(|md| md.dev()) == other.metadata().map(|md| md.dev()) } } @@ -1955,7 +1956,7 @@ impl Eq for PathData {} #[cfg(not(windows))] impl Hash for PathData { fn hash(&self, state: &mut H) { - if let Some(md) = self.get_metadata() { + if let Some(md) = self.metadata() { md.ino().hash(state); md.dev().hash(state); } @@ -1967,7 +1968,7 @@ impl Clone for PathData { Self { md: self.md.clone(), ft: self.ft.clone(), - de: None, + de: RefCell::new(None), display_name: self.display_name.clone(), p_buf: self.p_buf.clone(), must_dereference: self.must_dereference, @@ -2126,7 +2127,6 @@ pub fn list(locs: Vec<&Path>, config: &Config) -> UResult<()> { } recurse_directories(path_data, read_dir, config, &mut state, &mut dired)?; - } if config.dired && !config.hyperlink { dired::print_dired_output(config, &dired, &mut state.out)?; @@ -2326,19 +2326,6 @@ fn recurse_directories( state: &mut ListState, dired: &mut DiredOutput, ) -> UResult<()> { - #[cfg(target_os = "windows")] - let mut listed_ancestors = HashSet::new(); - #[cfg(target_os = "windows")] - listed_ancestors.insert(FileInformation::from_path( - path_data.p_buf, - path_data.must_dereference, - )); - - #[cfg(not(target_os = "windows"))] - let mut listed_ancestors: HashSet = HashSet::new(); - #[cfg(not(target_os = "windows"))] - listed_ancestors.insert(path_data.clone()); - let mut entries_stack: Vec = Vec::new(); enter_directory( @@ -2351,13 +2338,25 @@ fn recurse_directories( )?; if config.recursive { + #[cfg(target_os = "windows")] + let mut listed_ancestors: HashSet = HashSet::default(); + #[cfg(target_os = "windows")] + listed_ancestors.insert(FileInformation::from_path( + path_data.path(), + path_data.must_dereference, + )?); + + #[cfg(not(target_os = "windows"))] + let mut listed_ancestors: HashSet = HashSet::default(); + #[cfg(not(target_os = "windows"))] + listed_ancestors.insert(path_data.clone()); + while let Some(item) = entries_stack.pop() { match fs::read_dir(&item.p_buf) { Err(err) => { state.out.flush()?; show!(LsError::IOErrorContext( item.p_buf.clone(), - err, item.command_line )); @@ -2365,9 +2364,9 @@ fn recurse_directories( Ok(rd) => { #[cfg(target_os = "windows")] if !listed_ancestors.insert(FileInformation::from_path( - path_data.p_buf, - path_data.must_dereference, - )) { + item.path(), + item.must_dereference, + )?) { state.out.flush()?; show!(LsError::AlreadyListedError(item.p_buf.clone())); continue; @@ -2538,22 +2537,22 @@ fn display_items( // `-Z`, `--context`: // Display the SELinux security context or '?' if none is found. When used with the `-l` // option, print the security context to the left of the size column. - let quoted = items.iter().any(|item| { - let name = locale_aware_escape_name(item.display_name(), config.quoting_style); - os_str_starts_with(&name, b"'") - }); - - let padding_collection = calculate_padding_collection(items, config, state); + #[cfg(unix)] + let should_display_additional_info = config.inode || config.alloc_size; + #[cfg(not(unix))] + let should_display_additional_info = config.alloc_size; match config.format { Format::Long => { + let padding_collection = calculate_padding_collection(items, config, state); + + let quoted = items.iter().any(|item| { + let name = locale_aware_escape_name(item.display_name(), config.quoting_style); + os_str_starts_with(&name, b"'") + }); + for item in items { - #[cfg(unix)] - let should_display_leading_info = config.inode || config.alloc_size; - #[cfg(not(unix))] - let should_display_leading_info = config.alloc_size; - - if should_display_leading_info { + if should_display_additional_info { let more_info = display_additional_leading_info(item, &padding_collection, config)?; @@ -2564,11 +2563,24 @@ fn display_items( } } _ => { + let padding_collection = if should_display_additional_info { + Some(calculate_padding_collection(items, config, state)) + } else { + None + }; + let mut names = - display_short_common(items, config, state, &padding_collection)?.into_iter(); + display_short_common(items, config, state, padding_collection.as_ref())? + .into_iter(); match config.format { Format::Columns => { + let quoted = items.iter().any(|item| { + let name = + locale_aware_escape_name(item.display_name(), config.quoting_style); + os_str_starts_with(&name, b"'") + }); + display_grid( names, config.width, @@ -2579,6 +2591,12 @@ fn display_items( )?; } Format::Across => { + let quoted = items.iter().any(|item| { + let name = + locale_aware_escape_name(item.display_name(), config.quoting_style); + os_str_starts_with(&name, b"'") + }); + display_grid( names, config.width, @@ -2625,16 +2643,17 @@ fn display_items( Ok(()) } +#[inline] fn display_short_common( items: &[PathData], config: &Config, state: &mut ListState, - padding_collection: &PaddingCollection, + padding_collection: Option<&PaddingCollection>, ) -> Result, Box> { let mut longest_context_len = 1; let prefix_context = if config.context { for item in items { - let context_len = item.security_context.len(); + let context_len = item.security_context(config).len(); longest_context_len = context_len.max(longest_context_len); } Some(longest_context_len) @@ -2649,17 +2668,8 @@ fn display_short_common( let mut names_vec = Vec::new(); for i in items { - #[cfg(unix)] - let should_display_leading_info = config.inode || config.alloc_size; - #[cfg(not(unix))] - let should_display_leading_info = config.alloc_size; - - let more_info = if should_display_leading_info { - Some(display_additional_leading_info( - i, - padding_collection, - config, - )?) + let more_info = if let Some(padding) = padding_collection { + Some(display_additional_leading_info(i, padding, config)?) } else { None }; From 8808a0475c5227f1326568bcf075133cc2e950dc Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Mon, 29 Sep 2025 10:21:26 -0500 Subject: [PATCH 58/76] Fix lint --- src/uu/ls/src/ls.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index fe7e6aafc1c..b123dd17a03 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2508,6 +2508,10 @@ fn display_additional_leading_info( result.push(' '); } } + #[cfg(not(unix))] + { + let _ = item.metadata(); + } if config.alloc_size { let s = if let Some(md) = item.metadata() { From c690deb0326052b12e1bdb90c5334fb6004fbcae Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Mon, 29 Sep 2025 12:39:26 -0500 Subject: [PATCH 59/76] Fix suggestions --- src/uu/ls/src/ls.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index b123dd17a03..e28534a20c1 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1776,7 +1776,7 @@ struct PathData { ft: OnceCell>, // can be used to avoid reading the filetype. Can be also called d_type: // https://www.gnu.org/software/libc/manual/html_node/Directory-Entries.html - de: RefCell>>, + de: RefCell>, security_context: OnceCell>, // Name of the file - will be empty for . or .. display_name: OsString, @@ -1830,11 +1830,11 @@ impl PathData { let md: OnceCell> = OnceCell::new(); let security_context: OnceCell> = OnceCell::new(); - let de: RefCell>> = if let Some(de) = dir_entry { + let de: RefCell> = if let Some(de) = dir_entry { if must_dereference { if let Ok(md_pb) = p_buf.metadata() { - md.get_or_init(|| Some(md_pb.clone())); ft.get_or_init(|| Some(md_pb.file_type())); + md.get_or_init(|| Some(md_pb)); } } @@ -1842,7 +1842,7 @@ impl PathData { ft.get_or_init(|| Some(ft_de)); } - RefCell::new(Some(de.into())) + RefCell::new(Some(de)) } else { RefCell::new(None) }; @@ -1863,7 +1863,7 @@ impl PathData { self.md .get_or_init(|| { if !self.must_dereference { - if let Some(dir_entry) = RefCell::take(&self.de).as_deref() { + if let Some(dir_entry) = RefCell::take(&self.de) { return dir_entry.metadata().ok(); } } @@ -1871,7 +1871,7 @@ impl PathData { match get_metadata_with_deref_opt(self.path(), self.must_dereference) { Err(err) => { // FIXME: A bit tricky to propagate the result here - let mut out: std::io::StdoutLock<'static> = stdout().lock(); + let mut out = stdout(); let _ = out.flush(); let errno = err.raw_os_error().unwrap_or(1i32); // a bad fd will throw an error when dereferenced, @@ -2508,10 +2508,6 @@ fn display_additional_leading_info( result.push(' '); } } - #[cfg(not(unix))] - { - let _ = item.metadata(); - } if config.alloc_size { let s = if let Some(md) = item.metadata() { From f5953d4343daf913a7c47de84382498a3bc1835c Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Mon, 29 Sep 2025 14:29:36 -0500 Subject: [PATCH 60/76] Test should be Unix only as inode is Unix only --- tests/by-util/test_ls.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index a9ab7d7df2e..05cf7ed9d6f 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -4385,14 +4385,17 @@ fn test_ls_dangling_symlinks() { .succeeds() .stdout_contains("dangle"); - scene - .ucmd() - .arg("-Li") - .arg("temp_dir") - .fails_with_code(1) - .stderr_contains("cannot access") - .stderr_contains("No such file or directory") - .stdout_contains(if cfg!(windows) { "dangle" } else { "? dangle" }); + #[cfg(unix)] + { + scene + .ucmd() + .arg("-Li") + .arg("temp_dir") + .fails_with_code(1) + .stderr_contains("cannot access") + .stderr_contains("No such file or directory") + .stdout_contains(if cfg!(windows) { "dangle" } else { "? dangle" }); + } scene .ucmd() From 41a5e3a4b00c1be86cdf89c830c003f094db25c8 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Mon, 29 Sep 2025 15:40:12 -0500 Subject: [PATCH 61/76] Deref if deref set late as late as possible in the rudimentary display modes --- src/uu/ls/src/ls.rs | 9 ++++++++- tests/by-util/test_ls.rs | 19 ++++++++----------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index e28534a20c1..014d193c252 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2089,7 +2089,7 @@ pub fn list(locs: Vec<&Path>, config: &Config) -> UResult<()> { for (pos, path_data) in dirs.iter().enumerate() { // Do read_dir call here to match GNU semantics by printing // read_dir errors before directory headings, names and totals - let read_dir = match fs::read_dir(path_data.path()) { + let read_dir = match std::fs::read_dir(path_data.path()) { Err(err) => { // flush stdout buffer before the error to preserve formatting and order state.out.flush()?; @@ -2128,6 +2128,7 @@ pub fn list(locs: Vec<&Path>, config: &Config) -> UResult<()> { recurse_directories(path_data, read_dir, config, &mut state, &mut dired)?; } + if config.dired && !config.hyperlink { dired::print_dired_output(config, &dired, &mut state.out)?; } @@ -2674,6 +2675,12 @@ fn display_short_common( None }; + // if there has been no other reason to request metadata + // request now if must_deref is set + if i.must_dereference { + i.metadata(); + } + // it's okay to set current column to zero which is used to decide // whether text will wrap or not, because when format is grid or // column ls will try to place the item name in a new line if it diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 05cf7ed9d6f..a9ab7d7df2e 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -4385,17 +4385,14 @@ fn test_ls_dangling_symlinks() { .succeeds() .stdout_contains("dangle"); - #[cfg(unix)] - { - scene - .ucmd() - .arg("-Li") - .arg("temp_dir") - .fails_with_code(1) - .stderr_contains("cannot access") - .stderr_contains("No such file or directory") - .stdout_contains(if cfg!(windows) { "dangle" } else { "? dangle" }); - } + scene + .ucmd() + .arg("-Li") + .arg("temp_dir") + .fails_with_code(1) + .stderr_contains("cannot access") + .stderr_contains("No such file or directory") + .stdout_contains(if cfg!(windows) { "dangle" } else { "? dangle" }); scene .ucmd() From 94e69d68e0afdf2d6f1e4e3d32f9a6a02dd3f540 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Mon, 29 Sep 2025 17:06:59 -0500 Subject: [PATCH 62/76] Add too many levels of slinks error --- src/uu/ls/src/ls.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 014d193c252..6cd5b304b1f 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -193,6 +193,7 @@ enum LsError { }, }, _ => match .1.raw_os_error().unwrap_or(1) { + 40 => translate!("ls-error-cannot-access-too-many-levels-of-symbolic-links", "path" => .0.to_string_lossy(), "error" => format!("{:?}", .1)), 9 => translate!("ls-error-cannot-open-directory-bad-descriptor", "path" => .0.to_string_lossy()), _ => translate!("ls-error-unknown-io-error", "path" => .0.to_string_lossy(), "error" => format!("{:?}", .1)), }, From bb8f1b0b319a122a5c8879102642be91ee124eaf Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Mon, 29 Sep 2025 17:21:13 -0500 Subject: [PATCH 63/76] Cleanup --- src/uu/ls/src/ls.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 6cd5b304b1f..834aafae293 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2676,12 +2676,6 @@ fn display_short_common( None }; - // if there has been no other reason to request metadata - // request now if must_deref is set - if i.must_dereference { - i.metadata(); - } - // it's okay to set current column to zero which is used to decide // whether text will wrap or not, because when format is grid or // column ls will try to place the item name in a new line if it @@ -3268,10 +3262,9 @@ fn display_item_name( } } - if config.format == Format::Long - && path.file_type().is_some_and(|ft| ft.is_symlink()) - && !path.must_dereference - { + let is_symlink = path.file_type().is_some_and(|ft| ft.is_symlink()); + + if config.format == Format::Long && is_symlink && !path.must_dereference { match path.path().read_link() { Ok(target_path) => { name.push(" -> "); @@ -3325,6 +3318,13 @@ fn display_item_name( } } + // if there has been no other reason to request metadata + // request now if must_deref is set, because we must print an error + // for any dangling links + if is_symlink && path.must_dereference { + path.metadata(); + } + // Prepend the security context to the `name` and adjust `width` in order // to get correct alignment from later calls to`display_grid()`. if config.context { From c1a18cfcde223df4911fccae97149b6ad4e569ad Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Mon, 29 Sep 2025 17:47:30 -0500 Subject: [PATCH 64/76] Cleanup Cleanup --- src/uu/ls/src/ls.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 834aafae293..601586ed91a 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1785,6 +1785,7 @@ struct PathData { p_buf: PathBuf, must_dereference: bool, command_line: bool, + is_symlink: bool, } impl PathData { @@ -1830,6 +1831,7 @@ impl PathData { let ft: OnceCell> = OnceCell::new(); let md: OnceCell> = OnceCell::new(); let security_context: OnceCell> = OnceCell::new(); + let mut is_symlink = false; let de: RefCell> = if let Some(de) = dir_entry { if must_dereference { @@ -1841,6 +1843,7 @@ impl PathData { if let Ok(ft_de) = de.file_type() { ft.get_or_init(|| Some(ft_de)); + is_symlink = ft_de.is_symlink(); } RefCell::new(Some(de)) @@ -1857,6 +1860,7 @@ impl PathData { p_buf, must_dereference, command_line, + is_symlink, } } @@ -1905,7 +1909,7 @@ impl PathData { fn is_dangling_link(&self) -> bool { // deref enabled, self is real dir entry, self has metadata associated with link, but not with target - self.must_dereference && self.file_type().is_none() && self.metadata().is_none() + self.must_dereference && self.is_symlink && self.metadata().is_none() } #[cfg(unix)] @@ -1975,6 +1979,7 @@ impl Clone for PathData { must_dereference: self.must_dereference, security_context: self.security_context.clone(), command_line: self.command_line, + is_symlink: self.is_symlink, } } } @@ -3262,9 +3267,7 @@ fn display_item_name( } } - let is_symlink = path.file_type().is_some_and(|ft| ft.is_symlink()); - - if config.format == Format::Long && is_symlink && !path.must_dereference { + if config.format == Format::Long && path.is_symlink && !path.must_dereference { match path.path().read_link() { Ok(target_path) => { name.push(" -> "); @@ -3321,7 +3324,7 @@ fn display_item_name( // if there has been no other reason to request metadata // request now if must_deref is set, because we must print an error // for any dangling links - if is_symlink && path.must_dereference { + if path.must_dereference && path.is_symlink { path.metadata(); } From 8b3aa8217a933863dfbde2d14a6aa413d4774ba8 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Mon, 29 Sep 2025 19:05:11 -0500 Subject: [PATCH 65/76] Revert "Cleanup" This reverts commit c1a18cfcde223df4911fccae97149b6ad4e569ad. --- src/uu/ls/src/ls.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 601586ed91a..834aafae293 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1785,7 +1785,6 @@ struct PathData { p_buf: PathBuf, must_dereference: bool, command_line: bool, - is_symlink: bool, } impl PathData { @@ -1831,7 +1830,6 @@ impl PathData { let ft: OnceCell> = OnceCell::new(); let md: OnceCell> = OnceCell::new(); let security_context: OnceCell> = OnceCell::new(); - let mut is_symlink = false; let de: RefCell> = if let Some(de) = dir_entry { if must_dereference { @@ -1843,7 +1841,6 @@ impl PathData { if let Ok(ft_de) = de.file_type() { ft.get_or_init(|| Some(ft_de)); - is_symlink = ft_de.is_symlink(); } RefCell::new(Some(de)) @@ -1860,7 +1857,6 @@ impl PathData { p_buf, must_dereference, command_line, - is_symlink, } } @@ -1909,7 +1905,7 @@ impl PathData { fn is_dangling_link(&self) -> bool { // deref enabled, self is real dir entry, self has metadata associated with link, but not with target - self.must_dereference && self.is_symlink && self.metadata().is_none() + self.must_dereference && self.file_type().is_none() && self.metadata().is_none() } #[cfg(unix)] @@ -1979,7 +1975,6 @@ impl Clone for PathData { must_dereference: self.must_dereference, security_context: self.security_context.clone(), command_line: self.command_line, - is_symlink: self.is_symlink, } } } @@ -3267,7 +3262,9 @@ fn display_item_name( } } - if config.format == Format::Long && path.is_symlink && !path.must_dereference { + let is_symlink = path.file_type().is_some_and(|ft| ft.is_symlink()); + + if config.format == Format::Long && is_symlink && !path.must_dereference { match path.path().read_link() { Ok(target_path) => { name.push(" -> "); @@ -3324,7 +3321,7 @@ fn display_item_name( // if there has been no other reason to request metadata // request now if must_deref is set, because we must print an error // for any dangling links - if path.must_dereference && path.is_symlink { + if is_symlink && path.must_dereference { path.metadata(); } From ee29ce61849962f9e068a6546f074e0f1ffcbc41 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Mon, 29 Sep 2025 20:07:03 -0500 Subject: [PATCH 66/76] Increase initial capacity in hopes of speeding up small scale tests --- src/uu/ls/src/ls.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 834aafae293..c4a7dbf8ea1 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2257,7 +2257,7 @@ fn enter_directory( entries_stack: &mut Vec, ) -> UResult<()> { // Create vec of entries with initial dot files - let mut entries: Vec = if config.files == Files::All { + let mut new_entries: Vec = if config.files == Files::All { vec![ PathData::new( path_data.path().to_path_buf(), @@ -2292,24 +2292,24 @@ fn enter_directory( if should_display(&dir_entry, config) { let entry_path_data = PathData::new(dir_entry.path(), Some(dir_entry), None, config, false); - entries.push(entry_path_data); + new_entries.push(entry_path_data); } } - sort_entries(&mut entries, config); + sort_entries(&mut new_entries, config); // Print total after any error display if config.format == Format::Long || config.alloc_size { - let total = return_total(&entries, config, &mut state.out)?; + let total = return_total(&new_entries, config, &mut state.out)?; write!(state.out, "{}", total.as_str())?; if config.dired { dired::add_total(dired, total.len()); } } - display_items(&entries, config, state, dired)?; + display_items(&new_entries, config, state, dired)?; - let new_dirs = entries + let new_dirs = new_entries .into_iter() .skip(if config.files == Files::All { 2 } else { 0 }) .filter(|p| p.file_type().is_some_and(|ft| ft.is_dir())) @@ -2328,7 +2328,7 @@ fn recurse_directories( state: &mut ListState, dired: &mut DiredOutput, ) -> UResult<()> { - let mut entries_stack: Vec = Vec::new(); + let mut entries_stack: Vec = Vec::with_capacity(64); enter_directory( path_data, From 7a0c6afa75107774b11db8c0027f24ce14122170 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Mon, 29 Sep 2025 20:32:28 -0500 Subject: [PATCH 67/76] Boxed is slightly faster --- src/uu/ls/src/ls.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index c4a7dbf8ea1..1e1ce6db4ba 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1777,7 +1777,7 @@ struct PathData { ft: OnceCell>, // can be used to avoid reading the filetype. Can be also called d_type: // https://www.gnu.org/software/libc/manual/html_node/Directory-Entries.html - de: RefCell>, + de: RefCell>>, security_context: OnceCell>, // Name of the file - will be empty for . or .. display_name: OsString, @@ -1831,19 +1831,19 @@ impl PathData { let md: OnceCell> = OnceCell::new(); let security_context: OnceCell> = OnceCell::new(); - let de: RefCell> = if let Some(de) = dir_entry { + let de: RefCell>> = if let Some(de) = dir_entry { if must_dereference { if let Ok(md_pb) = p_buf.metadata() { ft.get_or_init(|| Some(md_pb.file_type())); md.get_or_init(|| Some(md_pb)); } + } else { + if let Ok(ft_de) = de.file_type() { + ft.get_or_init(|| Some(ft_de)); + } } - if let Ok(ft_de) = de.file_type() { - ft.get_or_init(|| Some(ft_de)); - } - - RefCell::new(Some(de)) + RefCell::new(Some(de.into())) } else { RefCell::new(None) }; @@ -1864,7 +1864,7 @@ impl PathData { self.md .get_or_init(|| { if !self.must_dereference { - if let Some(dir_entry) = RefCell::take(&self.de) { + if let Some(dir_entry) = self.de.take().as_deref() { return dir_entry.metadata().ok(); } } From b578125600719811b11553124cf15a732641267c Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Mon, 29 Sep 2025 20:37:05 -0500 Subject: [PATCH 68/76] Fix lints --- src/uu/ls/src/ls.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 1e1ce6db4ba..f676536f652 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1837,10 +1837,8 @@ impl PathData { ft.get_or_init(|| Some(md_pb.file_type())); md.get_or_init(|| Some(md_pb)); } - } else { - if let Ok(ft_de) = de.file_type() { - ft.get_or_init(|| Some(ft_de)); - } + } else if let Ok(ft_de) = de.file_type() { + ft.get_or_init(|| Some(ft_de)); } RefCell::new(Some(de.into())) From c2b80326d9534f20da63c9dd2b0b56f7cea3999a Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Tue, 30 Sep 2025 13:32:46 -0500 Subject: [PATCH 69/76] Perhaps fix test --- src/uu/ls/src/ls.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index f676536f652..c73f2dc46b0 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -193,7 +193,6 @@ enum LsError { }, }, _ => match .1.raw_os_error().unwrap_or(1) { - 40 => translate!("ls-error-cannot-access-too-many-levels-of-symbolic-links", "path" => .0.to_string_lossy(), "error" => format!("{:?}", .1)), 9 => translate!("ls-error-cannot-open-directory-bad-descriptor", "path" => .0.to_string_lossy()), _ => translate!("ls-error-unknown-io-error", "path" => .0.to_string_lossy(), "error" => format!("{:?}", .1)), }, From a8f49368a46e909ba8269d0858ca3f6bd1a4e77c Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 1 Oct 2025 01:49:25 -0500 Subject: [PATCH 70/76] Revert to pass GNU test --- src/uu/ls/src/ls.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index c73f2dc46b0..2b22e99f3aa 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2673,6 +2673,13 @@ fn display_short_common( None }; + // if there has been no other reason to request metadata + // request now if must_deref is set, because we must print an error + // for any dangling links + if i.must_dereference { + i.metadata(); + } + // it's okay to set current column to zero which is used to decide // whether text will wrap or not, because when format is grid or // column ls will try to place the item name in a new line if it @@ -3259,9 +3266,10 @@ fn display_item_name( } } - let is_symlink = path.file_type().is_some_and(|ft| ft.is_symlink()); - - if config.format == Format::Long && is_symlink && !path.must_dereference { + if config.format == Format::Long + && path.file_type().is_some_and(|ft| ft.is_symlink()) + && !path.must_dereference + { match path.path().read_link() { Ok(target_path) => { name.push(" -> "); @@ -3315,13 +3323,6 @@ fn display_item_name( } } - // if there has been no other reason to request metadata - // request now if must_deref is set, because we must print an error - // for any dangling links - if is_symlink && path.must_dereference { - path.metadata(); - } - // Prepend the security context to the `name` and adjust `width` in order // to get correct alignment from later calls to`display_grid()`. if config.context { From f1dc38b6b9b3ddd64c7dd37dcab0f224f74709d2 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 1 Oct 2025 16:25:48 -0500 Subject: [PATCH 71/76] Fix GNU test, add back correctness changes --- src/uu/ls/src/ls.rs | 48 +++++++++++-------- src/uu/mv/src/mv.rs | 2 +- src/uucore/src/lib/features/fsxattr.rs | 65 ++++++++++++++++++++------ 3 files changed, 81 insertions(+), 34 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 5dc881215ea..7b44f4ec52f 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1832,9 +1832,16 @@ impl PathData { let de: RefCell>> = if let Some(de) = dir_entry { if must_dereference { - if let Ok(md_pb) = p_buf.metadata() { - ft.get_or_init(|| Some(md_pb.file_type())); - md.get_or_init(|| Some(md_pb)); + match p_buf.metadata() { + Ok(pb_md) => { + ft.get_or_init(|| Some(pb_md.file_type())); + md.get_or_init(|| Some(pb_md)); + } + Err(err) => { + Self::handle_metadata_error(&p_buf, command_line, err); + ft.get_or_init(|| None); + md.get_or_init(|| None); + } } } else if let Ok(ft_de) = de.file_type() { ft.get_or_init(|| Some(ft_de)); @@ -1868,24 +1875,20 @@ impl PathData { match get_metadata_with_deref_opt(self.path(), self.must_dereference) { Err(err) => { - // FIXME: A bit tricky to propagate the result here - let mut out = stdout(); - let _ = out.flush(); let errno = err.raw_os_error().unwrap_or(1i32); + // a bad fd will throw an error when dereferenced, // but GNU will not throw an error until a bad fd "dir" // is entered, here we match that GNU behavior, by handing // back the non-dereferenced metadata upon an EBADF if self.must_dereference && errno == 9i32 { - if let Ok(file) = self.path().read_link() { - return file.symlink_metadata().ok(); + if let Ok(target) = self.path().read_link() { + return target.symlink_metadata().ok(); } } - show!(LsError::IOErrorContext( - self.path().to_path_buf(), - err, - self.command_line - )); + + Self::handle_metadata_error(self.path(), self.command_line, err); + None } Ok(md) => Some(md), @@ -1894,6 +1897,18 @@ impl PathData { .as_ref() } + fn handle_metadata_error(path: &Path, command_line: bool, err: std::io::Error) { + // FIXME: A bit tricky to propagate the result here + let mut out = stdout(); + let _ = out.flush(); + + show!(LsError::IOErrorContext( + path.to_path_buf(), + err, + command_line + )); + } + fn file_type(&self) -> Option<&FileType> { self.ft .get_or_init(|| self.metadata().map(|md| md.file_type())) @@ -2673,13 +2688,6 @@ fn display_short_common( None }; - // if there has been no other reason to request metadata - // request now if must_deref is set, because we must print an error - // for any dangling links - if i.must_dereference { - i.metadata(); - } - // it's okay to set current column to zero which is used to decide // whether text will wrap or not, because when format is grid or // column ls will try to place the item name in a new line if it diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index 820b912401a..3425eb5eb15 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -950,7 +950,7 @@ fn rename_dir_fallback( }; #[cfg(all(unix, not(any(target_os = "macos", target_os = "redox"))))] - let xattrs = fsxattr::retrieve_xattrs(from).unwrap_or_else(|_| HashMap::new()); + let xattrs = fsxattr::retrieve_xattrs(from, false).unwrap_or_else(|_| HashMap::new()); // Use directory copying (with or without hardlink support) let result = copy_dir_contents( diff --git a/src/uucore/src/lib/features/fsxattr.rs b/src/uucore/src/lib/features/fsxattr.rs index 1f1356ee5f6..3e351d7e92a 100644 --- a/src/uucore/src/lib/features/fsxattr.rs +++ b/src/uucore/src/lib/features/fsxattr.rs @@ -7,9 +7,13 @@ //! Set of functions to manage xattr on files and dirs use std::collections::HashMap; -use std::ffi::OsString; +use std::ffi::{OsStr, OsString}; use std::path::Path; +pub static POSIX_ACL_ACCESS_KEY: &str = "system.posix_acl_access"; +pub static POSIX_ACL_DEFAULT_KEY: &str = "system.posix_acl_default"; +pub static SET_CAPABILITY_KEY: &str = "security.capability"; + /// Copies extended attributes (xattrs) from one file or directory to another. /// /// # Arguments @@ -38,9 +42,19 @@ pub fn copy_xattrs>(source: P, dest: P) -> std::io::Result<()> { /// # Returns /// /// A result containing a HashMap of attributes names and values, or an error. -pub fn retrieve_xattrs>(source: P) -> std::io::Result>> { +pub fn retrieve_xattrs>( + source: P, + must_dereference: bool, +) -> std::io::Result>> { let mut attrs = HashMap::new(); - for attr_name in xattr::list(&source)? { + + let iter = if must_dereference { + xattr::list_deref(&source)? + } else { + xattr::list(&source)? + }; + + for attr_name in iter { if let Some(value) = xattr::get(&source, &attr_name)? { attrs.insert(attr_name, value); } @@ -79,10 +93,32 @@ pub fn apply_xattrs>( /// `true` if the file has extended attributes (indicating an ACL), `false` otherwise. pub fn has_acl>(file: P) -> bool { // don't use exacl here, it is doing more getxattr call then needed - xattr::list_deref(file).is_ok_and(|acl| { - // if we have extra attributes, we have an acl - acl.count() > 0 - }) + xattr::get_deref(&file, OsStr::new(POSIX_ACL_ACCESS_KEY)) + .ok() + .flatten() + .or_else(|| { + xattr::get_deref(&file, POSIX_ACL_DEFAULT_KEY) + .ok() + .flatten() + }) + .is_some_and(|vec| !vec.is_empty()) +} + +/// Checks if a file has an Capability set based on its extended attributes. +/// +/// # Arguments +/// +/// * `file` - A reference to the path of the file. +/// +/// # Returns +/// +/// `true` if the file has a capability extended attribute, `false` otherwise. +pub fn has_capability>(file: P) -> bool { + // don't use exacl here, it is doing more getxattr call then needed + xattr::get_deref(&file, OsStr::new(SET_CAPABILITY_KEY)) + .ok() + .flatten() + .is_some_and(|vec| !vec.is_empty()) } /// Returns the permissions bits of a file or directory which has Access Control List (ACL) entries based on its @@ -101,9 +137,9 @@ pub fn get_acl_perm_bits_from_xattr>(source: P) -> u32 { // Only default acl entries get inherited by objects under the path i.e. if child directories // will have their permissions modified. - if let Ok(entries) = retrieve_xattrs(source) { + if let Ok(entries) = retrieve_xattrs(source, true) { let mut perm: u32 = 0; - if let Some(value) = entries.get(&OsString::from("system.posix_acl_default")) { + if let Some(value) = entries.get(OsStr::new(POSIX_ACL_DEFAULT_KEY)) { // value is xattr byte vector // value follows a starts with a 4 byte header, and then has posix_acl_entries, each // posix_acl_entry is separated by a u32 sequence i.e. 0xFFFFFFFF @@ -177,7 +213,7 @@ mod tests { test_xattrs.insert(OsString::from(test_attr), test_value.to_vec()); apply_xattrs(&file_path, test_xattrs).unwrap(); - let retrieved_xattrs = retrieve_xattrs(&file_path).unwrap(); + let retrieved_xattrs = retrieve_xattrs(&file_path, true).unwrap(); assert!(retrieved_xattrs.contains_key(OsString::from(test_attr).as_os_str())); assert_eq!( retrieved_xattrs @@ -242,9 +278,12 @@ mod tests { assert!(!has_acl(&file_path)); - let test_attr = "user.test_acl"; - let test_value = b"test value"; - xattr::set(&file_path, test_attr, test_value).unwrap(); + let test_attr = "system.posix_acl_access"; + let test_value = "invalid_test_value"; + // perhaps can't set actual ACL in test environment? if so, return early + let Ok(_) = xattr::set(&file_path, test_attr, test_value.as_bytes()) else { + return; + }; assert!(has_acl(&file_path)); } From 0409f0a0e0694b77c471c877e19370a80214c7eb Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 1 Oct 2025 17:21:20 -0500 Subject: [PATCH 72/76] Fix cap test --- src/uu/ls/src/colors.rs | 2 +- src/uucore/src/lib/features/fsxattr.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/uu/ls/src/colors.rs b/src/uu/ls/src/colors.rs index f7acb2f5a30..8386245289b 100644 --- a/src/uu/ls/src/colors.rs +++ b/src/uu/ls/src/colors.rs @@ -163,7 +163,7 @@ pub(crate) fn color_name( let has_capabilities = if capabilities.is_none() { false } else { - uucore::fsxattr::has_acl(path.p_buf.as_path()) + uucore::fsxattr::has_capability(path.p_buf.as_path()) }; // If the file has capabilities, use a specific style for `ca` (capabilities) diff --git a/src/uucore/src/lib/features/fsxattr.rs b/src/uucore/src/lib/features/fsxattr.rs index 3e351d7e92a..b60e9576f8d 100644 --- a/src/uucore/src/lib/features/fsxattr.rs +++ b/src/uucore/src/lib/features/fsxattr.rs @@ -93,7 +93,7 @@ pub fn apply_xattrs>( /// `true` if the file has extended attributes (indicating an ACL), `false` otherwise. pub fn has_acl>(file: P) -> bool { // don't use exacl here, it is doing more getxattr call then needed - xattr::get_deref(&file, OsStr::new(POSIX_ACL_ACCESS_KEY)) + xattr::get_deref(&file, POSIX_ACL_ACCESS_KEY) .ok() .flatten() .or_else(|| { @@ -115,7 +115,7 @@ pub fn has_acl>(file: P) -> bool { /// `true` if the file has a capability extended attribute, `false` otherwise. pub fn has_capability>(file: P) -> bool { // don't use exacl here, it is doing more getxattr call then needed - xattr::get_deref(&file, OsStr::new(SET_CAPABILITY_KEY)) + xattr::get_deref(&file, SET_CAPABILITY_KEY) .ok() .flatten() .is_some_and(|vec| !vec.is_empty()) From 64dcdbd440d2371d586a9371d18ef2eb04c85541 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 1 Oct 2025 17:36:59 -0500 Subject: [PATCH 73/76] Fix GNU test --- src/uu/ls/src/ls.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 7b44f4ec52f..c29484d0ef8 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1831,7 +1831,7 @@ impl PathData { let security_context: OnceCell> = OnceCell::new(); let de: RefCell>> = if let Some(de) = dir_entry { - if must_dereference { + if must_dereference && command_line { match p_buf.metadata() { Ok(pb_md) => { ft.get_or_init(|| Some(pb_md.file_type())); From 70c329d5311f90211b1d6935a140f84a3f5ce827 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 1 Oct 2025 17:41:07 -0500 Subject: [PATCH 74/76] Fix the fix --- src/uu/ls/src/ls.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index c29484d0ef8..f9ebeb6a6a1 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1831,17 +1831,18 @@ impl PathData { let security_context: OnceCell> = OnceCell::new(); let de: RefCell>> = if let Some(de) = dir_entry { - if must_dereference && command_line { + if must_dereference { match p_buf.metadata() { Ok(pb_md) => { ft.get_or_init(|| Some(pb_md.file_type())); md.get_or_init(|| Some(pb_md)); } - Err(err) => { + Err(err) if command_line => { Self::handle_metadata_error(&p_buf, command_line, err); ft.get_or_init(|| None); md.get_or_init(|| None); } + Err(_) => (), } } else if let Ok(ft_de) = de.file_type() { ft.get_or_init(|| Some(ft_de)); From acf659bda382790931b42b545c4bfd11a99b3228 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 1 Oct 2025 18:46:01 -0500 Subject: [PATCH 75/76] FIx GNU test --- src/uu/ls/src/ls.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index f9ebeb6a6a1..177ba41e5aa 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2322,13 +2322,15 @@ fn enter_directory( display_items(&new_entries, config, state, dired)?; - let new_dirs = new_entries - .into_iter() - .skip(if config.files == Files::All { 2 } else { 0 }) - .filter(|p| p.file_type().is_some_and(|ft| ft.is_dir())) - .rev(); + if config.recursive { + let new_dirs = new_entries + .into_iter() + .skip(if config.files == Files::All { 2 } else { 0 }) + .filter(|p| p.file_type().is_some_and(|ft| ft.is_dir())) + .rev(); - entries_stack.extend(new_dirs); + entries_stack.extend(new_dirs); + } Ok(()) } From 5cacbafe48df37ce9a4422855787f6fb0c7d419d Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+kimono-koans@users.noreply.github.com> Date: Wed, 1 Oct 2025 19:03:51 -0500 Subject: [PATCH 76/76] Keeping the windows behavior, as is, conflicts with a GNU test --- tests/by-util/test_ls.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index a9ab7d7df2e..45c8a82658f 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -4385,14 +4385,17 @@ fn test_ls_dangling_symlinks() { .succeeds() .stdout_contains("dangle"); - scene - .ucmd() - .arg("-Li") - .arg("temp_dir") - .fails_with_code(1) - .stderr_contains("cannot access") - .stderr_contains("No such file or directory") - .stdout_contains(if cfg!(windows) { "dangle" } else { "? dangle" }); + #[cfg(unix)] + { + scene + .ucmd() + .arg("-Li") + .arg("temp_dir") + .fails_with_code(1) + .stderr_contains("cannot access") + .stderr_contains("No such file or directory") + .stdout_contains("? dangle"); + } scene .ucmd()