Subject: bug#33097: test has filetest -a but man page doesn't list it

From: Bernhard Voelker <[email-masked]>
Date: Mon, 22 Oct 2018 01:06:49 +0200

On 10/21/18 2:09 AM, Paul Eggert wrote:
> I have the opposite impression. Any scripts using this confusing -a operator are 
> already broken, and we should phase it out. Not that anybody actually *uses* 
> coreutils "test -a".

Done with the attached 1st patch.

The 2nd patch is a cleanup avoiding the redundant checking of unary operators
in test_unop and unary_operator.

The 3rd patch adds support for 'test -N FILE' as in bash.
Please check (on various platforms / file systems if possible).

Thanks & have a nice day,
Berny

From 3dc474f71275d5fdd18a028ede19c507e1ecd830 Mon Sep 17 00:00:00 2001
From: Bernhard Voelker <[email-masked]>
Date: Sun, 21 Oct 2018 21:17:31 +0200
Subject: [PATCH 1/3] test: remove support for the ambigous -a unary operator

* src/test.c (unary_operator): Remove case 'a'.
(test_unop): Likewise.
* NEWS (Changes in behavior): Document the change.

Discussed at https://bugs.gnu.org/33097
---
 NEWS       | 7 +++++++
 src/test.c | 5 ++---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/NEWS b/NEWS
index ae9667e61..a2b733d38 100644
--- a/NEWS
+++ b/NEWS
@@ -16,6 +16,13 @@ GNU coreutils NEWS                                    -*- outline -*-
   approach is still used in situations where hard links to directories
   are allowed (e.g., NetBSD when superuser).
 
+** Changes in behavior
+
+  'test -a FILE' is not supported anymore.  Long ago, there were concerns about
+  the high probability of humans confusing the -a primary with the -a binary
+  operator, so POSIX changed this to 'test -e FILE'.  Scripts using it were
+  already broken and non-portable; the -a unary operator was never documented.
+
 ** New features
 
   id now supports specifying multiple users.
diff --git a/src/test.c b/src/test.c
index aae45012a..339a84075 100644
--- a/src/test.c
+++ b/src/test.c
@@ -406,8 +406,7 @@ unary_operator (void)
          pos right past it.  This means that pos - 1 is the location of the
          argument. */
 
-    case 'a':			/* file exists in the file system? */
-    case 'e':
+    case 'e':			/* file exists in the file system? */
       unary_advance ();
       return stat (argv[pos - 1], &stat_buf) == 0;
 
@@ -586,7 +585,7 @@ test_unop (char const *op)
 
   switch (op[1])
     {
-    case 'a': case 'b': case 'c': case 'd': case 'e':
+    case 'b': case 'c': case 'd': case 'e':
     case 'f': case 'g': case 'h': case 'k': case 'n':
     case 'o': case 'p': case 'r': case 's': case 't':
     case 'u': case 'w': case 'x': case 'z':
-- 
2.19.1

From 6088beb33c64c1fad7f1ec607335bf227fd831d0 Mon Sep 17 00:00:00 2001
From: Bernhard Voelker <[email-masked]>
Date: Sun, 21 Oct 2018 21:56:43 +0200
Subject: [PATCH 2/3] test: simplify redundant code

Remove the function 'test_unop', as the cases therein are redundant to
those handled by 'unary_operator'; exception: the cases 'o' and 'N':
they had been present in test_unop and handling the commands
  test -N STR
  test -o STR
and
  test x = x -a -N STR
  test x = x -a -o STR
which ran into an error later on anyway.
With this commit, the error diagnostic will change from ...
  $ /usr/bin/test -N STR
  /usr/bin/test: extra argument '-N'
  $ /usr/bin/test -o STR
  /usr/bin/test: extra argument '-o'
... to ...
  $ src/test -N STR
  src/test: '-N': unary operator expected
  $ src/test -o STR
  src/test: '-o': unary operator expected

* src/test.c (test_unop): Remove.
(unary_operator): Fail with test_syntax_error in the default case.
(term): Directly call unary_operator.
(two_arguments): Likewise.
* tests/misc/test-diag.pl: Adjust error diagnostic.
---
 src/test.c              | 34 +++-------------------------------
 tests/misc/test-diag.pl |  4 ++--
 2 files changed, 5 insertions(+), 33 deletions(-)

diff --git a/src/test.c b/src/test.c
index 339a84075..9005b1962 100644
--- a/src/test.c
+++ b/src/test.c
@@ -72,7 +72,6 @@ static int pos;		/* The offset of the current argument in ARGV. */
 static int argc;	/* The number of arguments present in ARGV. */
 static char **argv;	/* The argument list. */
 
-static bool test_unop (char const *s);
 static bool unary_operator (void);
 static bool binary_operator (bool);
 static bool two_arguments (void);
@@ -258,12 +257,7 @@ term (void)
 
   /* It might be a switch type argument.  */
   else if (argv[pos][0] == '-' && argv[pos][1] && argv[pos][2] == '\0')
-    {
-      if (test_unop (argv[pos]))
-        value = unary_operator ();
-      else
-        test_syntax_error (_("%s: unary operator expected"), quote (argv[pos]));
-    }
+    value = unary_operator ();
   else
     {
       value = (argv[pos][0] != '\0');
@@ -399,6 +393,7 @@ unary_operator (void)
   switch (argv[pos][1])
     {
     default:
+      test_syntax_error (_("%s: unary operator expected"), quote (argv[pos]));
       return false;
 
       /* All of the following unary operators use unary_advance (), which
@@ -576,26 +571,6 @@ expr (void)
   return or ();		/* Same with this. */
 }
 
-/* Return true if OP is one of the test command's unary operators. */
-static bool
-test_unop (char const *op)
-{
-  if (op[0] != '-')
-    return false;
-
-  switch (op[1])
-    {
-    case 'b': case 'c': case 'd': case 'e':
-    case 'f': case 'g': case 'h': case 'k': case 'n':
-    case 'o': case 'p': case 'r': case 's': case 't':
-    case 'u': case 'w': case 'x': case 'z':
-    case 'G': case 'L': case 'O': case 'S': case 'N':
-      return true;
-    default:
-      return false;
-    }
-}
-
 static bool
 one_argument (void)
 {
@@ -616,10 +591,7 @@ two_arguments (void)
            && argv[pos][1] != '\0'
            && argv[pos][2] == '\0')
     {
-      if (test_unop (argv[pos]))
-        value = unary_operator ();
-      else
-        test_syntax_error (_("%s: unary operator expected"), quote (argv[pos]));
+      value = unary_operator ();
     }
   else
     beyond ();
diff --git a/tests/misc/test-diag.pl b/tests/misc/test-diag.pl
index 91356ef56..f543f9f09 100755
--- a/tests/misc/test-diag.pl
+++ b/tests/misc/test-diag.pl
@@ -26,8 +26,8 @@ use strict;
 my @Tests =
     (
      # In coreutils-5.93, this diagnostic lacked the newline.
-     ['o', '-o arg', {ERR => "test: extra argument '-o'\n"},
-      {ERR_SUBST => 's!^.*:!test:!'},
+     ['o', '-o arg', {ERR => "test: '-o': unary operator expected\n"},
+      {ERR_SUBST => 's!^.*test:!test:!'},
       {EXIT => 2}],
     );
 
-- 
2.19.1

From ab7719540668240baf734b44a63bff270557c0a6 Mon Sep 17 00:00:00 2001
From: Bernhard Voelker <[email-masked]>
Date: Mon, 22 Oct 2018 00:54:51 +0200
Subject: [PATCH 3/3] test: add -N unary operator

Bash knows 'test -N FILE'.  Add it to GNU 'test' as well.

* src/test.c (unary_operator): Add a case for 'N'.
(usage): Document it.
* doc/coreutils.texi (node File characteristic tests): Likewise.
* NEWS (New features): Likewise.
* tests/misc/test-N.sh: Add a test.
* tests/local.mk (all_tests): Reference it.
---
 NEWS                 |  3 +++
 doc/coreutils.texi   |  6 ++++++
 src/test.c           | 11 +++++++++++
 tests/local.mk       |  1 +
 tests/misc/test-N.sh | 42 ++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 63 insertions(+)
 create mode 100755 tests/misc/test-N.sh

diff --git a/NEWS b/NEWS
index a2b733d38..a8fa5f2e4 100644
--- a/NEWS
+++ b/NEWS
@@ -27,6 +27,9 @@ GNU coreutils NEWS                                    -*- outline -*-
 
   id now supports specifying multiple users.
 
+  test now supports the '-N FILE' unary operator (like e.g. bash) to check
+  whether FILE exists and has been modified since it was last read.
+
 
 * Noteworthy changes in release 8.30 (2018-07-01) [stable]
 
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 512443aa6..30155bb86 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -13099,6 +13099,12 @@ True if @var{file1} is older (according to modification date) than
 True if @var{file1} and @var{file2} have the same device and inode
 numbers, i.e., if they are hard links to each other.
 
+@item -N @var{file}
+@opindex -N
+@cindex mtime-greater-atime file check
+True if @var{file} exists and has been modified (mtime) since it was
+last read (atime).
+
 @end table

diff --git a/src/test.c b/src/test.c
index 9005b1962..7ed797f17 100644
--- a/src/test.c
+++ b/src/test.c
@@ -417,6 +417,16 @@ unary_operator (void)
       unary_advance ();
       return euidaccess (argv[pos - 1], X_OK) == 0;
 
+    case 'N':  /* File exists and has been modified since it was last read? */
+      {
+        unary_advance ();
+        if (stat (argv[pos - 1], &stat_buf) != 0)
+          return false;
+        struct timespec atime = get_stat_atime (&stat_buf);
+        struct timespec mtime = get_stat_mtime (&stat_buf);
+        return (timespec_cmp (mtime, atime) > 0);
+      }
+
     case 'O':			/* File is owned by you? */
       {
         unary_advance ();
@@ -741,6 +751,7 @@ EXPRESSION is true or false and sets exit status.  It is one of:\n\
 "), stdout);
       fputs (_("\
   -L FILE     FILE exists and is a symbolic link (same as -h)\n\
+  -N FILE     FILE exists and has been modified since it was last read\n\
   -O FILE     FILE exists and is owned by the effective user ID\n\
   -p FILE     FILE exists and is a named pipe\n\
   -r FILE     FILE exists and read permission is granted\n\
diff --git a/tests/local.mk b/tests/local.mk
index 47c6b9537..84d346979 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -408,6 +408,7 @@ all_tests =					\
   tests/misc/tac-2-nonseekable.sh		\
   tests/misc/tail.pl				\
   tests/misc/tee.sh				\
+  tests/misc/test-N.sh				\
   tests/misc/test-diag.pl			\
   tests/misc/time-style.sh			\
   tests/misc/timeout.sh				\
diff --git a/tests/misc/test-N.sh b/tests/misc/test-N.sh
new file mode 100755
index 000000000..36eae8e8c
--- /dev/null
+++ b/tests/misc/test-N.sh
@@ -0,0 +1,42 @@
+#!/bin/sh
+# Test 'test -N file'.
+
+# Copyright (C) 2018 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
+print_ver_ test
+
+# For a freshly touched file, atime should equal mtime: 'test -N' returns 1.
+touch file || framework_failure_
+stat file
+returns_ 1 env test -N file || fail=1
+
+# Set access time to yesterday at noon: 'test -N' returns 0.
+touch -a -d "12:00 today -1 days" file || framework_failure_
+stat file
+env test -N file || fail=1
+
+# Set mtime to the day before yesterday: 'test -N' returns 1;
+touch -m -d "12:00 today -2 days" file || framework_failure_
+stat file
+returns_ 1 env test -N file || fail=1
+
+# Now modify the file: 'test -N' returns 0.
+> file || framework_failure_
+stat file
+env test -N file || fail=1
+
+Exit $fail
-- 
2.19.1