artin/sourcegraph
1
0
Fork 0
mirror of https://github.com/sourcegraph/sourcegraph.git synced 2026-02-06 17:31:43 +00:00
Code Issues Packages Projects Releases Wiki Activity

gitserver: RawDiff checks if commits exist (#64245)

Browse Source 
ResolveRevision additionally adds a "^0" to the spec to make sure we
actually check if it exists. This is important, since most uses of the
ChangedFiles API pass in commit shas which do not get resolved by git
unless the "^0" is present.

I noticed this since hybrid search in searcher started to fail if the
commit was missing. In particular Zoekt for empty repositories uses a
fake SHA of 404aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa so when a repo
started getting cloned hybrid search would fail until it was indexed.
Hybrid search explicitly checks for the NotFound error, so hybrid search
failing was a a regression from moving away from the SymbolDiff API.
This commit is contained in:
Keegan Carruthers-Smith 2024-08-02 15:10:04 +02:00 committed by GitHub
parent ad4d7177ab
commit f7d62a3076
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 12 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
cmd/gitserver/internal/git/gitcli/diff.go
View File

@ -15,11 +15,11 @@ import (
)
func (g *gitCLIBackend) RawDiff(ctx context.Context, base string, head string, typ git.GitDiffComparisonType, opts git.RawDiffOpts, paths ...string) (io.ReadCloser, error) {
baseOID, err := g.revParse(ctx, base)
baseOID, err := g.ResolveRevision(ctx, base)
if err != nil {
return nil, err
}
headOID, err := g.revParse(ctx, head)
headOID, err := g.ResolveRevision(ctx, head)
if err != nil {
return nil, err
}

16
cmd/gitserver/internal/git/gitcli/diff_test.go
View File

@ -131,13 +131,17 @@ index 0ef51c52043997fdd257a0b77d761e9ca58bcc1f..58692a00a73d1f78df00014edf4ef39e
"git tag test",
)
_, err := backend.RawDiff(ctx, "unknown", "test", git.GitDiffComparisonTypeOnlyInHead, defaultOpts)
require.Error(t, err)
require.True(t, errors.HasType[*gitdomain.RevisionNotFoundError](err))
// Test with both an unknown ref that needs resolving and something
// that looks like a sha256 (hits different code paths inside of git)
for _, missing := range []string{"404aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "unknown"} {
_, err := backend.RawDiff(ctx, missing, "test", git.GitDiffComparisonTypeOnlyInHead, defaultOpts)
require.Error(t, err)
require.True(t, errors.HasType[*gitdomain.RevisionNotFoundError](err))
_, err = backend.RawDiff(ctx, "test", "unknown", git.GitDiffComparisonTypeOnlyInHead, defaultOpts)
require.Error(t, err)
require.True(t, errors.HasType[*gitdomain.RevisionNotFoundError](err))
_, err = backend.RawDiff(ctx, "test", missing, git.GitDiffComparisonTypeOnlyInHead, defaultOpts)
require.Error(t, err)
require.True(t, errors.HasType[*gitdomain.RevisionNotFoundError](err))
}
})
t.Run("files outside repository", func(t *testing.T) {
// We use git-diff-tree, but with git-diff you can diff any files on disk