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

Go filesystem implementations refactoring #27

Merged
merged 32 commits into from
Jul 11, 2023

Conversation

djdv
Copy link
Owner

@djdv djdv commented Nov 26, 2022

These are the various changes made to the Go fs.FS implementations we currently have.
As well as a new work in progress for IPFS' MFS API, which will be used to handle writable methods in KeyFS+IPNS later.
Currently being tested against the IPFS Files API, which itself will likely need its own implementation (later).

Edit: removed from this change set. We'll do writable stuff after the staging/refactor branch makes it to master.


PR meta:
Adds on top of #26 originally derived from #20
Initially targeting j/cgofuse-refactoring, which should target staging/refactor after that's merged.

@djdv djdv mentioned this pull request Nov 26, 2022
@djdv djdv force-pushed the j/cgofuse-refactoring branch 2 times, most recently from 115f41b to 5101a8a Compare December 2, 2022 21:16
@djdv djdv force-pushed the j/cgofuse-refactoring branch 2 times, most recently from 4a3a892 to 1dc571e Compare December 20, 2022 02:54
Base automatically changed from j/cgofuse-refactoring to staging/refactor December 21, 2022 02:42
Copy link
Owner Author

@djdv djdv left a comment

Choose a reason for hiding this comment

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

Preliminary partial review pass from last week.

internal/filesystem/ipfs/shared.go Show resolved Hide resolved
internal/filesystem/ipfs/ipfs.go Outdated Show resolved Hide resolved
internal/filesystem/ipfs/ipfs.go Show resolved Hide resolved
internal/filesystem/ipfs/shared.go Show resolved Hide resolved
internal/filesystem/ipfs/ipfs.go Show resolved Hide resolved
internal/filesystem/ipfs/ipfs.go Show resolved Hide resolved
internal/filesystem/ipfs/ufs.go Show resolved Hide resolved
@djdv djdv requested a review from aschmahmann May 19, 2023 13:05
djdv added 17 commits June 21, 2023 19:37
This only worked for canonical IPFS paths, not IPNS.
Standard implies we're supposed to return entries or an error, but never
both.
Previously, at the tail of a directory, we'd return ents + EOF.
Now we follow the same convention as standard `os` and `fstest`.
Standard implies we're supposed to return entries or an error, but never
both.
Previously, at the tail of a directory, we'd return ents + EOF.
Now we follow the same convention as standard `os` and `fstest`.
djdv and others added 10 commits June 21, 2023 19:37
- Add caching mechanism to IPFS, IPNS, and PinFS
- Change constructors for all, specifically how options are handled
- Change field parser logic for mount-point file
- Various minor bugfixes
PinFS: 104 pointer bytes -> 80
nodeInfo: 56 pointer bytes -> 32
Previously we were peeking the cache to avoid inflating the hit counter
in commonly seen sequences like `stat;stat;open`. However, it's not
likely to be detrimental / cause unwanted evictions if calls to `stat`
update the hit counter for a file. It may actually allow some paths to
fall out of cache if they're merely being queried/monitored frequently,
which is undesirable.
@djdv
Copy link
Owner Author

djdv commented Jul 11, 2023

This was mainly reviewed offline, and tested during the development of #28
Some issues were found and have been broken out to be resolved later, but the foundation here seems to work well enough to move forward with a merge.

@djdv djdv merged commit 4b264f5 into staging/refactor Jul 11, 2023
@djdv djdv deleted the j/fs-impl-refactoring branch July 11, 2023 14:50
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