ls: break a tie in the time sort with the name

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(),

tests/by-util/test_ls.rs

@@ -2529,6 +2529,57 @@
         .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!());
@@ -7692,7 +7743,7 @@
 
     let stats_of_some_dir = |args: &[&str]| -> Option<usize> {
         let output = Command::new("strace")
             .args(["-qq", "-e", "trace=stat,statx,lstat,newfstatat"])
             .arg(&scene.bin_path)
             .arg(scene.util_name.as_str())
             .args(args)