fsnotify update

376 views
Skip to first unread message

Nathan Youngman

unread,
Jun 13, 2014, 2:17:05 AM6/13/14
to golan...@googlegroups.com
Hi,

For tonights work, I began revising the API for fsnotify based on the draft at http://goo.gl/MrYxyA

I still need to convert the Errors channel to an Err() method and there is a tonne of internal cleanup left, but I will upload a CL to go.exp once the following are reviewed & submitted:

https://codereview.appspot.com/103300045/ (awaiting new test from tilaks)

If you would like a sneak peek, the code is here (v0.11.0):

And I'm using it from Looper's watch.go:

Nathan.

Nathan Youngman

unread,
Jun 20, 2014, 12:46:08 AM6/20/14
to golan...@googlegroups.com

Hi,

Would anyone be willing to review/submit this CL? https://codereview.appspot.com/100860043/

I made the changes back in January but only submitted the CL three weeks ago (and I think I did something wrong when mailing it out, so nobody saw it).

It removes the current implementation of WatchFlags. Hopefully we'll come up with something better that takes advantage of the OS to filter out events in the kernel, depending if we can reconcile the differences on Windows. Otherwise I can provide a much simpler utility function with the same functionality. 

~

I've been working on the next CL which focuses on the API. A nice side effect is that we should be able to unit test the library more easily because Event is a simple two-field struct.

While I'm already using this API, I hope to wrap up some loose ends this weekend before delving into the internals of each individual adapter. (the real work)

Thanks,
Nathan.

Nathan Youngman

unread,
Jun 22, 2014, 2:34:32 AM6/22/14
to golan...@googlegroups.com, Chris Howey, Russ Cox

Thanks to mikio and rsc for reviewing/submitting my last CL to go.exp/fsnotify.

My next CL brings the API in line with what has been discussed thus far in the doc: http://goo.gl/MrYxyA

Exceptions:
* Add doesn't have an argument to filter Ops, mainly due to differences on Windows (and I have an alternative to propose that may work)
* I haven't figured out the changes necessary to use an Err() method instead of an Errors channel, particularly for any non-fatal errors. Very little exists for testing the error cases right now. :-(

The CL also includes some internal cleanups, mostly renaming things. I'd like to better understand how each adapter works before doing any really big changes.


I fully expect the API to change again. This is closer to what we had in mind, and was an opportunity to clean up the code a little more. Now that Event is a simple two field struct, we may be able unit test some pieces that would've been challenging before.

Earlier today I was able to pair program with a friend who knows Linux and gdb much better than me. We reproduced a bug in inotify that "conveniently" disappears when adding a breakpoint. No solution yet. I have yet to consolidate all the fsnotify bugs in the Go issue tracker. I think that will be my next task while iterating on any comments to this CL.

Nathan.

P.S. tilaks also added a regression test to this CL: https://codereview.appspot.com/103300045/

Reply all
Reply to author
Forward
0 new messages