From 76959bc7357a5f287224adcdd47a0cf38415edf8 Mon Sep 17 00:00:00 2001
From: Martin Tournoij <martin@arp242.net>
Date: Sat, 15 Oct 2022 21:53:28 +0200
Subject: [PATCH] wip

---
 CHANGELOG.md      | 12 ++++++++++--
 backend_kqueue.go |  9 ++++-----
 fsnotify.go       |  2 +-
 fsnotify_test.go  | 22 +++++++++++++++-------
 4 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 31fa1a47..cda10ced 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -36,9 +36,17 @@ Unreleased
   Before it would merely return "short read", making it hard to detect this
   error.
 
-- all: return `ErrClosed` on `Add()` when the watcher is closed ([#516])
+- kqueue: make sure events for all files are delivered properly when removing a
+  watched directory ([#526])
 
-- kqueue: deal with `rm -rf watched-dir` better ([#526], [#537])
+  Previously they would get sent with "" or "." as the path name.
+
+- kqueue: don't emit spurious Create events for symbolic links ([#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 ([#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
diff --git a/backend_kqueue.go b/backend_kqueue.go
index 45aaf3ba..6a04ef7f 100644
--- a/backend_kqueue.go
+++ b/backend_kqueue.go
@@ -160,6 +160,7 @@ func NewWatcher() (*Watcher, error) {
 // all.
 func newKqueue() (kq int, closepipe [2]int, err error) {
 	kq, err = unix.Kqueue()
+	// fmt.Println("OPEN", kq, err)
 	if kq == -1 {
 		return kq, closepipe, err
 	}
@@ -500,13 +501,10 @@ func (w *Watcher) addWatch(name string, flags uint32) (string, error) {
 // Event values that it sends down the Events channel.
 func (w *Watcher) readEvents() {
 	defer func() {
-		err := unix.Close(w.kq)
-		if err != nil {
-			w.Errors <- err
-		}
-		unix.Close(w.closepipe[0])
 		close(w.Events)
 		close(w.Errors)
+		_ = unix.Close(w.kq)
+		unix.Close(w.closepipe[0])
 	}()
 
 	eventBuffer := make([]unix.Kevent_t, 10)
@@ -742,6 +740,7 @@ func (w *Watcher) register(fds []int, flags int, fflags uint32) error {
 // read retrieves pending events, or waits until an event occurs.
 func (w *Watcher) read(events []unix.Kevent_t) ([]unix.Kevent_t, error) {
 	n, err := unix.Kevent(w.kq, nil, events, nil)
+	// fmt.Printf("kevent(%d) -> %d %v\n", w.kq, n, err)
 	if err != nil {
 		return nil, err
 	}
diff --git a/fsnotify.go b/fsnotify.go
index 142169da..524f29dc 100644
--- a/fsnotify.go
+++ b/fsnotify.go
@@ -63,7 +63,7 @@ const (
 
 // Common errors that can be reported.
 var (
-	ErrNonExistentWatch = errors.New("fsnotify: can't remove non-existent watcher")
+	ErrNonExistentWatch = errors.New("fsnotify: can't remove non-existent watch")
 	ErrEventOverflow    = errors.New("fsnotify: queue or buffer overflow")
 	ErrClosed           = errors.New("fsnotify: watcher already closed")
 )
diff --git a/fsnotify_test.go b/fsnotify_test.go
index da3708f6..5cb76534 100644
--- a/fsnotify_test.go
+++ b/fsnotify_test.go
@@ -531,6 +531,16 @@ func TestWatchSymlink(t *testing.T) {
 
 		// Bug #277
 		{"277", func(t *testing.T, w *Watcher, tmp string) {
+			// TODO: there is some strange fuckery going on if I use go test
+			// -count=2; the second test run has unix.Kqueue() in newKqueue()
+			// return 0, which is a very odd fd number, but the first event does
+			// work (create /foo). After that we get EBADF (Bad file
+			// descriptor).
+			//
+			// This is *only* for this test, and *only* if we have the symlinks
+			// below. kqueue(2) doesn't document returning fd 0.
+			//
+			// This happens on both FreeBSD and NetBSD.
 			touch(t, tmp, "file1")
 			touch(t, tmp, "file2")
 			symlink(t, join(tmp, "file1"), tmp, "link1")
@@ -551,15 +561,13 @@ func TestWatchSymlink(t *testing.T) {
 			remove   /pear    # rm -r pear
 
 			kqueue:
-				create         /foo    # touch foo
-				remove         /foo    # rm foo
-				create         /apple  # mkdir apple
-				create         /pear   # mv apple pear
-				remove|rename  /apple
-				remove         /pear   # rm -r pear
+				create   /foo    # touch foo
+				remove   /foo    # rm foo
+				create   /apple  # mkdir apple
+				rename   /apple  # mv apple pear
+				# TODO: why no create /pear and remove /pear?
 		`},
 	}
-
 	for _, tt := range tests {
 		tt := tt
 		tt.run(t)