On 27/06/2024 16:05, Dave wrote:
The ls command without the -l option and with the --time=mtime option,
is incorrectly sorting by the name rather than by the modification
time.

ls               # sorts by name (ok)
ls --time=mtime  # sorts by name (should sort by mtime)

// The current statement in ls.c (lines 2383-2387)
   sort_type = (0 <= sort_opt ? sort_opt
                : (format != long_format
                   && (time_type == time_ctime || time_type == time_atime
                       || time_type == time_btime))
                ? sort_time : sort_name);

// Proposed correction (untested)
   sort_type = (0 <= sort_opt ? sort_opt
                : (format != long_format
                   && (time_type == time_ctime || time_type == time_atime
                       || time_type == time_btime || time_type == time_mtime))
                ? sort_time : sort_name);

Right, we should be applying this GNU extension to --time=mtime also.
I.e. sorting when not displaying time (-l not specified),
and no other sorting specific option is used.

The proposed fix wouldn't work as time_mtime is the default,
so we'd be sorting by mtime rather than name by default.

I'll apply the attached later to address this.

Marking this as done.

thanks,
Pádraig
From b95c6ac7d20084d9167bdb9d759bcebcb0c2b766 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Thu, 27 Jun 2024 18:15:02 +0100
Subject: [PATCH] ls: treat --time=mtime consistently with other time selectors

* src/ls.c: Track if --time=mtime is explicitly specified,
so that we can apply the GNU extension of sorting by the
specified time, when not displaying (-l not specified),
and not explicitly sorting (-t not specified).
* tests/ls/ls-time.sh: Add / Update test cases.
Fixes https://bugs.gnu.org/71803
---
 src/ls.c            | 18 ++++++++----------
 tests/ls/ls-time.sh | 30 ++++++++++++++++++++----------
 2 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/src/ls.c b/src/ls.c
index 5e2f7c3e7..f7ffe1086 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -468,6 +468,7 @@ enum time_type
   };
 
 static enum time_type time_type;
+static bool explicit_time;
 
 /* The file characteristic to sort by.  Controlled by -t, -S, -U, -X, -v.
    The values of each item of this enum are important since they are
@@ -1943,6 +1944,7 @@ decode_switches (int argc, char **argv)
 
         case 'c':
           time_type = time_ctime;
+          explicit_time = true;
           break;
 
         case 'd':
@@ -2017,6 +2019,7 @@ decode_switches (int argc, char **argv)
 
         case 'u':
           time_type = time_atime;
+          explicit_time = true;
           break;
 
         case 'v':
@@ -2146,6 +2149,7 @@ decode_switches (int argc, char **argv)
 
         case TIME_OPTION:
           time_type = XARGMATCH ("--time", optarg, time_args, time_types);
+          explicit_time = true;
           break;
 
         case FORMAT_OPTION:
@@ -2372,18 +2376,12 @@ decode_switches (int argc, char **argv)
   if (eolbyte < dired)
     error (LS_FAILURE, 0, _("--dired and --zero are incompatible"));
 
-  /* If -c or -u is specified and not -l (or any other option that implies -l),
-     and no sort-type was specified, then sort by the ctime (-c) or atime (-u).
-     The behavior of ls when using either -c or -u but with neither -l nor -t
-     appears to be unspecified by POSIX.  So, with GNU ls, '-u' alone means
-     sort by atime (this is the one that's not specified by the POSIX spec),
-     -lu means show atime and sort by name, -lut means show atime and sort
-     by atime.  */
+  /* If a time type is explicitly specified (with -c, -u, or --time=)
+     and we're not showing a time (-l not specified), then sort by that time,
+     rather than by name.  Note this behavior is unspecified by POSIX.  */
 
   sort_type = (0 <= sort_opt ? sort_opt
-               : (format != long_format
-                  && (time_type == time_ctime || time_type == time_atime
-                      || time_type == time_btime))
+               : (format != long_format && explicit_time)
                ? sort_time : sort_name);
 
   if (format == long_format)
diff --git a/tests/ls/ls-time.sh b/tests/ls/ls-time.sh
index f27a5dbbe..be8a83556 100755
--- a/tests/ls/ls-time.sh
+++ b/tests/ls/ls-time.sh
@@ -33,11 +33,15 @@ u2='1998-01-14 12:00'
 u3='1998-01-14 13:00'
 
 touch -m -d "$t3" a || framework_failure_
-touch -m -d "$t2" b || framework_failure_
+touch -m -d "$t2" B || framework_failure_  # Capital to distinguish name sort
 touch -m -d "$t1" c || framework_failure_
 
+# Check default name sorting works
+set $(ls a B c)
+test "$*" = 'B a c' || fail=1
+
 touch -a -d "$u3" c || framework_failure_
-touch -a -d "$u2" b || framework_failure_
+touch -a -d "$u2" B || framework_failure_
 # Make sure A has ctime at least 1 second more recent than C's.
 sleep 2
 touch -a -d "$u1" a || framework_failure_
@@ -47,7 +51,9 @@ touch -a -d "$u1" a || framework_failure_
 
 
 # A has ctime more recent than C.
-set $(ls -c a c)
+set $(ls -t -c a c)
+test "$*" = 'a c' || fail=1
+set $(ls -c a c)  # Not specified by POSIX
 test "$*" = 'a c' || fail=1
 
 # Sleep so long in an attempt to avoid spurious failures
@@ -93,13 +99,17 @@ EOF
   ;;
 esac
 
-set $(ls -ut a b c)
-test "$*" = 'c b a' && : || fail=1
-test $fail = 1 && ls -l --full-time --time=access a b c
-
-set $(ls -t a b c)
-test "$*" = 'a b c' && : || fail=1
-test $fail = 1 && ls -l --full-time a b c
+set $(ls -ut a B c)
+test "$*" = 'c B a' || fail=1
+set $(ls -u a B c)  # not specified by POSIX
+test "$*" = 'c B a' || fail=1
+test $fail = 1 && ls -l --full-time --time=access a B c
+
+set $(ls -t a B c)
+test "$*" = 'a B c' || fail=1
+set $(ls --time=mtime a B c)
+test "$*" = 'a B c' || fail=1
+test $fail = 1 && ls -l --full-time a B c
 
 # Now, C should have ctime more recent than A.
 set $(ls -ct a c)
-- 
2.45.1

Reply via email to