Hi Padraig,
On 04/03/2013 01:02 PM, Pádraig Brady wrote:
> Yes I agree that this is a regression associated with the inotify support.
> I take `tail --follow=descriptor --retry` to mean,
> Wait for the file to appear initially and follow even if renamed or unlinked.
The problem is that tail_forever_inotify() returns false when the
initial open in tail_file() failed. This makes tail exit immediately.
Without intense reading of the source, I guess it's not easy to
use inotify in such a case. Therefore, falling back to polling
seems to be quite okay.
The attached patch implement this in a one-liner:
@ -2189,7 +2192,8 @@ main (int argc, char **argv)
is recreated, then we don't recheck any new file when
follow_mode == Follow_name */
if (!disable_inotify && (tailable_stdin (F, n_files)
- || any_remote_file (F, n_files)))
+ || any_remote_file (F, n_files)
+ || !ok))
disable_inotify = true;
if (!disable_inotify)
>> Furthermore, I find this warning irritating
>> "warning: --retry is useful mainly when following by name"
>
> I agree. It's not imparting much info, about why that's supported.
> What we should do is print what tail will do in this case.
> I propose we do:
>
> if (retry)
> if (!follow_mode)
> warn ("--retry ignored");
> else if (follow_mode == DESC)
> warn ("--retry only effective for the initial open");
I'd go even one step further, and also remove the latter warning.
It fits better into the texinfo documentation.
The attached patch includes that, too.
Finally, the patch comes with a new test, NEWS etc.
WDYT?
Have a nice day,
Berny
>From d73c270013f2f70ae85fcb4cb634d946c728455e Mon Sep 17 00:00:00 2001
From: Bernhard Voelker <[email protected]>
Date: Thu, 4 Apr 2013 00:44:43 +0200
Subject: [PATCH] tail: let -f --retry wait for inaccessible files
The --retry option is indeed useful for both following modes
by name and by file descriptor. The difference is that in the
latter case, it is effective only during the initial open.
As a regression of the implementation of the inotify support,
tail -f --retry would immediately exit if the given file is
inaccessible.
* src/tail.c (usage): Change the description of the --retry option:
remove the note that this option would mainly be useful when
following by name.
(main): Instead of issuing a warning when the --retry option is used
together with --follow=descriptor, only issue a warning when it is
used without any following mode.
Disable inotify also in the case when the initial open in tail_file()
failed (which is the actual bug fix).
* tests/tail-2/retry.sh: Add new tests.
* tests/local.mk (all_tests): Mention it.
* doc/coreutils.texi (tail invocation): Enhance the documentation
of the --retry option. Clarify the difference in tail's behavior
regarding the --retry option when combined with the following modes
name versus descriptor.
* NEWS (Bug fixes): Mention the fix.
Reported by Noel Morrison in:
http://lists.gnu.org/archive/html/coreutils/2013-04/msg00003.html
---
NEWS | 5 ++++
doc/coreutils.texi | 14 +++++++--
src/tail.c | 16 ++++++----
tests/local.mk | 1 +
tests/tail-2/retry.sh | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 108 insertions(+), 9 deletions(-)
create mode 100644 tests/tail-2/retry.sh
diff --git a/NEWS b/NEWS
index 0c2daad..339b714 100644
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,11 @@ GNU coreutils NEWS -*- outline -*-
permissions.
[This bug was present in "the beginning".]
+ tail --retry -f now waits for the files specified to appear. Before, tail
+ would immediately exit when such a file is inaccessible during the initial
+ open.
+ [bug introduced in coreutils-7.5]
+
** New features
join accepts a new option: --zero-terminated (-z). As with the sort,uniq
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index dfa9b1c..1952f5f 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -3155,9 +3155,17 @@ will keep trying until it becomes accessible again.
@item --retry
@opindex --retry
-This option is useful mainly when following by name (i.e., with
-@option{--follow=name}).
-Without this option, when tail encounters a file that doesn't
+Indefinitely try to open the specified file.
+This option is useful mainly when following (and otherwise issues a warning).
+
+When following by file descriptor (i.e., with @option{--follow=descriptor}),
+this option only affects the initial open of the file, as after a successful
+open, @command{tail} will start following the file descriptor.
+
+When following by name (i.e., with @option{--follow=name}), @command{tail}
+infinitely retries to re-open the given files until killed.
+
+Without this option, when @command{tail} encounters a file that doesn't
exist or is otherwise inaccessible, it reports that fact and
never checks it again.
diff --git a/src/tail.c b/src/tail.c
index ca851ee..0e8295e 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -294,9 +294,7 @@ With no FILE, or when FILE is -, read standard input.\n\
fputs (_("\
--pid=PID with -f, terminate after process ID, PID dies\n\
-q, --quiet, --silent never output headers giving file names\n\
- --retry keep trying to open a file even when it is or\n\
- becomes inaccessible; useful when following by\n\
- name, i.e., with --follow=name\n\
+ --retry keep trying to open a file if it is inaccessible\n\
"), stdout);
fputs (_("\
-s, --sleep-interval=N with -f, sleep for approximately N seconds\n\
@@ -2030,8 +2028,9 @@ parse_options (int argc, char **argv,
}
}
- if (reopen_inaccessible_files && follow_mode != Follow_name)
- error (0, 0, _("warning: --retry is useful mainly when following by name"));
+ if (reopen_inaccessible_files && !forever)
+ error (0, 0, _("warning: --retry ignored; --retry is useful"
+ " only when following"));
if (pid && !forever)
error (0, 0,
@@ -2178,6 +2177,10 @@ main (int argc, char **argv)
in this case because it would miss any updates to the file
that were not initiated from the local system.
+ ok is false when one of the files specified could not be opened for
+ reading. In this case, tail_forever_inotify() cannot be used (in its
+ current implementation).
+
FIXME: inotify doesn't give any notification when a new
(remote) file or directory is mounted on top a watched file.
When follow_mode == Follow_name we would ideally like to detect that.
@@ -2189,7 +2192,8 @@ main (int argc, char **argv)
is recreated, then we don't recheck any new file when
follow_mode == Follow_name */
if (!disable_inotify && (tailable_stdin (F, n_files)
- || any_remote_file (F, n_files)))
+ || any_remote_file (F, n_files)
+ || !ok))
disable_inotify = true;
if (!disable_inotify)
diff --git a/tests/local.mk b/tests/local.mk
index dc87ef4..5e51085 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -389,6 +389,7 @@ all_tests = \
tests/misc/uniq-perf.sh \
tests/misc/xattr.sh \
tests/tail-2/wait.sh \
+ tests/tail-2/retry.sh \
tests/chmod/c-option.sh \
tests/chmod/equal-x.sh \
tests/chmod/equals.sh \
diff --git a/tests/tail-2/retry.sh b/tests/tail-2/retry.sh
new file mode 100644
index 0000000..a3617fa
--- /dev/null
+++ b/tests/tail-2/retry.sh
@@ -0,0 +1,81 @@
+#!/bin/sh
+# Exercise tail's behavior regarding missing files with/without --retry.
+
+# Copyright (C) 2013 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 <http://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
+print_ver_ tail
+
+# Retry without --follow results in a warning.
+touch file
+tail --retry file > out 2>&1 || fail=1
+[ $( wc -l < out ) = 1 ] || fail=1
+grep -F 'tail: warning: --retry ignored' out || fail=1
+
+
+# The same with a missing file: expect error message and exit 1.
+tail --retry missing > out 2>&1 && fail=1
+[ $( wc -l < out ) = 2 ] || fail=1
+grep -F 'tail: warning: --retry ignored' out || fail=1
+
+
+# Ensure that "tail --retry --follow=name" waits for the file to appear.
+tail -s.1 --follow=name --retry missing >out 2>&1 & pid=$!
+sleep .2 # Wait a bit ...
+echo "X" > missing # ... for the file to appear.
+sleep .2 # Then give tail enough time to recognize it
+ # ... and to output the content to out.
+kill $pid # Then kill tail ...
+wait $pid # ... and wait for it to complete.
+rm -f missing || fail=1
+# Expect 3 lines in the output file ("cannot open" + "has appeared" + "X").
+[ $( wc -l < out ) = 3 ] || { fail=1; cat out; }
+grep -F 'cannot open' out || { fail=1; cat out; }
+grep -F 'has appeared' out || { fail=1; cat out; }
+grep '^X$' out || { fail=1; cat out; }
+
+
+# Ensure that "tail --retry --follow=descriptor" waits for the file to appear.
+# tail-8.21 failed at this (since the implementation of the inotify support).
+tail -s.1 --follow=descriptor --retry missing >out 2>&1 & pid=$!
+sleep .2 # Wait a bit ...
+echo "X" > missing # ... for the file to appear.
+sleep .2 # Then give tail enough time to recognize it
+ # ... and to output the content to out.
+kill $pid # Then kill tail ...
+wait $pid # ... and wait for it to complete.
+rm -f missing || fail=1
+# Expect 3 lines in the output file ("cannot open" + "has appeared" + "X").
+[ $( wc -l < out ) = 3 ] || { fail=1; cat out; }
+grep -F 'cannot open' out || { fail=1; cat out; }
+grep -F 'has appeared' out || { fail=1; cat out; }
+grep '^X$' out || { fail=1; cat out; }
+
+# Ensure that --follow=descriptor (without --retry) does not wait for the
+# file to appear. Expect 2 lines in the output file ("cannot open" +
+# "no files remaining") and exit status 1.
+tail --follow=descriptor missing >out 2>&1 && fail=1
+[ $( wc -l < out ) = 2 ] || { fail=1; cat out; }
+grep -F 'cannot open' out || { fail=1; cat out; }
+grep -F 'no files remaining' out || { fail=1; cat out; }
+
+# Likewise for --follow=name (without --retry).
+tail --follow=name missing >out 2>&1 && fail=1
+[ $( wc -l < out ) = 2 ] || { fail=1; cat out; }
+grep -F 'cannot open' out || { fail=1; cat out; }
+grep -F 'no files remaining' out || { fail=1; cat out; }
+
+Exit $fail
--
1.8.1.3.619.g7b6e784
