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

Symlinked directories cause EISDIR error from vinyl-fs.src() #39

Closed
pghalliday opened this issue Oct 4, 2014 · 11 comments · Fixed by #57
Closed

Symlinked directories cause EISDIR error from vinyl-fs.src() #39

pghalliday opened this issue Oct 4, 2014 · 11 comments · Fixed by #57

Comments

@pghalliday
Copy link

The fix for #38 is causing me issues when I have symlinked directories. In my use case I want to follow symlinks and copy their contents, etc. However the use of lstat causes vinyl-fs to think symlinked directories are files and I think it tries to load their content resulting in EISDIR errors.

I have created the following project to demonstrate the problem

https://github.com/pghalliday/test-vinyl-fs

@rpendleton
Copy link

Has this been looked into? I'm running into this same problem.

@bolasblack
Copy link

👍

@heikki
Copy link

heikki commented Dec 2, 2014

Maybe this is related:

Note that symlinked directories are not crawled as part of a **, though their contents may match against subsequent portions of the pattern. This prevents infinite loops and duplicates and the like.

https://github.com/isaacs/node-glob#comparisons-to-other-fnmatchglob-implementations

--edit
Maybe not.

@yocontra
Copy link
Member

@heikki Any info on why "maybe not"?

@heikki
Copy link

heikki commented Dec 28, 2014

@contra On first read it seemed related and then I started to doubt because I'm not familiar with lstat etc.

@nahidakbar
Copy link

bump?

@tkalfigo
Copy link

tkalfigo commented Feb 8, 2015

I'm also running into this. Anyone figured out a workaround?

@valeriangalliat
Copy link
Contributor

We also reproduce this error in @SassDoc. There's no stack trace on this EISDIR error, but I could manually trace it here.

So it appears not to be a problem with node-glob; maybe this function should be returning true when the file is a symlink to a directory, or symlinks should be resolved to actual files while here?

Fixed on my side with the following (rather dirty) patch:

diff --git a/lib/src/index.js b/lib/src/index.js
index 01b5430..86f6ca7 100644
--- a/lib/src/index.js
+++ b/lib/src/index.js
@@ -6,6 +6,8 @@ var gs = require('glob-stream');
 var File = require('vinyl');
 var duplexify = require('duplexify');
 var merge = require('merge-stream');
+var fs = require('graceful-fs');
+var path = require('path');

 var filterSince = require('./filterSince');
 var getContents = require('./getContents');
@@ -15,6 +17,27 @@ function createFile(globFile, enc, cb) {
   cb(null, new File(globFile));
 }

+function resolveSymlinks(globFile, enc, cb) {
+  fs.lstat(globFile.path, function (err, stat) {
+    if (err) return cb(err);
+
+    if (!stat.isSymbolicLink()) {
+      return cb(null, globFile);
+    }
+
+    fs.realpath(globFile.path, function (err, filePath) {
+      if (err) return cb(err);
+
+      cb(null, {
+        cwd: globFile.cwd,
+        base: path.dirname(filePath),
+        path: filePath
+      });
+    });
+  });
+}
+
 function src(glob, opt) {
   var options = assign({
     read: true,
@@ -37,6 +60,7 @@ function src(glob, opt) {
   var globStream = gs.create(glob, options);

   var outputStream = globStream
+    .pipe(through.obj(resolveSymlinks))
     .pipe(through.obj(createFile))
     .pipe(getStats(options));

@yocontra
Copy link
Member

@valeriangalliat That runs a stat on each file twice (once in getStats, once in resolveSymlinks)

@valeriangalliat
Copy link
Contributor

Totally, hence me qualifying it as "rather dirty". :/

But maybe propagating the stats object in resolveSymlinks and getting rid of the getStats pipe is an acceptable solution? If so I can provide a PR for this.

@yocontra
Copy link
Member

@valeriangalliat Yep, or make getStats resolve symlinks

phated pushed a commit that referenced this issue Nov 27, 2017
* Removed `getStats` function.
* Added a `resolveSymlinks` function that exposes a `stat` property on
  file object as a side effect (making `getStats` obsolete).
* Added tests.
* Fixes #39.
phated pushed a commit that referenced this issue Nov 27, 2017
* Removed `getStats` function.
* Added a `resolveSymlinks` function that exposes a `stat` property on
  file object as a side effect (making `getStats` obsolete).
* Added tests.
* Fixes #39.
phated pushed a commit that referenced this issue Nov 27, 2017
* Removed `getStats` function.
* Added a `resolveSymlinks` function that exposes a `stat` property on
  file object as a side effect (making `getStats` obsolete).
* Added tests.
* Fixes #39.
phated pushed a commit that referenced this issue Nov 27, 2017
* Removed `getStats` function.
* Added a `resolveSymlinks` function that exposes a `stat` property on
  file object as a side effect (making `getStats` obsolete).
* Added tests.
* Fixes #39.
phated pushed a commit that referenced this issue Nov 27, 2017
* Removed `getStats` function.
* Added a `resolveSymlinks` function that exposes a `stat` property on
  file object as a side effect (making `getStats` obsolete).
* Added tests.
* Fixes #39.
phated pushed a commit that referenced this issue Nov 27, 2017
* Removed `getStats` function.
* Added a `resolveSymlinks` function that exposes a `stat` property on
  file object as a side effect (making `getStats` obsolete).
* Added tests.
* Fixes #39.
phated pushed a commit that referenced this issue Nov 27, 2017
* Removed `getStats` function.
* Added a `resolveSymlinks` function that exposes a `stat` property on
  file object as a side effect (making `getStats` obsolete).
* Added tests.
* Fixes #39.
phated pushed a commit that referenced this issue Nov 28, 2017
* Removed `getStats` function.
* Added a `resolveSymlinks` function that exposes a `stat` property on
  file object as a side effect (making `getStats` obsolete).
* Added tests.
* Fixes #39.
phated pushed a commit that referenced this issue Nov 28, 2017
* Removed `getStats` function.
* Added a `resolveSymlinks` function that exposes a `stat` property on
  file object as a side effect (making `getStats` obsolete).
* Added tests.
* Fixes #39.
phated pushed a commit that referenced this issue Nov 28, 2017
* Removed `getStats` function.
* Added a `resolveSymlinks` function that exposes a `stat` property on
  file object as a side effect (making `getStats` obsolete).
* Added tests.
* Fixes #39.
phated pushed a commit that referenced this issue Nov 28, 2017
* Removed `getStats` function.
* Added a `resolveSymlinks` function that exposes a `stat` property on
  file object as a side effect (making `getStats` obsolete).
* Added tests.
* Fixes #39.
phated pushed a commit that referenced this issue Nov 28, 2017
* Removed `getStats` function.
* Added a `resolveSymlinks` function that exposes a `stat` property on
  file object as a side effect (making `getStats` obsolete).
* Added tests.
* Fixes #39.
phated pushed a commit that referenced this issue Nov 28, 2017
* Removed `getStats` function.
* Added a `resolveSymlinks` function that exposes a `stat` property on
  file object as a side effect (making `getStats` obsolete).
* Added tests.
* Fixes #39.
phated pushed a commit that referenced this issue Nov 28, 2017
* Removed `getStats` function.
* Added a `resolveSymlinks` function that exposes a `stat` property on
  file object as a side effect (making `getStats` obsolete).
* Added tests.
* Fixes #39.
phated pushed a commit that referenced this issue Nov 28, 2017
* Removed `getStats` function.
* Added a `resolveSymlinks` function that exposes a `stat` property on
  file object as a side effect (making `getStats` obsolete).
* Added tests.
* Fixes #39.
phated pushed a commit that referenced this issue Nov 28, 2017
* Removed `getStats` function.
* Added a `resolveSymlinks` function that exposes a `stat` property on
  file object as a side effect (making `getStats` obsolete).
* Added tests.
* Fixes #39.
phated pushed a commit that referenced this issue Nov 28, 2017
* Removed `getStats` function.
* Added a `resolveSymlinks` function that exposes a `stat` property on
  file object as a side effect (making `getStats` obsolete).
* Added tests.
* Fixes #39.
phated pushed a commit that referenced this issue Dec 5, 2017
* Removed `getStats` function.
* Added a `resolveSymlinks` function that exposes a `stat` property on
  file object as a side effect (making `getStats` obsolete).
* Added tests.
* Fixes #39.
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

Successfully merging a pull request may close this issue.

8 participants