refactor: decouple git.Repository(ctx) from git.Commit & git.Tree (#38464)

1. Storing "ctx" in a long-living object is wrong
2. Make the commit & tree cacheable (for the future performance
optimization)
3. Also fix some bad designs like `// FIXME: bad design, this field can
be nil if the commit is from "last commit cache"`

ref:
* #33893
This commit is contained in:
wxiaoguang
2026-07-15 17:30:01 +00:00
committed by GitHub
parent ed678b9d45
commit 82edc3da01
115 changed files with 732 additions and 864 deletions
+1 -1
View File
@@ -83,7 +83,7 @@ func testPackageCargo(t *testing.T, _ *neturl.URL) {
commit, err := gitRepo.GetBranchCommit(repo.DefaultBranch)
assert.NoError(t, err)
blob, err := commit.GetBlobByPath(path)
blob, err := commit.GetBlobByPath(t.Context(), gitRepo, path)
assert.NoError(t, err)
content, err := blob.GetBlobContent(1024)
@@ -120,7 +120,7 @@ func testAPIGetContentsList(t *testing.T, u *url.URL) {
assert.NotNil(t, contentsListResponse)
branchCommit, err := gitRepo.GetBranchCommit(ref)
assert.NoError(t, err)
lastCommit, err = branchCommit.GetCommitByPath("README.md")
lastCommit, err = branchCommit.GetCommitByPath(gitRepo, "README.md")
assert.NoError(t, err)
expectedContentsListResponse = getExpectedContentsListResponseForContents(ref, refType, lastCommit.ID.String())
assert.Equal(t, expectedContentsListResponse, contentsListResponse)
@@ -134,7 +134,7 @@ func testAPIGetContentsList(t *testing.T, u *url.URL) {
assert.NotNil(t, contentsListResponse)
tagCommit, err := gitRepo.GetTagCommit(ref)
assert.NoError(t, err)
lastCommit, err = tagCommit.GetCommitByPath("README.md")
lastCommit, err = tagCommit.GetCommitByPath(gitRepo, "README.md")
assert.NoError(t, err)
expectedContentsListResponse = getExpectedContentsListResponseForContents(ref, refType, lastCommit.ID.String())
assert.Equal(t, expectedContentsListResponse, contentsListResponse)
@@ -125,7 +125,7 @@ func testAPIGetContents(t *testing.T, _ *url.URL) {
resp = MakeRequest(t, req, http.StatusOK)
contentsResponse = DecodeJSON(t, resp, &api.ContentsResponse{})
branchCommit, _ := gitRepo.GetBranchCommit(ref)
lastCommit, _ = branchCommit.GetCommitByPath("README.md")
lastCommit, _ = branchCommit.GetCommitByPath(gitRepo, "README.md")
expectedContentsResponse = getExpectedContentsResponseForContents(ref, refType, lastCommit.ID.String())
assert.Equal(t, *expectedContentsResponse, *contentsResponse)
@@ -136,7 +136,7 @@ func testAPIGetContents(t *testing.T, _ *url.URL) {
resp = MakeRequest(t, req, http.StatusOK)
contentsResponse = DecodeJSON(t, resp, &api.ContentsResponse{})
tagCommit, _ := gitRepo.GetTagCommit(ref)
lastCommit, _ = tagCommit.GetCommitByPath("README.md")
lastCommit, _ = tagCommit.GetCommitByPath(gitRepo, "README.md")
expectedContentsResponse = getExpectedContentsResponseForContents(ref, refType, lastCommit.ID.String())
assert.Equal(t, *expectedContentsResponse, *contentsResponse)
+2 -2
View File
@@ -50,10 +50,10 @@ func TestDataAsyncDoubleRead_Issue29101(t *testing.T) {
commit, err := gitRepo.GetCommit(sha)
assert.NoError(t, err)
entry, err := commit.GetTreeEntryByPath("test.txt")
entry, err := commit.GetTreeEntryByPath(t.Context(), gitRepo, "test.txt")
assert.NoError(t, err)
b := entry.Blob()
b := entry.Blob(gitRepo)
r1, err := b.DataAsync()
assert.NoError(t, err)
defer r1.Close()
+6 -6
View File
@@ -403,7 +403,7 @@ func TestChangeRepoFilesForUpdate(t *testing.T) {
defer gitRepo.Close()
commit, _ := gitRepo.GetBranchCommit(opts.NewBranch)
lastCommit, _ := commit.GetCommitByPath(opts.Files[0].TreePath)
lastCommit, _ := commit.GetCommitByPath(gitRepo, opts.Files[0].TreePath)
expectedFileResponse := getExpectedFileResponseForRepoFilesUpdate(commit.ID.String(), opts.Files[0].TreePath, lastCommit.ID.String(), lastCommit.Committer.When, lastCommit.Author.When)
assert.Equal(t, expectedFileResponse.Content, filesResponse.Files[0])
assert.Equal(t, expectedFileResponse.Commit.SHA, filesResponse.Commit.SHA)
@@ -439,17 +439,17 @@ func TestChangeRepoFilesForUpdateWithFileMove(t *testing.T) {
defer gitRepo.Close()
commit, _ := gitRepo.GetBranchCommit(opts.NewBranch)
lastCommit, _ := commit.GetCommitByPath(opts.Files[0].TreePath)
lastCommit, _ := commit.GetCommitByPath(gitRepo, opts.Files[0].TreePath)
expectedFileResponse := getExpectedFileResponseForRepoFilesUpdate(commit.ID.String(), opts.Files[0].TreePath, lastCommit.ID.String(), lastCommit.Committer.When, lastCommit.Author.When)
// assert that the old file no longer exists in the last commit of the branch
fromEntry, err := commit.GetTreeEntryByPath(opts.Files[0].FromTreePath)
fromEntry, err := commit.GetTreeEntryByPath(ctx, gitRepo, opts.Files[0].FromTreePath)
switch err.(type) {
case git.ErrNotExist:
// correct, continue
default:
t.Fatalf("expected git.ErrNotExist, got:%v", err)
}
toEntry, err := commit.GetTreeEntryByPath(opts.Files[0].TreePath)
toEntry, err := commit.GetTreeEntryByPath(ctx, gitRepo, opts.Files[0].TreePath)
assert.NoError(t, err)
assert.Nil(t, fromEntry) // Should no longer exist here
assert.NotNil(t, toEntry) // Should exist here
@@ -485,7 +485,7 @@ func TestChangeRepoFilesForUpdateWithFileRename(t *testing.T) {
defer gitRepo.Close()
commit, _ := gitRepo.GetBranchCommit(repo.DefaultBranch)
lastCommit, _ := commit.GetCommitByPath(opts.Files[0].TreePath)
lastCommit, _ := commit.GetCommitByPath(gitRepo, opts.Files[0].TreePath)
expectedFileResponse := getExpectedFileResponseForRepoFilesUpdateRename(commit.ID.String(), lastCommit.ID.String())
for _, file := range filesResponse.Files {
file.LastCommitterDate, file.LastAuthorDate = nil, nil // there might be different time in one operation, so we ignore them
@@ -522,7 +522,7 @@ func TestChangeRepoFilesWithoutBranchNames(t *testing.T) {
defer gitRepo.Close()
commit, _ := gitRepo.GetBranchCommit(repo.DefaultBranch)
lastCommit, _ := commit.GetCommitByPath(opts.Files[0].TreePath)
lastCommit, _ := commit.GetCommitByPath(gitRepo, opts.Files[0].TreePath)
expectedFileResponse := getExpectedFileResponseForRepoFilesUpdate(commit.ID.String(), opts.Files[0].TreePath, lastCommit.ID.String(), lastCommit.Committer.When, lastCommit.Author.When)
assert.Equal(t, expectedFileResponse.Content, filesResponse.Files[0])
})