rjeczalik / notify

File system event notification library on steroids.
MIT License
903 stars 128 forks source link

epoll_wait syscall doesn't exist on arm64. #116

Closed harshavardhana closed 7 years ago

harshavardhana commented 7 years ago

Implement it with by calling epoll_pwait(). According to man epoll_pwait, calling epoll_pwait with sigmask of NULL is identical to epoll_wait.

Bring the fixes from golang.org/x/sys/unix to support these transparently on arm64.

This is needed to fix https://github.com/minio/mc/issues/2047

rjeczalik commented 7 years ago

/cc @ppknap

rjeczalik commented 7 years ago

@harshavardhana You'd also need to add missing import to event_kqueue.go:

# github.com/rjeczalik/notify
./event_kqueue.go:29: undefined: unix in unix.NOTE_DELETE

Goimports seems to not like build tags.

Thanks for the PR! 😄

rjeczalik commented 7 years ago

One more:

# github.com/rjeczalik/notify
./watcher_kqueue.go:53: undefined: unix in unix.Write
./watcher_kqueue.go:59: undefined: unix in unix.Close
./watcher_kqueue.go:64: undefined: unix in unix.Open
./watcher_kqueue.go:78: undefined: unix in unix.Close

You can test build with:

$ go build -tag kqueue ./...
$ go test -run none -tag kqueue ./...
ghost commented 7 years ago

With 465c9e8 fen does not work anymore. Tests hang:

$ go test -v ./...
=== RUN   TestEventString
--- PASS: TestEventString (0.00s)
=== RUN   TestNotifyExample
rjeczalik commented 7 years ago

Regression in x/sys? Do you have a chance to debug on which syscall does it hang? Is it epoll_pwait?

pblaszczyk notifications@github.com schrieb am Di. 28. Feb. 2017 um 17:09:

With 465c9e8 https://github.com/rjeczalik/notify/commit/465c9e8a9ced2f99ac0fa92b0b9d103e9d0f46ba fen does not work anymore. Tests hang:

$ go test -v ./... === RUN TestEventString --- PASS: TestEventString (0.00s) === RUN TestNotifyExample

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rjeczalik/notify/pull/116#issuecomment-283083267, or mute the thread https://github.com/notifications/unsubscribe-auth/ABG7ITnpDNCHhWTUIPsfaOIHXptF6IZTks5rhEahgaJpZM4MODN- .

harshavardhana commented 7 years ago

With 465c9e8 fen does not work anymore. Tests hang:

$ go test -v ./... === RUN TestEventString --- PASS: TestEventString (0.00s) === RUN TestNotifyExample

What is the operating system you are using? @pblaszczyk

ghost commented 7 years ago

@harshavardhana I'm using OmniOS.

harshavardhana commented 7 years ago

@harshavardhana I'm using OmniOS.

@pblaszczyk can you try the latest push?

ghost commented 7 years ago

Yes, reverting all changes to fen makes it work.

harshavardhana commented 7 years ago

Yes, reverting all changes to fen makes it work.

yeah that's what i expected sys/unix perhaps has bugs for Solaris.

harshavardhana commented 7 years ago

S3 is down guys :-) so Travis not conclusive.

harshavardhana commented 7 years ago

Anything else pending on this @rjeczalik @pblaszczyk @ppknap ?

rjeczalik commented 7 years ago

@harshavardhana Nope, merged. Thanks for the PR!

ppknap commented 7 years ago

Thanks @harshavardhana :100: