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.

Sign up for GitHub

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: fix caching git commit through multiple refs #5444

Merged
merged 1 commit into from
Oct 28, 2024
Merged

git: fix caching git commit through multiple refs #5444

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions source/git/source.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,13 @@ type gitSourceHandler struct {
authArgs []string
}

func (gs *gitSourceHandler) shaToCacheKey(sha string) string {
func (gs *gitSourceHandler) shaToCacheKey(sha, ref string) string {
key := sha
if gs.src.KeepGitDir {
key += ".git"
if ref != "" {
key += "#" + ref
}
}
if gs.src.Subdir != "" {
key += ":" + gs.src.Subdir
Expand Down Expand Up @@ -341,7 +344,7 @@ func (gs *gitSourceHandler) CacheKey(ctx context.Context, g session.Group, index
defer gs.locker.Unlock(remote)

if ref := gs.src.Ref; ref != "" && gitutil.IsCommitSHA(ref) {
cacheKey := gs.shaToCacheKey(ref)
cacheKey := gs.shaToCacheKey(ref, "")
gs.cacheKey = cacheKey
return cacheKey, ref, nil, true, nil
}
Expand Down Expand Up @@ -377,6 +380,7 @@ func (gs *gitSourceHandler) CacheKey(ctx context.Context, g session.Group, index
annotatedTagRef = tagRef + "^{}"
)
var sha, headSha, tagSha string
var usedRef string
for _, line := range lines {
lineSha, lineRef, _ := strings.Cut(line, "\t")
switch lineRef {
Expand All @@ -386,15 +390,18 @@ func (gs *gitSourceHandler) CacheKey(ctx context.Context, g session.Group, index
tagSha = lineSha
case partialRef:
sha = lineSha
usedRef = lineRef
}
}

// git-checkout prefers branches in case of ambiguity
if sha == "" {
sha = headSha
usedRef = headRef
}
if sha == "" {
sha = tagSha
usedRef = tagRef
}
if sha == "" {
return "", "", nil, false, errors.Errorf("repository does not contain ref %s, output: %q", ref, string(buf))
Expand All @@ -403,7 +410,7 @@ func (gs *gitSourceHandler) CacheKey(ctx context.Context, g session.Group, index
return "", "", nil, false, errors.Errorf("invalid commit sha %q", sha)
}

cacheKey := gs.shaToCacheKey(sha)
cacheKey := gs.shaToCacheKey(sha, usedRef)
gs.cacheKey = cacheKey
return cacheKey, sha, nil, true, nil
}
Expand Down
141 changes: 127 additions & 14 deletions source/git/source_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package git
import (
"bytes"
"context"
"fmt"
"io"
"net/http"
"net/http/cgi"
Expand Down Expand Up @@ -69,9 +70,10 @@ func testRepeatedFetch(t *testing.T, keepGitDir bool) {
expLen := 40
if keepGitDir {
expLen += 4
require.GreaterOrEqual(t, len(key1), expLen)
} else {
require.Equal(t, expLen, len(key1))
}

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

ref1, err := g.Snapshot(ctx, nil)
Expand Down Expand Up @@ -189,9 +191,10 @@ func testFetchBySHA(t *testing.T, keepGitDir bool) {
expLen := 40
if keepGitDir {
expLen += 4
require.GreaterOrEqual(t, len(key1), expLen)
} else {
require.Equal(t, expLen, len(key1))
}

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

ref1, err := g.Snapshot(ctx, nil)
Expand Down Expand Up @@ -276,9 +279,10 @@ func testFetchUnreferencedRefSha(t *testing.T, ref string, keepGitDir bool) {
expLen := 40
if keepGitDir {
expLen += 4
require.GreaterOrEqual(t, len(key1), expLen)
} else {
require.Equal(t, expLen, len(key1))
}

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

ref1, err := g.Snapshot(ctx, nil)
Expand Down Expand Up @@ -372,9 +376,10 @@ func testFetchByTag(t *testing.T, tag, expectedCommitSubject string, isAnnotated
expLen := 40
if keepGitDir {
expLen += 4
require.GreaterOrEqual(t, len(key1), expLen)
} else {
require.Equal(t, expLen, len(key1))
}

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

ref1, err := g.Snapshot(ctx, nil)
Expand Down Expand Up @@ -447,6 +452,105 @@ func testFetchByTag(t *testing.T, tag, expectedCommitSubject string, isAnnotated
}
}

func TestMultipleTagAccessKeepGitDir(t *testing.T) {
testMultipleTagAccess(t, true)
}

func TestMultipleTagAccess(t *testing.T) {
testMultipleTagAccess(t, false)
}

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

t.Parallel()
ctx := namespaces.WithNamespace(context.Background(), "buildkit-test")
ctx = logProgressStreams(ctx, t)

gs := setupGitSource(t, t.TempDir())

repo := setupGitRepo(t)

id := &GitIdentifier{Remote: repo.mainURL, KeepGitDir: keepGitDir, Ref: "a/v1.2.3"}

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

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

key1, pin1, _, _, err := g.CacheKey(ctx, nil, 0)
require.NoError(t, err)
if keepGitDir {
require.GreaterOrEqual(t, len(key1), expLen)
} else {
require.Equal(t, expLen, len(key1))
}
require.Equal(t, 40, len(pin1))

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

id2 := &GitIdentifier{Remote: repo.mainURL, KeepGitDir: keepGitDir, Ref: "a/v1.2.3-same"}
g2, err := gs.Resolve(ctx, id2, nil, nil)
require.NoError(t, err)

key2, pin2, _, _, err := g2.CacheKey(ctx, nil, 0)
require.NoError(t, err)
if keepGitDir {
require.GreaterOrEqual(t, len(key1), expLen)
} else {
require.Equal(t, expLen, len(key1))
}
require.Equal(t, 40, len(pin2))

require.Equal(t, pin1, pin2)
if !keepGitDir {
require.Equal(t, key1, key2)
return
}
// key should be different because of the ref
require.NotEqual(t, key1, key2)

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

mount1, err := ref2.Mount(ctx, true, nil)
require.NoError(t, err)

lm1 := snapshot.LocalMounter(mount1)
dir1, err := lm1.Mount()
require.NoError(t, err)
defer lm1.Unmount()

workDir := t.TempDir()

runShell(t, dir1, fmt.Sprintf(`git rev-parse a/v1.2.3 > %s/ref1`, workDir))

dt1, err := os.ReadFile(filepath.Join(workDir, "ref1"))
require.NoError(t, err)

mount2, err := ref2.Mount(ctx, true, nil)
require.NoError(t, err)

lm2 := snapshot.LocalMounter(mount2)
dir2, err := lm2.Mount()
require.NoError(t, err)
defer lm2.Unmount()

runShell(t, dir2, fmt.Sprintf(`git rev-parse a/v1.2.3-same > %s/ref2`, workDir))

dt2, err := os.ReadFile(filepath.Join(workDir, "ref2"))
require.NoError(t, err)
require.Equal(t, string(dt1), string(dt2))
}

func TestMultipleRepos(t *testing.T) {
testMultipleRepos(t, false)
}
Expand Down Expand Up @@ -496,12 +600,20 @@ func testMultipleRepos(t *testing.T, keepGitDir bool) {

key1, pin1, _, _, err := g.CacheKey(ctx, nil, 0)
require.NoError(t, err)
require.Equal(t, expLen, len(key1))
if keepGitDir {
require.GreaterOrEqual(t, len(key1), expLen)
} else {
require.Equal(t, expLen, len(key1))
}
require.Equal(t, 40, len(pin1))

key2, pin2, _, _, err := g2.CacheKey(ctx, nil, 0)
require.NoError(t, err)
require.Equal(t, expLen, len(key2))
if keepGitDir {
require.GreaterOrEqual(t, len(key2), expLen)
} else {
require.Equal(t, expLen, len(key2))
}
require.Equal(t, 40, len(pin2))

require.NotEqual(t, key1, key2)
Expand Down Expand Up @@ -608,9 +720,10 @@ func testSubdir(t *testing.T, keepGitDir bool) {
expLen := 44
if keepGitDir {
expLen += 4
require.GreaterOrEqual(t, len(key1), expLen)
} else {
require.Equal(t, expLen, len(key1))
}

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

ref1, err := g.Snapshot(ctx, nil)
Expand Down Expand Up @@ -712,7 +825,7 @@ func setupGitRepo(t *testing.T) gitRepoFixture {
// | * (tag: refs/tags/v1.2.3-special) tagonly-leaf
// |/
// * (tag: refs/tags/v1.2.3) second
// * (tag: refs/tags/a/v1.2.3) initial
// * (tag: refs/tags/a/v1.2.3, refs/tags/a/v1.2.3-same) initial
runShell(t, fixture.mainPath,
"git -c init.defaultBranch=master init",
"git config --local user.email test",
Expand All @@ -722,7 +835,7 @@ func setupGitRepo(t *testing.T) gitRepoFixture {
"git add abc",
"git commit -m initial",
"git tag --no-sign a/v1.2.3",

"git tag --no-sign a/v1.2.3-same",
"echo bar > def",
"mkdir subdir",
"echo subcontents > subdir/subfile",
Expand Down
Loading