diff --git a/source/git/source.go b/source/git/source.go index cb56eeed5986..12b381196b06 100644 --- a/source/git/source.go +++ b/source/git/source.go @@ -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 @@ -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 } @@ -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 { @@ -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)) @@ -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 } diff --git a/source/git/source_test.go b/source/git/source_test.go index 5955e697848c..863cf3090954 100644 --- a/source/git/source_test.go +++ b/source/git/source_test.go @@ -3,6 +3,7 @@ package git import ( "bytes" "context" + "fmt" "io" "net/http" "net/http/cgi" @@ -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) @@ -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) @@ -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) @@ -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) @@ -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) } @@ -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) @@ -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) @@ -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", @@ -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",