MFS Root vs MFS File/Directory

[schomatis]

I would like to understand (and possibly decouple) the relationship between those two.

I'm looking at the NewRoot function,

https://github.com/ipfs/go-ipfs/blob/7853e53860805e08a212d78c4baa5d59bff99ba8/mfs/system.go#L69-L104

The Root is created from a ProtoNode (instead of an ipld.Node, why?) that is usually an empty directory and stores a node reference to it, which doesn't seem to be used anywhere in the code.

They all share the reference to (I'm assuming the same) DAGService, is that necessary?

The Type field of the Root (which one could mistakenly associate with the Type() function of the FSNode interface to which Root doesn't belong) also doesn't seem to be used anywhere in the code. This also make it harder to conceptually distinguish between the Root and the File/Dir types (coupled with other minor details like the Flush() function that is part of the FSNode interface that Root also implements, besides File and Dir, that one could associate with those types).

The switch in NewRoot seems to allow the possibility that the root value (val) can either be a directory or a file but other functions like Mkdir assume the root is a Directory,

https://github.com/ipfs/go-ipfs/blob/7853e53860805e08a212d78c4baa5d59bff99ba8/mfs/ops.go#L132

Another thing that may generate confusion is that the Context argument received by NewRoot is named parent and then passed to NewFile and NewDirectory functions which use the name parent not for the context passed (which uses the more standardized name ctx) but rather for the ipld.Node (which seems to make more sense). Could the parent context.Context be renamed ctx context.Context? What is the rationale behind that name?

[schomatis]

/cc @Stebalien

[schomatis]

/cc @Stebalien :)

[Stebalien]

The Root is created from a ProtoNode (instead of an ipld.Node, why?)

Probably for historical reasons. This is now technically incorrect as we now support Raw IPLS objects as well.

that is usually an empty directory and stores a node reference to it, which doesn't seem to be used anywhere in the code.

We may have used that field at one point but yes, it appears unused. Feel free to remove it for now.

They all share the reference to (I'm assuming the same) DAGService, is that necessary?

Yes? That is, they all need access to some DAG service.

The Type field of the Root (which one could mistakenly associate with the Type() function of the FSNode interface to which Root doesn't belong) also doesn't seem to be used anywhere in the code.

I have no idea what that was for. If it's unused, remove it.

The switch in NewRoot seems to allow the possibility that the root value (val) can either be a directory or a file but other functions like Mkdir assume the root is a Directory,

I believe the root is supposed to be able to be either a file or a directory but I'm not sure why. I also agree that it probably doesn't work as a file.

If I had to guess, I'd say that we probably wanted MFS to be the way to create/modify files in IPFS. We may have planned on deprecating some of our other file builder utilities. However, that's purely speculation. @whyrusleeping?

Another thing that may generate confusion is that the Context argument...

So, in general, this is an abuse of contexts. We shouldn't be storing them like that...

Putting that aside...

Could the parent context.Context be renamed ctx context.Context? What is the rationale behind that name?

Yes, and 🤷‍♂️.

[whyrusleeping]

The switch in NewRoot seems to allow the possibility that the root value (val) can either be a directory or a file but other functions like Mkdir assume the root is a Directory,

I think we can probably move to assuming its always a directory. That might clean up a few things.

So, in general, this is an abuse of contexts. We shouldn't be storing them like that...

Yeah... This was written before I knew better. The only reason its still useful is that Read and Write calls don't pass a context, but we need one inside those calls. I figured it was better to store the master context than to just use a background or some fixed timeout. That assumption could be wrong.

other than that, agree with @Stebalien
Thanks for the investigation @schomatis :)

[schomatis]

With #5170 making explicit that the Root holds a Directory I'm now leaning even more to the idea of removing the structure altogether in favor of having the coreunix.Adder hold a pointer to a Directory (instead of a Root) that it considers the root of the MFS tree, making more clear the fact that this is just relative concept, that (root) directory could be the child of another directory which would be beyond the MFS tree the Adder cares about. I need to figure out how the Republisher (the only other surviving member of Root) fits into this scheme (so I'll continue the work in #5092).

[schomatis]

It is needed to better understand the relationship between a Directory and a Root structure in the context of the use of the Republisher, an example code flow is studied.

In the ipfs files rm command the Flush() function from the directory being modified is called which in turns calls the parent's closeChild() with the directory's node,

https://github.com/ipfs/go-ipfs/blob/7853e53860805e08a212d78c4baa5d59bff99ba8/mfs/dir.go#L342-L349

Assuming (for the sake of the example) that its parent is also a Directory, its closeChild() function will update itself and then propagate its own DAG node to its parent through closeChild(),

https://github.com/ipfs/go-ipfs/blob/7853e53860805e08a212d78c4baa5d59bff99ba8/mfs/dir.go#L75-L87

The closeChildUpdate function uses updateChild to replace the DAG node's link with the (new) child's name and ipld.Node,

https://github.com/ipfs/go-ipfs/blob/7853e53860805e08a212d78c4baa5d59bff99ba8/mfs/dir.go#L89-L103

(flushCurrentNode() adds the new node to the DAGService.)

Now this continues repeatedly through the (parent) directories of this tree until the Root (disguised in the childCloser interface) is reached. At this point the Republisher is used to "publish" (the meaning of which should be clarified in #5092) the new CID of the root directory of the MFS tree,

https://github.com/ipfs/go-ipfs/blob/7853e53860805e08a212d78c4baa5d59bff99ba8/mfs/system.go#L160-L172

Looking closer at the undocumented childCloser interface,

https://github.com/ipfs/go-ipfs/blob/7853e53860805e08a212d78c4baa5d59bff99ba8/mfs/system.go#L32-L35

it now becomes more clear that it's a way for a child to transmit information to its parent about its own modification, since normally at the DAG and UnixFS layers a child has no link to its parent to send it any signal (this last part is the key aspect that should be documented to help understand the necessity for this connection at the MFS layer).

[whyrusleeping]

since normally at the DAG and UnixFS layers a child has no link to its parent to send it any signal

Exactly. This is how we 'bubble up' changes. 'childCloser' definitely needs a better name...

[schomatis]

This will be continued in ipfs/go-mfs#3.