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

file watching doesn't work as expected on Windows #69

Closed
heycalmdown opened this issue Aug 4, 2012 · 3 comments
Closed

file watching doesn't work as expected on Windows #69

heycalmdown opened this issue Aug 4, 2012 · 3 comments

Comments

@heycalmdown
Copy link
Contributor

fs.watch on Windows lies on below function:
https://github.com/joyent/libuv/blob/95e89c6a0efec2d2228377cfdf55cd14b1b83536/src/win/fs-event.c#L69

as you can see on the fourth option(FALSE) it doesn't watch any changes of subtree's.

I suggest use the same function - watchFile - as used for non-win32 implements.

@iangreenleaf
Copy link
Collaborator

Do you mean it's no longer recursively watching everything in a directory? It was working at some point - we couldn't always get the exact filename that had changed, but could at least detect change events.

I would like to unify the system calls. I'm hesitant though because as noted in nodejs/node-v0.x-archive#2049, it's not totally clear which option is preferred - the core guys stated at one point that watchFile will be going away, and the docs instruct you to use watch when available. But watch still has some bugs and doesn't always pass a filename, which would be a problem for a couple of our features. And just to compound matters, it's unclear from the docs if watchFile makes use of inotify etc when available, or if it is a purely polling solution (at one point it was using inotify, but maybe that's only happening in watch now).

So my point is that neither of the options looks totally great to me right now. Thoughts?

@heycalmdown
Copy link
Contributor Author

As you know node-supervisor watch the root directory only via fs.watch on Windows. And according to the implement of the fs.watch the fourth option is FALSE that means ReadDirectoryChangesW doesn't watch files recursively at all.
http://msdn.microsoft.com/en-us/library/windows/desktop/aa365465(v=vs.85).aspx

Not just leaking exact filename, but leaking the event too.

I can understand why the implementation of fs.watch use the option, because making too big size of buffer for watch huge number of files leads to get the BSOD. And also understand your concern about polling many files with fs.watchFile.

So here is my thought: seek directories recursively like non-windows implementation and watch directory only not each files.

pseudo code:

var findAllWatchFiles = function(dir, callback) {
  dir = path.resolve(dir);
  if (ignoredPaths[dir])
    return;
  fs.stat(dir, function(err, stats){
    if (err) {
      util.error('Error retrieving stats for file: ' + dir);
    } else {
      if (stats.isDirectory()) {
        if (isWindows) callback(dir);
        fs.readdir(dir, function(err, fileNames) {
          if(err) {
            util.error('Error reading path: ' + dir);
          }
          else {
            fileNames.forEach(function (fileName) {
              findAllWatchFiles(dir + '/' + fileName, callback);
            });
          }
        });
      } else {
        if (!isWindows && dir.match(fileExtensionPattern)) {
          callback(dir);
        }
      }
    }
  });
};

@iangreenleaf
Copy link
Collaborator

Closed by #70.

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

No branches or pull requests

2 participants