Skip to content

Commit 6c9c38a

Browse files
zeripathzeripath
committed
Propagate context and ensure git commands run in request context
This PR continues the work in go-gitea#17125 by progressively ensuring that git commands run within the request context. Signed-off-by: Andrew Thornton <[email protected]>
1 parent 4f81c7d commit 6c9c38a

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

44 files changed

+209
-196
lines changed

‎cmd/admin.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -565,7 +565,7 @@ func runRepoSyncReleases(_ *cli.Context) error {
565565
log.Trace("Processing next %d repos of %d", len(repos), count)
566566
for _, repo := range repos {
567567
log.Trace("Synchronizing repo %s with path %s", repo.FullName(), repo.RepoPath())
568-
gitRepo, err := git.OpenRepository(repo.RepoPath())
568+
gitRepo, err := git.OpenRepositoryCtx(ctx, repo.RepoPath())
569569
if err != nil {
570570
log.Warn("OpenRepository: %v", err)
571571
continue

‎integrations/api_repo_get_contents_list_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ func testAPIGetContentsList(t *testing.T, u *url.URL) {
7272

7373
// Make a new branch in repo1
7474
newBranch := "test_branch"
75-
err := repo_service.CreateNewBranch(user2, repo1, repo1.DefaultBranch, newBranch)
75+
err := repo_service.CreateNewBranch(git.DefaultContext, user2, repo1, repo1.DefaultBranch, newBranch)
7676
assert.NoError(t, err)
7777
// Get the commit ID of the default branch
7878
gitRepo, err := git.OpenRepository(repo1.RepoPath())

‎integrations/api_repo_get_contents_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ func testAPIGetContents(t *testing.T, u *url.URL) {
7373

7474
// Make a new branch in repo1
7575
newBranch := "test_branch"
76-
err := repo_service.CreateNewBranch(user2, repo1, repo1.DefaultBranch, newBranch)
76+
err := repo_service.CreateNewBranch(git.DefaultContext, user2, repo1, repo1.DefaultBranch, newBranch)
7777
assert.NoError(t, err)
7878
// Get the commit ID of the default branch
7979
gitRepo, err := git.OpenRepository(repo1.RepoPath())

‎integrations/git_helper_for_declarative_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ func doGitCloneFail(u *url.URL) func(*testing.T) {
128128
tmpDir, err := os.MkdirTemp("", "doGitCloneFail")
129129
assert.NoError(t, err)
130130
defer util.RemoveAll(tmpDir)
131-
assert.Error(t, git.Clone(u.String(), tmpDir, git.CloneRepoOptions{}))
131+
assert.Error(t, git.Clone(git.DefaultContext, u.String(), tmpDir, git.CloneRepoOptions{}))
132132
exist, err := util.IsExist(filepath.Join(tmpDir, "README.md"))
133133
assert.NoError(t, err)
134134
assert.False(t, exist)
@@ -138,7 +138,7 @@ func doGitCloneFail(u *url.URL) func(*testing.T) {
138138
func doGitInitTestRepository(dstPath string) func(*testing.T) {
139139
return func(t *testing.T) {
140140
// Init repository in dstPath
141-
assert.NoError(t, git.InitRepository(dstPath, false))
141+
assert.NoError(t, git.InitRepository(git.DefaultContext, dstPath, false))
142142
assert.NoError(t, os.WriteFile(filepath.Join(dstPath, "README.md"), []byte(fmt.Sprintf("# Testing Repository\n\nOriginally created in: %s", dstPath)), 0644))
143143
assert.NoError(t, git.AddChanges(dstPath, true))
144144
signature := git.Signature{

‎integrations/pull_merge_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -240,11 +240,11 @@ func TestCantMergeConflict(t *testing.T) {
240240
gitRepo, err := git.OpenRepository(models.RepoPath(user1.Name, repo1.Name))
241241
assert.NoError(t, err)
242242

243-
err = pull.Merge(pr, user1, gitRepo, models.MergeStyleMerge, "CONFLICT")
243+
err = pull.Merge(git.DefaultContext, pr, user1, gitRepo, models.MergeStyleMerge, "CONFLICT")
244244
assert.Error(t, err, "Merge should return an error due to conflict")
245245
assert.True(t, models.IsErrMergeConflicts(err), "Merge error is not a conflict error")
246246

247-
err = pull.Merge(pr, user1, gitRepo, models.MergeStyleRebase, "CONFLICT")
247+
err = pull.Merge(git.DefaultContext, pr, user1, gitRepo, models.MergeStyleRebase, "CONFLICT")
248248
assert.Error(t, err, "Merge should return an error due to conflict")
249249
assert.True(t, models.IsErrRebaseConflicts(err), "Merge error is not a conflict error")
250250
gitRepo.Close()
@@ -328,7 +328,7 @@ func TestCantMergeUnrelated(t *testing.T) {
328328
BaseBranch: "base",
329329
}).(*models.PullRequest)
330330

331-
err = pull.Merge(pr, user1, gitRepo, models.MergeStyleMerge, "UNRELATED")
331+
err = pull.Merge(git.DefaultContext, pr, user1, gitRepo, models.MergeStyleMerge, "UNRELATED")
332332
assert.Error(t, err, "Merge should return an error due to unrelated")
333333
assert.True(t, models.IsErrMergeUnrelatedHistories(err), "Merge error is not a unrelated histories error")
334334
gitRepo.Close()

‎integrations/pull_update_test.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"code.gitea.io/gitea/models"
1414
"code.gitea.io/gitea/models/unittest"
1515
user_model "code.gitea.io/gitea/models/user"
16+
"code.gitea.io/gitea/modules/git"
1617
pull_service "code.gitea.io/gitea/services/pull"
1718
repo_service "code.gitea.io/gitea/services/repository"
1819
files_service "code.gitea.io/gitea/services/repository/files"
@@ -28,7 +29,7 @@ func TestAPIPullUpdate(t *testing.T) {
2829
pr := createOutdatedPR(t, user, org26)
2930

3031
//Test GetDiverging
31-
diffCount, err := pull_service.GetDiverging(pr)
32+
diffCount, err := pull_service.GetDiverging(git.DefaultContext, pr)
3233
assert.NoError(t, err)
3334
assert.EqualValues(t, 1, diffCount.Behind)
3435
assert.EqualValues(t, 1, diffCount.Ahead)
@@ -41,7 +42,7 @@ func TestAPIPullUpdate(t *testing.T) {
4142
session.MakeRequest(t, req, http.StatusOK)
4243

4344
//Test GetDiverging after update
44-
diffCount, err = pull_service.GetDiverging(pr)
45+
diffCount, err = pull_service.GetDiverging(git.DefaultContext, pr)
4546
assert.NoError(t, err)
4647
assert.EqualValues(t, 0, diffCount.Behind)
4748
assert.EqualValues(t, 2, diffCount.Ahead)
@@ -56,7 +57,7 @@ func TestAPIPullUpdateByRebase(t *testing.T) {
5657
pr := createOutdatedPR(t, user, org26)
5758

5859
//Test GetDiverging
59-
diffCount, err := pull_service.GetDiverging(pr)
60+
diffCount, err := pull_service.GetDiverging(git.DefaultContext, pr)
6061
assert.NoError(t, err)
6162
assert.EqualValues(t, 1, diffCount.Behind)
6263
assert.EqualValues(t, 1, diffCount.Ahead)
@@ -69,7 +70,7 @@ func TestAPIPullUpdateByRebase(t *testing.T) {
6970
session.MakeRequest(t, req, http.StatusOK)
7071

7172
//Test GetDiverging after update
72-
diffCount, err = pull_service.GetDiverging(pr)
73+
diffCount, err = pull_service.GetDiverging(git.DefaultContext, pr)
7374
assert.NoError(t, err)
7475
assert.EqualValues(t, 0, diffCount.Behind)
7576
assert.EqualValues(t, 1, diffCount.Ahead)
@@ -160,7 +161,7 @@ func createOutdatedPR(t *testing.T, actor, forkOrg *user_model.User) *models.Pul
160161
BaseRepo: baseRepo,
161162
Type: models.PullRequestGitea,
162163
}
163-
err = pull_service.NewPullRequest(baseRepo, pullIssue, nil, nil, pullRequest, nil)
164+
err = pull_service.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil)
164165
assert.NoError(t, err)
165166

166167
issue := unittest.AssertExistsAndLoadBean(t, &models.Issue{Title: "Test Pull -to-update-"}).(*models.Issue)

‎models/admin/notice.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ func CreateNotice(ctx context.Context, tp NoticeType, desc string, args ...inter
5656

5757
// CreateRepositoryNotice creates new system notice with type NoticeRepository.
5858
func CreateRepositoryNotice(desc string, args ...interface{}) error {
59+
// Note we use the db.DefaultContext here rather than passing in a context as the context may be cancelled
5960
return CreateNotice(db.DefaultContext, NoticeRepository, desc, args...)
6061
}
6162

@@ -65,7 +66,8 @@ func RemoveAllWithNotice(ctx context.Context, title, path string) {
6566
if err := util.RemoveAll(path); err != nil {
6667
desc := fmt.Sprintf("%s [%s]: %v", title, path, err)
6768
log.Warn(title+" [%s]: %v", path, err)
68-
if err = CreateNotice(ctx, NoticeRepository, desc); err != nil {
69+
// Note we use the db.DefaultContext here rather than passing in a context as the context may be cancelled
70+
if err = CreateNotice(db.DefaultContext, NoticeRepository, desc); err != nil {
6971
log.Error("CreateRepositoryNotice: %v", err)
7072
}
7173
}
@@ -77,7 +79,9 @@ func RemoveStorageWithNotice(ctx context.Context, bucket storage.ObjectStorage,
7779
if err := bucket.Delete(path); err != nil {
7880
desc := fmt.Sprintf("%s [%s]: %v", title, path, err)
7981
log.Warn(title+" [%s]: %v", path, err)
80-
if err = CreateNotice(ctx, NoticeRepository, desc); err != nil {
82+
83+
// Note we use the db.DefaultContext here rather than passing in a context as the context may be cancelled
84+
if err = CreateNotice(db.DefaultContext, NoticeRepository, desc); err != nil {
8185
log.Error("CreateRepositoryNotice: %v", err)
8286
}
8387
}

‎modules/git/commit_info_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func cloneRepo(url, dir, name string) (string, error) {
2424
if _, err := os.Stat(repoDir); err == nil {
2525
return repoDir, nil
2626
}
27-
return repoDir, Clone(url, repoDir, CloneRepoOptions{
27+
return repoDir, Clone(DefaultContext, url, repoDir, CloneRepoOptions{
2828
Mirror: false,
2929
Bare: false,
3030
Quiet: true,

‎modules/git/repo.go

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -63,13 +63,13 @@ func IsRepoURLAccessible(url string) bool {
6363
}
6464

6565
// InitRepository initializes a new Git repository.
66-
func InitRepository(repoPath string, bare bool) error {
66+
func InitRepository(ctx context.Context, repoPath string, bare bool) error {
6767
err := os.MkdirAll(repoPath, os.ModePerm)
6868
if err != nil {
6969
return err
7070
}
7171

72-
cmd := NewCommand("init")
72+
cmd := NewCommandContext(ctx, "init")
7373
if bare {
7474
cmd.AddArguments("--bare")
7575
}
@@ -104,12 +104,7 @@ type CloneRepoOptions struct {
104104
}
105105

106106
// Clone clones original repository to target path.
107-
func Clone(from, to string, opts CloneRepoOptions) error {
108-
return CloneWithContext(DefaultContext, from, to, opts)
109-
}
110-
111-
// CloneWithContext clones original repository to target path.
112-
func CloneWithContext(ctx context.Context, from, to string, opts CloneRepoOptions) error {
107+
func Clone(ctx context.Context, from, to string, opts CloneRepoOptions) error {
113108
cargs := make([]string, len(GlobalCommandArgs))
114109
copy(cargs, GlobalCommandArgs)
115110
return CloneWithArgs(ctx, from, to, cargs, opts)

‎modules/git/repo_branch.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,8 @@ func (repo *Repository) GetBranch(branch string) (*Branch, error) {
8888

8989
// GetBranchesByPath returns a branch by it's path
9090
// if limit = 0 it will not limit
91-
func GetBranchesByPath(path string, skip, limit int) ([]*Branch, int, error) {
92-
gitRepo, err := OpenRepository(path)
91+
func GetBranchesByPath(ctx context.Context, path string, skip, limit int) ([]*Branch, int, error) {
92+
gitRepo, err := OpenRepositoryCtx(ctx, path)
9393
if err != nil {
9494
return nil, 0, err
9595
}

0 commit comments

Comments
 (0)