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

fix: Throw an error when readdir is called on a non-directory #492

Merged
merged 1 commit into from
Jun 24, 2024

Conversation

AtkinsSJ
Copy link
Contributor

This replaces a72ec97 as a fix for #417.

That change meant that calling readdir on a file path would return a single stat entry, for that file. The problem is, to callers that looks like the file is a directory containing itself. This was causing problems in isomorphic-git, as it would readdir foo.txt, receive the listing of [ {path: foo.txt', ... } ], and then try to read foo.txt/foo.txt and get confused that the file doesn't exist.

This change instead returns an error equivalent to ENOTDIR to the caller, which matches the behaviour of Node.js and Posix.

cc @KernelDeimos

@KernelDeimos
Copy link
Contributor

KernelDeimos commented Jun 24, 2024

This was causing problems in isomorphic-git, as it would readdir foo.txt, receive the listing of [ {path: foo.txt', ... } ], and then try to read foo.txt/foo.txt and get confused that the file doesn't exist.

I was really hoping I could come to the conclusion that we don't need two endpoints that stat filesystem nodes, but it looks like this elegance breaks other software so I'm going to merge this fix.

@KernelDeimos KernelDeimos merged commit 46eb4ed into HeyPuter:main Jun 24, 2024
4 of 5 checks passed
@AtkinsSJ
Copy link
Contributor Author

I was really hoping I could come to the conclusion that we don't need two endpoints that stat filesystem nodes, but it looks like this elegance breaks other software so I'm going to merge this fix.

Yeah that would definitely be convenient! Maybe there's some other way of signalling that it's not a directory.

@AtkinsSJ AtkinsSJ deleted the readdir-enotdir branch June 25, 2024 16:49
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 this pull request may close these issues.

2 participants