diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 356e4c0f5..079dbfb94 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -29,7 +29,7 @@ use std::{ error::Error, fmt::Display, fs::{self, DirEntry, FileType, Metadata}, - io::{stdout, BufWriter, Stdout, Write}, + io::{stdout, BufWriter, ErrorKind, Stdout, Write}, path::{Path, PathBuf}, time::{SystemTime, UNIX_EPOCH}, }; @@ -142,14 +142,16 @@ const DEFAULT_TERM_WIDTH: u16 = 80; #[derive(Debug)] enum LsError { InvalidLineWidth(String), - NoMetadata(PathBuf), + IOError(std::io::Error), + IOErrorContext(std::io::Error, PathBuf), } impl UError for LsError { fn code(&self) -> i32 { match self { LsError::InvalidLineWidth(_) => 2, - LsError::NoMetadata(_) => 1, + LsError::IOError(_) => 1, + LsError::IOErrorContext(_, _) => 1, } } } @@ -160,7 +162,39 @@ impl Display for LsError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { LsError::InvalidLineWidth(s) => write!(f, "invalid line width: {}", s.quote()), - LsError::NoMetadata(p) => write!(f, "could not open file: {}", p.quote()), + LsError::IOError(e) => write!(f, "general io error: {}", e), + LsError::IOErrorContext(e, p) => { + let error_kind = e.kind(); + + match error_kind { + ErrorKind::NotFound => write!( + f, + "cannot access '{}': No such file or directory", + p.to_string_lossy() + ), + ErrorKind::PermissionDenied => { + if p.is_dir() { + write!( + f, + "cannot open directory '{}': Permission denied", + p.to_string_lossy() + ) + } else { + write!( + f, + "cannot open file '{}': Permission denied", + p.to_string_lossy() + ) + } + } + _ => write!( + f, + "unknown io error: '{:?}', '{:?}'", + p.to_string_lossy(), + e + ), + } + } } } } @@ -259,6 +293,8 @@ struct LongFormat { } struct PaddingCollection { + #[cfg(unix)] + longest_inode_len: usize, longest_link_count_len: usize, longest_uname_len: usize, longest_group_len: usize, @@ -633,6 +669,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let matches = app.get_matches_from(args); let config = Config::from(&matches)?; + let locs = matches .values_of_os(options::PATHS) .map(|v| v.map(Path::new).collect()) @@ -1205,6 +1242,7 @@ only ignore '.' and '..'.", /// Represents a Path along with it's associated data /// Any data that will be reused several times makes sense to be added to this structure /// Caching data here helps eliminate redundant syscalls to fetch same information +#[derive(Debug)] struct PathData { // Result got from symlink_metadata() or metadata() based on config md: OnceCell>, @@ -1253,6 +1291,7 @@ impl PathData { } Dereference::None => false, }; + let ft = match file_type { Some(ft) => OnceCell::from(ft.ok()), None => OnceCell::new(), @@ -1290,24 +1329,22 @@ impl PathData { fn list(locs: Vec<&Path>, config: Config) -> UResult<()> { let mut files = Vec::::new(); let mut dirs = Vec::::new(); - let mut out = BufWriter::new(stdout()); + let initial_locs_len = locs.len(); - for loc in &locs { - let p = PathBuf::from(loc); - let path_data = PathData::new(p, None, None, &config, true); + for loc in locs { + let path_data = PathData::new(PathBuf::from(loc), None, None, &config, true); + // Getting metadata here is no big deal as it's just the CWD + // and we really just want to know if the strings exist as files/dirs if path_data.md().is_none() { - // FIXME: Would be nice to use the actual error instead of hardcoding it - // Presumably other errors can happen too? - show_error!( - "cannot access {}: No such file or directory", - path_data.p_buf.quote() - ); - set_exit_code(1); - // We found an error, no need to continue the execution + let _ = out.flush(); + show!(LsError::IOErrorContext( + std::io::Error::new(ErrorKind::NotFound, "NotFound"), + path_data.p_buf + )); continue; - } + }; let show_dir_contents = match path_data.file_type() { Some(ft) => !config.directory && ft.is_dir(), @@ -1323,16 +1360,14 @@ fn list(locs: Vec<&Path>, config: Config) -> UResult<()> { files.push(path_data); } } + sort_entries(&mut files, &config); + sort_entries(&mut dirs, &config); + display_items(&files, &config, &mut out); - sort_entries(&mut dirs, &config); - for dir in dirs { - if locs.len() > 1 || config.recursive { - // FIXME: This should use the quoting style and propagate errors - let _ = writeln!(out, "\n{}:", dir.p_buf.display()); - } - enter_directory(&dir, &config, &mut out); + for dir in &dirs { + enter_directory(dir, &config, initial_locs_len, &mut out); } Ok(()) @@ -1347,9 +1382,7 @@ fn sort_entries(entries: &mut Vec, config: &Config) { .unwrap_or(UNIX_EPOCH), ) }), - Sort::Size => { - entries.sort_by_key(|k| Reverse(k.md().as_ref().map(|md| md.len()).unwrap_or(0))) - } + Sort::Size => entries.sort_by_key(|k| Reverse(k.md().map(|md| md.len()).unwrap_or(0))), // The default sort in GNU ls is case insensitive Sort::Name => entries.sort_by(|a, b| a.display_name.cmp(&b.display_name)), Sort::Version => entries @@ -1376,7 +1409,7 @@ fn is_hidden(file_path: &DirEntry) -> bool { let attr = metadata.file_attributes(); (attr & 0x2) > 0 } - #[cfg(unix)] + #[cfg(not(windows))] { file_path .file_name() @@ -1399,43 +1432,90 @@ fn should_display(entry: &DirEntry, config: &Config) -> bool { }; continue; } + // else default to display true } -fn enter_directory(dir: &PathData, config: &Config, out: &mut BufWriter) { - let mut entries: Vec<_> = if config.files == Files::All { +fn enter_directory( + dir: &PathData, + config: &Config, + initial_locs_len: usize, + out: &mut BufWriter, +) { + // Create vec of entries with initial dot files + let mut entries: Vec = if config.files == Files::All { vec![ - PathData::new( - dir.p_buf.clone(), - Some(Ok(*dir.file_type().unwrap())), - Some(".".into()), - config, - false, - ), + PathData::new(dir.p_buf.clone(), None, Some(".".into()), config, false), PathData::new(dir.p_buf.join(".."), None, Some("..".into()), config, false), ] } else { vec![] }; - let mut temp: Vec<_> = crash_if_err!(1, fs::read_dir(&dir.p_buf)) - .map(|res| crash_if_err!(1, res)) - .filter(|res| should_display(res, config)) - .map(|res| { - PathData::new( - DirEntry::path(&res), - Some(res.file_type()), - None, - config, - false, - ) - }) - .collect(); + // Convert those entries to the PathData struct + let mut vec_path_data = Vec::new(); - sort_entries(&mut temp, config); + // check for errors early, and ignore entries with errors + let read_dir = match fs::read_dir(&dir.p_buf) { + Err(err) => { + // flush buffer because the error may get printed in the wrong order + let _ = out.flush(); + show!(LsError::IOErrorContext(err, dir.p_buf.clone())); + return; + } + Ok(res) => res, + }; - entries.append(&mut temp); + for entry in read_dir { + let unwrapped = match entry { + Ok(path) => path, + Err(error) => { + let _ = out.flush(); + show!(LsError::IOError(error)); + continue; + } + }; + if should_display(&unwrapped, config) { + // why check the DirEntry file_type()? B/c the call is + // nearly free compared to a metadata() or file_type() call on a dir/file + let path_data = match unwrapped.file_type() { + Err(_err) => { + let _ = out.flush(); + show!(LsError::IOErrorContext( + std::io::Error::new(ErrorKind::NotFound, "NotFound"), + unwrapped.path() + )); + continue; + } + Ok(dir_ft) => { + let res = + PathData::new(unwrapped.path(), Some(Ok(dir_ft)), None, config, false); + if dir_ft.is_symlink() && res.md().is_none() { + let _ = out.flush(); + show!(LsError::IOErrorContext( + std::io::Error::new(ErrorKind::NotFound, "NotFound"), + unwrapped.path() + )); + } + res + } + }; + vec_path_data.push(path_data); + } + } + + sort_entries(&mut vec_path_data, config); + entries.append(&mut vec_path_data); + + // Print dir heading - name... + if initial_locs_len > 1 || config.recursive { + let _ = writeln!(out, "\n{}:", dir.p_buf.display()); + } + // ...and total + if config.format == Format::Long { + display_total(&entries, config, out); + } display_items(&entries, config, out); @@ -1445,21 +1525,23 @@ fn enter_directory(dir: &PathData, config: &Config, out: &mut BufWriter) .skip(if config.files == Files::All { 2 } else { 0 }) .filter(|p| p.file_type().map(|ft| ft.is_dir()).unwrap_or(false)) { - let _ = writeln!(out, "\n{}:", e.p_buf.display()); - enter_directory(e, config, out); + enter_directory(e, config, 0, out); } } } -fn get_metadata(entry: &Path, dereference: bool) -> std::io::Result { +fn get_metadata(p_buf: &Path, dereference: bool) -> std::io::Result { if dereference { - entry.metadata() + p_buf.metadata() } else { - entry.symlink_metadata() + p_buf.symlink_metadata() } } -fn display_dir_entry_size(entry: &PathData, config: &Config) -> (usize, usize, usize, usize) { +fn display_dir_entry_size( + entry: &PathData, + config: &Config, +) -> (usize, usize, usize, usize, usize) { // TODO: Cache/memorize the display_* results so we don't have to recalculate them. if let Some(md) = entry.md() { ( @@ -1467,9 +1549,10 @@ fn display_dir_entry_size(entry: &PathData, config: &Config) -> (usize, usize, u display_uname(md, config).len(), display_group(md, config).len(), display_size_or_rdev(md, config).len(), + display_inode(md).len(), ) } else { - (0, 0, 0, 0) + (0, 0, 0, 0, 0) } } @@ -1481,12 +1564,34 @@ fn pad_right(string: &str, count: usize) -> String { format!("{:) { + let mut total_size = 0; + for item in items { + total_size += item + .md() + .as_ref() + .map_or(0, |md| get_block_size(md, config)); + } + let _ = writeln!(out, "total {}", display_size(total_size, config)); +} + fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter) { // `-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. if config.format == Format::Long { + #[cfg(unix)] + let ( + mut longest_inode_len, + mut longest_link_count_len, + mut longest_uname_len, + mut longest_group_len, + mut longest_context_len, + mut longest_size_len, + ) = (1, 1, 1, 1, 1, 1); + + #[cfg(not(unix))] let ( mut longest_link_count_len, mut longest_uname_len, @@ -1494,11 +1599,27 @@ fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter 0 { - let _ = writeln!(out, "total {}", display_size(total_size, config)); } for item in items { display_item_long( item, PaddingCollection { + #[cfg(unix)] + longest_inode_len, longest_link_count_len, longest_uname_len, longest_group_len, @@ -1540,9 +1658,12 @@ fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter = items .iter() - .filter_map(|i| display_file_name(i, config, prefix_context)); + .map(|i| display_file_name(i, config, prefix_context, out)) + .collect::>() + .into_iter(); match config.format { Format::Columns => display_grid(names, config.width, Direction::TopToBottom, out), @@ -1671,85 +1792,138 @@ fn display_grid( /// longest_size_len: usize, /// ``` /// that decide the maximum possible character count of each field. +#[allow(clippy::write_literal)] fn display_item_long( item: &PathData, padding: PaddingCollection, config: &Config, out: &mut BufWriter, ) { - let md = match item.md() { - None => { - show!(LsError::NoMetadata(item.p_buf.clone())); - return; + if let Some(md) = item.md() { + #[cfg(unix)] + { + if config.inode { + let _ = write!( + out, + "{} ", + pad_left(&get_inode(md), padding.longest_inode_len), + ); + } } - Some(md) => md, - }; - #[cfg(unix)] - { - if config.inode { - let _ = write!(out, "{} ", get_inode(md)); + let _ = write!( + out, + "{}{} {}", + display_permissions(md, true), + if item.security_context.len() > 1 { + // GNU `ls` uses a "." character to indicate a file with a security context, + // but not other alternate access method. + "." + } else { + "" + }, + pad_left(&display_symlink_count(md), padding.longest_link_count_len), + ); + + if config.long.owner { + let _ = write!( + out, + " {}", + pad_right(&display_uname(md, config), padding.longest_uname_len), + ); } - } - let _ = write!( - out, - "{}{} {}", - display_permissions(md, true), - if item.security_context.len() > 1 { - // GNU `ls` uses a "." character to indicate a file with a security context, - // but not other alternate access method. - "." - } else { - "" - }, - pad_left(&display_symlink_count(md), padding.longest_link_count_len), - ); + if config.long.group { + let _ = write!( + out, + " {}", + pad_right(&display_group(md, config), padding.longest_group_len), + ); + } + + if config.context { + let _ = write!( + out, + " {}", + pad_right(&item.security_context, padding.longest_context_len), + ); + } + + // Author is only different from owner on GNU/Hurd, so we reuse + // the owner, since GNU/Hurd is not currently supported by Rust. + if config.long.author { + let _ = write!( + out, + " {}", + pad_right(&display_uname(md, config), padding.longest_uname_len), + ); + } + + let dfn = display_file_name(item, config, None, out).contents; + + let _ = writeln!( + out, + " {} {} {}", + pad_left(&display_size_or_rdev(md, config), padding.longest_size_len), + display_date(md, config), + dfn, + ); + } else { + // this 'else' is expressly for the case of a dangling symlink + #[cfg(unix)] + { + if config.inode { + let _ = write!(out, "{} ", pad_left("?", padding.longest_inode_len),); + } + } - if config.long.owner { let _ = write!( out, - " {}", - pad_right(&display_uname(md, config), padding.longest_uname_len) + "{}{} {}", + "l?????????".to_string(), + if item.security_context.len() > 1 { + // GNU `ls` uses a "." character to indicate a file with a security context, + // but not other alternate access method. + "." + } else { + "" + }, + pad_left("", padding.longest_link_count_len), ); - } - if config.long.group { - let _ = write!( + if config.long.owner { + let _ = write!(out, " {}", pad_right("?", padding.longest_uname_len)); + } + + if config.long.group { + let _ = write!(out, " {}", pad_right("?", padding.longest_group_len)); + } + + if config.context { + let _ = write!( + out, + " {}", + pad_right(&item.security_context, padding.longest_context_len) + ); + } + + // Author is only different from owner on GNU/Hurd, so we reuse + // the owner, since GNU/Hurd is not currently supported by Rust. + if config.long.author { + let _ = write!(out, " {}", pad_right("?", padding.longest_uname_len)); + } + + let dfn = display_file_name(item, config, None, out).contents; + let date_len = 12; + + let _ = writeln!( out, - " {}", - pad_right(&display_group(md, config), padding.longest_group_len) + " {} {} {}", + pad_left("?", padding.longest_size_len), + pad_left("?", date_len), + dfn, ); } - - if config.context { - let _ = write!( - out, - " {}", - pad_right(&item.security_context, padding.longest_context_len) - ); - } - - // Author is only different from owner on GNU/Hurd, so we reuse - // the owner, since GNU/Hurd is not currently supported by Rust. - if config.long.author { - let _ = write!( - out, - " {}", - pad_right(&display_uname(md, config), padding.longest_uname_len) - ); - } - - let _ = writeln!( - out, - " {} {} {}", - pad_left(&display_size_or_rdev(md, config), padding.longest_size_len), - display_date(md, config), - // unwrap is fine because it fails when metadata is not available - // but we already know that it is because it's checked at the - // start of the function. - display_file_name(item, config, None).unwrap().contents, - ); } #[cfg(unix)] @@ -1911,7 +2085,7 @@ fn display_size_or_rdev(metadata: &Metadata, config: &Config) -> String { let dev: u64 = metadata.rdev(); let major = (dev >> 8) as u8; let minor = dev as u8; - return format!("{}, {}", major, minor); + return format!("{}, {}", major, minor,); } } @@ -1952,7 +2126,7 @@ fn classify_file(path: &PathData) -> Option { Some('=') } else if file_type.is_fifo() { Some('|') - } else if file_type.is_file() && file_is_executable(path.md()?) { + } else if file_type.is_file() && file_is_executable(path.md().as_ref().unwrap()) { Some('*') } else { None @@ -1976,27 +2150,44 @@ fn classify_file(path: &PathData) -> Option { /// /// Note that non-unicode sequences in symlink targets are dealt with using /// [`std::path::Path::to_string_lossy`]. +#[allow(unused_variables)] fn display_file_name( path: &PathData, config: &Config, prefix_context: Option, -) -> Option { + out: &mut BufWriter, +) -> Cell { // This is our return value. We start by `&path.display_name` and modify it along the way. let mut name = escape_name(&path.display_name, &config.quoting_style); - #[cfg(unix)] - { - if config.format != Format::Long && config.inode { - name = path.md().map_or_else(|| "?".to_string(), get_inode) + " " + &name; - } - } - // We need to keep track of the width ourselves instead of letting term_grid // infer it because the color codes mess up term_grid's width calculation. let mut width = name.width(); if let Some(ls_colors) = &config.color { - name = color_name(ls_colors, &path.p_buf, name, path.md()?); + if let Ok(metadata) = path.p_buf.symlink_metadata() { + name = color_name(ls_colors, &path.p_buf, name, &metadata); + } + } + + #[cfg(unix)] + { + if config.inode && config.format != Format::Long { + let inode = if let Some(md) = path.md() { + get_inode(md) + } else { + let _ = out.flush(); + let show_error = show!(LsError::IOErrorContext( + std::io::Error::new(ErrorKind::NotFound, "NotFound"), + path.p_buf.clone(), + )); + "?".to_string() + }; + // increment width here b/c name was given colors and name.width() is now the wrong + // size for display + width += inode.width(); + name = inode + " " + &name; + } } if config.indicator_style != IndicatorStyle::None { @@ -2027,7 +2218,10 @@ fn display_file_name( } } - if config.format == Format::Long && path.file_type()?.is_symlink() { + if config.format == Format::Long + && path.file_type().is_some() + && path.file_type().unwrap().is_symlink() + { if let Ok(target) = path.p_buf.read_link() { name.push_str(" -> "); @@ -2050,17 +2244,21 @@ fn display_file_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. - let target_metadata = match target_data.md() { - Some(md) => md, - None => path.md()?, - }; + if path.md().is_none() && target_data.md().is_none() { + name.push_str(&path.p_buf.read_link().unwrap().to_string_lossy()); + } else { + let target_metadata = match target_data.md() { + Some(md) => md, + None => path.md().unwrap(), + }; - name.push_str(&color_name( - ls_colors, - &target_data.p_buf, - target.to_string_lossy().into_owned(), - target_metadata, - )); + name.push_str(&color_name( + ls_colors, + &target_data.p_buf, + target.to_string_lossy().into_owned(), + target_metadata, + )); + } } else { // If no coloring is required, we just use target as is. name.push_str(&target.to_string_lossy()); @@ -2082,10 +2280,10 @@ fn display_file_name( } } - Some(Cell { + Cell { contents: name, width, - }) + } } fn color_name(ls_colors: &LsColors, path: &Path, name: String, md: &Metadata) -> String { @@ -2107,6 +2305,18 @@ fn display_symlink_count(metadata: &Metadata) -> String { metadata.nlink().to_string() } +#[allow(unused_variables)] +fn display_inode(metadata: &Metadata) -> String { + #[cfg(unix)] + { + get_inode(metadata) + } + #[cfg(not(unix))] + { + "".to_string() + } +} + // This returns the SELinux security context as UTF8 `String`. // In the long term this should be changed to `OsStr`, see discussions at #2621/#2656 #[allow(unused_variables)] @@ -2115,7 +2325,7 @@ fn get_security_context(config: &Config, p_buf: &Path, must_dereference: bool) - if config.selinux_supported { #[cfg(feature = "selinux")] { - match selinux::SecurityContext::of_path(p_buf, must_dereference, false) { + match selinux::SecurityContext::of_path(p_buf, must_dereference.to_owned(), false) { Err(_r) => { // TODO: show the actual reason why it failed show_warning!("failed to get security context of: {}", p_buf.quote()); diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index b3234ce54..4749e2c29 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -11,7 +11,6 @@ use std::collections::HashMap; use std::path::Path; use std::thread::sleep; use std::time::Duration; - #[cfg(not(windows))] extern crate libc; #[cfg(not(windows))] @@ -39,6 +38,75 @@ fn test_ls_i() { new_ucmd!().arg("-il").succeeds(); } +#[test] +fn test_ls_ordering() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.mkdir("some-dir1"); + at.mkdir("some-dir2"); + at.mkdir("some-dir3"); + at.mkdir("some-dir4"); + at.mkdir("some-dir5"); + at.mkdir("some-dir6"); + + scene + .ucmd() + .arg("-Rl") + .succeeds() + .stdout_matches(&Regex::new("some-dir1:\\ntotal 0").unwrap()); +} + +#[cfg(all(feature = "chmod"))] +#[test] +fn test_ls_io_errors() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.mkdir("some-dir1"); + at.mkdir("some-dir2"); + at.symlink_file("does_not_exist", "some-dir2/dangle"); + at.mkdir("some-dir3"); + at.mkdir("some-dir3/some-dir4"); + at.mkdir("some-dir3/some-dir5"); + at.mkdir("some-dir3/some-dir6"); + at.mkdir("some-dir3/some-dir7"); + at.mkdir("some-dir3/some-dir8"); + + scene.ccmd("chmod").arg("000").arg("some-dir1").succeeds(); + + scene + .ucmd() + .arg("-1") + .arg("some-dir1") + .fails() + .stderr_contains("cannot open directory") + .stderr_contains("Permission denied"); + + scene + .ucmd() + .arg("-Li") + .arg("some-dir2") + .fails() + .stderr_contains("cannot access") + .stderr_contains("No such file or directory") + .stdout_contains(if cfg!(windows) { "dangle" } else { "? dangle" }); + + scene + .ccmd("chmod") + .arg("000") + .arg("some-dir3/some-dir4") + .succeeds(); + + scene + .ucmd() + .arg("-laR") + .arg("some-dir3") + .fails() + .stderr_contains("some-dir4") + .stderr_contains("cannot open directory") + .stderr_contains("Permission denied") + .stdout_contains("some-dir4"); +} + #[test] fn test_ls_walk_glob() { let scene = TestScenario::new(util_name!()); @@ -2303,8 +2371,16 @@ fn test_ls_dangling_symlinks() { .ucmd() .arg("-Li") .arg("temp_dir") - .succeeds() // this should fail, though at the moment, ls lacks a way to propagate errors encountered during display + .fails() + .stderr_contains("cannot access") .stdout_contains(if cfg!(windows) { "dangle" } else { "? dangle" }); + + scene + .ucmd() + .arg("-Ll") + .arg("temp_dir") + .fails() + .stdout_contains("l?????????"); } #[test]