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

[feature request] auto-detect network mounts and switch to polling #242

Closed
nfvs opened this issue Feb 26, 2015 · 31 comments
Closed

[feature request] auto-detect network mounts and switch to polling #242

nfvs opened this issue Feb 26, 2015 · 31 comments

Comments

@nfvs
Copy link

nfvs commented Feb 26, 2015

Only by passing usePolling=true will chokidar watch for changes inside mounted folders. Could be that this happens with all VM shared folders and not just with docker.

Grunt-contrib-watch (which uses gaze) works, perhaps by falling back to polling automatically, so perhaps finding a way to fallback could be an option too.

@es128
Copy link
Contributor

es128 commented Feb 26, 2015

I think grunt-contrib-watch, like gulp, uses an old version of gaze that relies on polling by default.

Falling back to polling is easy, but do you have any ideas about how to automatically detect mounted drives and determine a fallback is necessary (without, for instance, just making assumptions about paths like /mnt)?

@es128
Copy link
Contributor

es128 commented Feb 26, 2015

Btw, the need to set usePolling for mounted drives is documented in the README.

@nfvs
Copy link
Author

nfvs commented Feb 26, 2015

Very good point about watch using an old polling-only version of gaze, I just tried it with a recent version and indeed it doesn't work.

I'm aware we should use polling, unfortunately my use case involves using grunt-browserify, which uses watchify, which in turn uses chokidar. I was hoping we could use a fallback method instead of configuration parameters, which may very well be impossible. Currently waiting on two PRs that allow passing an option from grunt -> watchify -> chokidar.

@es128
Copy link
Contributor

es128 commented Feb 26, 2015

I'd be happy to implement it if we can find a way to automatically detect mounted paths.

@nfvs
Copy link
Author

nfvs commented Feb 26, 2015

I don't know of any that doesn't involve stat:

[root@46ea7f60201f ~]# stat -fc%T app/
nfs
[root@46ea7f60201f ~]# stat -fc%T .
aufs

@es128
Copy link
Contributor

es128 commented Feb 26, 2015

Well that's something at least - which I hadn't been aware of. I actually don't understand all the fields in http://nodejs.org/api/fs.html#fs_class_fs_stats - is this kind of thing represented in there in any way?

@nfvs
Copy link
Author

nfvs commented Feb 26, 2015

Trying to find that out right now...

@nfvs
Copy link
Author

nfvs commented Feb 26, 2015

Don't think so tbh. stat -f gets information about the filesystem and not the file (man), which seems to be the only information that fs returns.

@es128
Copy link
Contributor

es128 commented Feb 26, 2015

So any objection to closing this issue?

@nfvs
Copy link
Author

nfvs commented Feb 26, 2015

So close to a solution... How about making running stat on start up and checking only the root folder being watched? Would be a good compromise at least, as long as the performance penalty isn't so great (provided it's even possible too).

@es128
Copy link
Contributor

es128 commented Feb 26, 2015

Maybe...

@es128 es128 changed the title Not watching mounted folders inside docker containers [feature request] auto-detect network mounts and switch to polling Feb 26, 2015
@nfvs
Copy link
Author

nfvs commented Feb 26, 2015

Simple example, in my docker container ./app is a shared folder:

var exec = require('child_process').exec;
function check(error, stdout, stderr) {
  if (stdout.trim() == 'nfs')
    console.info('MOUNT POINT');
  else
    console.info('FOLDER');
}
exec("stat -fc%T .", check);
exec("stat -fc%T ./app", check);

Output:

FOLDER
MOUNT POINT

@nfvs
Copy link
Author

nfvs commented Feb 26, 2015

I looked a little more into this, turns out stat output is not the same across Linux/OSX. This was the best way I could find.

Linux: df -T nfs,sshfs,cifs,vboxsf $dir
Will output anything if one of the given filesystems is found. Return code 0 means the dir has one of those types, so it's a shared dir.

OS X: df -PT $dir | sed 1d | awk '{print $2}'
Will print the type (nfs, sshfs...). Can be combined with grep so we can get rid of sed/awk and match by return code too:
df -PT $dir | grep 'vboxsf\|sshfs\|nfs\|cifs'

@es128
Copy link
Contributor

es128 commented Feb 26, 2015

@nfvs Thanks for figuring that stuff out. I'll see what I can do regarding using it to make chokidar more intelligent.

@blopker
Copy link

blopker commented Feb 28, 2015

I was thinking about this the other day because Watchify refuses to let us use polling or pass options to chokidar so I'm stuck on an old version. :(

One solution I was thinking of: what if at startup you initiated two watchers. One with polling, one using fs events. Then wait for an event. If they both catch it, kill the polling watcher. If only the polling watcher picks up the change then the filesystem doesn't support events, so kill that one.

To me this seems like the only sure-fire way of determining if events will work. Keep in mind it's not just network mounts that must use polling so using something like stat won't catch all cases.

@nfvs
Copy link
Author

nfvs commented Feb 28, 2015

because Watchify refuses to let us use polling or pass options to chokidar

@blopker actually there is an existing pull request exactly for that. If you don't want to wait, you can use this fork.

If you use grunt-browserify, support for passing options to Watchify was also just added.

@es128
Copy link
Contributor

es128 commented Feb 28, 2015

what if at startup you initiated two watchers

Sounds like an invitation for 100 new bugs and problems.

But let's keep trying to think of elegant ways to detect that a fallback to polling is needed.

@blopker
Copy link

blopker commented Feb 28, 2015

I'm aware of the fixes (there are many of them), Watchify just needs to add one.

Sounds like an invitation for 100 new bugs and problems.

You must be an exceptionally bad coder, my condolences. It's funny you're looking for an 'elegant' solution for a library which is essentially a giant hack in the first place. Hopefully this io.js business will fix the fs module and this library can go away.

Anyway, good luck with this. 🍺

@es128
Copy link
Contributor

es128 commented Feb 28, 2015

Yup, I'm an exceptionally bad coder. Go away before my horribleness rubs off on you.

@paulmillr
Copy link
Owner

@blopker race conditions in some cases are indeed very hard to fix. Elan is totally right here. Running two watchers at once would never be a good solution for a problem.

Anyway this is not a place to discuss whether someone is a good or bad coder.

@gillesdemey
Copy link

There's no mention of mount here, but doesn't it accomplish pretty much the same thing?

On Linux and OSX:
mount -t vboxsf,sshfs,nfs,cifs

It seems trivial to figure out if the cwd is on one of these mount paths, or am I missing something obvious here?

@nfvs
Copy link
Author

nfvs commented Apr 4, 2015

@gillesdemey would that also work for symlinks?

@gillesdemey
Copy link

We can resolve symlinks using fs.realpath().

So I think we can solve this in two steps:

  • figure out what NFS drives are mounted on the system
  • figure out if the watched directory lives on one of those mounted drives

EDIT: I've created a quick proof-of-concept and it works on my vagrant box
https://gist.github.com/gillesdemey/c6f2d7d33b4cbe3c4f6c

There's probably lots of room for improvement though.

@es128
Copy link
Contributor

es128 commented Aug 28, 2015

A thought that just occurred to me is that rather than solve NFS detection, which would still have holes, a potential solution here would be for chokidar to execute something similar to what it does in the unit tests - quickly write, change, and delete a temp file in the watched directory, listening for the events and switching watch modes if the test fails. This could only become default behavior with a major version bump, but maybe it can first be tried out behind an option in a minor release.

Interested in anyone else's thoughts on that idea.

@ahomatthew
Copy link

+1 to the idea above

@Xample
Copy link

Xample commented Aug 29, 2015

In some environments like within a Docker container you have a special filesystem. Moving a file is therefore similar to removing it and adding it back. There is no reliable way to know if the file is the same or not, the 'ino' of the file also changes because the filesystem is virtualised. A way could be to compare the file size and make a hash of it but you can imagine how slow it will be. If anyone could provide a better option I am all ears :-)

@es128
Copy link
Contributor

es128 commented Aug 29, 2015

@Xample I'm not sure I understand your point. Seems like what you're saying is more relevant to the discussion about rename events.

My proposal did not involve moving a file. Just adding it, changing its contents, and deleting it.

@Xample
Copy link

Xample commented Aug 31, 2015

@es128 okay I probably mixed up 2 threads, I thought only computer were able to do so :-)

@jaalzateolaya
Copy link

I'm still having this issue on a vagrant box.

@jakiestfu
Copy link

@paulmillr Was there a resolution to this issue?

@paulmillr
Copy link
Owner

No, i've closed it because no feedback has been posted for 1.5 yrs. If you want to resolve it, go ahead and submit a PR. Don't want to keep a list of 100 issues that are unpopular.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants