Conversation
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
|
This will be really useful for what I've been working on, thanks! Do we need to back port this? Could it not just land in the next minor release? |
| return nil, errors.Wrapf(err, "failed to create temporary checkout dir") | ||
| } | ||
| } | ||
| _, err = gitWithinDir(ctx, gitDir, cd, sock, knownHosts, nil, "checkout", ref, "--", ".") |
There was a problem hiding this comment.
Wonder is a sparse-checkout wouldn't be more efficient in this case but would require at least Git 2.25.0 which is not widely used atm unfortunately. Maybe this could be fixed with #1048 when go-git/go-git#90 is implemented.
There was a problem hiding this comment.
This can be looked at later as an optimization but may be tricky because of the shared local repository buildkit uses for incremental updates(or this would maybe need to be turned off for subdirs then). Also, not quite sure how cleanly fallback can be done for servers that don't support it.
| } | ||
| } | ||
| if err := d.Close(); err != nil { | ||
| return nil, err |
There was a problem hiding this comment.
Wouldn't d.Close() be called again here?
There was a problem hiding this comment.
The close in defer is skipped in here
|
I think we can go on with this as is. Adding llb fallback for this is tricky because caps are not known until marshaling and completely different llb is needed for fallback. I did make a commit with fallback in the frontend but it is kind of pointless because to parse |
|
That sounds good to me @tonistiigi! |
AkihiroSuda
left a comment
There was a problem hiding this comment.
The path separator stuff probably does not work on Windows, so perhaps we should have "!windows" build tag?
|
Why would it not work on windows? |
fixes #1684
wip: trying to see if I can also add backward compatibility to support old buildkit versions in https://github.com/moby/buildkit/compare/tonistiigi:git-subdir-wip. Looks tricky but may be possible.
@crazy-max @chris-crone
Signed-off-by: Tonis Tiigi tonistiigi@gmail.com