support extraction of unixfs content stored in car files #263
Conversation
willscott
commented
Nov 4, 2021
•
willscott
commented
- support symlinks
- round-trip (create then extract) tests
|
Naively writing out files with |
|
Okay. There's better accounting to not get in trouble with symlinks or paths outside of the chosen directory now. |
|
|
||
| ls := cidlink.DefaultLinkSystem() | ||
| ls.TrustedStorage = true | ||
| ls.StorageReadOpener = func(_ ipld.LinkContext, l ipld.Link) (io.Reader, error) { |
There was a problem hiding this comment.
graphsync's storeutil.LinkSystemForBlockstore does this quite pleasantly -- sets trusted storage, adds a good storage read opener, even includes some optimizations for bytes.Buffer. We should really, really consider putting this in ipld-prime, or somewhere that's obviously accessable.
There was a problem hiding this comment.
There's a glue module for this coming in ipld/go-ipld-prime#279 (which I haven't merged yet, but don't actually know what I'm waiting for, either), so we could use that shortly.
cmd/car/car.go
Outdated
| { | ||
| Name: "extract", | ||
| Aliases: []string{"x"}, | ||
| Usage: "Extract the contents of a car", |
There was a problem hiding this comment.
Looking at the code, it looks like if you give anything other than a car where the roots are DagPB/Raw, it will error. Perhaps it is worth putting a comment like this ("Extracts the contents of a car when the car encodes UnixFS data")
There was a problem hiding this comment.
We should think about other conventions that we should try to interpret, like hamts, structured data, etc.
| } | ||
| pbnode := pbn.(dagpb.PBNode) | ||
|
|
||
| ufn, err := unixfsnode.Reify(ipld.LinkContext{}, pbnode, ls) |
There was a problem hiding this comment.
Not using a the Reify feature of LinkSystem for some reason?
There was a problem hiding this comment.
because in extractDir we look at the dagpb level data in understanding how and when to do the reification
There was a problem hiding this comment.
have not entirely wrapped my head around all the code here, but it's possible that you could use NodeReifier's LinkContext parameter to pass down relevant hints, either via the .LinkPath or .LinkNode or .ParentNode properties. I'd think one of those should provide enough info for the decision making. Unless the relevant info is super nonlocal.
I have not analyzed if that would make this code easier to reason about or reuse; just offering the possibility.
There was a problem hiding this comment.
that would be a new coordination with unixfsnode somehow that seems harder to reason about though. I think this is a reasonable setup for applying the reification explicitly when needed.
| return nil | ||
| } | ||
|
|
||
| pbn, err := ls.Load(ipld.LinkContext{}, cidlink.Link{Cid: root}, dagpb.Type.PBNode) |
There was a problem hiding this comment.
This code seems to assume that if you don't have a raw node, you have a root that is a UnixFS Directory, not a regular file (even though extractFile exists as a function). Is this the intent?
There was a problem hiding this comment.
I believe this is generally the case. if i export a single file it'll be implicitly in a directory in order to provide the file name of the file.
There was a problem hiding this comment.
fwiw, to compare to other unix stuff, tar commands do allow single file no-dir contents, iirc. Rare and a bit arcane, but possible.
There was a problem hiding this comment.
okay for that to go in a follow-up?
