Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os/fsnotify: add new package #4068

Closed
rsc opened this issue Sep 12, 2012 · 33 comments
Closed

os/fsnotify: add new package #4068

rsc opened this issue Sep 12, 2012 · 33 comments

Comments

@rsc
Copy link
Contributor

rsc commented Sep 12, 2012

Issue for creating standard fsnotify package (current exp/inotify)
@gopherbot
Copy link
Contributor

Comment 1 by howeyc:

I created a library that does this and tried to be as consistent between operating
systems as I could. It works on OSX/BSD/Linux/Windows.
I could attempt to merge it in, or someone more familiar with your contributing process
could do so with my blessing.
Take a look:
https://github.com/howeyc/fsnotify
Generated GoDoc:
http://go.pkgdoc.org/github.com/howeyc/fsnotify
I'll follow this issue to get your feedback.

@alberts
Copy link
Contributor

alberts commented Sep 13, 2012

Comment 2:

exp/inotify has some open issues: issue #2483, issue #3132

@gopherbot
Copy link
Contributor

Comment 3 by uriel@berlinblue.org:

Quite a few people are using howeyc's fsnotify package quite successfully, the API might
need a bit of tuning, but seems to be relatively close to exp/inotify.
Any advice on what would be the best approach to merge fsnotify into mainline?

@gopherbot
Copy link
Contributor

Comment 4 by jnwhiteh:

There are still data races in fsnotify, in particular between addWatch and the
readEvents goroutine simultaneously accessing the map.
This same race has been fixed in exp/inotify, but I don't see the corresponding fix in
fsnotify.

@rsc
Copy link
Contributor Author

rsc commented Oct 6, 2012

Comment 5:

Labels changed: added go1.1.

@gopherbot
Copy link
Contributor

Comment 6 by cpisto:

While I like the idea of fsnotify, it seems to me a more idiomatic go approach would be
to design a file system event interface (FileWatcher?) around the existing inotify &
winfsnotify exp packages (in addition to some future kevent based package for the bsds).
fsnotify as it stands now hides much of the detail provided by the underlying interfaces
and definitely has some oddities in its current implementation. For instance it seems to
register to receive all event types it supports with the underlying OS backend
regardless of the event mask specified by the user.

@gopherbot
Copy link
Contributor

Comment 7 by peter.waller:

In addition to the above comment, fsnotify does not allow detecting when a file is
modified due to a rename. This means you can't see configuration files modified by many
types of text editor which use the atomicity of a rename to wholly update the file on
save.
This seems like one of the primary use cases of fsnotify for which it unfortunately
doesn't currently work.
howeyc/fsnotify#15

@gopherbot
Copy link
Contributor

Comment 8 by howeyc:

@ Comment 6
What you have to remember is that go hides some of the underlying interfaces for
consistency as well. The net package for example, you don't write apps that do epoll on
linux, kevent/kqueue on OSX/BSD and winsock on Windows, you write against net. I concede
my interface may not be optimal, I just went with whatever exp/inotify was doing, but
providing every available os-specific mask for each OS does not exactly encourage
portability.
The goal I had in mind is that you could write against fsnotify on linux, copy your
source tree to windows, build and not have to change anything. In order to do that I did
have to impose some limitations to attempt consistency across operating systems.

@gopherbot
Copy link
Contributor

Comment 9 by cpisto:

Fully understood, go hides things when it makes sense. In certain cases, it still offers
access to underlying OS specific features in the interests of being a useful system
programming language (look at os.FileInfo.Sys() for example). Filesystem event
notification looks to present enough cross platform differences to warrant some
exploration in that direction.
Don't get me wrong, fsnotify is great stuff, I just think inclusion of it or something
like it could use a little back and forth :)
At the minimum, refactoring it to only register with the underlying APIs for the user
requested event mask instead of all supported events, and cleaning up the various data
races that have been fixed or identified in the exp inotify/winfsnotify packages seems
prudent.

@gopherbot
Copy link
Contributor

Comment 10 by howeyc:

I agree, the API probably does need some work to provide something similar to
os.FileInfo.Sys() so that access is there if you really want it.

@gopherbot
Copy link
Contributor

Comment 11 by aaron.l.france:

Is this something that really needs to be in the standard library? It doesn't really
seem to gel with the rest of the standard library packages, to me.
I'm not trying to poo-poo it or anything, it seems like it would be quite useful.
However, in the interests of fighting complexity it would seem like a bit of a sore
thumb in the standard library.
As a sort of side-point to this, why isn't there a "contrib" set of packages? It seems
like this would be perfect for a quasi-sanctioned contrib package.
Just my $0.02. I know that one of the biggest draws for me to Go is it's incredibly
consistent standard library, having yet more packages worries me about increasing
entropy.
Regards,
Aaron

@minux
Copy link
Member

minux commented Dec 16, 2012

Comment 12:

i don't understand why adding this package to the standard library adds that much
complexity?
if i read correctly, you mean the smaller the standard library the better?
or the proposed new package isn't consistent with the other standard packages?

@gopherbot
Copy link
Contributor

Comment 13 by aaron.l.france:

My point was that it's prudent to be extra sure this is required when adding it. Once
added there's little chance of removing it.

@nathany
Copy link
Contributor

nathany commented Jan 16, 2013

Comment 14:

What I like about including fsevents, etc. in the stdlib is that it opens up
possibilities to use it from within. One example would be to have built in
autotest/guard (Ruby) or SBT (Scala) like functionality built into the go (test) tool.

@rsc
Copy link
Contributor Author

rsc commented Mar 11, 2013

Comment 15:

I regret dropping the ball on this, but we won't have time to get this into Go 1.1.
I replied on howeyc's CL thread. I'd gladly accept a CL putting this in the go.exp
subrepo so that we can get more experience using it.
Re #12, one source of complexity is that it's not uniformly supported on all systems.

Labels changed: removed go1.1.

@rsc
Copy link
Contributor Author

rsc commented Jul 30, 2013

Comment 16:

Labels changed: added go1.3.

@robpike
Copy link
Contributor

robpike commented Aug 20, 2013

Comment 17:

Labels changed: removed go1.3.

@nathany
Copy link
Contributor

nathany commented Sep 24, 2013

Comment 18:

Just an update... there are currently 56 packages the import fsnotify on GitHub and an
additional 7 importing the mirror at go.exp.
We are starting on an FSEvents adapter for OS X which is more efficient at watching
large directory structures than kqueue. FSEvents provides two options that are
frequently requested by fsnotify users:
1. It does recursive watches on a directory structure, which isn't unique, but
apparently it doesn't have a non-recursive option.
2. It can coalesce events within a given latency, such as when an editor saves a file
twice.
The fsnotify API is becoming even more high-level to offer consistent behaviour across
OSes.
We currently have a git branch (pull request) that replaces the previous event mask
filtering (FSN_CREATE, etc.) with a more general purpose event processing pipeline. The
idea being that if an adapter (eg. inotify) doesn't support a given option, we will
perform our own filtering on the incoming events. 
It's certainly going to take some time to for all this to come together and to settle on
a good high-level API.
As far as a low-level APIs, I suspect the ideal situation will be to have packages for
inotify, kqueue, winfsnotify, and fsevents at the "syscall" level that are implemented
in such a way that os.fsnotify can build on top of them. Having spent some time in the
fsnotify code, I suspect that getting to this state will be quite an undertaking!
Feel free to join in on the fun. :-)
P.S. Aaron, is https://github.com/golang the new contrib?

@rsc
Copy link
Contributor Author

rsc commented Nov 27, 2013

Comment 19:

Labels changed: added go1.3maybe.

@rsc
Copy link
Contributor Author

rsc commented Dec 4, 2013

Comment 20:

Labels changed: added release-none, removed go1.3maybe.

@rsc
Copy link
Contributor Author

rsc commented Dec 4, 2013

Comment 21:

Labels changed: added repo-main.

@rsc
Copy link
Contributor Author

rsc commented Dec 4, 2013

Comment 22:

Labels changed: added release-go1.3, removed release-none.

@nathany
Copy link
Contributor

nathany commented Jan 30, 2014

Comment 24:

The API document is at http://goo.gl/MrYxyA.

@bradfitz
Copy link
Contributor

Comment 26:

Labels changed: added release-go1.4, removed release-go1.3.

@nathany
Copy link
Contributor

nathany commented Aug 17, 2014

Comment 27:

Please push this to Go1.5. Thanks.

@nathany
Copy link
Contributor

nathany commented Aug 17, 2014

Comment 28:

FYI, the equivalent issue in the fsnotify issue tracker is: 
go-fsnotify/fsnotify#1

@rsc
Copy link
Contributor Author

rsc commented Sep 16, 2014

Comment 29:

Labels changed: added release-none, removed release-go1.4.

@nathany
Copy link
Contributor

nathany commented Dec 9, 2014

We have the start of FSEvents for OS X, but it isn't yet rolled into fsnotify. Still lots of work to do on the existing code base (error handling, bug fixing), as well as research into the behaviour on each OS.

@rjeczalik
Copy link

Just in case someone lands here from the search there's also another library with API similar to os/signal - https://github.com/rjeczalik/notify.

@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@baptistedonaux
Copy link

Well ! It's a good new because only two libraries work but each has specific running (recursive or not…). Make a « official » lib is good to unify the golang environment. +1

@nathany
Copy link
Contributor

nathany commented Dec 17, 2015

At this point I don't know if it makes sense to have os/fsnotify in the standard library. Maybe an x/fsnotify package to benefit from Gerrit, the cross-platform builders, and the CLA process. That would also make it more official, but without have a compatibility promise.

Whether that's based on the current revision of fsnotify or @rjeczalik's library, I'm not sure. I've been meaning to take a closer look at notify.

@nathany
Copy link
Contributor

nathany commented Oct 2, 2016

At this point, and in light of burnout among the core team, I think this issue can be safely closed. Please correct me if I'm wrong.

  • With internal/ packages, tools like guru, godoc, and even the go command can technically make use of fsnotify or @rjeczalik's notify without needing a public API in standard library or x/.
  • Fsnotify will remain under a BSD license (3-clause or 2-clause), but perhaps there is a caveat with the CLA situation if it is a non-Google project? (the Google CLA is still requested at this time, but not enforced)
  • In time, we will hopefully have a way to ensure that (foundational) libraries like fsnotify and grpc-go run everywhere that Go does. x/build: go builders and trybots for third-party repositories #17312

If anyone reading this has the bandwidth and interest in file system notifications, you are certainly welcome to contribute as much or little as you like over at https://github.com/fsnotify. Thanks.

@ALTree
Copy link
Member

ALTree commented Jul 25, 2017

Closing this, as requested.

@ALTree ALTree closed this as completed Jul 25, 2017
@golang golang locked and limited conversation to collaborators Jul 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

10 participants