tail: improve inotify handling of symlinks

Previous behavior failed to read contents of a (re)appearing file,
when symlinked by tail's watched file.  Also we now diagnose other
edge cases when running in inotify mode, where an initially
missing or regular file changes to a symlink.

* src/tail.c (main): If any arg is a symlink, use polling mode.
(recheck): Diagnose the edge case where a symlink appears during
inotify processing.
* tests/tail-2/symlink.sh: Test the fix. Mention the edge cases.
* tests/local.mk: Reference the new test.
* NEWS: Mention the fix.
Reported by: Ondrej Oprala

4 files changed, 123 insertions, 4 deletions

diff --git a/NEWS b/NEWS
index 5ea592c55..5791a2701 100644
--- a/NEWS
+++ b/NEWS
@@ -43,8 +43,11 @@ GNU coreutils NEWS                                    -*- outline -*-
    by the alignment bug introduced in coreutils-6.0]
 
   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.
+  would immediately exit when such a file is initially inaccessible.
+  [This bug was introduced when inotify support was added in coreutils-7.5]
+
+  tail -F has improved handling of symlinks.  Previously tail didn't respond
+  to the symlink target (re)appearing after being (re)created.
   [This bug was introduced when inotify support was added in coreutils-7.5]
 
 ** New features
diff --git a/src/tail.c b/src/tail.c
index e7fefda71..c781dc2fa 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -945,7 +945,20 @@ recheck (struct File_spec *f, bool blocking)
      then mark the file as not tailable.  */
   f->tailable = !(reopen_inaccessible_files && fd == -1);
 
-  if (fd == -1 || fstat (fd, &new_stats) < 0)
+  if (! disable_inotify && ! lstat (f->name, &new_stats)
+      && S_ISLNK (new_stats.st_mode))
+    {
+      /* Diagnose the edge case where a regular file is changed
+         to a symlink.  We avoid inotify with symlinks since
+         it's awkward to match between symlink name and target.  */
+      ok = false;
+      f->errnum = -1;
+      f->ignore = true;
+
+      error (0, 0, _("%s has been replaced with a symbolic link. "
+                     "giving up on this name"), quote (pretty_name (f)));
+    }
+  else if (fd == -1 || fstat (fd, &new_stats) < 0)
     {
       ok = false;
       f->errnum = errno;
@@ -1251,6 +1264,23 @@ any_remote_file (const struct File_spec *f, size_t n_files)
   return false;
 }
 
+/* Return true if any of the N_FILES files in F is a symlink.
+   Note we don't worry about the edge case where "-" exists,
+   since that will have the same consequences for inotify,
+   which is the only context this function is currently used.  */
+
+static bool
+any_symlinks (const struct File_spec *f, size_t n_files)
+{
+  size_t i;
+
+  struct stat st;
+  for (i = 0; i < n_files; i++)
+    if (lstat (f[i].name, &st) == 0 && S_ISLNK (st.st_mode))
+      return true;
+  return false;
+}
+
 /* Return true if any of the N_FILES files in F represents
    stdin and is tailable.  */
 
@@ -1531,6 +1561,7 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files,
           int new_wd = inotify_add_watch (wd, f[j].name, inotify_wd_mask);
           if (new_wd < 0)
             {
+              /* Can get ENOENT for a dangling symlink for example.  */
               error (0, errno, _("cannot watch %s"), quote (f[j].name));
               continue;
             }
@@ -2206,6 +2237,11 @@ 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.
 
+         any_symlinks() checks if the user has specified any symbolic links.
+         inotify is not used in this case because it returns updated _targets_
+         which would not match the specified names.  If we tried to always
+         use the target names, then we would miss changes to the symlink itself.
+
          ok is false when one of the files specified could not be opened for
          reading.  In this case and when following by descriptor,
          tail_forever_inotify() cannot be used (in its current implementation).
@@ -2222,6 +2258,7 @@ main (int argc, char **argv)
          follow_mode == Follow_name  */
       if (!disable_inotify && (tailable_stdin (F, n_files)
                                || any_remote_file (F, n_files)
+                               || any_symlinks (F, n_files)
                                || (!ok && follow_mode == Follow_descriptor)))
         disable_inotify = true;
 
diff --git a/tests/local.mk b/tests/local.mk
index 59bf07f01..2dd7e2063 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -388,7 +388,8 @@ all_tests =					\
   tests/misc/uniq-perf.sh			\
   tests/misc/xattr.sh				\
   tests/tail-2/wait.sh				\
-  tests/tail-2/retry.sh			\
+  tests/tail-2/retry.sh				\
+  tests/tail-2/symlink.sh				\
   tests/chmod/c-option.sh			\
   tests/chmod/equal-x.sh			\
   tests/chmod/equals.sh				\
diff --git a/tests/tail-2/symlink.sh b/tests/tail-2/symlink.sh
new file mode 100755
index 000000000..bca0797fa
--- /dev/null
+++ b/tests/tail-2/symlink.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+# Ensure tail tracks symlinks properly.
+
+# 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
+
+# Function to check the expected line count in 'out'.
+# Called via retry_delay_().  Sleep some time - see retry_delay_() - if the
+# line count is still smaller than expected.
+wait4lines_ ()
+{
+  local delay=$1
+  local elc=$2   # Expected line count.
+  [ "$( wc -l < out )" -ge "$elc" ] || { sleep $delay; return 1; }
+}
+
+# Ensure changing targets of cli specified symlinks are handled.
+# Prior to v8.22, inotify would fail to recognize changes in the targets.
+# Clear 'out' so that we can check its contents without races.
+:>out                           || framework_failure_
+ln -nsf target symlink          || framework_failure_
+timeout 10 tail -s.1 -F symlink >out 2>&1 & pid=$!
+retry_delay_ wait4lines_ .1 6 1 || fail=1  # Wait for "cannot open..."
+echo "X" > target               || fail=1
+retry_delay_ wait4lines_ .1 6 3 || fail=1  # Wait for the expected output.
+kill $pid
+wait $pid
+# Expect 3 lines in the output file.
+[ $( 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; }
+rm -f target out           || framework_failure_
+
+# Ensure we correctly handle the source symlink itself changing.
+# I.E. that we don't operate solely on the targets.
+# Clear 'out' so that we can check its contents without races.
+:>out                           || framework_failure_
+echo "X1" > target1             || framework_failure_
+ln -nsf target1 symlink         || framework_failure_
+timeout 10 tail -s.1 -F symlink >out 2>&1 & pid=$!
+retry_delay_ wait4lines_ .1 6 1 || fail=1  # Wait for the expected output.
+ln -nsf target2 symlink         || framework_failure_
+retry_delay_ wait4lines_ .1 6 2 || fail=1  # Wait for "become inaccess..."
+echo "X2" > target2             || fail=1
+retry_delay_ wait4lines_ .1 6 4 || fail=1  # Wait for the expected output.
+kill $pid
+wait $pid
+# Expect 4 lines in the output file.
+[ $( wc -l < out ) = 4 ]    || { fail=1; cat out; }
+grep -F 'become inacce' out || { fail=1; cat out; }
+grep -F 'has appeared' out  || { fail=1; cat out; }
+grep '^X1$' out             || { fail=1; cat out; }
+grep '^X2$' out             || { fail=1; cat out; }
+rm -f target1 target2 out   || framework_failure_
+
+# Note other symlink edge cases are currently just diagnosed
+# rather than being handled. I.E. if you specify a missing item,
+# or existing file that later change to a symlink, if inotify
+# is in use, you'll get a diagnostic saying that link will
+# no longer be tailed.
+
+Exit $fail