code review 5248062: os/kevent: implements a wrapper for the BSD kevent/kque... (issue 5248062)

19 views
Skip to first unread message

how...@gmail.com

unread,
Oct 12, 2011, 10:08:12 AM10/12/11
to da...@cheney.net, golan...@googlegroups.com, re...@codereview.appspotmail.com
Reviewers: dfc,

Message:
Hello da...@cheney.net (cc: golan...@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/


Description:
os/kevent: implements a wrapper for the BSD kevent/kqueue system.

This is meant to be a file watcher package for BSD systems.
See os/inotify for a similar package on Linux systems.

Please review this at http://codereview.appspot.com/5248062/

Affected files:
M src/pkg/Makefile
A src/pkg/os/kevent/Makefile
A src/pkg/os/kevent/kevent_bsd.go
A src/pkg/os/kevent/kevent_bsd_test.go


hect...@gmail.com

unread,
Oct 13, 2011, 8:57:31 AM10/13/11
to how...@gmail.com, da...@cheney.net, golan...@googlegroups.com, re...@codereview.appspotmail.com
I think it will be necessary to define a common interface to the file
watcher systems across the various OSes. I tried to submit a change to
implement file watching for Windows and rsc prompted that we should be
doing this. Can we try to hammer out a proposal for such an interface
on golang-dev? The reference to my changelist is
http://codereview.appspot.com/4188047/.

http://codereview.appspot.com/5248062/

Russ Cox

unread,
Oct 13, 2011, 9:50:53 AM10/13/11
to how...@gmail.com, da...@cheney.net, golan...@googlegroups.com, re...@codereview.appspotmail.com
Thanks for working on this. I agree with Hector that we
need to figure out an API that works for all the systems.
However, trying to design the API without code to look at
has not worked in the past, so let's try something else.
Please change the CL to make this package exp/kevent,
and we'll move os/inotify to exp/inotify, and then we
can have Windows's exp/whatever, and then once all three
are there it will be easier to compare APIs and decide
what the eventual, mostly portable os/fsnotify should be.

Thanks.
Russ

how...@gmail.com

unread,
Oct 13, 2011, 9:37:21 AM10/13/11
to da...@cheney.net, hect...@gmail.com, golan...@googlegroups.com, re...@codereview.appspotmail.com

That sounds like a good idea to me. I was actually thinking about a
unified file monitoring package for all supported OSs, but I don't have
a windows or linux box to test those platforms. It looks like you have a
windows box based on your changelist, and I can get a Linux box up and
running pretty quick if you don't have one, as we'll need to modify its
interface a bit to make a common API.

Are you thinking of starting another thread to talk about the interface?
Here are my thoughts that you can use/disregard if you start one:

Create a FileEvent type similar to FileInfo returned from Stat() that
has common functions between OSs (Path(), IsDelete(), IsWrite(), ...)

As we cannot push an interface over a channel I was thinking we may need
to make a blocking NextFileEvent() (FileEvent, Error) function with
simple code (not valid, off the top my of head):

func Nextfileevent(watch w) ( fileevent, error) {
select {
case err := <-w.Error:
case event := <-w.Event:
}
return event, err
}

The channels would then become private.

I'm also thinking of removing an external facing AddWatch(name, flags)
as the flags are OS dependent, just have AddWatch(name) as an external
function that adds all flags. The user of package can use the interface
functions of FileEvent to disregard the events they don't care about.
Also, removing the flag consts from being public would be something to
discuss.

I'd also like to change the package name from inotify to something more
OS-agnostic, perhaps filemon or filewatch? I'm not attached to any name
in particular, I'm okay with whatever others feel is appropriate.

http://codereview.appspot.com/5248062/

roger peppe

unread,
Oct 13, 2011, 11:15:44 AM10/13/11
to how...@gmail.com, da...@cheney.net, hect...@gmail.com, golan...@googlegroups.com, re...@codereview.appspotmail.com
On 13 October 2011 14:37, <how...@gmail.com> wrote:
> As we cannot push an interface over a channel [...]

er, why not?

how...@gmail.com

unread,
Oct 13, 2011, 11:34:17 AM10/13/11
to da...@cheney.net, hect...@gmail.com, r...@golang.org, rogp...@gmail.com, golan...@googlegroups.com, re...@codereview.appspotmail.com
On 2011/10/13 15:15:50, rog wrote:

> On 13 October 2011 14:37, <mailto:how...@gmail.com> wrote:
> > As we cannot push an interface over a channel [...]

> er, why not?

in my head I was thinking of something like this...
http://groups.google.com/group/golang-nuts/browse_thread/thread/71bba09a30918319/3a15c4e509f3d2ef?#3a15c4e509f3d2ef

http://codereview.appspot.com/5248062/

Reply all
Reply to author
Forward
0 new messages