fs: clean up filesystem import state #44
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The CI failures look like the actions runners got the 2024-enabled 1.85 rust release and clippy isn't following our |
This also requires Rust 1.83.0 (for the lifetime elisions), so bump our dependency. Signed-off-by: Allison Karlitskaya <[email protected]>
Clean up two aspects of importing filesystems: First: instead of trying to avoid crossing mountpoints (by checking `.st_dev`) in order to use `.st_ino` values as a "unique key" for tracking hardlinks, just take the `(.st_dev, .st_ino)` pair. This resolves some issues with overlayfs where adjacent files can end up with different `.st_dev`. Second: we were abusing the first setting of `st_dev` as a flag field to determine if we were `stat()`-ing the root inode or not, which was always a bit evil. We did this to avoid collecting the mtime of the root directory for determining the newest file in the filesystem. Let's change our approach here: we can collect the mtime when adding items to their parent directories (which is never done for the root directory). Use a impl on `Inode` to make it easier to collect that information. These changes together turn .stat() into a pure operation (with respect to the state of the reader), so remove the `&mut self` reference. Fixes #41 Signed-off-by: Allison Karlitskaya <[email protected]>
allisonkarlitskaya
force-pushed
the
fs-dev-ino
branch
from
73e9e0d to
314fbec
Compare
|
Okay, so it's not an edition thing, it's just a version thing: this got added in 1.83.0. I didn't know that changes like this could happen between releases... The new version is in Fedora now, in any case, so let's just depend on it. |
|
Oh and of course it's not that the actions runners got the new version — it's that our fedora container did. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Clean up two aspects of importing filesystems:
First: instead of trying to avoid crossing mountpoints (by checking
.st_dev) in order to use.st_inovalues as a "unique key" for tracking hardlinks, just take the(.st_dev, .st_ino)pair. This resolves some issues with overlayfs where adjacent files can end up with different.st_dev.Second: we were abusing the first setting of
st_devas a flag field to determine if we werestat()-ing the root inode or not, which was always a bit evil. We did this to avoid collecting the mtime of the root directory for determining the newest file in the filesystem. Let's change our approach here: we can collect the mtime when adding items to their parent directories (which is never done for the root directory). Use a impl onInodeto make it easier to collect that information.These changes together turn .stat() into a pure operation (with respect to the state of the reader), so remove the
&mut selfreference.Fixes #41