diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 2317e37c4a..03ae84a7a8 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -20,7 +20,6 @@ use std::cell::RefCell; use std::os::unix::fs::{FileTypeExt, MetadataExt}; use std::{ cell::OnceCell, - cmp::Reverse, ffi::{OsStr, OsString}, fs::{self, DirEntry, FileType, Metadata, ReadDir}, io::{BufWriter, ErrorKind, Stdout, Write, stdout}, @@ -1471,13 +1470,30 @@ pub fn list(locs: Vec<&Path>, config: &Config) -> UResult<()> { } fn sort_entries(entries: &mut [PathData], config: &Config) { + // The order the name sort uses. Sorting by time falls back on it so that + // entries sharing a timestamp come out in a fixed order rather than in + // whatever order the directory was read in, which is what GNU ls does and + // what every other arm of this match already does. + let use_locale = uucore::i18n::collator::should_use_locale_collation(); + let name_cmp = |a: &PathData, b: &PathData| { + if use_locale { + uucore::i18n::collator::locale_cmp( + os_str_as_bytes_lossy(a.display_name()).as_ref(), + os_str_as_bytes_lossy(b.display_name()).as_ref(), + ) + } else { + a.display_name().cmp(b.display_name()) + } + }; + match config.sort { - Sort::Time => entries.sort_unstable_by_key(|k| { - Reverse( - k.metadata() + Sort::Time => entries.sort_unstable_by(|a, b| { + let time = |p: &PathData| { + p.metadata() .and_then(|md| metadata_get_time(md, config.time)) - .unwrap_or(UNIX_EPOCH), - ) + .unwrap_or(UNIX_EPOCH) + }; + time(b).cmp(&time(a)).then_with(|| name_cmp(a, b)) }), Sort::Size => { entries.sort_unstable_by(|a, b| { @@ -1488,18 +1504,7 @@ fn sort_entries(entries: &mut [PathData], config: &Config) { }); } // The default sort in GNU ls is case insensitive - Sort::Name => { - if uucore::i18n::collator::should_use_locale_collation() { - entries.sort_unstable_by(|a, b| { - uucore::i18n::collator::locale_cmp( - os_str_as_bytes_lossy(a.display_name()).as_ref(), - os_str_as_bytes_lossy(b.display_name()).as_ref(), - ) - }); - } else { - entries.sort_unstable_by(|a, b| a.display_name().cmp(b.display_name())); - } - } + Sort::Name => entries.sort_unstable_by(name_cmp), Sort::Version => entries.sort_unstable_by(|a, b| { version_cmp( os_str_as_bytes_lossy(a.file_name()).as_ref(), diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 186f942e1a..1d3d29b3b3 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -2529,6 +2529,57 @@ fn test_ls_time_recent_future() { .stdout_contains("RECENT"); } +#[test] +fn test_ls_order_time_breaks_ties_by_name() { + // Every other sort in this utility falls back on the name, and GNU ls does + // the same for -t. Without the fallback, entries sharing a timestamp come + // out in whatever order the directory happened to be read in. + use filetime::{FileTime, set_file_times}; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let names = ["zulu", "alpha", "Mike", "bravo"]; + for name in names { + at.touch(name); + at.append(name, "x"); + } + let same = FileTime::from_unix_time(1_700_000_000, 0); + for name in names { + set_file_times(at.plus_as_string(name), same, same).unwrap(); + } + + scene + .ucmd() + .env("LC_ALL", "C") + .arg("-t") + .succeeds() + .stdout_only("Mike\nalpha\nbravo\nzulu\n"); + + scene + .ucmd() + .env("LC_ALL", "C") + .arg("-tr") + .succeeds() + .stdout_only("zulu\nbravo\nalpha\nMike\n"); + + // The tie is broken with the same order the name sort uses, so a UTF-8 + // locale puts `alpha` before `Mike` where the C locale does the reverse. + #[cfg(unix)] + { + use uutests::util::is_locale_available; + let locale = "en_US.UTF-8"; + if is_locale_available(locale) { + scene + .ucmd() + .env("LC_ALL", locale) + .arg("-t") + .succeeds() + .stdout_only("alpha\nbravo\nMike\nzulu\n"); + } + } +} + #[test] fn test_ls_order_time() { let scene = TestScenario::new(util_name!());