Repository: gulpjs/vinyl-fs (js)

Issue:
Symlink following logic in src() seems wrong to me
Basically, the generated vinyl file base should be what's set to to the symlink itself:

Say the src directory structure is like:
```
|-- foo
    |-- bar -> baz
    |-- baz
        |-- qux.txt
```
the symlink should produce: ./target/foo/bar at the end with qux.txt in it if dest is set to target. In practice it doesn't.

The reason is the file base should always be set to ./foo for anything linked by bar whereas the original recursive logic would produce foo/baz.

Such incorrect logic would yield a flattened file structure and empty directories at the end:

```
pipe(gulp.dest('target'))

the original logic would generate:

|-- target
    |-- foo
    |-- baz (empty)
    |-- qux.txt

whereas the fixed logic generates:

|-- target
    |-- foo
        |-- bar
            |-- qux.txt
```

I've opened a PR here: https://github.com/gulpjs/vinyl-fs/pull/131
Please check it out.

Pull request:
Fixes #132 The symlink following logic in src() was incorrect resulting in wrong file.base being generated.
Basically, the generated vinyl file base should be what's set to to the symlink itself:

foo/bar -> baz/qux.txt
this should produce: foo/baz/qux.txt

but the file base should still default to ./foo whereas the original recursive logic would produce foo/baz.

Such incorrect logic would yield a flattened file structure and empty directories at the end.

Patch:
diff --git a/lib/src/index.js b/lib/src/index.js
index a95ac41..06cbe75 100644
--- a/lib/src/index.js
+++ b/lib/src/index.js
@@ -3,7 +3,6 @@
 var assign = require('object-assign');
 var through2 = require('through2');
 var gs = require('glob-stream');
-var File = require('vinyl');
 var duplexify = require('duplexify');
 var merge = require('merge-stream');
 var sourcemaps = require('gulp-sourcemaps');
@@ -11,11 +10,7 @@ var filterSince = require('../filterSince');
 var isValidGlob = require('is-valid-glob');
 
 var getContents = require('./getContents');
-var resolveSymlinks = require('./resolveSymlinks');
-
-function createFile(globFile, enc, cb) {
-  cb(null, new File(globFile));
-}
+var wrapWithVinylFile = require('./wrapWithVinylFile');
 
 function src(glob, opt) {
   var options = assign({
@@ -40,8 +35,7 @@ function src(glob, opt) {
   var globStream = gs.create(glob, options);
 
   var outputStream = globStream
-    .pipe(resolveSymlinks(options))
-    .pipe(through2.obj(options, createFile));
+    .pipe(wrapWithVinylFile(options));
 
   if (options.since != null) {
     outputStream = outputStream
diff --git a/lib/src/resolveSymlinks.js b/lib/src/resolveSymlinks.js
deleted file mode 100644
index 9bcc8d4..0000000
--- a/lib/src/resolveSymlinks.js
+++ /dev/null
@@ -1,39 +0,0 @@
-'use strict';
-
-var through2 = require('through2');
-var fs = require('graceful-fs');
-var path = require('path');
-
-function resolveSymlinks(options) {
-
-  // A stat property is exposed on file objects as a (wanted) side effect
-  function resolveFile(globFile, enc, cb) {
-    fs.lstat(globFile.path, function(err, stat) {
-      if (err) {
-        return cb(err);
-      }
-
-      globFile.stat = stat;
-
-      if (!stat.isSymbolicLink() || !options.followSymlinks) {
-        return cb(null, globFile);
-      }
-
-      fs.realpath(globFile.path, function(err, filePath) {
-        if (err) {
-          return cb(err);
-        }
-
-        globFile.base = path.dirname(filePath);
-        globFile.path = filePath;
-
-        // Recurse to get real file stat
-        resolveFile(globFile, enc, cb);
-      });
-    });
-  }
-
-  return through2.obj(options, resolveFile);
-}
-
-module.exports = resolveSymlinks;
diff --git a/lib/src/wrapWithVinylFile.js b/lib/src/wrapWithVinylFile.js
new file mode 100644
index 0000000..5201e48
--- /dev/null
+++ b/lib/src/wrapWithVinylFile.js
@@ -0,0 +1,50 @@
+'use strict';
+
+var through2 = require('through2');
+var fs = require('graceful-fs');
+var File = require('vinyl');
+
+function wrapWithVinylFile(options) {
+
+  // A stat property is exposed on file objects as a (wanted) side effect
+  function resolveFile(globFile, enc, cb) {
+    fs.lstat(globFile.path, function(err, stat) {
+      if (err) {
+        return cb(err);
+      }
+
+      globFile.stat = stat;
+
+      if (!stat.isSymbolicLink() || !options.followSymlinks) {
+        var vinylFile = new File(globFile);
+        if (globFile.originalSymlinkPath) {
+          // If we reach here, it means there is at least one
+          // symlink on the path and we need to rewrite the path
+          // to its original value.
+          // Updated file stats will tell getContents() to actually read it.
+          vinylFile.path = globFile.originalSymlinkPath;
+        }
+        return cb(null, vinylFile);
+      }
+      if (!globFile.originalSymlinkPath) {
+        // Store the original symlink path before the recursive call so later we can rewrite it back.
+        globFile.originalSymlinkPath = globFile.path;
+      }
+
+      fs.realpath(globFile.path, function(err, filePath) {
+        if (err) {
+          return cb(err);
+        }
+
+        globFile.path = filePath;
+
+        // Recurse to get real file stat
+        resolveFile(globFile, enc, cb);
+      });
+    });
+  }
+
+  return through2.obj(options, resolveFile);
+}
+
+module.exports = wrapWithVinylFile;


Test patch:
diff --git a/test/fixtures/foo/bar/baz.txt b/test/fixtures/foo/bar/baz.txt
new file mode 100644
index 0000000..999e9ec
--- /dev/null
+++ b/test/fixtures/foo/bar/baz.txt
@@ -0,0 +1,1 @@
+symlink works
diff --git a/test/src.js b/test/src.js
index d92cb9d..1601e17 100644
--- a/test/src.js
+++ b/test/src.js
@@ -428,34 +428,60 @@ describe('.src() symlinks', function() {
   var filePath = path.join(__dirname, './fixtures/test.coffee');
   var fileSymlinkPath = path.join(__dirname, './fixtures/test-symlink');
 
+  var symlinkFirstLayer = path.join(__dirname, './fixtures/test-multi-layer-symlink');
+  var symlinkSecondLayer = path.join(__dirname, './fixtures/foo/baz-link.txt');
+  var symlinkSecondLayerTarget = path.join(__dirname, './fixtures/foo/bar/baz.txt');
+
   beforeEach(function(done) {
     fs.symlinkSync(dirPath, dirSymlinkPath);
     fs.symlinkSync(filePath, fileSymlinkPath);
+    fs.symlinkSync(symlinkSecondLayerTarget, symlinkSecondLayer);
+    fs.symlinkSync(symlinkSecondLayer, symlinkFirstLayer);
     done();
   });
 
   afterEach(function(done) {
     fs.unlinkSync(dirSymlinkPath);
     fs.unlinkSync(fileSymlinkPath);
+    fs.unlinkSync(symlinkSecondLayer);
+    fs.unlinkSync(symlinkFirstLayer);
     done();
   });
 
-  it('should follow file symlinks', function(done) {
-    var stream = vfs.src(fileSymlinkPath, { cwd: __dirname });
+  it('should follow symlinks correctly', function(done) {
+    var linkTarget = './fixtures/test-multi-layer-symlink';
+    var expectedPath = path.join(__dirname, linkTarget);
+
+    var stream = vfs.src(linkTarget, { cwd: __dirname, base: __dirname });
     stream.on('data', function(file) {
-      file.path.should.equal(filePath);
+      file.base.should.equal(__dirname);
+      // The path should be the symlink itself
+      file.path.should.equal(expectedPath);
+      // But the content should be what's in the actual file
+      file.contents.toString().should.equal('symlink works\n');
+      file.stat.isSymbolicLink().should.equal(false);
+      file.stat.isFile().should.equal(true);
       done();
     });
   });
 
-  it('should follow dir symlinks', function(done) {
-    var stream = vfs.src(dirSymlinkPath, { cwd: __dirname });
+  it('should follow dir symlinks correctly', function(done) {
+    var linkTarget = './fixtures/test-symlink-dir';
+    var expectedPath = path.join(__dirname, linkTarget);
+
+    var stream = vfs.src(linkTarget, { cwd: __dirname, base: __dirname });
     stream.on('data', function(file) {
-      file.path.should.equal(dirPath);
+      file.base.should.equal(__dirname);
+      // The path should be the symlink itself
+      file.path.should.equal(expectedPath);
+      // But the stats should have been updated
+      file.stat.isSymbolicLink().should.equal(false);
+      file.stat.isDirectory().should.equal(true);
       done();
     });
   });
 
+
   it('should preserve file symlinks with followSymlinks option set to false', function(done) {
     fs.readlink(fileSymlinkPath, function(err, expectedRelativeSymlinkPath) {
       if (err) {