Skip to content

Conversation

@arp242
Copy link
Member

@arp242 arp242 commented Oct 15, 2022

There were two problems here:

  • When we see a link inside a directory the resolved version would get added to w.watches etc. but the link won't, resulting in many spurious create events for the link. This fixes Spurious symlink CREATE events on macOS when watching a directory #277; I'm surprised there aren't more reports for this).

  • filepath.EvalSymlinks() will resolve any symlinks in the entire path, rather than just the link itself. This would cause paths such as:

    /path/to/LINK/dir/dir/WATCH-THIS
    

    To have the wrong Event.Name; many of the test cases failed because of this because /tmp is a link to /private/tmp on macOS, but curiously no one reported it AFAIK (I guess many people don't use symlinks all that much).

    Example:

    % mkdir /tmp/xxx
    % touch /tmp/xxx/FILE
    
    % ln -s /tmp/xxx/LINK /tmp/xxx/FILE   # 25 years of Unix experience, and still...
    ln: /tmp/xxx/FILE: File exists
    % ln -s /tmp/xxx/FILE /tmp/xxx/LINK
    

    Before it would do:

    % go run ./cmd/fsnotify watch /tmp/xxx &
    % touch /tmp/xxx/FILE
    03:23:03.2731   1 CHMOD         "/private/tmp/xxx/FILE"
    03:23:03.2744   2 CHMOD         "/tmp/xxx/FILE"
    ^C
    
    % fsnotify watch /tmp/xxx/LINK &
    % touch /tmp/xxx/LINK
    03:26:37.3576   1 CHMOD         "/private/tmp/xxx/FILE"
    

    And now it does:

    % go run ./cmd/fsnotify watch /tmp/xxx &
    03:23:47.8911   1 CHMOD         "/tmp/xxx/FILE"
    ^C
    
    % fsnotify watch /tmp/xxx/LINK &
    % touch /tmp/xxx/LINK
    03:27:38.5227   1 CHMOD         "/tmp/xxx/FILE"
    

@arp242
Copy link
Member Author

arp242 commented Oct 15, 2022

Still got some build failures in CI.

macOS 11 (after restart it did work):

--- FAIL: TestWatchSymlink/277 (1.18s)
    fsnotify_test.go:558: 
        have:
        	CREATE               "/foo"
        	WRITE                ""
        	WRITE                ""
        	WRITE                ""
        	REMOVE               "/foo"
        	CREATE               "/apple"
        	CREATE               "/pear"
        	REMOVE|RENAME        "/apple"
        	REMOVE               "/pear"
        want:
        	CREATE               "/foo"
        	REMOVE               "/foo"
        	CREATE               "/apple"
        	CREATE               "/pear"
        	REMOVE|RENAME        "/apple"
        	REMOVE               "/pear"

FreeBSD:

        	CREATE               "/foo"
        	REMOVE               "/foo"
        	CREATE               "/apple"
        	CREATE               "/pear"
        	REMOVE|RENAME        "/apple"
        	REMOVE               "/pear"
        	REMOVE|WRITE         ""
        	CREATE               "."

        	# After restart
        	CREATE               "/foo"
        	REMOVE               "/foo"
        	CREATE               "/apple"
        	CREATE               "/pear"
        	REMOVE|RENAME        "/apple"
        	REMOVE               "/pear"

NetBSD:

        	CREATE               "/foo"
        	REMOVE               "/foo"
        	CREATE               "/apple"
        	REMOVE|RENAME        "/apple"

        	# After restart
        	CREATE               "/foo"
        	REMOVE               "/foo"
        	CREATE               "/apple"
        	CREATE               "/pear"
        	REMOVE|RENAME        "/apple"
        	REMOVE               "/pear"

@arp242 arp242 force-pushed the kq-symlink branch 3 times, most recently from 76959bc to 3dabc49 Compare November 16, 2022 04:36
@arp242 arp242 force-pushed the kq-symlink branch 4 times, most recently from fa77cc2 to 37e4300 Compare July 12, 2023 17:15
There were two problems here:

- When we see a link inside a directory the resolved version would get
  added to w.watches etc. but the link won't, resulting in many spurious
  create events for the link. This fixes #277; I'm surprised there
  aren't more reports for this).

- filepath.EvalSymlinks() will resolve any symlinks in the entire path,
  rather than just the link itself. This would cause paths such as:

      /path/to/LINK/dir/dir/WATCH-THIS

  To have the wrong Event.Name; many of the test cases failed because of
  this because /tmp is a link to /private/tmp on macOS, but curiously no
  one reported it AFAIK (I guess many people don't use symlinks all that
  much).

  Example:

      % mkdir /tmp/xxx
      % touch /tmp/xxx/FILE

      % ln -s /tmp/xxx/LINK /tmp/xxx/FILE   # 25 years of Unix experience, and still...
      ln: /tmp/xxx/FILE: File exists
      % ln -s /tmp/xxx/FILE /tmp/xxx/LINK

  Before it would do:

      % go run ./cmd/fsnotify watch /tmp/xxx &
      % touch /tmp/xxx/FILE
      03:23:03.2731   1 CHMOD         "/private/tmp/xxx/FILE"
      03:23:03.2744   2 CHMOD         "/tmp/xxx/FILE"
      ^C

      % fsnotify watch /tmp/xxx/LINK &
      % touch /tmp/xxx/LINK
      03:26:37.3576   1 CHMOD         "/private/tmp/xxx/FILE"

  And now it does:

      % go run ./cmd/fsnotify watch /tmp/xxx &
      03:23:47.8911   1 CHMOD         "/tmp/xxx/FILE"
      ^C

      % fsnotify watch /tmp/xxx/LINK &
      % touch /tmp/xxx/LINK
      03:27:38.5227   1 CHMOD         "/tmp/xxx/FILE"
@arp242 arp242 merged commit c35de00 into main Jul 12, 2023
@arp242 arp242 deleted the kq-symlink branch July 12, 2023 18:11
@shogo82148 shogo82148 mentioned this pull request Mar 6, 2024
25 tasks
belimawr added a commit to belimawr/fsnotify that referenced this pull request Sep 25, 2024
Release 1.7.0

This version of fsnotify needs Go 1.17.

Additions:

- illumos: add FEN backend to support illumos and Solaris. ([fsnotify#371])

- all: add `NewBufferedWatcher()` to use a buffered channel, which can
  be useful in cases where you can't control the kernel buffer and
  receive a large number of events in bursts. ([fsnotify#550], [fsnotify#572])

- all: add `AddWith()`, which is identical to `Add()` but allows passing
  options. ([fsnotify#521])

- windows: allow setting the ReadDirectoryChangesW() buffer size with
  `fsnotify.WithBufferSize()`; the default of 64K is the highest value
  that works on all platforms and is enough for most purposes, but in
  some cases a highest buffer is needed. ([fsnotify#521])

Changes and fixes:

- inotify: remove watcher if a watched path is renamed ([fsnotify#518])

  After a rename the reported name wasn't updated, or even an empty
  string. Inotify doesn't provide any good facilities to update it, so
  just remove the watcher. This is already how it worked on kqueue and
  FEN.

  On Windows this does work, and remains working.

- windows: don't listen for file attribute changes ([fsnotify#520])

  File attribute changes are sent as `FILE_ACTION_MODIFIED` by the
  Windows API, with no way to see if they're a file write or attribute
  change, so would show up as a fsnotify.Write event. This is never
  useful, and could result in many spurious Write events.

- windows: return `ErrEventOverflow` if the buffer is full ([fsnotify#525])

  Before it would merely return "short read", making it hard to detect
  this error.

- kqueue: make sure events for all files are delivered properly when
  removing a watched directory ([fsnotify#526])

  Previously they would get sent with `""` (empty string) or `"."` as
  the path name.

- kqueue: don't emit spurious Create events for symbolic links ([fsnotify#524])

  The link would get resolved but kqueue would "forget" it already saw
  the link itself, resulting on a Create for every Write event for the
  directory.

- all: return `ErrClosed` on `Add()` when the watcher is closed ([fsnotify#516])

- other: add `Watcher.Errors` and `Watcher.Events` to the no-op
  `Watcher` in `backend_other.go`, making it easier to use on
  unsupported platforms such as WASM, AIX, etc. ([fsnotify#528])

- other: use the `backend_other.go` no-op if the `appengine` build tag
  is set; Google AppEngine forbids usage of the unsafe package so the
  inotify backend won't compile there.

[fsnotify#371]: fsnotify#371
[fsnotify#516]: fsnotify#516
[fsnotify#518]: fsnotify#518
[fsnotify#520]: fsnotify#520
[fsnotify#521]: fsnotify#521
[fsnotify#524]: fsnotify#524
[fsnotify#525]: fsnotify#525
[fsnotify#526]: fsnotify#526
[fsnotify#528]: fsnotify#528
[fsnotify#537]: fsnotify#537
[fsnotify#550]: fsnotify#550
[fsnotify#572]: fsnotify#572
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Spurious symlink CREATE events on macOS when watching a directory

2 participants