Skip to content

Conversation

@kylo5aby
Copy link
Contributor

@kylo5aby kylo5aby commented Apr 16, 2024

  • Add a skipSymlinkDir option to the fs.readdir to allow for behavior similar to the find [path] -print and find -L [path] -print commands. This option would control whether symbolic link directories are skipped or traversed during directory listing.
  • Ensure that checking if the current directory is a symlink leads to consistent file listings in fs.readdir([path], {withFileTypes: false}) and fs.readdir([path], {withFileTypes: true})

Refs: #51858

@nodejs-github-bot nodejs-github-bot added fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels Apr 16, 2024
@avivkeller
Copy link
Member

/cc @nodejs/fs

const skipSymlinkDir = options.skipSymlinkDir || false;

if (skipSymlinkDir) {
const stats = await lstat(originalPath);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm no CPP expert, but benchmark-wise, wouldn't this call make us get the file twice? Is there a CPP way to achieve this same result?

Disclaimer: I'm not a CPP expert (or even a novice)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't find an effective way to acquire stat in cpp way directly to avoid this dilemma, it would be great if anyone could help, and this will only affect efficiency when using skipSymlinkDir to skip symbolic link dirs.

pathModule.relative(originalPath, direntPath),
);
if (skipSymlinkDir) {
const stats = await lstat(direntPath);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See previous comment

@richardlau
Copy link
Member

#49255 would implement a more general solution (provision of a filtering function to limit traversal).

@kylo5aby kylo5aby closed this Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants