From 2053e350f3d39e312a6a4b18c05fe8abefc5ee95 Mon Sep 17 00:00:00 2001 From: Michael Albinus Date: Fri, 16 Sep 2022 17:23:59 +0200 Subject: [PATCH] Enable `dont-follow' for inotify file notifications * doc/lispref/os.texi (File Notifications): Symlinks aren't followed. * lisp/filenotify.el (file-notify--add-watch-inotify): Add `dont-follow' flag. * lisp/net/tramp.el (tramp-handle-file-notify-rm-watch): Suppress errors when reading process output. * test/lisp/filenotify-tests.el (file-notify-test11-symlinks) (file-notify-test11-symlinks-remote): New tests. --- doc/lispref/os.texi | 3 + lisp/filenotify.el | 1 + lisp/net/tramp.el | 6 +- test/lisp/filenotify-tests.el | 109 ++++++++++++++++++++++++++++++++++ 4 files changed, 117 insertions(+), 2 deletions(-) diff --git a/doc/lispref/os.texi b/doc/lispref/os.texi index 35828018417..3e16ac0eb49 100644 --- a/doc/lispref/os.texi +++ b/doc/lispref/os.texi @@ -3175,6 +3175,9 @@ This is not detected by this function, and so a non-@code{nil} return value does not guarantee that changes on @var{file} will be actually notified. +If @var{file} is a symlink, it doesn't follow that link. Just +@var{file} itself will be watched. + @var{flags} is a list of conditions to set what will be watched for. It can include the following symbols: diff --git a/lisp/filenotify.el b/lisp/filenotify.el index 94e07289e32..6b13ed0b725 100644 --- a/lisp/filenotify.el +++ b/lisp/filenotify.el @@ -339,6 +339,7 @@ DESC is the back-end descriptor. ACTIONS is a list of: "Add a watch for FILE in DIR with FLAGS, using inotify." (inotify-add-watch dir (append + '(dont-follow) (and (memq 'change flags) '(create delete delete-self modify move-self move)) (and (memq 'attribute-change flags) diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el index 90cc03c188e..f18e4c41c3c 100644 --- a/lisp/net/tramp.el +++ b/lisp/net/tramp.el @@ -5173,8 +5173,10 @@ of." ;; The descriptor must be a process object. (unless (processp proc) (tramp-error proc 'file-notify-error "Not a valid descriptor %S" proc)) - ;; There might be pending output. - (while (tramp-accept-process-output proc 0)) + ;; There might be pending output. Avoid problems with reentrant + ;; call of Tramp. + (ignore-errors + (while (tramp-accept-process-output proc 0))) (tramp-message proc 6 "Kill %S" proc) (delete-process proc)) diff --git a/test/lisp/filenotify-tests.el b/test/lisp/filenotify-tests.el index 2d147e900d7..fef3ab80f9a 100644 --- a/test/lisp/filenotify-tests.el +++ b/test/lisp/filenotify-tests.el @@ -1571,6 +1571,115 @@ the file watch." (file-notify--deftest-remote file-notify-test10-sufficient-resources "Check `file-notify-test10-sufficient-resources' for remote files.") +(ert-deftest file-notify-test11-symlinks () + "Check that file notification do not follow symbolic links." + :tags '(:expensive-test) + (skip-unless (file-notify--test-local-enabled)) + + (setq file-notify--test-tmpfile (file-notify--test-make-temp-name) + file-notify--test-tmpfile1 (file-notify--test-make-temp-name)) + + ;; Symlink a file. + (unwind-protect + (progn + (write-region "any text" nil file-notify--test-tmpfile1 nil 'no-message) + (make-symbolic-link file-notify--test-tmpfile1 file-notify--test-tmpfile) + (should + (setq file-notify--test-desc + (file-notify--test-add-watch + file-notify--test-tmpfile + '(attribute-change change) #'file-notify--test-event-handler))) + (should (file-notify-valid-p file-notify--test-desc)) + + ;; Writing to either the symlink or the target should not + ;; raise any event. + (file-notify--test-with-actions nil + (write-region + "another text" nil file-notify--test-tmpfile nil 'no-message) + (write-region + "another text" nil file-notify--test-tmpfile1 nil 'no-message)) + ;; Sanity check. + (file-notify--test-wait-for-events + (file-notify--test-timeout) + (not (input-pending-p))) + (should-not file-notify--test-events) + + ;; Changing timestamp of the target should not raise any + ;; event. We don't use `nofollow'. + (file-notify--test-with-actions nil + (set-file-times file-notify--test-tmpfile1 '(0 0)) + (set-file-times file-notify--test-tmpfile '(0 0))) + ;; Sanity check. + (file-notify--test-wait-for-events + (file-notify--test-timeout) + (not (input-pending-p))) + (should-not file-notify--test-events) + + ;; Changing timestamp of the symlink shows the event. + (file-notify--test-with-actions '(attribute-changed) + (set-file-times file-notify--test-tmpfile '(0 0) 'nofollow)) + + ;; Deleting the target should not raise any event. + (file-notify--test-with-actions nil + (delete-file file-notify--test-tmpfile1) + (delete-file file-notify--test-tmpfile)) + ;; Sanity check. + (file-notify--test-wait-for-events + (file-notify--test-timeout) + (not (input-pending-p))) + (should-not file-notify--test-events) + + ;; The environment shall be cleaned up. + (file-notify-rm-watch file-notify--test-desc) + (file-notify--test-cleanup-p)) + + ;; Cleanup. + (file-notify--test-cleanup)) + + (setq file-notify--test-tmpfile1 (file-notify--test-make-temp-name) + file-notify--test-tmpfile (file-notify--test-make-temp-name)) + + ;; Symlink a directory. + (unwind-protect + (let ((tmpfile (expand-file-name "foo" file-notify--test-tmpfile)) + (tmpfile1 (expand-file-name "foo" file-notify--test-tmpfile1))) + (make-directory file-notify--test-tmpfile1) + (make-symbolic-link file-notify--test-tmpfile1 file-notify--test-tmpfile) + (write-region "any text" nil tmpfile1 nil 'no-message) + (should + (setq file-notify--test-desc + (file-notify--test-add-watch + file-notify--test-tmpfile + '(attribute-change change) #'file-notify--test-event-handler))) + (should (file-notify-valid-p file-notify--test-desc)) + + ;; None of the actions on a file in the symlinked directory will be reported. + (file-notify--test-with-actions nil + (write-region "another text" nil tmpfile nil 'no-message) + (write-region "another text" nil tmpfile1 nil 'no-message) + (set-file-times tmpfile '(0 0)) + (set-file-times tmpfile '(0 0) 'nofollow) + (set-file-times tmpfile1 '(0 0)) + (set-file-times tmpfile1 '(0 0) 'nofollow) + (delete-file tmpfile) + (delete-file tmpfile1)) + ;; Sanity check. + (file-notify--test-wait-for-events + (file-notify--test-timeout) + (not (input-pending-p))) + (should-not file-notify--test-events) + + ;; The environment shall be cleaned up. + (delete-directory file-notify--test-tmpdir 'recursive) + (file-notify-rm-watch file-notify--test-desc) + (file-notify--test-cleanup-p)) + + ;; Cleanup. + (file-notify--test-cleanup))) + +(file-notify--deftest-remote file-notify-test11-symlinks + "Check `file-notify-test11-symlinks' for remote files.") + (defun file-notify-test-all (&optional interactive) "Run all tests for \\[file-notify]." (interactive "p") -- 2.39.2