From d9461b08122827ac72881d01a819b1309e7f83f9 Mon Sep 17 00:00:00 2001
From: Luan Taraschi <130802253+luantaraschi@users.noreply.github.com>
Date: Thu, 20 Aug 2026 16:48:47 -0300
Subject: [PATCH] ls: break a tie in the time sort with the name

Every other sort in sort_entries falls back on the name when its key
ties. Sorting by time did not, so entries sharing a timestamp came out
in the order the directory was read in, and since the sort is unstable
that order was not guaranteed either.

GNU ls breaks the tie with the collation rather than with the bytes, so
the comparison Sort::Name already builds is hoisted out of the match and
the fallback uses that same one.
---
 src/uu/ls/src/ls.rs      | 41 ++++++++++++++++++--------------
 tests/by-util/test_ls.rs | 51 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+), 18 deletions(-)

diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs
index 2317e37c4a5..03ae84a7a8a 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 186f942e1ae..1d3d29b3b3b 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!());