Skip to content

Commit

Permalink
Merge pull request #5444 from tonistiigi/git-cachekey-fix
Browse files Browse the repository at this point in the history 
git: fix caching git commit through multiple refs
  • Loading branch information
@tonistiigi
tonistiigi authored Oct 28, 2024
2 parents bc6f7be + 44b1aca commit 94f0ff8
Show file tree
Hide file tree
Showing 2 changed files with 137 additions and 17 deletions.
13 changes: 10 additions & 3 deletions source/git/source.go
View file
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
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

0 comments on commit 94f0ff8

Please sign in to comment.