gin icon indicating copy to clipboard operation
gin copied to clipboard

Add fsnotify support to gin

Open losinggeneration opened this issue 10 years ago • 13 comments

This, at build time, enables fsnotify support for OS's that support it (e.g. Linux.) The this has a slight advantage over the default scanner in that there is almost zero delay between saving and triggering a rebuild.

losinggeneration avatar Mar 13 '14 21:03 losinggeneration

https://github.com/howeyc/fsnotify <- ~~may want to actually use that instead of go.exp~~ (misread that) it also sounds like fsnotify is proposed for Go 1.4 inclusion.

losinggeneration avatar Mar 13 '14 21:03 losinggeneration

http://code.google.com/p/go/source/browse/?repo=exp#hg%2Ffsnotify I didn't really look, but it looks like most OS's are supported. Let me know if anything needs cleaned up or changed before you're ready to pull.

losinggeneration avatar Mar 13 '14 21:03 losinggeneration

This looks pretty good! Using fsnotify I seem to remember that it does not recurse into subdirectories. Is this still the case with this code? If so then this unfortunately does not replicate the filewalker functionality exactly. We could maintain a list of directories from an initial filewalk, and also react to CREATE and DELETE events to add/remove the directories from the list while gin is running.

Again, I don't seem to remember whether or not that is a real concern. I would love to have fsnotify instead of a filewalker since it is way more efficient and nearly instant for recompiles.

Sidenote - It would be cool to block serving in the proxy until the app is done compiling and running again, this way a browser refresh after a save will block until until we are ready to process the request from the new server.

On Thu, Mar 13, 2014 at 2:16 PM, Harley Laue [email protected]:

http://code.google.com/p/go/source/browse/?repo=exp#hg%2Ffsnotify I didn't really look, but it looks like most OS's are supported. Let me know if anything needs cleaned up or changed before you're ready to pull.

Reply to this email directly or view it on GitHubhttps://github.com/codegangsta/gin/pull/15#issuecomment-37587831 .

codegangsta avatar Mar 13 '14 21:03 codegangsta

That's a good point. I have a feeling you're right about it not recursing. However, it shouldn't be too hard to make sure all paths are watched (adding/deleting paths as needed.) I'll probably work on that a little later tonight.

Also, like the idea of the sidenote. I guess I would imagine that would be done via a channel. Write down the channel to tell it to block, and then another write tells it to unblock, but I haven't really given it any more thought than that.

losinggeneration avatar Mar 13 '14 21:03 losinggeneration

Sounds good. Thanks!

On Thu, Mar 13, 2014 at 2:38 PM, Harley Laue [email protected]:

That's a good point. I have a feeling you're right about it not recursing. However, it shouldn't be too hard to make sure all paths are watched (adding/deleting paths as needed.) I'll probably work on that a little later tonight.

Also, like the idea of the sidenote. I guess I would imagine that would be done via a channel. Write down the channel to tell it to block, and then another write tells it to unblock, but I haven't really given it any more thought than that.

Reply to this email directly or view it on GitHubhttps://github.com/codegangsta/gin/pull/15#issuecomment-37590151 .

codegangsta avatar Mar 13 '14 22:03 codegangsta

That was a fair bit of work to ensure rebuilds didn't happen multiple times. I think the solution I came up with works well enough. There's basically a flush of the fsnotify events after a build. I noticed the problem when VIM would save a file it would do lots of file movement between the file and temporary files.

losinggeneration avatar Mar 14 '14 07:03 losinggeneration

It would be really nice to have a config option to ignore certain directories.

My use case is that I have fairly big bower_components directory, with a tons of small files. I think there's a limit on the number of files you can watch through fsnotify.

tamasd avatar Apr 26 '14 23:04 tamasd

@tamasd are you talking about tons of small files that are all Go files? IIRC (and a quick look at the code seems to indicate this as well) is that it's only adding watches to directories and .go files. Would this still be an issue for your browser_components directory? Perhaps I should ask, is this an issue you're already experiencing based on this pull request?

losinggeneration avatar Apr 27 '14 20:04 losinggeneration

@losinggeneration My bower_components directory (~60 MB, more than 11000 files) contains mostly JavaScript files. When I start gin with your modifications, after a second or two, it exits with the error too many open files. The original gin runs well, but it eats around ~30% CPU.

tamasd avatar Apr 27 '14 21:04 tamasd

I agree that it would be awesome to have a ginignore file to track these kinds of things. Please an issue and I will get around implementing it

Sent from my iPhone

On Apr 27, 2014, at 2:33 PM, Tamás Demeter-Haludka [email protected] wrote:

@losinggeneration My bower_components directory (~60 MB, more than 11000 files) contains mostly JavaScript files. When I start gin with your modifications, after a second or two, it exits with the error too many open files. The original gin runs well, but it eats around ~30% CPU.

— Reply to this email directly or view it on GitHub.

codegangsta avatar Apr 27 '14 21:04 codegangsta

Ping :question:

toqueteos avatar Jun 25 '14 17:06 toqueteos

I'm still not sure about this. It doesn't looks like it works for everybody. I may have to try running it again when I get the chance to

codegangsta avatar Jun 30 '14 23:06 codegangsta

I'm specifically using gin because the other similar tools that use fsnotify don't work over network shares. It'd be nice if both methods were retained if this is ever merged.

nalanj avatar Jun 27 '15 00:06 nalanj