-
-
Notifications
You must be signed in to change notification settings - Fork 34.4k
fs: add followSymlinks option to glob and globSync #61317
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,8 +16,8 @@ const { | |
| StringPrototypeEndsWith, | ||
| } = primordials; | ||
|
|
||
| const { lstatSync, readdirSync } = require('fs'); | ||
| const { lstat, readdir } = require('fs/promises'); | ||
| const { lstatSync, readdirSync, statSync: fsStatSync } = require('fs'); | ||
| const { lstat, readdir, stat: fsStat } = require('fs/promises'); | ||
|
Comment on lines
+19
to
+20
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are these being renamed?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If you believe that I should make the change, do let me know, I'll commit and push ASAP :) |
||
| const { join, resolve, basename, isAbsolute, dirname } = require('path'); | ||
|
|
||
| const { | ||
|
|
@@ -264,12 +264,14 @@ class Glob { | |
| #subpatterns = new SafeMap(); | ||
| #patterns; | ||
| #withFileTypes; | ||
| #followSymlinks; | ||
| #isExcluded = () => false; | ||
| constructor(pattern, options = kEmptyObject) { | ||
| validateObject(options, 'options'); | ||
| const { exclude, cwd, withFileTypes } = options; | ||
| const { exclude, cwd, withFileTypes, followSymlinks } = options; | ||
| this.#root = toPathIfFileURL(cwd) ?? '.'; | ||
| this.#withFileTypes = !!withFileTypes; | ||
| this.#followSymlinks = !!followSymlinks; | ||
| if (exclude != null) { | ||
| validateStringArrayOrFunction(exclude, 'options.exclude'); | ||
| if (ArrayIsArray(exclude)) { | ||
|
|
@@ -429,6 +431,16 @@ class Glob { | |
| const entryPath = join(path, entry.name); | ||
| this.#cache.addToStatCache(join(fullpath, entry.name), entry); | ||
|
|
||
| let isDirectory = entry.isDirectory(); | ||
| if (entry.isSymbolicLink() && this.#followSymlinks) { | ||
| try { | ||
| const stat = fsStatSync(join(fullpath, entry.name)); | ||
| isDirectory = stat.isDirectory(); | ||
| } catch { | ||
| // ignore | ||
| } | ||
|
||
| } | ||
|
|
||
| const subPatterns = new SafeSet(); | ||
| const nSymlinks = new SafeSet(); | ||
| for (const index of pattern.indexes) { | ||
|
|
@@ -456,7 +468,7 @@ class Glob { | |
| (this.#exclude && this.#exclude(this.#withFileTypes ? entry : entry.name))) { | ||
| continue; | ||
| } | ||
| if (!fromSymlink && entry.isDirectory()) { | ||
| if (!fromSymlink && isDirectory) { | ||
| // If directory, add ** to its potential patterns | ||
| subPatterns.add(index); | ||
| } else if (!fromSymlink && index === last) { | ||
|
|
@@ -469,24 +481,24 @@ class Glob { | |
| if (nextMatches && nextIndex === last && !isLast) { | ||
| // If next pattern is the last one, add to results | ||
| this.#results.add(entryPath); | ||
| } else if (nextMatches && entry.isDirectory()) { | ||
| } else if (nextMatches && isDirectory) { | ||
| // Pattern matched, meaning two patterns forward | ||
| // are also potential patterns | ||
| // e.g **/b/c when entry is a/b - add c to potential patterns | ||
| subPatterns.add(index + 2); | ||
| } | ||
| if ((nextMatches || pattern.at(0) === '.') && | ||
| (entry.isDirectory() || entry.isSymbolicLink()) && !fromSymlink) { | ||
| (isDirectory || entry.isSymbolicLink()) && !fromSymlink) { | ||
| // If pattern after ** matches, or pattern starts with "." | ||
| // and entry is a directory or symlink, add to potential patterns | ||
| subPatterns.add(nextIndex); | ||
| } | ||
|
|
||
| if (entry.isSymbolicLink()) { | ||
| if (entry.isSymbolicLink() && !this.#followSymlinks) { | ||
| nSymlinks.add(index); | ||
| } | ||
|
|
||
| if (next === '..' && entry.isDirectory()) { | ||
| if (next === '..' && isDirectory) { | ||
| // In case pattern is "**/..", | ||
| // both parent and current directory should be added to the queue | ||
| // if this is the last pattern, add to results instead | ||
|
|
@@ -529,7 +541,7 @@ class Glob { | |
| // add next pattern to potential patterns, or to results if it's the last pattern | ||
| if (index === last) { | ||
| this.#results.add(entryPath); | ||
| } else if (entry.isDirectory()) { | ||
| } else if (isDirectory) { | ||
| subPatterns.add(nextIndex); | ||
| } | ||
| } | ||
|
|
@@ -639,6 +651,16 @@ class Glob { | |
| const entryPath = join(path, entry.name); | ||
| this.#cache.addToStatCache(join(fullpath, entry.name), entry); | ||
|
|
||
| let isDirectory = entry.isDirectory(); | ||
| if (entry.isSymbolicLink() && this.#followSymlinks) { | ||
| try { | ||
| const s = await fsStat(join(fullpath, entry.name)); | ||
| isDirectory = s.isDirectory(); | ||
| } catch { | ||
| // ignore | ||
| } | ||
| } | ||
|
|
||
| const subPatterns = new SafeSet(); | ||
| const nSymlinks = new SafeSet(); | ||
| for (const index of pattern.indexes) { | ||
|
|
@@ -666,7 +688,7 @@ class Glob { | |
| (this.#exclude && this.#exclude(this.#withFileTypes ? entry : entry.name))) { | ||
| continue; | ||
| } | ||
| if (!fromSymlink && entry.isDirectory()) { | ||
| if (!fromSymlink && isDirectory) { | ||
| // If directory, add ** to its potential patterns | ||
| subPatterns.add(index); | ||
| } else if (!fromSymlink && index === last) { | ||
|
|
@@ -683,24 +705,24 @@ class Glob { | |
| if (!this.#results.has(entryPath) && this.#results.add(entryPath)) { | ||
| yield this.#withFileTypes ? entry : entryPath; | ||
| } | ||
| } else if (nextMatches && entry.isDirectory()) { | ||
| } else if (nextMatches && isDirectory) { | ||
| // Pattern matched, meaning two patterns forward | ||
| // are also potential patterns | ||
| // e.g **/b/c when entry is a/b - add c to potential patterns | ||
| subPatterns.add(index + 2); | ||
| } | ||
| if ((nextMatches || pattern.at(0) === '.') && | ||
| (entry.isDirectory() || entry.isSymbolicLink()) && !fromSymlink) { | ||
| (isDirectory || entry.isSymbolicLink()) && !fromSymlink) { | ||
| // If pattern after ** matches, or pattern starts with "." | ||
| // and entry is a directory or symlink, add to potential patterns | ||
| subPatterns.add(nextIndex); | ||
| } | ||
|
|
||
| if (entry.isSymbolicLink()) { | ||
| if (entry.isSymbolicLink() && !this.#followSymlinks) { | ||
| nSymlinks.add(index); | ||
| } | ||
|
|
||
| if (next === '..' && entry.isDirectory()) { | ||
| if (next === '..' && isDirectory) { | ||
| // In case pattern is "**/..", | ||
| // both parent and current directory should be added to the queue | ||
| // if this is the last pattern, add to results instead | ||
|
|
@@ -759,7 +781,7 @@ class Glob { | |
| yield this.#withFileTypes ? entry : entryPath; | ||
| } | ||
| } | ||
| } else if (entry.isDirectory()) { | ||
| } else if (isDirectory) { | ||
| subPatterns.add(nextIndex); | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -543,3 +543,22 @@ describe('glob - with restricted directory', function() { | |
| } | ||
| }); | ||
| }); | ||
|
|
||
| describe('glob follow', () => { | ||
| test('should return matched files in symlinked directory when follow is true', async () => { | ||
| if (common.isWindows) return; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you explain why we completely bail on this test if we're on Windows?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The setup function excluded it specifically as well. I did not question why. From what I am understanding, symlinks need admin permissions or developer mode enabled on Windows which is perhaps why the test does not exist for it. |
||
| const relFilesPromise = asyncGlob('**', { cwd: fixtureDir, followSymlinks: true }); | ||
| let count = 0; | ||
| // eslint-disable-next-line no-unused-vars | ||
| for await (const file of relFilesPromise) { | ||
| count++; | ||
| } | ||
| assert.ok(count > 0); | ||
| }); | ||
|
|
||
| test('should return matched files in symlinked directory when follow is true (sync)', () => { | ||
| if (common.isWindows) return; | ||
| const relFiles = globSync('**', { cwd: fixtureDir, followSymlinks: true }); | ||
| assert.ok(relFiles.length > 0); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure to update the changes entry above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What should be the version tag for the change?