-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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-path integrated #8028
go-path integrated #8028
Changes from 6 commits
52094b1
e11af3a
f5bfb04
3f7626b
a56fbf3
e3211c0
5077fd3
63b6593
bae6c8a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,12 +6,12 @@ import ( | |
gopath "path" | ||
|
||
"github.com/ipfs/go-ipfs/namesys/resolve" | ||
"github.com/ipfs/go-unixfsnode" | ||
|
||
"github.com/ipfs/go-cid" | ||
ipld "github.com/ipfs/go-ipld-format" | ||
ipfspath "github.com/ipfs/go-path" | ||
"github.com/ipfs/go-path/resolver" | ||
uio "github.com/ipfs/go-unixfs/io" | ||
coreiface "github.com/ipfs/interface-go-ipfs-core" | ||
path "github.com/ipfs/interface-go-ipfs-core/path" | ||
) | ||
|
@@ -49,22 +49,12 @@ func (api *CoreAPI) ResolvePath(ctx context.Context, p path.Path) (path.Resolved | |
return nil, err | ||
} | ||
|
||
var resolveOnce resolver.ResolveOnce | ||
|
||
switch ipath.Segments()[0] { | ||
case "ipfs": | ||
resolveOnce = uio.ResolveUnixfsOnce | ||
case "ipld": | ||
resolveOnce = resolver.ResolveSingle | ||
default: | ||
if ipath.Segments()[0] != "ipfs" && ipath.Segments()[0] != "ipld" { | ||
return nil, fmt.Errorf("unsupported path namespace: %s", p.Namespace()) | ||
} | ||
|
||
r := &resolver.Resolver{ | ||
DAG: api.dag, | ||
ResolveOnce: resolveOnce, | ||
} | ||
|
||
r := resolver.NewBasicResolver(api.blocks) | ||
r.FetchConfig.NodeReifier = unixfsnode.Reify | ||
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. I guess this is no big deal, but this seems to exactly match the Resolve function declared in core/node/core.go. Was there a dependency issue or are we just copying the code bc it's only two lines? |
||
node, rest, err := r.ResolveToLastNode(ctx, ipath) | ||
if err != nil { | ||
return nil, err | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,14 +11,16 @@ import ( | |
"github.com/ipfs/go-cid" | ||
"github.com/ipfs/go-datastore" | ||
"github.com/ipfs/go-filestore" | ||
"github.com/ipfs/go-ipfs-blockstore" | ||
"github.com/ipfs/go-ipfs-exchange-interface" | ||
"github.com/ipfs/go-ipfs-pinner" | ||
blockstore "github.com/ipfs/go-ipfs-blockstore" | ||
exchange "github.com/ipfs/go-ipfs-exchange-interface" | ||
pin "github.com/ipfs/go-ipfs-pinner" | ||
"github.com/ipfs/go-ipfs-pinner/dspinner" | ||
"github.com/ipfs/go-ipld-format" | ||
format "github.com/ipfs/go-ipld-format" | ||
"github.com/ipfs/go-merkledag" | ||
"github.com/ipfs/go-mfs" | ||
"github.com/ipfs/go-path/resolver" | ||
"github.com/ipfs/go-unixfs" | ||
"github.com/ipfs/go-unixfsnode" | ||
"github.com/libp2p/go-libp2p-core/host" | ||
"github.com/libp2p/go-libp2p-core/routing" | ||
"go.uber.org/fx" | ||
|
@@ -82,6 +84,13 @@ func (s *syncDagService) Session(ctx context.Context) format.NodeGetter { | |
return merkledag.NewSession(ctx, s.DAGService) | ||
} | ||
|
||
// Resolver returns a resolver that's configured to look up unixfs paths | ||
func Resolver(bs blockservice.BlockService) *resolver.Resolver { | ||
rs := resolver.NewBasicResolver(bs) | ||
rs.FetchConfig.NodeReifier = unixfsnode.Reify | ||
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. This pattern of initialization seems a little weird. I'll make some comments as I get to that part of the stack when reviewing 😄. 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. I agree. I think we should have a FetchConfig top level dependency in the DI stack, and pass it to NewBasicResolver rather than the block service. I think I've said this elsewhere as well. |
||
return rs | ||
} | ||
|
||
// Dag creates new DAGService | ||
func Dag(bs blockservice.BlockService) format.DAGService { | ||
return merkledag.NewDAGService(bs) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,20 +6,21 @@ package readonly | |
import ( | ||
"context" | ||
"fmt" | ||
"github.com/ipfs/go-cid" | ||
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. let's group the dependencies here and have separate groupings for builtins, fuse, ipfs+ipld |
||
"io" | ||
"os" | ||
"syscall" | ||
|
||
fuse "bazil.org/fuse" | ||
fs "bazil.org/fuse/fs" | ||
core "github.com/ipfs/go-ipfs/core" | ||
ipld "github.com/ipfs/go-ipld-format" | ||
logging "github.com/ipfs/go-log" | ||
mdag "github.com/ipfs/go-merkledag" | ||
path "github.com/ipfs/go-path" | ||
ft "github.com/ipfs/go-unixfs" | ||
uio "github.com/ipfs/go-unixfs/io" | ||
|
||
fuse "bazil.org/fuse" | ||
fs "bazil.org/fuse/fs" | ||
ipld "github.com/ipfs/go-ipld-format" | ||
logging "github.com/ipfs/go-log" | ||
cidlink "github.com/ipld/go-ipld-prime/linking/cid" | ||
) | ||
|
||
var log = logging.Logger("fuse/ipfs") | ||
|
@@ -65,20 +66,41 @@ func (s *Root) Lookup(ctx context.Context, name string) (fs.Node, error) { | |
return nil, fuse.ENOENT | ||
} | ||
|
||
nd, err := s.Ipfs.Resolver.ResolvePath(ctx, p) | ||
nd, ndLnk, err := s.Ipfs.Resolver.ResolvePath(ctx, p) | ||
if err != nil { | ||
// todo: make this error more versatile. | ||
return nil, fuse.ENOENT | ||
} | ||
|
||
switch nd := nd.(type) { | ||
case *mdag.ProtoNode, *mdag.RawNode: | ||
return &Node{Ipfs: s.Ipfs, Nd: nd}, nil | ||
cidLnk, ok := ndLnk.(cidlink.Link) | ||
if !ok { | ||
log.Debugf("non-cidlink returned from ResolvePath: %v", ndLnk) | ||
return nil, fuse.ENOENT | ||
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.
|
||
} | ||
|
||
// convert ipld-prime node to universal node | ||
blk, err := s.Ipfs.Blockstore.Get(cidLnk.Cid) | ||
if err != nil { | ||
log.Debugf("fuse failed to retrieve block: %v: %s", cidLnk, err) | ||
return nil, fuse.ENOENT | ||
} | ||
|
||
var fnd ipld.Node | ||
switch cidLnk.Cid.Prefix().Codec { | ||
case cid.DagProtobuf: | ||
fnd, err = mdag.ProtoNodeConverter(blk, nd) | ||
case cid.Raw: | ||
fnd, err = mdag.RawNodeConverter(blk, nd) | ||
Comment on lines
+81
to
+93
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. This seems confusing/bad in that it'll result in multiple blockstore fetches for the same data. Perhaps if the underlying datastore has caching this doesn't end up being so bad since we likely just fetched the data, but if this is a recurring pattern in the associated set of PRs here then we might need to make some changes. I added some comments in the go-merkledag PR ipfs/go-merkledag#67 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. @aschmahmann I don't actually see those :( Can you point me to them? 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. or maybe you need to submit review or something? 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. In terms of the more general question -- it's a bit tricky. In IPLD Prime land, we don't always have a block to go with a node, cause nodes don't always match block boundaries. In a future pure IPLD Prime world, this isn't a problem -- we wouldn't load the block here, cause we wouldn't try to produce a legacy merkledag node, cause UnixFS would already work with prime nodes natively. However to convert back to legacy node, we have to do this. Hopefully, it's just a temporary step till we make more progress on go-ipld-prime conversion 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. Some of my comments seem to have disappeared, but this is the one I was thinking of ipfs/go-merkledag#67 (comment) |
||
default: | ||
log.Error("fuse node was not a protobuf node") | ||
return nil, fuse.ENOTSUP | ||
return nil, fuse.ENOENT | ||
} | ||
if err != nil { | ||
log.Error("could not convert protobuf node") | ||
return nil, fuse.ENOENT | ||
Comment on lines
+96
to
+100
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. Why did we switch these from 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. that's a great question -- I didn't actually write this bit of code here (Alex C did but he's not working on PL stuff any more). Seems odd. I can change it back. |
||
} | ||
|
||
return &Node{Ipfs: s.Ipfs, Nd: fnd}, nil | ||
} | ||
|
||
// ReadDirAll reads a particular directory. Disallowed for root. | ||
|
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.
minor: It would be nice if there were some
const
values for the segment indexes. If those do not already exist, that probably better in a separate future PR.