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 <m...@bernhard-voelker.de>
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 <m...@bernhard-voelker.de>
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 <m...@bernhard-voelker.de>
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
