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

vfs.src glob does not match symlinks #349

Closed
4 tasks done
joshhunt opened this issue Mar 19, 2024 · 2 comments · Fixed by gulpjs/glob-stream#122
Closed
4 tasks done

vfs.src glob does not match symlinks #349

joshhunt opened this issue Mar 19, 2024 · 2 comments · Fixed by gulpjs/glob-stream#122

Comments

@joshhunt
Copy link

Before you open this issue, please complete the following tasks:

  • use the search bar at the top of the page to search this repository for similar issues or discussions that have already been opened.
  • if you are looking for help from the gulp team or community, open a discussion.
  • if you think there is a problem with the plugin you're using, open a discussion.
  • if you think there is a bug in our code, open this issue.

What were you expecting to happen?

vfs.src(["./test/**/*.js"]) to match files that are in symlinked directories inside of test

What actually happened?

It doesn't.

Please give us a sample of your gulpfile

I'm not using gulp, using vinyl-fs directly

// ln -s -v -f -n "$PWD"/symlink-src ./test/symlink-dest

vfs.src(["./test/**/*.js"]).pipe(
  map((file, cb) => {
    console.log(file.path.replace(process.cwd(), ""));
    cb(null, file);
  })
);

See also this reproduction https://github.com/joshhunt/vinyl-fs-symlink-repro

Terminal output / screenshots

$ node run.js
vinyl-fs:
/test/top-file.js
/test/folder-a/file-a.js
/test/folder-b/file-b.js
/test/folder-b/folder-c/file-c.js

glob-stream:
/test/top-file.js
/test/folder-a/file-a.js
/test/folder-b/file-b.js
/test/folder-b/folder-c/file-c.js

Please provide the following information:

  • OS & version [e.g. MacOS Catalina 10.15.4]: Ubuntu 20.04.5 LTS
  • node version (run node -v): v20.9.0
  • npm version (run npm -v): 10.1.0

Additional information

@joshhunt
Copy link
Author

@phated It appears this is a regression in glob-stream 8, presumably introduced by gulpjs/glob-stream#118

But, even in glob-stream 7 the behaviour with symlinks is.... odd.

I have the following directory structure

$ tree test -l
test
├── folder-a
│   └── file-a.js
├── folder-b
│   ├── file-b.js
│   └── folder-c
│       └── file-c.js
├── symlink-dest -> ~/vinyl-fs-symlink-repro/symlink-src
│   ├── s-folder-a
│   │   └── s-file-a.js
│   ├── s-folder-b
│   │   ├── folder-c
│   │   │   └── s-file-c.js
│   │   └── s-file-b.js
│   └── s-top-file.js
└── top-file.js

And this is my test file

gs("./test/**/*.js").pipe(
  map((file, cb) => {
    console.log(file.path.replace(process.cwd(), ""));
    cb(null, file);
  })
);

With glob-stream 7, the glob "./test/**/*.js" will match:

glob: ./test/**/*.js

/test/top-file.js
/test/folder-a/file-a.js
/test/folder-b/file-b.js
/test/symlink-dest/s-top-file.js
/test/folder-b/folder-c/file-c.js

With glob-stream 7, the glob "./test/**/**/*.js" will match:

glob: ./test/**/**/*.js

/test/top-file.js
/test/folder-a/file-a.js
/test/folder-b/file-b.js
/test/symlink-dest/s-top-file.js
/test/folder-b/folder-c/file-c.js
/test/symlink-dest/s-folder-a/s-file-a.js
/test/symlink-dest/s-folder-b/s-file-b.js
/test/symlink-dest/s-folder-b/folder-c/s-file-c.js

With glob-stream 8, the glob "./test/**/*.js" will match:

glob: ./test/**/*.js

/test/top-file.js
/test/folder-a/file-a.js
/test/folder-b/file-b.js
/test/folder-b/folder-c/file-c.js

With glob-stream 8, the glob "./test/**/**/*.js" will match:

glob: ./test/**/**/*.js

/test/top-file.js
/test/folder-a/file-a.js
/test/folder-b/file-b.js
/test/folder-b/folder-c/file-c.js

So - there's definitely a regression in glob-stream 8, but the behaviour in 7 was also a bit wonky. I would expect that the **/* glob still traverses symlinks.

I believe this is caused by the new walkdir implementation containing an insufficient check for symlinks

https://github.com/gulpjs/glob-stream/blob/8450d1674c1d9e5c7bff32fdd0f26062ac205a7e/index.js#L75-L77

Is this something you would accept a PR to fix, or add an option to traverse into symlinks?

@phated
Copy link
Member

phated commented Mar 20, 2024

Thanks for diving in so deeply. 🙏 This definitely seems like a bug I introduced in the new implementation. I'm happy to receive a patch!

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.

2 participants