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 DirEntryInner::is_same_file at a mountpoint #333

Merged
merged 1 commit into from
Oct 11, 2023

Conversation

asomers
Copy link
Contributor

@asomers asomers commented Sep 18, 2023

If a direntry comes from a mountpoint's parent, then its ino will be the ino of the underlying directory, not the mounted file system's root directory. So ignore the direntry's ino, and just look at the ino in its metadata.

Fixes #330

If a direntry comes from a mountpoint's parent, then its ino will be the
ino of the underlying directory, not the mounted file system's root
directory.  So ignore the direntry's ino, and just look at the ino in
its metadata.

Fixes bytecodealliance#330
@asomers
Copy link
Contributor Author

asomers commented Sep 18, 2023

The test failure is on OSX, and I don't have a Mac to debug it on. Could one of the maintainers please help?

@sunfishcode
Copy link
Member

I apologize for the slow response here, I lost track of this. The CI was restarted so it now passed.

I think if DirEntry::is_same_file were a public API, I might have different thoughts about this, because it might sometimes be desirable to compare with the ino within the underlying directory rather than any filesystem that might happen to be mounted on top of it. However, this isn't a public API, and it's currently only used for these file_path_by_searching and remove_open_dir_by_searching functions which don't need that behavior, so it seems good.

@sunfishcode sunfishcode merged commit 9c1e426 into bytecodealliance:main Oct 11, 2023
22 checks passed
@asomers asomers deleted the is_same_file_mountpoint branch October 11, 2023 19:58
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.

file_path_by_searching does not work across mountpoint boundaries
2 participants