-
Notifications
You must be signed in to change notification settings - Fork 314
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
Add fsnotify support to gin #15
base: master
Are you sure you want to change the base?
Conversation
* The only downside to this is that the fsnotify is in the go.exp repository which may break API or move at some point.
https://github.com/howeyc/fsnotify <- |
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. |
This looks pretty good! Using fsnotify I seem to remember that it does not Again, I don't seem to remember whether or not that is a real concern. I Sidenote - It would be cool to block serving in the proxy until the app is On Thu, Mar 13, 2014 at 2:16 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. |
Sounds good. Thanks! On Thu, Mar 13, 2014 at 2:38 PM, Harley Laue [email protected]:
|
* To avoid multiple rebuilds on saving a file, some extra work was needed.
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. |
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 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 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 |
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
|
Ping ❓ |
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 |
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. |
@@ -0,0 +1,103 @@ | |||
//+build fsnotify |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@commondream The default for this PR is actually to be without fsnotify. It has to be built with go build -tags fsnotify
Though, it seems to have a conflict currently.
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.