Core API: Directory and File types

[shykes]

Implement new Directory and File types to make the core API more complete and graphql-ey.

Design proposal and discussion: dagger/cloak#163

[sipsma]

Yeah I like the with pattern a lot. Most of my thoughts are on all the other fields we'd want to add (or that could be added by loaded extensions), but this looks like a great base and general pattern.

I do wonder if we'll end up wanting to make this an interface too. I'm imagining e.g. there could be an Alpine object that implements the Directory interface but then also has additional fields like withPackages(pkgs: [String]).

Not necessary to figure that out immediately though, ship it as is.

[shykes]

Update: added diff / merge support, top-level query extension

[sipsma]

nit: File should have an id. withFile is good to have, but doesn't work if e.g. your file is 1GB in size. Need another one that accepts an id instead of contents.

[shykes]

nit: File should have an id. withFile is good to have, but doesn't work if e.g. your file is 1GB in size. Need another one that accepts an id instead of contents.

I don't understand how this would work? Doesn't the client need to send the full contents of the file in the query one way or the other?

[sipsma]

Update: added diff / merge support, top-level query extension

I was thinking about that too but actually we could get away with just having withDirectory, no need for both that and plus I don't think. withDirectory would just be optimized such that if your path is / then MergeOp is used instead of copy.

[shykes]

I was thinking about that too but actually we could get away with just having withDirectory, no need for both that and plus I don't think. withDirectory would just be optimized such that if your path is / then MergeOp is used instead of copy.

Makes sense, I removed plus and left minus.

[sipsma]

I don't understand how this would work? Doesn't the client need to send the full contents of the file in the query one way or the other?

Not necessarily, you could do something like:
{ host { workdir { file(path:"/foo") { id } } } }

And then pass that output as $id to
{ host { workdir { withFile(path: "/bar", id: $id) {id}}}}

That case is a bit contrived since you're just moving files within one filesystem, but makes more sense if you are moving a file between different filesystems. If you tried to do the same but just using contents and the file was very large, you'd be transferring enormous amounts of data (to the point where it actually matters and isn't just an optimization).

I'm also presuming this same type would be used with exec, in which case you would want to sometimes mount a single file at a location for the exec (wouldn't even need to copy anything). That would be the same situation where providing contents directly could be nice sometimes, but not if the file is enormous.

I think in general, if Directory has an ID, then File should too. LLB can refer to individual files just like it can refer to directories.

[shykes]

Ok I understand now.

I think in general, if Directory has an ID, then File should too. LLB can refer to individual files just like it can refer to directories.

We would still be cheating a little bit, though, since llb WriteFile itself requires passing the contents, I don't think you can pass the llb reference to the file instead, can you?

[shykes]

@sipsma would this Directory type be a good place to chain mounts as you mention in #3081? Should I give a try at adding withMount, mounts? Since llb state doesn't actually persist mounts, we would have to carry that state ourselves alongside the llb. Did I understand what you're trying to achieve correctly?

[sipsma]

We would still be cheating a little bit, though, since llb WriteFile itself requires passing the contents, I don't think you can pass the llb reference to the file instead, can you?

You can use copy op, i.e. baseState.File(llb.Copy(sourceState, "path/to/file", "dest/path")). In this case sourceState+"path/to/file" operate as the reference to the file. The id of a File would essentially encode that information. Can also optimize w/ mergeop when paths align correctly, etc.

@sipsma would this Directory type be a good place to chain mounts as you mention in #3081? Should I give a try at adding withMount, mounts? Since llb state doesn't actually persist mounts, we would have to carry that state ourselves alongside the llb. Did I understand what you're trying to achieve correctly?

Yep was thinking about that but was going to save it for later to keep this minimal. But yeah what you said is exactly right. Our internal object representing this (i.e. in the cloak server) would keep state holding what is mounted and not mounted.

I think we would want withMount and withoutMount. These would be very similar to withDirectory and withoutDirectory, except that if you do withMount on an existing path, the mount would block out the original directory and withoutMount would result in that original directory being returned. And they wouldn't need to copy data in the underlying implementation.

One thing though, due to what you said (Buildkit only knows about mounts during a single ExecOp), we'd have to put in some extra effort to make a lot of the other fields work. For instance, if you try to read file contents from a path that has been mounted over, then we'll need to handle the logic of finding the right mount to read from ourselves due to the fact that ReadFile doesn't know about multiple mounts.

That is highly possible, but will probably be tricky when dealing with symlinks and other annoying corner cases. Pretty much implementing a VFS at that point (not a bad thing, just a lot). So maybe we should aspire to have withMount someday but not implement it immediately.

[shykes]

One thing though, due to what you said (Buildkit only knows about mounts during a single ExecOp), we'd have to put in some extra effort to make a lot of the other fields work. For instance, if you try to read file contents from a path that has been mounted over, then we'll need to handle the logic of finding the right mount to read from ourselves due to the fact that ReadFile doesn't know about multiple mounts.

That is highly possible, but will probably be tricky when dealing with symlinks and other annoying corner cases. Pretty much implementing a VFS at that point (not a bad thing, just a lot). So maybe we should aspire to have withMount someday but not implement it immediately.

We could start with a simpler implementation: withExecMount and withoutExecMount that is only applied at exec, then later introduce withMount and withoutMount with the more advanced behavior you describe.

[shykes]

Update: I erased specific design proposal from this issue, to avoid drift from dagger/cloak#163

This issue tracks implementation of one piece of that design (or whatever design ends up getting consensus).