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

Gulp no longer follows symlinks past the first level #792

Closed
wesleycho opened this issue Nov 27, 2014 · 19 comments
Closed

Gulp no longer follows symlinks past the first level #792

wesleycho opened this issue Nov 27, 2014 · 19 comments

Comments

@wesleycho
Copy link

I have noticed a regression in Gulp where when using gulp.src, if a directory that is matched is symlinked, it will no longer try to follow nested folders of that directory. This is due to this change in vinyl-fs.

@yocontra
Copy link
Member

@wesleycho I'm not confident that it has to do with that change. node-glob is ultimately what walks the fs and emits stuff, this change just makes sure that when we get results from the glob module that we collect stats on links too

@yuvilio
Copy link

yuvilio commented Dec 9, 2014

Noticing a similar non following of symlinks when using gulp.dest() to write to a symlinked directory. The files doesn't end up written there. Same task works fine when the directory is not symlinked.

@yocontra
Copy link
Member

yocontra commented Dec 9, 2014

Is this still happening after an npm update? glob just fixed some bugs

@wesleycho
Copy link
Author

Yup, just updated global gulp and local to the project I saw it in - still occurring.

@wesleycho
Copy link
Author

@contra: FWIW, lstat in graceful-fs is coming straight from the Node.js fs.lstat, which states here the difference of behavior when dealing with symlinks - http://nodejs.org/api/fs.html#fs_fs_lstat_path_callback

Asynchronous lstat(2). The callback gets two arguments (err, stats) where stats is a fs.Stats object. lstat() is identical to stat(), except that if path is a symbolic link, then the link itself is stat-ed, not the file that it refers to.

This appears to be the cause of the issue to me, since it is the link that is stat'ed and not the files anymore, so gulp.src cannot read the actual contents of the symlinked files.

@yocontra
Copy link
Member

@wesleycho I think that was done for a reason - people wanted to take a folder of symlinks and move them somewhere else, which I is the correct behavior IMO. There is no way to support both of these cases, one will have to be behind an option.

@wesleycho
Copy link
Author

Well, this problem affects all of our build processes with development at my company since we cannot symlink component libraries and do a build for development since none of our files are present in the stream. It's a legitimate use case I believe.

Someone here is willing to open PRs in vinyl-fs & here to add a config option if that's the solution that is most desired.

@yocontra
Copy link
Member

@wesleycho I think an option to src that says if it should read symlinks or not would work best. Default should be not to read symlinks

@wesleycho
Copy link
Author

Looks like you were right - the problem is in node-glob: https://github.com/isaacs/node-glob/blob/master/glob.js#L532-L534 .

There even already is a reported issue for this. I'll look at subbing a patch there, and then subbing a patch here for hooking into this option when it makes sense.

@stringparser
Copy link

@wesleycho let me know if you are busy I can take one of the request if you want

@yocontra
Copy link
Member

@wesleycho AFAIK the options to src should get passed straight through to glob-stream which passes them straight through to glob. Let me know if that isn't the case and I'll take a PR for adding the new option. Even if it is passed through and already functional, we should still add some info to the docs with that option

@tkalfigo
Copy link

tkalfigo commented Feb 8, 2015

Running into this problem as well. Any updates regarding passing options to src()?

@heikki heikki added the bug label Feb 14, 2015
@heikki
Copy link
Contributor

heikki commented Feb 15, 2015

Shortcut to node-glob PR -> isaacs/node-glob#148

@evenfrost
Copy link

Will like this feature as well.

@phated
Copy link
Member

phated commented Mar 22, 2015

isaacs/node-glob#148 has been closed by a different commit by @isaacs - is this solved?

@phated
Copy link
Member

phated commented Mar 23, 2015

Can someone write a test for this against the vinyl-fs repo? I believe it is fixed now that @contra bumped the glob dependency (vinyl-fs 5.0.0)

@phated phated added this to the gulp 4 milestone Mar 23, 2015
@isaacs
Copy link

isaacs commented Mar 23, 2015

Note that the glob symlink followin behavior is opt-in, so you need to pass follow:true in the options object.

@yocontra
Copy link
Member

@isaacs Ah thanks, I need to add that then

@phated
Copy link
Member

phated commented Apr 1, 2015

@contra I'm assuming we still want it to be opt-in since it is an edge case. I've added a note to the 4.0 docs in 725da2a

@phated phated closed this as completed Apr 1, 2015
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