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

Globs don't match symlinks #530

Closed
thetrevdev opened this issue Sep 6, 2016 · 13 comments
Closed

Globs don't match symlinks #530

thetrevdev opened this issue Sep 6, 2016 · 13 comments

Comments

@thetrevdev
Copy link

thetrevdev commented Sep 6, 2016

If I have the paths:

origin
symlink-to-origin -> ./origin

I can watch ./symlink-to-origin/subdir/*.txt but I can't watch ./symlink-*/subdir/*.txt

Edit: I wrote these tests to show the specific issue and boundary cases
Passing / failing test cases:

describe('watch paths with symlinks', function() {
    if (os === 'win32') return;
    before(closeWatchers);
    var symLinkPath, originPath;
    beforeEach(function(done) {
      originPath = getFixturePath('origin');
      symLinkPath = getFixturePath('symlink-to-origin');
       fs.mkdir(originPath, 0x1ed, function() {
        fs.mkdir(getFixturePath('origin/subdir'), 0x1ed, function() {
          fs.writeFile(getFixturePath('origin/subdir/add.txt'), 'b', function(){
            fs.symlink(originPath, symLinkPath, done);
          });          
        })
      });
    });
    afterEach(function(done) {
      fs.unlink(symLinkPath, done);
    });

    //fails
    it('should watch files inside a globbed symlink path', function(done) {
      var addSpy = sinon.spy(function addSpy(){});
      watcher = chokidar.watch(getFixturePath('symlink*/subdir/*.txt'), options)
        .on('add', addSpy)
        .on('ready', function() {
          addSpy.should.have.been.calledWith(sysPath.join(symLinkPath, 'subdir/add.txt'));
          done();
        });
    });

    //passes
    it('should watch files inside explicit symlink path', function(done) {
      var addSpy = sinon.spy(function addSpy(){});
      watcher = chokidar.watch(getFixturePath('symlink-to-origin/subdir/*.txt'), options)
        .on('add', addSpy)
        .on('ready', function() {
          addSpy.should.have.been.calledWith(sysPath.join(symLinkPath, 'subdir/add.txt'));
          done();
        });
    });
    it('should watch files inside a globbed path', function(done) {
      var addSpy = sinon.spy(function addSpy(){});
      watcher = chokidar.watch(getFixturePath('orig*/subdir/*.txt'), options)
        .on('add', addSpy)
        .on('ready', function() {
          addSpy.should.have.been.calledWith(sysPath.join(originPath, 'subdir/add.txt'));
          done();
        });
    });
  });

@es128
Copy link
Contributor

es128 commented Sep 7, 2016

So, as you've pointed out, there is a test covering your same exact scenario which passes on my machine and in Travis.

Can you provide details such as your OS and anything else that might be noteworthy about your environment to help explain why it's different for you?

@thetrevdev
Copy link
Author

thetrevdev commented Sep 7, 2016

-Edit: To be clear, I wrote the tests above and they aren't in the actual test suite.

You don't have a test that covers the scenario and test above. Can you try running the first failing test above.

I only saw 1 test that covers both symlinks and globs:

'should properly match glob patterns that include a symlinked dir'
...
var watchDir = sysPath.relative(process.cwd(), linkedDir);
      watcher = chokidar.watch(sysPath.join(watchDir, '**/*'), options)

This would cover the ./symlink-to-origin/* scenario I described above but not the ./symlink-*/* scenario

@es128
Copy link
Contributor

es128 commented Sep 7, 2016

Ah I see. Apologies, I misunderstood. Will check it out.

In the meantime if you want to PR those tests you wrote, would probably be a good idea.

@es128
Copy link
Contributor

es128 commented Sep 7, 2016

I've tracked down a problem in the checkGlobSymlink method. It needs additional logic to detect this scenario and do the appropriate path substitutions.

This only impacts the initial filtering of the readdirp scan, once this is resolved there may possibly wind up being other areas that need fixing as well for the actual event handling (but maybe not).

@es128
Copy link
Contributor

es128 commented Sep 7, 2016

Well, actually I'm uncertain if that's the right way to fix it.

What happens if you set the watcher to just ./symlink-* without the trailing /*? Locally for me that seems to be a functioning workaround to the issue.

@thetrevdev
Copy link
Author

Well for my particular use case I am checking both extensions and subdirs after the symlink

@es128
Copy link
Contributor

es128 commented Sep 7, 2016

Understand, just wanted to verify you're seeing the same thing I am.

Perhaps it would help if you provided a closer equivalent of your real pattern and path structure.

@thetrevdev
Copy link
Author

thetrevdev commented Sep 7, 2016

I updated the original comment / tests to use .txt.
I will do 1 more test case of symlink-
/folder/*.txt

@thetrevdev
Copy link
Author

Updated the test case to use the full example of searching a subdir after a globbed symlink.

@es128
Copy link
Contributor

es128 commented Sep 8, 2016

I am currently evaluating this patch, which so far appears to solve the problem when in fsevents mode

diff --git a/index.js b/index.js
index 9841db2..af63ccc 100644
--- a/index.js
+++ b/index.js
@@ -408,6 +408,7 @@ FSWatcher.prototype._getWatchHelpers = function(path, depth) {
   };

   var filterPath = function(entry) {
+    if (entry.stat.isSymbolicLink()) return filterDir(entry);
     var resolvedPath = entryPath(entry);
     return (!hasGlob || globFilter(resolvedPath)) &&
       this._isntIgnored(resolvedPath, entry.stat) &&

@thetrevdev
Copy link
Author

Awesome! Confirmed working for fsevents.

@thetrevdev
Copy link
Author

Will this get cut into a build any time soon?

@paulmillr
Copy link
Owner

@thetrevdev 1.6.1 is out

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

3 participants