]> go.fuhry.dev Git - fsnotify.git/log
fsnotify.git
2 years agoUpdate issue templates
Martin Tournoij [Tue, 13 Jun 2023 15:29:48 +0000 (17:29 +0200)]
Update issue templates

2 years agoMerge pull request #566 from shogo82148/bump-actions-setup-go-v4
Martin Tournoij [Tue, 6 Jun 2023 02:22:14 +0000 (04:22 +0200)]
Merge pull request #566 from shogo82148/bump-actions-setup-go-v4

bump actions/setup-go v4

2 years agobump actions/setup-go v4
ICHINOSE Shogo [Wed, 17 May 2023 20:03:47 +0000 (05:03 +0900)]
bump actions/setup-go v4

2 years agoupdate x/sys (#554)
kylie [Thu, 9 Feb 2023 17:32:20 +0000 (09:32 -0800)]
update x/sys (#554)

Co-authored-by: k.a <k.a@email.com>
2 years agoRun iOS tests against newer Go version (#558)
Martin Tournoij [Thu, 9 Feb 2023 14:47:04 +0000 (15:47 +0100)]
Run iOS tests against newer Go version (#558)

Was still using Go 1.16. Just update the image: updating Homebrew takes
~2 minutes and slows down the CI a lot.

2 years agoUpdate CI to Go 1.17 - 1.20 (#557)
Martin Tournoij [Thu, 9 Feb 2023 14:31:23 +0000 (15:31 +0100)]
Update CI to Go 1.17 - 1.20 (#557)

I have no idea if anyone is using Go 1.16 with fsnotify.

Go 1.16 (released August 2021) became mandatory with v1.5.1 (August
2021). Go 1.17 was released just a few days before it was tagged. Some
people did experience problems (#400), but overall the feedback was
limited.

So based on this, I think it's probably safe to just follow the Go
release policy: "Each major Go release is supported until there are two
newer major releases."

This will allow merging #554

2 years agoTest adding a path more than once; update documentation (#549)
Martin Tournoij [Tue, 17 Jan 2023 17:20:23 +0000 (18:20 +0100)]
Test adding a path more than once; update documentation (#549)

Seems like this doesn't return an error with any backend. Maybe it
should, but we can't really change it because it's not compatible. I
tested with v1.5.4 and v1.4.8 too to make sure it's not a recent
regression, and it behaves identical there too.

Fixes #548

2 years agoiOS is tested (via CircleCI)
Martin Tournoij [Sun, 15 Jan 2023 16:05:09 +0000 (17:05 +0100)]
iOS is tested (via CircleCI)

2 years agoMention aix branch in README
Martin Tournoij [Sun, 15 Jan 2023 16:04:18 +0000 (17:04 +0100)]
Mention aix branch in README

2 years agoIncrease wait on TestClose()
Martin Tournoij [Sat, 14 Jan 2023 21:09:21 +0000 (22:09 +0100)]
Increase wait on TestClose()

Should hopefully help with:
https://github.com/fsnotify/fsnotify/issues/538#issuecomment-1318173851

2 years agoAdd basic benchmark for Add() and Remove()
Martin Tournoij [Sat, 14 Jan 2023 20:19:28 +0000 (21:19 +0100)]
Add basic benchmark for Add() and Remove()

2 years agoTest Add() on non-existent path (#546)
Martin Tournoij [Sat, 14 Jan 2023 20:13:13 +0000 (21:13 +0100)]
Test Add() on non-existent path (#546)

Ref: #144

2 years agoRemove file immediately in TestWatchStress (#545)
Martin Tournoij [Sat, 14 Jan 2023 19:37:03 +0000 (20:37 +0100)]
Remove file immediately in TestWatchStress (#545)

Previously it would create all the files and then remove it; remove the
file immediately instead.

My /tmp has ~500k free inodes and it will try to create >1 million
files, so it will error out on that.

2 years agoinotify: simplify bookkeeping of watched paths (#543)
Martin Tournoij [Sat, 14 Jan 2023 17:28:20 +0000 (18:28 +0100)]
inotify: simplify bookkeeping of watched paths (#543)

Create a new watches type to keep track of the watches instead of
keeping two maps on the Watcher and accessing these directly.

This makes the bookkeeping a bit easier to follow, and we no longer need
to worry about locking map access as the watcher type takes care of that
now.

Came up in #472 where I want to keep track if a path was added
recursively, and this makes that a bit easier.

Also seems a bit faster:

BenchmarkWatch-2          903709              7122 ns/op             194 B/op          3 allocs/op
BenchmarkWatch-2          923980              6322 ns/op             196 B/op          3 allocs/op

Although that benchmark is very simply and only tests one code path;
just want to make sure it's not a horrible regression.

2 years agoWait a bit longer in TestInotifyDeleteOpenFile()
Martin Tournoij [Wed, 21 Dec 2022 21:18:19 +0000 (22:18 +0100)]
Wait a bit longer in TestInotifyDeleteOpenFile()

Solves some intermittent test failures on my laptop.

2 years agoAdd a basic benchmark
Martin Tournoij [Tue, 20 Dec 2022 14:16:22 +0000 (15:16 +0100)]
Add a basic benchmark

2 years agoAdd recursive watcher for Windows backend (#540)
Martin Tournoij [Tue, 20 Dec 2022 12:50:49 +0000 (13:50 +0100)]
Add recursive watcher for Windows backend (#540)

Recursive watches can be added by using a "/..." parameter, similar to
the Go command:

w.Add("dir")         // Behaves as before.
w.Add("dir/...")     // Watch dir and all paths underneath it.

w.Remove("dir")      // Remove the watch for dir and, if
                     // recursive, all paths underneath it too

w.Remove("dir/...")  // Behaves like just "dir" if the path was
                     // recursive, error otherwise (probably
                     // want to add recursive remove too at some
                     // point).

The advantage of using "/..." vs. an option is that it can be easily
specified in configuration files and the like; for example from a TOML
file:

[watches]
dirs = ["/tmp/one", "/tmp/two/..."]

Options for this were previously discussed at:
https://github.com/fsnotify/fsnotify/pull/339#discussion_r788246013

This should be expanded to other backends too; I started with Windows
because the implementation is the both the easiest and has the least
amount of control (just setting a boolean parameter), and we can focus
mostly on writing tests and documentation and the for it, and we can
then match the inotify and kqueue behaviour to the Windows one.

Fixes #21

Co-authored-by: Milas Bowman <milasb@gmail.com>
3 years agoRemove TestExchangedata* tests for old macOS
Martin Tournoij [Thu, 17 Nov 2022 06:45:49 +0000 (07:45 +0100)]
Remove TestExchangedata* tests for old macOS

The exchangedata() system call has never been supported on APFS
introduced in macOS 10.13 (2017, unsupported since late 2020) and since
macOS 10.14 (2018, unsupported since mid 2021) all HFS+ drives are
converted to APFS. As far as I can find you can't use HFS+ drives with
current versions of macOS:

https://old.reddit.com/r/macsysadmin/comments/tk5lsi/use_hfs_instead_of_apfs_with_monterey/
https://old.reddit.com/r/MacOS/comments/jgodi9/i_cannot_update_because_i_cannot_convert_to_apfs/

So it seems we can safely remove this test. It wasn't run in the CI
anyway, as this only tests macOS 11 and 12.

3 years agoDocument that Watcher.Remove returns nil if the Watcher is closed
Martin Tournoij [Wed, 16 Nov 2022 04:17:23 +0000 (05:17 +0100)]
Document that Watcher.Remove returns nil if the Watcher is closed

This was already tested in TestClose/error_after_closed; just wasn't
documented.

3 years agoA few staticcheck fixes (#539)
Martin Tournoij [Wed, 16 Nov 2022 04:17:00 +0000 (05:17 +0100)]
A few staticcheck fixes (#539)

This fixes a few small staticcheck issues; turns out that staticcheck
only compiles the files for the current GOOS:
https://staticcheck.io/docs/running-staticcheck/cli/build-tags/

You can run all systems with:

staticcheck -matrix <<EOF
windows: GOOS=windows
linux: GOOS=linux
freebsd: GOOS=freebsd
openbsd: GOOS=openbsd
netbsd: GOOS=netbsd
darwin: GOOS=darwin
illumos: GOOS=illumos
EOF

The CI runner doesn't really support that, but it's little more than a
small wrapper around "go install" with some variables, so just use that
directly instead of the action.

3 years agoMerge pull request #537 from fsnotify/fix-526
Martin Tournoij [Wed, 16 Nov 2022 03:11:22 +0000 (04:11 +0100)]
Merge pull request #537 from fsnotify/fix-526

kqueue: fix removing a directory

3 years agoRun FreeBSD CI on Cirrus
Martin Tournoij [Wed, 16 Nov 2022 02:26:34 +0000 (03:26 +0100)]
Run FreeBSD CI on Cirrus

3 years agoFix BSD CI ... hopefully
Martin Tournoij [Wed, 16 Nov 2022 02:11:40 +0000 (03:11 +0100)]
Fix BSD CI ... hopefully

3 years agokqueue: fix removing a directory
Martin Tournoij [Sun, 16 Oct 2022 19:28:33 +0000 (21:28 +0200)]
kqueue: fix removing a directory

Fix regression from #526, which would sometimes fail with something
along the lines of:

    --- FAIL: TestWatchRm/remove_watched_directory (1.30s)
        helpers_test.go:384: fsnotify.sendDirectoryChangeEvents: open /tmp/TestWatchRmremove_watched_directory2055111636/001: no such file or directory
        fsnotify_test.go:750:
            have:
             REMOVE               "/a"
             REMOVE               "/b"
             REMOVE               "/c"
             REMOVE               "/d"
             REMOVE               "/e"
             REMOVE               "/f"
             REMOVE               "/g"
            want:
             REMOVE               "/"
             REMOVE               "/a"
             REMOVE               "/b"
             REMOVE               "/c"
             REMOVE               "/d"
             REMOVE               "/e"
             REMOVE               "/f"
             REMOVE               "/g"
             REMOVE               "/h"
             REMOVE               "/i"
             REMOVE               "/j"

We can just always ignore a directory no longer existing; kqueue should
still send the correct events.

Also noticed the error on sendFileCreatedEventIfNew() wasn't really
being sent properly, so deal with that as well.

3 years agoRemove TestInotifyNoBlockingSyscalls (#532)
Martin Tournoij [Tue, 8 Nov 2022 20:43:19 +0000 (21:43 +0100)]
Remove TestInotifyNoBlockingSyscalls (#532)

This test checks if no additional OS threads are created; this was added
after the old epoll-based inotify code was removed (which would create
additional threads). It's always been a bit flaky: it currently already
runs the test twice and "fails" only if both fail.

We don't really control the Go scheduler and when it creates/destroys OS
threads, or have all that much insight in it (the "threadcreate" profile
in pprof has been broken for years). I tried to make this test a bit
more reliable a few months ago, and the run-it-twice "solution" was the
best I could come up.

It still causes some failures though; see the links below. I don't know
why it consistently fails on armle (and not e.g. arm64), but that seems
a Go thing, rather than a fsnotify thing.

I'm not sure if this test adds all that much value; I don't really
foresee a regression where we accidentally create more OS threads than
we should.

Fixes #531
Also see: https://bugs.launchpad.net/ubuntu/+source/golang-fsnotify/+bug/1971967

3 years agoDon't build on appengine
Martin Tournoij [Fri, 4 Nov 2022 21:54:40 +0000 (22:54 +0100)]
Don't build on appengine

AppEngine forbids usage of the unsafe package,

Fixes #534
Fixes https://github.com/spf13/viper/issues/538

3 years agoUpdate ChangeLog
Martin Tournoij [Thu, 3 Nov 2022 20:04:01 +0000 (21:04 +0100)]
Update ChangeLog

3 years agoUpdate ChangeLog
Martin Tournoij [Sun, 30 Oct 2022 10:52:36 +0000 (11:52 +0100)]
Update ChangeLog

3 years agoAdd AddWith() to pass options, allow controlling Windows buffer size (#521)
Martin Tournoij [Sun, 30 Oct 2022 10:47:55 +0000 (11:47 +0100)]
Add AddWith() to pass options, allow controlling Windows buffer size (#521)

This is similar to Add(), except that you can pass options. Ideally this
should just be Add(), but that's not a compatible change, so we're stuck
with this until we do a v2.

There are quite a few enhancements that depend on *some* way to pass
options; as an example I added WithBufferSize() to control the buffer
size for (see #72) for the Windows backend, because that one is fairly
trivial to implement:

w, err := fsnotify.NewWatcher()
err = w.AddWith("/path", fsnotify.WithBufferSize(65536*4))

Some other options we might want to add:

err = w.AddWith("/path",
fsnotify.WithEvents(fsnotify.Open | fsnotify.Close),  // Filter events
fsnotify.WithPoll(time.Second),                       // Use poll watcher
fsnotify.WithFanotify(),                              // Prefer fanotify on Linux
fsnotify.WithFollowLinks(true),                       // Control symlink follow behaviour
fsnotify.WithDebounce(100*time.Milliseconds),         // Automatically debounce duplicate events
fsnotify.WithRetry(1*time.Second, 1*time.Minute),     // Retry every second if the path disappears for a minute
)

These are just some ideas, nothing fixed here yet. Some of these options
are likely to change once I get around to actually working on it.

This uses "functional options" so we can add more later. Options are
passed to Add() rather than the Watcher itself, so the behaviour can be
modified for every watch, rather than being global. This way you can do
things like watch /nfs-drive with a poll backend, and use the regular OS
backend for ~/dir, without having to create two watchers.

This upgrades fairly nicely to v2 where we rename AddWith() to Add():

err = w.Add("/path",
fsnotify.WithBufferSize(65536*4),
fsnotify.WithEvents(fsnotify.Open | fsnotify.Close))

Folks will just have to s/fsnotify.AddWith/fsnotify.Add/, without having
to change all the option names. Plus having a consistent prefix
autocompletes nicely in editors.

Fixes #72

3 years agoDocument events a bit better
Martin Tournoij [Mon, 17 Oct 2022 06:41:35 +0000 (08:41 +0200)]
Document events a bit better

Fixes #529

3 years agogofmt 1.19
Martin Tournoij [Sat, 29 Oct 2022 18:23:14 +0000 (20:23 +0200)]
gofmt 1.19

3 years agoCheck privileges for symlink tests on Windows (#530)
mattn [Thu, 20 Oct 2022 02:22:52 +0000 (11:22 +0900)]
Check privileges for symlink tests on Windows (#530)

Tests for symlink always fail when run the tests as non-admin users on Windows since Windows require privileges to create symlinks. This CL change to check the privileges.

3 years agoEh, also update the other workflows
Martin Tournoij [Thu, 20 Oct 2022 01:25:14 +0000 (03:25 +0200)]
Eh, also update the other workflows

3 years agoTrigger Actions on PR
Martin Tournoij [Thu, 20 Oct 2022 01:23:16 +0000 (03:23 +0200)]
Trigger Actions on PR

3 years agoAdd CI for linux/arm64, darwin/arm64 (#528)
Martin Tournoij [Mon, 17 Oct 2022 01:57:01 +0000 (03:57 +0200)]
Add CI for linux/arm64, darwin/arm64 (#528)

Add CI for linux/arm64, darwin/arm64

Use CircleCI to add CI for this.

3 years agoActually, just return the first error
Martin Tournoij [Sat, 15 Oct 2022 20:58:27 +0000 (22:58 +0200)]
Actually, just return the first error

Now I got thousands of:

    helpers_test.go:384: fsnotify.readEvents: bad file descriptor
    helpers_test.go:384: fsnotify.readEvents: bad file descriptor
    helpers_test.go:384: fsnotify.readEvents: bad file descriptor
    helpers_test.go:384: fsnotify.readEvents: bad file descriptor
    helpers_test.go:384: fsnotify.readEvents: bad file descriptor
    helpers_test.go:384: fsnotify.readEvents: bad file descriptor
    helpers_test.go:384: fsnotify.readEvents: bad file descriptor
    helpers_test.go:384: fsnotify.readEvents: bad file descriptor
    helpers_test.go:384: fsnotify.readEvents: bad file descriptor
    helpers_test.go:384: fsnotify.readEvents: bad file descriptor
    helpers_test.go:384: fsnotify.readEvents: bad file descriptor
    helpers_test.go:384: fsnotify.readEvents: bad file descriptor
    helpers_test.go:384: fsnotify.readEvents: bad file descriptor
    helpers_test.go:384: fsnotify.readEvents: bad file descriptor
    helpers_test.go:384: fsnotify.readEvents: bad file descriptor
    helpers_test.go:384: fsnotify.readEvents: bad file descriptor
    helpers_test.go:384: fsnotify.readEvents: bad file descriptor

That's not helpful.

3 years agoDon't return in collect test helper on errors
Martin Tournoij [Sat, 15 Oct 2022 20:54:04 +0000 (22:54 +0200)]
Don't return in collect test helper on errors

Would return without calling close, so on errors you'd get a meaningless
"event stream was not closed after 1s":

--- FAIL: TestWatchRm (0.00s)
--- FAIL: TestWatchRm/remove_watched_directory (2.43s)
helpers_test.go:384: fsnotify.sendDirectoryChangeEvents: open /tmp/TestWatchRmremove_watched_directory2304249565/001: no such file or directory
helpers_test.go:354: event stream was not closed after 1s

3 years agoRun gccgo in CI (#527)
Martin Tournoij [Sat, 15 Oct 2022 20:33:56 +0000 (22:33 +0200)]
Run gccgo in CI (#527)

Fixes #190

3 years agokqueue: don't immediately remove watches for all files in a directory on Delete event...
Martin Tournoij [Sat, 15 Oct 2022 08:33:09 +0000 (10:33 +0200)]
kqueue: don't immediately remove watches for all files in a directory on Delete event (#526)

The problem was that that on Delete events for directories it would call
Watcher.Remove() for all files in that directory too. This is fine if you call
w.Remove() from the application, but if you rm -rf a directory the directory
itself tends to be the first remove event (on FreeBSD at least):

DELETE WRITE  /tmp/xxx
DELETE        /tmp/xxx/a
DELETE        /tmp/xxx/b
DELETE        /tmp/xxx/c
DELETE        /tmp/xxx/d
DELETE        /tmp/xxx/e
DELETE        /tmp/xxx/f
DELETE        /tmp/xxx/g

So what would happen is that the internal state for /tmp/xxx/a etc. would get
removed, and when the event gets processed we no longer have access to it.

Move Remove() to remove() with a flag to control removing files. If a watched
directory is removed then the files will emit their own delete event, so we
don't really need to do this.

Fixes #514

Also:

- Don't send Remove|Write events; that's never really useful: if it's
  gone, then it's gone. No other backends do this.

- Save a stat call by checking the readdir error instead.

- Don't stat every directory; this isn't really needed, and getting a
  write event before a delete is fine (and this didn't suppress the
  write even anyway).

- Add test/kqueue.c; just a simple implementation to test things.

3 years agowindows: use ErrEventOverflow instead of "short read in readEvents()" (#525)
Martin Tournoij [Sat, 15 Oct 2022 01:57:47 +0000 (03:57 +0200)]
windows: use ErrEventOverflow instead of "short read in readEvents()" (#525)

Extracted from #521; we want to do this no matter what.

Fixes #383

3 years agoTweak docs a bit
Martin Tournoij [Fri, 14 Oct 2022 22:28:33 +0000 (00:28 +0200)]
Tweak docs a bit

3 years agoReformat comments to textwidth=80
Martin Tournoij [Fri, 14 Oct 2022 22:20:54 +0000 (00:20 +0200)]
Reformat comments to textwidth=80

It just looks so *off* to me otherwise.

3 years agofilepath.Join is so much work to type
Martin Tournoij [Fri, 14 Oct 2022 22:18:41 +0000 (00:18 +0200)]
filepath.Join is so much work to type

3 years agoisKqueue() would include illumos, run chmod tests on Windows
Martin Tournoij [Fri, 14 Oct 2022 21:10:32 +0000 (23:10 +0200)]
isKqueue() would include illumos, run chmod tests on Windows

Instead of a new TestWindowsNoAttributeChanges, just run the regular
chmod tests with a windows: block.

3 years agoUpdate changelog, add funding
Martin Tournoij [Fri, 14 Oct 2022 16:59:50 +0000 (18:59 +0200)]
Update changelog, add funding

3 years agowindows: ignore file attributes (#520)
Martin Tournoij [Fri, 14 Oct 2022 16:34:54 +0000 (18:34 +0200)]
windows: ignore file attributes (#520)

On Windows a FILE_ACTION_MODIFIED event (i.e. a Write event) is
triggered on file attribute changes, rather than some dedicates
ATTRIBUTE_CHANGED event. Looking at the docs, I don't really see a way
to distinguish between "real" write events and attribute changes. This is
very odd, but seems to be how the ReadDirectoryChangesW() API works.

The only way I can see to distinguish between the two events is to set
up two filters: one with a FILE_NOTIFY_CHANGE_ATTRIBUTES, and one
without. But that seems overly complex, and no one asked to get Chmod
events for Windows; it's not really all that interesting on Windows
anyway.

The problem is that some software (anti-virus, backup software, etc.)
can issue lots of attribute changes, causing a lot of "fake" Write
events.

So remove the FILE_NOTIFY_CHANGE_ATTRIBUTES and sysFSATTRIB flags.

This was adapted from the tilt-dev/fsnotify fork:
https://github.com/tilt-dev/fsnotify/pull/8

Fixes #487

3 years agoinotify: remove watch when renaming a watched path (#518)
Martin Tournoij [Fri, 14 Oct 2022 16:34:36 +0000 (18:34 +0200)]
inotify: remove watch when renaming a watched path (#518)

Renaming a watched path is problematic; on inotify we just get a
IN_MOVED_SELF event with the old filename and that's it; no more events
for you! So if you do:

watch one
mv    one two
cat   asd >two

You still continue to get events for the file "one", even though it's
now named "two" (the file descriptor doesn't care about the rename).
There is no way we can know the new event as far as I can tell,
inotifywait(1) also behaves like this. So instead of continuing in a
semi-broken state just remove the watcher, like we do for deletes.

On kqueue and FEN the situation is similar, and we actually already
removed watchers on renames.

On Windows this all works nicely; the watch is preserved and the
filename is updated. I decided to keep this as-is for now, even though
it's inconsistent. We actually fixed the Windows behaviour for the 1.6.0
release in #370 , so people do seem to care about it and use it, and
experience with the symlink change in 1.5.0 shows it's better to keep
inconsistent behaviour rather than change it. Fixing this up is
something for a v2.

Fixes #172
Fixes #503

3 years agoAdd failing test for #277 (#522)
Martin Tournoij [Fri, 14 Oct 2022 16:26:55 +0000 (18:26 +0200)]
Add failing test for #277 (#522)

3 years agoSkip TestWatchStress in the CI
Martin Tournoij [Thu, 13 Oct 2022 18:27:27 +0000 (20:27 +0200)]
Skip TestWatchStress in the CI

It fails too often, and it's not *that* important of a test.

3 years agoReturn ErrClosed on Add() when the watcher is closed (#516)
Martin Tournoij [Thu, 13 Oct 2022 18:17:08 +0000 (20:17 +0200)]
Return ErrClosed on Add() when the watcher is closed (#516)

Also consistently return nil for Remove() and WatchList(). For Remove()
we could return an error, but I don't really see the point: if the
watcher is closed it's basically a no-op so returning nil is fine.

For WatchList() I'd like to return an error, but there is no error
return, so explicitly returning nil is the best we can do.

Fixes #511

3 years agoDon't use unkeyed structs in fen watcher
Martin Tournoij [Thu, 13 Oct 2022 06:08:46 +0000 (08:08 +0200)]
Don't use unkeyed structs in fen watcher

Might as well change sendEvent() to just accept the name and Op.

3 years agoDon't run illumos as root in the CI (#517)
Martin Tournoij [Thu, 13 Oct 2022 04:56:56 +0000 (06:56 +0200)]
Don't run illumos as root in the CI (#517)

Fixes #515

3 years agoUpdate changelog, README
Martin Tournoij [Thu, 13 Oct 2022 01:27:31 +0000 (03:27 +0200)]
Update changelog, README

3 years agoAdd FEN backend to support illumos and Solaris (#371)
Nahum Shalman [Thu, 13 Oct 2022 01:25:12 +0000 (21:25 -0400)]
Add FEN backend to support illumos and Solaris (#371)

Add support for illumos and Solaris.

Co-authored-by: Martin Tournoij <martin@arp242.net>
3 years agoUpdate ChangeLog
Martin Tournoij [Thu, 13 Oct 2022 01:18:11 +0000 (03:18 +0200)]
Update ChangeLog

3 years agoTweak the docs a bit
Martin Tournoij [Thu, 13 Oct 2022 01:10:35 +0000 (03:10 +0200)]
Tweak the docs a bit

3 years agoAdd test for re-adding a renamed file (#508)
Martin Tournoij [Thu, 15 Sep 2022 16:42:57 +0000 (18:42 +0200)]
Add test for re-adding a renamed file (#508)

Ref: #503

3 years agoUpdate x/sys
Martin Tournoij [Thu, 8 Sep 2022 16:47:14 +0000 (18:47 +0200)]
Update x/sys

3 years agoUpdate x/sys
Martin Tournoij [Fri, 26 Aug 2022 00:18:56 +0000 (02:18 +0200)]
Update x/sys

3 years agoAdd test to see what happens if you watch a symlink (#498)
Martin Tournoij [Wed, 10 Aug 2022 17:33:57 +0000 (19:33 +0200)]
Add test to see what happens if you watch a symlink (#498)

3 years agoClarify doc comment on WatchList() (#499)
Martin Tournoij [Wed, 10 Aug 2022 17:03:32 +0000 (19:03 +0200)]
Clarify doc comment on WatchList() (#499)

3 years agoAdd note about README version
Martin Tournoij [Wed, 10 Aug 2022 15:11:24 +0000 (17:11 +0200)]
Add note about README version

Fixes #497

3 years agoUpdate documentation and examples (#496)
Martin Tournoij [Wed, 10 Aug 2022 15:10:01 +0000 (17:10 +0200)]
Update documentation and examples (#496)

Update some documentation and add various examples. I added the examples
as subcommands of ./cmd/fsnotify, so they're directly runnable, and it's
at least guaranteed they compile.

This adds some simpler test cases so it's easier to verify it actually
works as documented on all platforms, and it adds internal.Debug() for
all platforms, which is useful in dev.

Fixes 49
Fixes 74
Fixes 94
Fixes 122
Fixes 238
Fixes 372
Fixes 401

3 years agoMove some inotify-tests to run on all backends; test that state is cleaned up after...
Martin Tournoij [Mon, 8 Aug 2022 13:56:08 +0000 (15:56 +0200)]
Move some inotify-tests to run on all backends; test that state is cleaned up after Remove (#494)

This moves a number of tests from backend_inotify_test.go to
fsnotify_test.go, as they're not really inotify-specific. In particular,
it moves the "stress test", which creates a bunch of events/files. It
also expands this test to (potentially) create many more than just 1,000
files, depending on how many the system will allow.

Unfortunately these tests seem pretty flaky on kqueue platforms, where
they're allowed to fail for now (failing the test won't fail the test
run). This seems to expose some existing limits/problems that need to
fixed in a future PR.

Also test that the internal state is cleaned up with TestRemoveState().
The Windows backend doesn't have a test for it (or rather, it doesn't
run) as it *doesn't* clean the state properly, but I found it too
confusing to fix ðŸ¤· Need to spend some time on that in the future.

Reorder/rename some GitHub Actions test runs to show nicer in the UI.

Fixes #42
Fixes #268

3 years agoMove some files around
Martin Tournoij [Sat, 6 Aug 2022 18:24:59 +0000 (20:24 +0200)]
Move some files around

This will be useful once we start splitting out "systems" and "backends"
a bit more, e.g. when polling support is added.

3 years agoPort minor test changes from fen-v2 branch; make LICENSE text not ugly
Martin Tournoij [Sat, 6 Aug 2022 18:03:28 +0000 (20:03 +0200)]
Port minor test changes from fen-v2 branch; make LICENSE text not ugly

Also remove the copyright headers on the files; the license text
doesn't require them, forbids removing them, and they add nothing IMHO,
and it's a few more lines to scroll past every time you open a file.

3 years agowindows: simplify a bit (#493)
Martin Tournoij [Sat, 6 Aug 2022 17:56:40 +0000 (19:56 +0200)]
windows: simplify a bit (#493)

The sys* constants were used in the old golang.org/x/exp/winfsnotify
package on which this is based, but haven't been in use for a long time.

Remove the "one shot", "dir only", and "ignore" options as they were
never set. Also don't send "sysFSQOVERFLOW" as an event if the buffer is
too long; we already send an error and it's just a weird magical number
you get.

This could be simplified greatly more by removing the whole
sysFS… â†’ windows.FILE_NOTIFY… â†’ sysFS… conversions, but that's for
another day.

3 years agoall: add Watcher.{sendEvent,sendError} (#492)
Martin Tournoij [Sat, 6 Aug 2022 17:10:41 +0000 (19:10 +0200)]
all: add Watcher.{sendEvent,sendError} (#492)

Add a helper function instead of doing a select on w.done and w.Errors
or w.Events every time.

Also move most functions to methods on the Watcher.

3 years agokqueue: don't set up watchers on unreadable files (#479)
Martin Tournoij [Sat, 6 Aug 2022 16:40:15 +0000 (18:40 +0200)]
kqueue: don't set up watchers on unreadable files (#479)

Watching an entire directory would fail if it contains a file we can't
read due to permission errors. That's obviously not ideal, so just skip
the file.

Also add a test for trying to watch a directory where the permissions
are denied, and test the error is what's expected; this prevents
regressions like in #393.

Fixes #439

3 years agoUpdate changelog
Martin Tournoij [Sat, 6 Aug 2022 16:29:03 +0000 (18:29 +0200)]
Update changelog

3 years agokqueue: remove timeout from unix.Kevent() (#480)
Martin Tournoij [Sat, 6 Aug 2022 16:17:18 +0000 (18:17 +0200)]
kqueue: remove timeout from unix.Kevent() (#480)

The timeout for unix.Kevent() is causing issues; every 100ms it will do a new
unix.Kevent() syscall, which isn't too efficient: even if you have just one
change an hour, you will still keep calling kevent() ten times per second,
resulting in a needlessly high CPU usage.

Without a timeout, kevent() will block indefinitely until there are some events,
which is much more efficient. We can't just remove the timout however, since we
can't interrupt the kevent() call on FreeBSD and NetBSD, and it will hang
forever. This issue is described in more detail here:
https://github.com/fsnotify/fsnotify/pull/262#issuecomment-1201655890

To solve this we register a new kevent() with the file descriptor set to the
closepipe; when this pipe is closed an event is sent and kevent() will stop
blocking.

This is a rebased version of #124.

Fixes #89
Fixes #237
Fixes #333
Supersedes and closes #124
Supersedes and closes #262
Supersedes and closes #334

Co-authored-by: Felix Lange <fjl@twurst.com>
3 years agowindows: test symlinks (#491)
Martin Tournoij [Fri, 5 Aug 2022 21:45:16 +0000 (23:45 +0200)]
windows: test symlinks (#491)

Symlinks are supported since Windows Vista; there's no reason to not
test it.

3 years agowindows: run TestWatchRename/rename_overwriting_existing_file (#490)
Martin Tournoij [Fri, 5 Aug 2022 19:52:45 +0000 (21:52 +0200)]
windows: run TestWatchRename/rename_overwriting_existing_file (#490)

This Skip() was added in 2013 (c825c6d) with "Windows cannot overwrite
file on rename", but it seems to work fine. os.Rename() is also
documented as "If newpath already exists and is not a directory, Rename
replaces it", so if it *doesn't* work it's a bug in os.Rename(). Maybe
os.Rename() behaved different nine years ago.

3 years agoUse "os.Rename()" in tests instead of "mv"
Martin Tournoij [Fri, 5 Aug 2022 15:03:18 +0000 (17:03 +0200)]
Use "os.Rename()" in tests instead of "mv"

This was added in 00f2dfd (2011): "BSD Test: Need to use outside program
to trigger some events". All tests pass fine with just os.Rename(), so
just use that.

Might add another real-world test in the future for common tools like
mv, cp, vi, etc. because they don't always behave the same as library
calls, but calling mv in *only* the mv() helper is rather odd.

This also renames a few things in the kqueue helper for clarity, and
uses map[string]struct{} if we just use them for lookups to an
allocation (I originally tried to fix #488, but gave up on that for now,
hence the kqueue changes).

3 years agocmd/fsnotify: fix time.Format() string
Martin Tournoij [Thu, 4 Aug 2022 21:23:11 +0000 (23:23 +0200)]
cmd/fsnotify: fix time.Format() string

3 years agowindows: replace syscall with golang.org/x/sys/windows
Martin Tournoij [Thu, 4 Aug 2022 00:38:52 +0000 (02:38 +0200)]
windows: replace syscall with golang.org/x/sys/windows

Specifically to address #364; syscall.GetQueuedCompletionStatus()
contains a bug that can't be fixed in a backwards-compatible way, so
they didn't. Once you use sys/windows for one thing, you gotta use it
for everything.

I ran the tests with -count=100 locally, and it all seems fine.

Fixes #364

3 years agowindows: replace "e" with "err" for error variables
Martin Tournoij [Thu, 4 Aug 2022 00:36:16 +0000 (02:36 +0200)]
windows: replace "e" with "err" for error variables

3 years agowindows: increase buffer size from 4K to 64K (#485)
Martin Tournoij [Thu, 4 Aug 2022 01:25:38 +0000 (03:25 +0200)]
windows: increase buffer size from 4K to 64K (#485)

People are running in to trouble because the 4K buffer can overflow;
see: #72.

This is not a "real" fix, but I think a 64K buffer is acceptable even on
memory-limited machines; no one is running the Windows on an Arduino,
and even with something like 128M of memory, the extra 124K is basically
negligible. There is also no real performance difference between
allocating a large array vs. a small array: they're both comparable.

It should probably be enough for most applications. Need to look in the
future to either dynamically grow the size, or allow setting it similar
to what tilt does as mentioned in #72.

3 years agoci: update to use Go 1.19, kick off fewer builds, update x/sys (#484)
Martin Tournoij [Wed, 3 Aug 2022 13:36:21 +0000 (15:36 +0200)]
ci: update to use Go 1.19, kick off fewer builds, update x/sys (#484)

We need to update x/sys to support GOARCH=loong64, which is new in Go
1.19.

Also kick off fewer builds; they regularly get hung in the queue as we
have quite a few test jobs now.

3 years agoRemove CLA section in contributing
Martin Tournoij [Wed, 3 Aug 2022 12:50:33 +0000 (14:50 +0200)]
Remove CLA section in contributing

Not really needed anymore as far as I can tell:
https://github.com/fsnotify/fsnotify/issues/193#issuecomment-1203909130

3 years agoNeed Linux 5.9 for a useful fanotify we can use
Martin Tournoij [Wed, 3 Aug 2022 12:13:14 +0000 (14:13 +0200)]
Need Linux 5.9 for a useful fanotify we can use

3 years agoUpdate CONTRIBUTING.md
Martin Tournoij [Tue, 2 Aug 2022 08:09:56 +0000 (10:09 +0200)]
Update CONTRIBUTING.md

3 years agoRemove PULL_REQUEST_TEMPLATE
Martin Tournoij [Tue, 2 Aug 2022 07:11:07 +0000 (09:11 +0200)]
Remove PULL_REQUEST_TEMPLATE

It's annoying as I need to keep deleting the template, and it doesn't provide that much value IMO.

3 years agoRun tests in illumos (#481)
Martin Tournoij [Mon, 1 Aug 2022 22:58:19 +0000 (00:58 +0200)]
Run tests in illumos (#481)

Doesn't really test anything yet, but should do soon.

3 years agoUpdate ChangeLog
Martin Tournoij [Sun, 31 Jul 2022 20:44:24 +0000 (22:44 +0200)]
Update ChangeLog

3 years ago[bugfix] close handle when remWatch open in getIno (#288)
jie [Sun, 31 Jul 2022 20:42:51 +0000 (04:42 +0800)]
[bugfix] close handle when remWatch open in getIno (#288)

* [bugfix] close handle when remWatch open in getIno

* Use new tests

Note: the tests pass even without the syscall.CloseHandle()

Co-authored-by: jie <jie@tt.com>
Co-authored-by: Martin Tournoij <martin@arp242.net>
3 years agowindows: update watch paths when renaming directories with sub-watches (#370)
Arne Groskurth [Sun, 31 Jul 2022 19:16:05 +0000 (21:16 +0200)]
windows: update watch paths when renaming directories with sub-watches (#370)

Fixes #259
Fixes #243

Co-authored-by: Martin Tournoij <martin@arp242.net>
3 years agoRewrite tests (#478)
Martin Tournoij [Sun, 31 Jul 2022 18:03:10 +0000 (20:03 +0200)]
Rewrite tests (#478)

Rewrite tests

This rewrites quite a lot of tests to be much more easily readable.

While working on #472 I wanted to check "how do renames behave now?",
and found this quite hard as most test cases were >90% "plumbing", and
seeing "what file operations does this do?" and "what events do we get?"
was not very easy.

So refactor the lot, based on some work I did in #472:

- Add a bunch of "shell-like" helper functions so you're not forever
  typing error checks, filepath.Join(), and eventSeparator(). Just
  touch(t, tmp, "file") will create a file in tmp.

- Add eventCollector type which will collect all events in in a slice,
  replacing the previous "counter". This also ensures that the Watcher
  is closed within a second (this removes a lot of duplicate code).

  This is also much more precise than merely counting events; before
  random events could get emitted but if you weren't counting those then
  you'd never know.

  Downside is that some tests are a bit more flaky now as some
  behaviours are not always consistent in various edge cases; these are
  pre-existing bugs.

- Add Events (plural) type (only for tests), and helper function to
  create this from a string like:

      REMOVE  /link
      CREATE  /link
      WRITE   /link

  Which makes seeing which events are received, diffing them, etc. much
  easier.

- Add Parallel() for most tests; reduces runtime on my system from ~12
  seconds to ~6 seconds.

All in all it reduces the integrations_test.go from 1279 lines to 405
lines and it's quite easy to see which events are expected for which
operations, which should make things a lot easier going forward.

3 years agoAdd {Event,Op}.Has() (#477)
Martin Tournoij [Sat, 30 Jul 2022 15:28:03 +0000 (17:28 +0200)]
Add {Event,Op}.Has() (#477)

Using:

if event.Op&fsnotify.Create == fsnotify.Create {
}

is overly verbose and awkward; a Has() method makes this much nicer:

if event.Has(fsnotify.Create) {
}

PR #76 was previously rejected; I think that HasCreate() etc. is
overkill, but a Has() method makes the library quite a bit more
convenient IMO.

I added it to both the Event and Op to avoid confusion; with this we can
change the Op field to be private in a future v2.

Fixes #107

3 years agoDocument that /proc and /sys won't work
Martin Tournoij [Sat, 30 Jul 2022 12:31:21 +0000 (14:31 +0200)]
Document that /proc and /sys won't work

Fixes #78

3 years agoClarify FAQ on goroutines
Martin Tournoij [Sat, 30 Jul 2022 11:15:01 +0000 (13:15 +0200)]
Clarify FAQ on goroutines

Fixes #71

3 years agomacos: retry if open() returns EINTR (#475)
Martin Tournoij [Sat, 30 Jul 2022 10:54:27 +0000 (12:54 +0200)]
macos: retry if open() returns EINTR (#475)

Retry the unix.Open() if the error returned is EINTR; looking around the
web it seems many systems handle it like this. This is also what
os.Open() does:
https://github.com/golang/go/commit/50d0ee0c98ea21f818d2daa9bc21ef51861a2ef9

Fixes #354

3 years agoFix lint (#476)
Martin Tournoij [Sat, 30 Jul 2022 10:53:47 +0000 (12:53 +0200)]
Fix lint (#476)

Didn't run for some reasons; something with the actions cache:

/usr/bin/tar --use-compress-program zstd -d -xf /home/runner/work/_temp/b9a1abd7-273e-449f-b255-5e8e947b3f97/cache.tzst -P -C /home/runner/work/fsnotify/fsnotify
Error: /usr/bin/tar: ../../../go/pkg/mod/golang.org/x/sys@v0.0.0-20220412211240-33da011f77ad/windows/race0.go: Cannot open: File exists
Error: /usr/bin/tar: ../../../go/pkg/mod/golang.org/x/sys@v0.0.0-20220412211240-33da011f77ad/windows/types_windows.go: Cannot open: File exists

Replace it with staticcheck; which is nicer anyway.

3 years agodebian 6 test: deal with multiple packages (#474)
Martin Tournoij [Fri, 29 Jul 2022 23:33:19 +0000 (01:33 +0200)]
debian 6 test: deal with multiple packages (#474)

* debian 6 test: deal with multiple packages

The ./cmd/fsnotify doesn't have any tests, but sooner or later we'll
have subpackages so deal with it properly now.

* Cache vagrant boxes

3 years agoRemove AUTHORS file
Martin Tournoij [Fri, 29 Jul 2022 21:00:44 +0000 (23:00 +0200)]
Remove AUTHORS file

This goes back to when this library was part of golang.org/x/...; the "Google lawyers" said that an AUTHORS file was needed, but seems they changed their minds and it got removed from Go last week and decided that the git log is enough:
https://github.com/golang/go/commit/774fa58d1d3c9926709e108afdf83af7010bde72

I don't think there's much value keeping it here, and it's one more thing that can be forgotten (it's already out of date). The authorship is preserved in the git log, and if that's good enough for Google's lawyers then it's good enough for me.

3 years agoUpdate README: split out FAQ to "Platform-specific notes"
Martin Tournoij [Fri, 29 Jul 2022 20:15:40 +0000 (22:15 +0200)]
Update README: split out FAQ to "Platform-specific notes"

It's a bit clearer to document it like this.

Also some other minor changes to the README.

Fixes #457

3 years agoinotify: don't ignore events for files that don't exist (#470)
Martin Tournoij [Fri, 29 Jul 2022 18:31:09 +0000 (20:31 +0200)]
inotify: don't ignore events for files that don't exist (#470)

This is quite an odd check, leading to an inconsistent event stream,
which also doesn't match what other platforms do. If you do a "CREATE +
MODIFY + REMOVE" in quick succession then you probably *want* all three
events. If you don't want to operate on non-existing files, then you can
check this in your application code.

You need to do that already, since this check is far from reliable. In
the time between this check and the application code doing something
with an event the file may have been deleted already.

I looked a bit at the history of this, and looks like it was added in
2013 with cc2c34e; issue 36 refers to this issue on the old repo, which
mentions it fixes a memory leak: https://github.com/howeyc/fsnotify/issues/36

I can't reproduce that at all; using the CLI from #463 modified to print
the memory and running:

for i in $(seq 0 10000); { touch $i; rm $i }

Memory stays at about 100/110K in both the current main branch and this.

So I think it should be safe to remove.

3 years agoTweak comment regarding relative paths (#466)
Martin Tournoij [Fri, 29 Jul 2022 18:29:25 +0000 (20:29 +0200)]
Tweak comment regarding relative paths (#466)

Clarify what "relative" means here.

Fixes #362

3 years agoAdd cmd/fsnotify (#463)
Martin Tournoij [Fri, 29 Jul 2022 18:28:45 +0000 (20:28 +0200)]
Add cmd/fsnotify (#463)

This adds a little example that can be run:

% go run ./cmd/fsnotify **/*
12:76:11.4710 watching; press ^C to exit
12:76:13.6549   1 REMOVE                "cmd/fsnotify/4913"
12:76:13.6550   2 RENAME                "cmd/fsnotify/main.go"
12:76:13.6550   3 RENAME                "cmd/fsnotify/main.go"
12:76:13.6612   4 CREATE                "cmd/fsnotify/main.go"
12:76:13.6612   5 WRITE      (modified) "cmd/fsnotify/main.go"
12:76:13.6614   6 WRITE      (modified) "cmd/fsnotify/main.go"
12:76:13.6648   7 CHMOD                 "cmd/fsnotify/main.go"
12:76:15.2919   8 CREATE                "cmd/fsnotify/4913"
12:76:15.2919   9 REMOVE                "cmd/fsnotify/4913"
12:76:15.2922  10 CHMOD                 "cmd/fsnotify/main.go"
12:76:15.2923  11 REMOVE                "cmd/fsnotify/main.go"
12:76:15.2925  12 RENAME                "cmd/fsnotify/main.go"
12:76:15.2986  13 CREATE                "cmd/fsnotify/main.go"
12:76:15.2986  14 WRITE      (modified) "cmd/fsnotify/main.go"
12:76:15.2988  15 WRITE      (modified) "cmd/fsnotify/main.go"
12:76:15.3024  16 CHMOD                 "cmd/fsnotify/main.go"
^C

It's mostly the same as the example in the README, except that it takes
paths from the commandline and aligns things a bit nicer.

This is useful for documentation and experimentation. For example, while
reviewing some PRs I found it useful to quickly get an overview of "what
does fsnotify do now, and what does it do with this patch?"

3 years agokqueue: better error if watching a file fails (#471)
Martin Tournoij [Fri, 29 Jul 2022 18:28:22 +0000 (20:28 +0200)]
kqueue: better error if watching a file fails (#471)

kqueue requires opening a file descriptor for every file; so we read the
directory and do that. If this failed the error was quite unclear; for
example:

"/tmp/fsnotify_permission_denied/": permission denied

This puts the filepath in there:

"/tmp/fsnotify_permission_denied/": "/tmp/fsnotify_permission_denied/test2.log": permission denied

Fixes #270