On 27/09/16 21:16, Bernhard Voelker wrote: > On 09/21/2016 08:15 PM, Pádraig Brady wrote: >> We can get expected behavior option 1 with the attached patch. >> Note that's inconsistent with current inotify behavior which does >> _not_ actually give up on the name, as can be seen when starting >> with a (non existent) file: >> >> $ touch foo >> $ tail -F foo& >> [1] 13624 >> $ rm foo; mkdir foo >> tail: ‘foo’ has become inaccessible: No such file or directory >> tail: ‘foo’ has been replaced with an untailable file; giving up on this name >> $ rmdir foo; echo foo > foo >> tail: ‘foo’ has become inaccessible: No such file or directory >> tail: ‘foo’ has appeared; following new file >> foo >> >> The attached patch also removes the "; giving up on this name" >> message in the inotify case as that's not the case. >> >> Ideally we'd have expected behavior option 2 >> both with and without inotify. >> I'll need to look a bit more as to why we have that >> limitation without inotify. > > The new behavior is nice, but it would really be better to have > consistent behavior in inotify and polling mode.
Yes consistency would be nice. Looking at the polling behavior I see it was not fundamental to the initial implementation and only changed later: http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=FILEUTILS-4_0q-68-g54d12f7 The attached patch how has the preferred behavior option 2 both with and without inotify. thanks, Pádraig
From d76beda19d6c181915bb5323a19d965b899b20dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com> Date: Wed, 21 Sep 2016 18:42:40 +0100 Subject: [PATCH] tail: -F now always processes initially untailable files This was not the case when inotify was not available. * src/tail.c (any_live_files): Simplify, since the IGNORE flag is now only set when a file should be ignored indefinitely. (recheck): Only output the "giving up on name" message when that's actually the case. Only set the IGNORE flag when ignoring a file indefinitely. (tail_file): Likewise. * tests/tail-2/retry.sh: Add a test case. Also run all existing test cases with and without inotify. NEWS: Mention the fix. Fixes http://bugs.gnu.org/24495 --- NEWS | 5 +++++ src/tail.c | 41 ++++++++++++++++++++--------------------- tests/tail-2/retry.sh | 34 +++++++++++++++++++++++++++++----- 3 files changed, 54 insertions(+), 26 deletions(-) diff --git a/NEWS b/NEWS index beba774..c3554d0 100644 --- a/NEWS +++ b/NEWS @@ -42,6 +42,11 @@ GNU coreutils NEWS -*- outline -*- seq now immediately exits upon write errors. [This bug was present in "the beginning".] + tail -F now continues to process initially untailable files that are replaced + by a tailable file. This was handled correctly when inotify was available, + and is now handled correctly in all cases. + [bug introduced in fileutils-4.0h] + yes now handles short writes, rather than assuming all writes complete. [bug introduced in coreutils-8.24] diff --git a/src/tail.c b/src/tail.c index caa5407..c2982f5 100644 --- a/src/tail.c +++ b/src/tail.c @@ -956,8 +956,8 @@ recheck (struct File_spec *f, bool blocking) f->errnum = -1; f->ignore = true; - error (0, 0, _("%s has been replaced with a symbolic link. " - "giving up on this name"), quoteaf (pretty_name (f))); + error (0, 0, _("%s has been replaced with a symbolic link"), + quoteaf (pretty_name (f))); } else if (fd == -1 || fstat (fd, &new_stats) < 0) { @@ -986,17 +986,20 @@ recheck (struct File_spec *f, bool blocking) { ok = false; f->errnum = -1; - error (0, 0, _("%s has been replaced with an untailable file;\ - giving up on this name"), - quoteaf (pretty_name (f))); - f->ignore = true; + f->tailable = false; + if (! (reopen_inaccessible_files && follow_mode == Follow_name)) + f->ignore = true; + if (was_tailable || prev_errnum != f->errnum) + error (0, 0, _("%s has been replaced with an untailable file%s"), + quoteaf (pretty_name (f)), + f->ignore ? _("; giving up on this name") : ""); } else if (!disable_inotify && fremote (fd, pretty_name (f))) { ok = false; f->errnum = -1; - error (0, 0, _("%s has been replaced with a remote file. " - "giving up on this name"), quoteaf (pretty_name (f))); + error (0, 0, _("%s has been replaced with a remote file"), + quoteaf (pretty_name (f))); f->ignore = true; f->remote = true; } @@ -1075,20 +1078,14 @@ any_live_files (const struct File_spec *f, size_t n_files) { size_t i; - if (reopen_inaccessible_files && follow_mode == Follow_name) - return true; /* continue following for -F option */ - for (i = 0; i < n_files; i++) { if (0 <= f[i].fd) - { - return true; - } + return true; else { - if (reopen_inaccessible_files && follow_mode == Follow_descriptor) - if (! f[i].ignore) - return true; + if (! f[i].ignore && reopen_inaccessible_files) + return true; } } @@ -1930,12 +1927,14 @@ tail_file (struct File_spec *f, uintmax_t n_units) } else if (!IS_TAILABLE_FILE_TYPE (stats.st_mode)) { - error (0, 0, _("%s: cannot follow end of this type of file;\ - giving up on this name"), - quotef (pretty_name (f))); ok = false; f->errnum = -1; - f->ignore = true; + f->tailable = false; + f->ignore = ! (reopen_inaccessible_files + && follow_mode == Follow_name); + error (0, 0, _("%s: cannot follow end of this type of file%s"), + quotef (pretty_name (f)), + f->ignore ? _("; giving up on this name") : ""); } if (!ok) diff --git a/tests/tail-2/retry.sh b/tests/tail-2/retry.sh index 37e01ed..a4cff11 100755 --- a/tests/tail-2/retry.sh +++ b/tests/tail-2/retry.sh @@ -55,11 +55,14 @@ tail --retry missing > out 2>&1 && fail=1 [ "$(countlines_)" = 2 ] || { cat out; fail=1; } grep -F 'tail: warning: --retry ignored' out || { cat out; fail=1; } +for mode in '' '---disable-inotify'; do + # === Test: # Ensure that "tail --retry --follow=name" waits for the file to appear. # Clear 'out' so that we can check its contents without races >out || framework_failure_ -timeout 10 tail $fastpoll --follow=name --retry missing >out 2>&1 & pid=$! +timeout 10 \ + tail $mode $fastpoll --follow=name --retry missing >out 2>&1 & pid=$! # Wait for "cannot open" error. retry_delay_ wait4lines_ .1 6 1 || { cat out; fail=1; } echo "X" > missing || framework_failure_ @@ -76,7 +79,8 @@ rm -f missing out || framework_failure_ # === Test: # 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). -timeout 10 tail $fastpoll --follow=descriptor --retry missing >out 2>&1 & pid=$! +timeout 10 \ + tail $mode $fastpoll --follow=descriptor --retry missing >out 2>&1 & pid=$! # Wait for "cannot open" error. retry_delay_ wait4lines_ .1 6 2 || { cat out; fail=1; } echo "X" > missing || framework_failure_ @@ -95,7 +99,8 @@ rm -f missing out || framework_failure_ # === Test: # Ensure that tail --follow=descriptor --retry exits when the file appears # untailable. Expect exit status 1. -timeout 10 tail $fastpoll --follow=descriptor --retry missing >out 2>&1 & pid=$! +timeout 10 \ + tail $mode $fastpoll --follow=descriptor --retry missing >out 2>&1 & pid=$! # Wait for "cannot open" error. retry_delay_ wait4lines_ .1 6 2 || { cat out; fail=1; } mkdir missing || framework_failure_ # Create untailable @@ -116,16 +121,35 @@ rm -fd missing out || framework_failure_ # 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 +tail $mode --follow=descriptor missing >out 2>&1 && fail=1 [ "$(countlines_)" = 2 ] || { fail=1; cat out; } grep -F 'cannot open' out || { fail=1; cat out; } grep -F 'no files remaining' out || { fail=1; cat out; } # === Test: # Likewise for --follow=name (without --retry). -tail --follow=name missing >out 2>&1 && fail=1 +tail $mode --follow=name missing >out 2>&1 && fail=1 [ "$(countlines_)" = 2 ] || { fail=1; cat out; } grep -F 'cannot open' out || { fail=1; cat out; } grep -F 'no files remaining' out || { fail=1; cat out; } +# === Test: +# Ensure that tail -F retries when the file is initally untailable. +mkdir untailable +timeout 10 \ + tail $mode $fastpoll -F untailable >out 2>&1 & pid=$! +# Wait for "cannot open" error. +retry_delay_ wait4lines_ .1 6 2 || { cat out; fail=1; } +{ rmdir untailable; echo foo > untailable; } || framework_failure_ +# Wait for the expected output. +retry_delay_ wait4lines_ .1 6 4 || { cat out; fail=1; } +cleanup_ +[ "$(countlines_)" = 4 ] || { fail=1; cat out; } +grep -F 'cannot follow' out || { fail=1; cat out; } +grep -F 'has become accessible' out || { fail=1; cat out; } +grep -F 'foo' out || { fail=1; cat out; } +rm -fd untailable out || framework_failure_ + +done + Exit $fail -- 2.5.5
