This repository has been archived by the owner on Jun 19, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 55
Closed
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
0c19a87
MFS interface draft
magik6k d8751b5
mfs: add contexts
magik6k 39b8d13
mfs: attach to the core interface
magik6k c6ece89
mfs: drop ReaderAt from File, add WriterAt
magik6k 177e67f
mfs: expose as Files on CoreAPI
magik6k 26f5aa1
mfs: implement FilePath func
magik6k 9b1d915
mfs: Flush call
magik6k cbc70f4
mfs: Setup tests
magik6k ea1ef0f
mfs: Copy Call
magik6k 48d6eda
mfs: remove Name from File
magik6k 91883fc
[WIP] drop MfsPath
magik6k File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,84 @@ | ||
package iface | ||
|
||
import ( | ||
"context" | ||
"io" | ||
"os" | ||
"strings" | ||
) | ||
|
||
// MfsPath | ||
// | ||
// Note that Path and ResolvedPath interfaces satisfy this interface | ||
// | ||
// This interface is inspired by https://godoc.org/github.com/src-d/go-billy, | ||
// but doesn't implement it | ||
type MfsPath interface { | ||
// String returns the path as a string. | ||
String() string | ||
|
||
// Mutable returns false if the data pointed to by this path in guaranteed | ||
// to not change. | ||
// | ||
// Note that resolved mutable path can be immutable. | ||
Mutable() bool | ||
} | ||
|
||
type filesPath struct { | ||
p string | ||
} | ||
|
||
func (p *filesPath) String() string { | ||
return p.p | ||
} | ||
|
||
func (p *filesPath) Mutable() bool { | ||
return !(strings.HasPrefix(p.p, "/ipfs") || strings.HasPrefix(p.p, "/ipld")) | ||
} | ||
|
||
func FilePath(p string) MfsPath { | ||
// TODO: more validation | ||
return &filesPath{ | ||
p: p, | ||
} | ||
} | ||
|
||
type MfsAPI interface { | ||
magik6k marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Create(ctx context.Context, path MfsPath) (File, error) | ||
Open(ctx context.Context, path MfsPath) (File, error) | ||
OpenFile(ctx context.Context, path MfsPath, flag int, perm os.FileMode) (File, error) | ||
|
||
Stat(ctx context.Context, path MfsPath) (os.FileInfo, error) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TODO: change this to match There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe but I'd consider not materializing the CID here for performance reasons. IMO, (although... there are some atomicity issues there...) |
||
|
||
Rename(ctx context.Context, oldpath, newpath MfsPath) error | ||
Remove(ctx context.Context, path MfsPath) error | ||
|
||
// ReadDir reads the directory named by dirname and returns a list of | ||
// directory entries sorted by filename. | ||
ReadDir(ctx context.Context, path MfsPath) ([]os.FileInfo, error) | ||
// MkdirAll creates a directory named path, along with any necessary | ||
// parents, and returns nil, or else returns an error. The permission bits | ||
// perm are used for all directories that MkdirAll creates. If path is | ||
// already a directory, MkdirAll does nothing and returns nil. | ||
MkdirAll(ctx context.Context, path MfsPath, perm os.FileMode) error | ||
|
||
Flush(ctx context.Context, path MfsPath) error | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We might want to make this return the CID. |
||
// TODO: ChCid | ||
// TODO: Symlink stuff (is it implemented in mfs?) | ||
} | ||
|
||
type File interface { | ||
io.Writer | ||
io.WriterAt | ||
io.Reader | ||
io.Seeker | ||
io.Closer | ||
|
||
Name() MfsPath | ||
magik6k marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// Truncate the file. | ||
Truncate(size int64) error | ||
} | ||
|
||
var _ MfsPath = (Path)(nil) | ||
var _ MfsPath = (ResolvedPath)(nil) |
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
package tests | ||
|
||
import ( | ||
"context" | ||
"testing" | ||
|
||
"github.com/ipfs/interface-go-ipfs-core" | ||
) | ||
|
||
func (tp *provider) TestMfs(t *testing.T) { | ||
t.Run("TestFilesDirs", tp.TestFilesDirs) | ||
} | ||
|
||
func (tp *provider) TestFilesDirs(t *testing.T) { | ||
ctx, cancel := context.WithCancel(context.Background()) | ||
defer cancel() | ||
api, err := tp.makeAPI(ctx) | ||
if err != nil { | ||
t.Fatal(err) | ||
return | ||
} | ||
|
||
l, err := api.Files().ReadDir(ctx, iface.FilePath("/test")) | ||
if len(l) != 0 { | ||
return | ||
} | ||
} |
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't have to do this now, but I'd like to prefix all MFS paths with
/local
. If we do that, we won't have to have this special type (and we'll get rid of a bunch of namespace conflict issues.(the upgrade path will be interesting but we may have to save this for an API refactor)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/local
outside Files API?ipfs files
commands handle those?/local
?/ipfs
equal/local/ipfs
everywhere?I slowly starting to think that using fancy types for paths isn't the best way to handle them (#16, #15), but using raw strings introduces another set of problems (CID serialization to name one). There is some trade-off to be made here..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, in the mount implementation I have the Files API mounted under
/files
, I almost feel like/local
is a bit of a misnomer given that files placed there are still published/accessible to the network.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(for a bit of context, I think
/local
initially appeared in ipfs/kubo#4666 (comment))There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"files" is fine, really. or maybe just "file" to match the
file:///
schema? My goal is just to disambiguate paths.See ipfs/specs#98 and ipfs/specs#98 (comment). The hope was to eventually find a way to unify these APIs.
For now, let's just push forward with what we have. I think this is a bikeshed for another day.