Skip to content
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

git: support subdir component #2116

Merged
merged 1 commit into from
Jun 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions solver/pb/caps.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const (
CapSourceGitHTTPAuth apicaps.CapID = "source.git.httpauth"
CapSourceGitKnownSSHHosts apicaps.CapID = "source.git.knownsshhosts"
CapSourceGitMountSSHSock apicaps.CapID = "source.git.mountsshsock"
CapSourceGitSubdir apicaps.CapID = "source.git.subdir"

CapSourceHTTP apicaps.CapID = "source.http"
CapSourceHTTPChecksum apicaps.CapID = "source.http.checksum"
Expand Down Expand Up @@ -152,6 +153,12 @@ func init() {
Status: apicaps.CapStatusExperimental,
})

Caps.Init(apicaps.Cap{
ID: CapSourceGitSubdir,
Enabled: true,
Status: apicaps.CapStatusExperimental,
})

Caps.Init(apicaps.Cap{
ID: CapSourceHTTP,
Enabled: true,
Expand Down
47 changes: 45 additions & 2 deletions source/git/gitsource.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"os"
"os/exec"
"os/user"
"path"
"path/filepath"
"regexp"
"strconv"
Expand Down Expand Up @@ -170,6 +171,9 @@ func (gs *gitSourceHandler) shaToCacheKey(sha string) string {
if gs.src.KeepGitDir {
key += ".git"
}
if gs.src.Subdir != "" {
key += ":" + gs.src.Subdir
}
return key
}

Expand Down Expand Up @@ -480,7 +484,12 @@ func (gs *gitSourceHandler) Snapshot(ctx context.Context, g session.Group) (out
}
}()

if gs.src.KeepGitDir {
subdir := path.Clean(gs.src.Subdir)
if subdir == "/" {
subdir = "."
}

if gs.src.KeepGitDir && subdir == "." {
checkoutDirGit := filepath.Join(checkoutDir, ".git")
if err := os.MkdirAll(checkoutDir, 0711); err != nil {
return nil, err
Expand Down Expand Up @@ -513,10 +522,44 @@ func (gs *gitSourceHandler) Snapshot(ctx context.Context, g session.Group) (out
}
gitDir = checkoutDirGit
} else {
_, err = gitWithinDir(ctx, gitDir, checkoutDir, sock, knownHosts, nil, "checkout", ref, "--", ".")
cd := checkoutDir
if subdir != "." {
cd, err = ioutil.TempDir(cd, "checkout")
if err != nil {
return nil, errors.Wrapf(err, "failed to create temporary checkout dir")
}
}
_, err = gitWithinDir(ctx, gitDir, cd, sock, knownHosts, nil, "checkout", ref, "--", ".")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 != nil {
return nil, errors.Wrapf(err, "failed to checkout remote %s", redactCredentials(gs.src.Remote))
}
if subdir != "." {
d, err := os.Open(filepath.Join(cd, subdir))
if err != nil {
return nil, errors.Wrapf(err, "failed to open subdir %v", subdir)
}
defer func() {
if d != nil {
d.Close()
}
}()
names, err := d.Readdirnames(0)
if err != nil {
return nil, err
}
for _, n := range names {
if err := os.Rename(filepath.Join(cd, subdir, n), filepath.Join(checkoutDir, n)); err != nil {
return nil, err
}
}
if err := d.Close(); err != nil {
return nil, err
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't d.Close() be called again here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The close in defer is skipped in here

}
d = nil // reset defer
if err := os.RemoveAll(cd); err != nil {
return nil, err
}
}
}

_, err = gitWithinDir(ctx, gitDir, checkoutDir, sock, knownHosts, gs.auth, "submodule", "update", "--init", "--recursive", "--depth=1")
Expand Down
77 changes: 77 additions & 0 deletions source/git/gitsource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,82 @@ func TestCredentialRedaction(t *testing.T) {
require.False(t, strings.Contains(err.Error(), "keepthissecret"))
}

func TestSubdir(t *testing.T) {
testSubdir(t, false)
}
func TestSubdirKeepGitDir(t *testing.T) {
testSubdir(t, true)
}

func testSubdir(t *testing.T, keepGitDir bool) {
if runtime.GOOS == "windows" {
t.Skip("Depends on unimplemented containerd bind-mount support on Windows")
}

t.Parallel()
ctx := context.TODO()

tmpdir, err := ioutil.TempDir("", "buildkit-state")
require.NoError(t, err)
defer os.RemoveAll(tmpdir)

gs := setupGitSource(t, tmpdir)

repodir, err := ioutil.TempDir("", "buildkit-gitsource")
require.NoError(t, err)
defer os.RemoveAll(repodir)

err = runShell(repodir,
"git init",
"git config --local user.email test",
"git config --local user.name test",
"echo foo > abc",
"mkdir sub",
"echo abc > sub/bar",
"git add abc sub",
"git commit -m initial",
)
require.NoError(t, err)

id := &source.GitIdentifier{Remote: repodir, KeepGitDir: keepGitDir, Subdir: "sub"}

g, err := gs.Resolve(ctx, id, nil, nil)
require.NoError(t, err)

key1, _, done, err := g.CacheKey(ctx, nil, 0)
require.NoError(t, err)
require.True(t, done)

expLen := 44
if keepGitDir {
expLen += 4
}

require.Equal(t, expLen, len(key1))

ref1, err := g.Snapshot(ctx, nil)
require.NoError(t, err)
defer ref1.Release(context.TODO())

mount, err := ref1.Mount(ctx, false, nil)
require.NoError(t, err)

lm := snapshot.LocalMounter(mount)
dir, err := lm.Mount()
require.NoError(t, err)
defer lm.Unmount()

fis, err := ioutil.ReadDir(dir)
require.NoError(t, err)

require.Equal(t, 1, len(fis))

dt, err := ioutil.ReadFile(filepath.Join(dir, "bar"))
require.NoError(t, err)

require.Equal(t, "abc\n", string(dt))
}

func setupGitSource(t *testing.T, tmpdir string) source.Source {
snapshotter, err := native.NewSnapshotter(filepath.Join(tmpdir, "snapshots"))
assert.NoError(t, err)
Expand Down Expand Up @@ -437,6 +513,7 @@ func runShell(dir string, cmds ...string) error {
cmd = exec.Command("sh", "-c", args)
}
cmd.Dir = dir
cmd.Stderr = os.Stderr
if err := cmd.Run(); err != nil {
return errors.Wrapf(err, "error running %v", args)
}
Expand Down
6 changes: 3 additions & 3 deletions source/gitidentifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ package source

import (
"net/url"
"path"
"strings"

"github.com/moby/buildkit/util/sshutil"
"github.com/pkg/errors"
)

type GitIdentifier struct {
Expand Down Expand Up @@ -46,8 +46,8 @@ func NewGitIdentifier(remoteURL string) (*GitIdentifier, error) {
u.Fragment = ""
repo.Remote = u.String()
}
if repo.Subdir != "" {
return nil, errors.Errorf("subdir not supported yet")
if sd := path.Clean(repo.Subdir); sd == "/" || sd == "." {
repo.Subdir = ""
}
return &repo, nil
}
Expand Down