Skip to content

Commit 94f0ff8

Browse files
tonistiigitonistiigi
authored
Merge pull request moby#5444 from tonistiigi/git-cachekey-fix
git: fix caching git commit through multiple refs
2 parents bc6f7be + 44b1aca commit 94f0ff8

File tree

2 files changed

+137
-17
lines changed

2 files changed

+137
-17
lines changed

‎source/git/source.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -196,10 +196,13 @@ type gitSourceHandler struct {
196196
authArgs []string
197197
}
198198

199-
func (gs *gitSourceHandler) shaToCacheKey(sha string) string {
199+
func (gs *gitSourceHandler) shaToCacheKey(sha, ref string) string {
200200
key := sha
201201
if gs.src.KeepGitDir {
202202
key += ".git"
203+
if ref != "" {
204+
key += "#" + ref
205+
}
203206
}
204207
if gs.src.Subdir != "" {
205208
key += ":" + gs.src.Subdir
@@ -341,7 +344,7 @@ func (gs *gitSourceHandler) CacheKey(ctx context.Context, g session.Group, index
341344
defer gs.locker.Unlock(remote)
342345

343346
if ref := gs.src.Ref; ref != "" && gitutil.IsCommitSHA(ref) {
344-
cacheKey := gs.shaToCacheKey(ref)
347+
cacheKey := gs.shaToCacheKey(ref, "")
345348
gs.cacheKey = cacheKey
346349
return cacheKey, ref, nil, true, nil
347350
}
@@ -377,6 +380,7 @@ func (gs *gitSourceHandler) CacheKey(ctx context.Context, g session.Group, index
377380
annotatedTagRef = tagRef + "^{}"
378381
)
379382
var sha, headSha, tagSha string
383+
var usedRef string
380384
for _, line := range lines {
381385
lineSha, lineRef, _ := strings.Cut(line, "\t")
382386
switch lineRef {
@@ -386,15 +390,18 @@ func (gs *gitSourceHandler) CacheKey(ctx context.Context, g session.Group, index
386390
tagSha = lineSha
387391
case partialRef:
388392
sha = lineSha
393+
usedRef = lineRef
389394
}
390395
}
391396

392397
// git-checkout prefers branches in case of ambiguity
393398
if sha == "" {
394399
sha = headSha
400+
usedRef = headRef
395401
}
396402
if sha == "" {
397403
sha = tagSha
404+
usedRef = tagRef
398405
}
399406
if sha == "" {
400407
return "", "", nil, false, errors.Errorf("repository does not contain ref %s, output: %q", ref, string(buf))
@@ -403,7 +410,7 @@ func (gs *gitSourceHandler) CacheKey(ctx context.Context, g session.Group, index
403410
return "", "", nil, false, errors.Errorf("invalid commit sha %q", sha)
404411
}
405412

406-
cacheKey := gs.shaToCacheKey(sha)
413+
cacheKey := gs.shaToCacheKey(sha, usedRef)
407414
gs.cacheKey = cacheKey
408415
return cacheKey, sha, nil, true, nil
409416
}

‎source/git/source_test.go

Lines changed: 127 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package git
33
import (
44
"bytes"
55
"context"
6+
"fmt"
67
"io"
78
"net/http"
89
"net/http/cgi"
@@ -69,9 +70,10 @@ func testRepeatedFetch(t *testing.T, keepGitDir bool) {
6970
expLen := 40
7071
if keepGitDir {
7172
expLen += 4
73+
require.GreaterOrEqual(t, len(key1), expLen)
74+
} else {
75+
require.Equal(t, expLen, len(key1))
7276
}
73-
74-
require.Equal(t, expLen, len(key1))
7577
require.Equal(t, 40, len(pin1))
7678

7779
ref1, err := g.Snapshot(ctx, nil)
@@ -189,9 +191,10 @@ func testFetchBySHA(t *testing.T, keepGitDir bool) {
189191
expLen := 40
190192
if keepGitDir {
191193
expLen += 4
194+
require.GreaterOrEqual(t, len(key1), expLen)
195+
} else {
196+
require.Equal(t, expLen, len(key1))
192197
}
193-
194-
require.Equal(t, expLen, len(key1))
195198
require.Equal(t, 40, len(pin1))
196199

197200
ref1, err := g.Snapshot(ctx, nil)
@@ -276,9 +279,10 @@ func testFetchUnreferencedRefSha(t *testing.T, ref string, keepGitDir bool) {
276279
expLen := 40
277280
if keepGitDir {
278281
expLen += 4
282+
require.GreaterOrEqual(t, len(key1), expLen)
283+
} else {
284+
require.Equal(t, expLen, len(key1))
279285
}
280-
281-
require.Equal(t, expLen, len(key1))
282286
require.Equal(t, 40, len(pin1))
283287

284288
ref1, err := g.Snapshot(ctx, nil)
@@ -372,9 +376,10 @@ func testFetchByTag(t *testing.T, tag, expectedCommitSubject string, isAnnotated
372376
expLen := 40
373377
if keepGitDir {
374378
expLen += 4
379+
require.GreaterOrEqual(t, len(key1), expLen)
380+
} else {
381+
require.Equal(t, expLen, len(key1))
375382
}
376-
377-
require.Equal(t, expLen, len(key1))
378383
require.Equal(t, 40, len(pin1))
379384

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

455+
func TestMultipleTagAccessKeepGitDir(t *testing.T) {
456+
testMultipleTagAccess(t, true)
457+
}
458+
459+
func TestMultipleTagAccess(t *testing.T) {
460+
testMultipleTagAccess(t, false)
461+
}
462+
463+
func testMultipleTagAccess(t *testing.T, keepGitDir bool) {
464+
if runtime.GOOS == "windows" {
465+
t.Skip("Depends on unimplemented containerd bind-mount support on Windows")
466+
}
467+
468+
t.Parallel()
469+
ctx := namespaces.WithNamespace(context.Background(), "buildkit-test")
470+
ctx = logProgressStreams(ctx, t)
471+
472+
gs := setupGitSource(t, t.TempDir())
473+
474+
repo := setupGitRepo(t)
475+
476+
id := &GitIdentifier{Remote: repo.mainURL, KeepGitDir: keepGitDir, Ref: "a/v1.2.3"}
477+
478+
g, err := gs.Resolve(ctx, id, nil, nil)
479+
require.NoError(t, err)
480+
481+
expLen := 40
482+
if keepGitDir {
483+
expLen += 4
484+
}
485+
486+
key1, pin1, _, _, err := g.CacheKey(ctx, nil, 0)
487+
require.NoError(t, err)
488+
if keepGitDir {
489+
require.GreaterOrEqual(t, len(key1), expLen)
490+
} else {
491+
require.Equal(t, expLen, len(key1))
492+
}
493+
require.Equal(t, 40, len(pin1))
494+
495+
ref1, err := g.Snapshot(ctx, nil)
496+
require.NoError(t, err)
497+
defer ref1.Release(context.TODO())
498+
499+
id2 := &GitIdentifier{Remote: repo.mainURL, KeepGitDir: keepGitDir, Ref: "a/v1.2.3-same"}
500+
g2, err := gs.Resolve(ctx, id2, nil, nil)
501+
require.NoError(t, err)
502+
503+
key2, pin2, _, _, err := g2.CacheKey(ctx, nil, 0)
504+
require.NoError(t, err)
505+
if keepGitDir {
506+
require.GreaterOrEqual(t, len(key1), expLen)
507+
} else {
508+
require.Equal(t, expLen, len(key1))
509+
}
510+
require.Equal(t, 40, len(pin2))
511+
512+
require.Equal(t, pin1, pin2)
513+
if !keepGitDir {
514+
require.Equal(t, key1, key2)
515+
return
516+
}
517+
// key should be different because of the ref
518+
require.NotEqual(t, key1, key2)
519+
520+
ref2, err := g2.Snapshot(ctx, nil)
521+
require.NoError(t, err)
522+
defer ref1.Release(context.TODO())
523+
524+
mount1, err := ref2.Mount(ctx, true, nil)
525+
require.NoError(t, err)
526+
527+
lm1 := snapshot.LocalMounter(mount1)
528+
dir1, err := lm1.Mount()
529+
require.NoError(t, err)
530+
defer lm1.Unmount()
531+
532+
workDir := t.TempDir()
533+
534+
runShell(t, dir1, fmt.Sprintf(`git rev-parse a/v1.2.3 > %s/ref1`, workDir))
535+
536+
dt1, err := os.ReadFile(filepath.Join(workDir, "ref1"))
537+
require.NoError(t, err)
538+
539+
mount2, err := ref2.Mount(ctx, true, nil)
540+
require.NoError(t, err)
541+
542+
lm2 := snapshot.LocalMounter(mount2)
543+
dir2, err := lm2.Mount()
544+
require.NoError(t, err)
545+
defer lm2.Unmount()
546+
547+
runShell(t, dir2, fmt.Sprintf(`git rev-parse a/v1.2.3-same > %s/ref2`, workDir))
548+
549+
dt2, err := os.ReadFile(filepath.Join(workDir, "ref2"))
550+
require.NoError(t, err)
551+
require.Equal(t, string(dt1), string(dt2))
552+
}
553+
450554
func TestMultipleRepos(t *testing.T) {
451555
testMultipleRepos(t, false)
452556
}
@@ -496,12 +600,20 @@ func testMultipleRepos(t *testing.T, keepGitDir bool) {
496600

497601
key1, pin1, _, _, err := g.CacheKey(ctx, nil, 0)
498602
require.NoError(t, err)
499-
require.Equal(t, expLen, len(key1))
603+
if keepGitDir {
604+
require.GreaterOrEqual(t, len(key1), expLen)
605+
} else {
606+
require.Equal(t, expLen, len(key1))
607+
}
500608
require.Equal(t, 40, len(pin1))
501609

502610
key2, pin2, _, _, err := g2.CacheKey(ctx, nil, 0)
503611
require.NoError(t, err)
504-
require.Equal(t, expLen, len(key2))
612+
if keepGitDir {
613+
require.GreaterOrEqual(t, len(key2), expLen)
614+
} else {
615+
require.Equal(t, expLen, len(key2))
616+
}
505617
require.Equal(t, 40, len(pin2))
506618

507619
require.NotEqual(t, key1, key2)
@@ -608,9 +720,10 @@ func testSubdir(t *testing.T, keepGitDir bool) {
608720
expLen := 44
609721
if keepGitDir {
610722
expLen += 4
723+
require.GreaterOrEqual(t, len(key1), expLen)
724+
} else {
725+
require.Equal(t, expLen, len(key1))
611726
}
612-
613-
require.Equal(t, expLen, len(key1))
614727
require.Equal(t, 40, len(pin1))
615728

616729
ref1, err := g.Snapshot(ctx, nil)
@@ -712,7 +825,7 @@ func setupGitRepo(t *testing.T) gitRepoFixture {
712825
// | * (tag: refs/tags/v1.2.3-special) tagonly-leaf
713826
// |/
714827
// * (tag: refs/tags/v1.2.3) second
715-
// * (tag: refs/tags/a/v1.2.3) initial
828+
// * (tag: refs/tags/a/v1.2.3, refs/tags/a/v1.2.3-same) initial
716829
runShell(t, fixture.mainPath,
717830
"git -c init.defaultBranch=master init",
718831
"git config --local user.email test",
@@ -722,7 +835,7 @@ func setupGitRepo(t *testing.T) gitRepoFixture {
722835
"git add abc",
723836
"git commit -m initial",
724837
"git tag --no-sign a/v1.2.3",
725-
838+
"git tag --no-sign a/v1.2.3-same",
726839
"echo bar > def",
727840
"mkdir subdir",
728841
"echo subcontents > subdir/subfile",

0 commit comments

Comments
 (0)