Skip to content

Commit

Permalink
Propagate context and ensure git commands run in request context (go-…
Browse files Browse the repository at this point in the history 
…gitea#17868)

This PR continues the work in go-gitea#17125 by progressively ensuring that git
commands run within the request context.

This now means that the if there is a git repo already open in the context it will be used instead of reopening it.

Signed-off-by: Andrew Thornton <art27@cantab.net>
  • Loading branch information
@zeripath
zeripath authored and Stelios Malathouras committed Mar 28, 2022
1 parent cae6953 commit d06314a
Show file tree
Hide file tree
Showing 193 changed files with 1,262 additions and 1,152 deletions.
2 changes: 1 addition & 1 deletion cmd/admin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,7 @@ func runRepoSyncReleases(_ *cli.Context) error {
log.Trace("Processing next %d repos of %d", len(repos), count)
for _, repo := range repos {
log.Trace("Synchronizing repo %s with path %s", repo.FullName(), repo.RepoPath())
gitRepo, err := git.OpenRepository(repo.RepoPath())
gitRepo, err := git.OpenRepositoryCtx(ctx, repo.RepoPath())
if err != nil {
log.Warn("OpenRepository: %v", err)
continue
Expand Down
3 changes: 1 addition & 2 deletions cmd/doctor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
package cmd

import (
"context"
"fmt"
golog "log"
"os"
Expand Down Expand Up @@ -116,7 +115,7 @@ func runRecreateTable(ctx *cli.Context) error {
}
recreateTables := migrations.RecreateTables(beans...)

return db.InitEngineWithMigration(context.Background(), func(x *xorm.Engine) error {
return db.InitEngineWithMigration(stdCtx, func(x *xorm.Engine) error {
if err := migrations.EnsureUpToDate(x); err != nil {
return err
}
Expand Down
3 changes: 2 additions & 1 deletion integrations/api_repo_file_helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package integrations
import (
repo_model "code.gitea.io/gitea/models/repo"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/git"
api "code.gitea.io/gitea/modules/structs"
files_service "code.gitea.io/gitea/services/repository/files"
)
Expand All @@ -20,7 +21,7 @@ func createFileInBranch(user *user_model.User, repo *repo_model.Repository, tree
Author: nil,
Committer: nil,
}
return files_service.CreateOrUpdateRepoFile(repo, user, opts)
return files_service.CreateOrUpdateRepoFile(git.DefaultContext, repo, user, opts)
}

func createFile(user *user_model.User, repo *repo_model.Repository, treePath string) (*api.FileResponse, error) {
Expand Down
2 changes: 1 addition & 1 deletion integrations/api_repo_get_contents_list_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func testAPIGetContentsList(t *testing.T, u *url.URL) {

// Make a new branch in repo1
newBranch := "test_branch"
err := repo_service.CreateNewBranch(user2, repo1, repo1.DefaultBranch, newBranch)
err := repo_service.CreateNewBranch(git.DefaultContext, user2, repo1, repo1.DefaultBranch, newBranch)
assert.NoError(t, err)
// Get the commit ID of the default branch
gitRepo, err := git.OpenRepository(repo1.RepoPath())
Expand Down
2 changes: 1 addition & 1 deletion integrations/api_repo_get_contents_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func testAPIGetContents(t *testing.T, u *url.URL) {

// Make a new branch in repo1
newBranch := "test_branch"
err := repo_service.CreateNewBranch(user2, repo1, repo1.DefaultBranch, newBranch)
err := repo_service.CreateNewBranch(git.DefaultContext, user2, repo1, repo1.DefaultBranch, newBranch)
assert.NoError(t, err)
// Get the commit ID of the default branch
gitRepo, err := git.OpenRepository(repo1.RepoPath())
Expand Down
4 changes: 2 additions & 2 deletions integrations/git_helper_for_declarative_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func doGitCloneFail(u *url.URL) func(*testing.T) {
tmpDir, err := os.MkdirTemp("", "doGitCloneFail")
assert.NoError(t, err)
defer util.RemoveAll(tmpDir)
assert.Error(t, git.Clone(u.String(), tmpDir, git.CloneRepoOptions{}))
assert.Error(t, git.Clone(git.DefaultContext, u.String(), tmpDir, git.CloneRepoOptions{}))
exist, err := util.IsExist(filepath.Join(tmpDir, "README.md"))
assert.NoError(t, err)
assert.False(t, exist)
Expand All @@ -138,7 +138,7 @@ func doGitCloneFail(u *url.URL) func(*testing.T) {
func doGitInitTestRepository(dstPath string) func(*testing.T) {
return func(t *testing.T) {
// Init repository in dstPath
assert.NoError(t, git.InitRepository(dstPath, false))
assert.NoError(t, git.InitRepository(git.DefaultContext, dstPath, false))
// forcibly set default branch to master
_, err := git.NewCommand("symbolic-ref", "HEAD", git.BranchPrefix+"master").RunInDir(dstPath)
assert.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion integrations/mirror_pull_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func TestMirrorPull(t *testing.T) {

release, err := models.GetRelease(repo.ID, "v0.2")
assert.NoError(t, err)
assert.NoError(t, release_service.DeleteReleaseByID(release.ID, user, true))
assert.NoError(t, release_service.DeleteReleaseByID(ctx, release.ID, user, true))

ok = mirror_service.SyncPullMirror(ctx, mirror.ID)
assert.True(t, ok)
Expand Down
6 changes: 3 additions & 3 deletions integrations/pull_merge_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,11 +241,11 @@ func TestCantMergeConflict(t *testing.T) {
gitRepo, err := git.OpenRepository(repo_model.RepoPath(user1.Name, repo1.Name))
assert.NoError(t, err)

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

err = pull.Merge(pr, user1, gitRepo, repo_model.MergeStyleRebase, "", "CONFLICT")
err = pull.Merge(git.DefaultContext, pr, user1, gitRepo, repo_model.MergeStyleRebase, "", "CONFLICT")
assert.Error(t, err, "Merge should return an error due to conflict")
assert.True(t, models.IsErrRebaseConflicts(err), "Merge error is not a conflict error")
gitRepo.Close()
Expand Down Expand Up @@ -329,7 +329,7 @@ func TestCantMergeUnrelated(t *testing.T) {
BaseBranch: "base",
}).(*models.PullRequest)

err = pull.Merge(pr, user1, gitRepo, repo_model.MergeStyleMerge, "", "UNRELATED")
err = pull.Merge(git.DefaultContext, pr, user1, gitRepo, repo_model.MergeStyleMerge, "", "UNRELATED")
assert.Error(t, err, "Merge should return an error due to unrelated")
assert.True(t, models.IsErrMergeUnrelatedHistories(err), "Merge error is not a unrelated histories error")
gitRepo.Close()
Expand Down
15 changes: 8 additions & 7 deletions integrations/pull_update_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"code.gitea.io/gitea/models"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/git"
pull_service "code.gitea.io/gitea/services/pull"
repo_service "code.gitea.io/gitea/services/repository"
files_service "code.gitea.io/gitea/services/repository/files"
Expand All @@ -28,7 +29,7 @@ func TestAPIPullUpdate(t *testing.T) {
pr := createOutdatedPR(t, user, org26)

//Test GetDiverging
diffCount, err := pull_service.GetDiverging(pr)
diffCount, err := pull_service.GetDiverging(git.DefaultContext, pr)
assert.NoError(t, err)
assert.EqualValues(t, 1, diffCount.Behind)
assert.EqualValues(t, 1, diffCount.Ahead)
Expand All @@ -41,7 +42,7 @@ func TestAPIPullUpdate(t *testing.T) {
session.MakeRequest(t, req, http.StatusOK)

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

//Test GetDiverging
diffCount, err := pull_service.GetDiverging(pr)
diffCount, err := pull_service.GetDiverging(git.DefaultContext, pr)
assert.NoError(t, err)
assert.EqualValues(t, 1, diffCount.Behind)
assert.EqualValues(t, 1, diffCount.Ahead)
Expand All @@ -69,7 +70,7 @@ func TestAPIPullUpdateByRebase(t *testing.T) {
session.MakeRequest(t, req, http.StatusOK)

//Test GetDiverging after update
diffCount, err = pull_service.GetDiverging(pr)
diffCount, err = pull_service.GetDiverging(git.DefaultContext, pr)
assert.NoError(t, err)
assert.EqualValues(t, 0, diffCount.Behind)
assert.EqualValues(t, 1, diffCount.Ahead)
Expand Down Expand Up @@ -98,7 +99,7 @@ func createOutdatedPR(t *testing.T, actor, forkOrg *user_model.User) *models.Pul
assert.NotEmpty(t, headRepo)

//create a commit on base Repo
_, err = files_service.CreateOrUpdateRepoFile(baseRepo, actor, &files_service.UpdateRepoFileOptions{
_, err = files_service.CreateOrUpdateRepoFile(git.DefaultContext, baseRepo, actor, &files_service.UpdateRepoFileOptions{
TreePath: "File_A",
Message: "Add File A",
Content: "File A",
Expand All @@ -121,7 +122,7 @@ func createOutdatedPR(t *testing.T, actor, forkOrg *user_model.User) *models.Pul
assert.NoError(t, err)

//create a commit on head Repo
_, err = files_service.CreateOrUpdateRepoFile(headRepo, actor, &files_service.UpdateRepoFileOptions{
_, err = files_service.CreateOrUpdateRepoFile(git.DefaultContext, headRepo, actor, &files_service.UpdateRepoFileOptions{
TreePath: "File_B",
Message: "Add File on PR branch",
Content: "File B",
Expand Down Expand Up @@ -160,7 +161,7 @@ func createOutdatedPR(t *testing.T, actor, forkOrg *user_model.User) *models.Pul
BaseRepo: baseRepo,
Type: models.PullRequestGitea,
}
err = pull_service.NewPullRequest(baseRepo, pullIssue, nil, nil, pullRequest, nil)
err = pull_service.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil)
assert.NoError(t, err)

issue := unittest.AssertExistsAndLoadBean(t, &models.Issue{Title: "Test Pull -to-update-"}).(*models.Issue)
Expand Down
6 changes: 3 additions & 3 deletions integrations/repo_tag_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func TestCreateNewTagProtected(t *testing.T) {
t.Run("API", func(t *testing.T) {
defer PrintCurrentTest(t)()

err := release.CreateNewTag(owner, repo, "master", "v-1", "first tag")
err := release.CreateNewTag(git.DefaultContext, owner, repo, "master", "v-1", "first tag")
assert.NoError(t, err)

err = models.InsertProtectedTag(&models.ProtectedTag{
Expand All @@ -44,11 +44,11 @@ func TestCreateNewTagProtected(t *testing.T) {
})
assert.NoError(t, err)

err = release.CreateNewTag(owner, repo, "master", "v-2", "second tag")
err = release.CreateNewTag(git.DefaultContext, owner, repo, "master", "v-2", "second tag")
assert.Error(t, err)
assert.True(t, models.IsErrProtectedTagName(err))

err = release.CreateNewTag(owner, repo, "master", "v-1.1", "third tag")
err = release.CreateNewTag(git.DefaultContext, owner, repo, "master", "v-1.1", "third tag")
assert.NoError(t, err)
})

Expand Down
17 changes: 9 additions & 8 deletions integrations/repofiles_delete_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unittest"
"code.gitea.io/gitea/modules/git"
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/test"
files_service "code.gitea.io/gitea/services/repository/files"
Expand Down Expand Up @@ -80,7 +81,7 @@ func testDeleteRepoFile(t *testing.T, u *url.URL) {
opts := getDeleteRepoFileOptions(repo)

t.Run("Delete README.md file", func(t *testing.T) {
fileResponse, err := files_service.DeleteRepoFile(repo, doer, opts)
fileResponse, err := files_service.DeleteRepoFile(git.DefaultContext, repo, doer, opts)
assert.NoError(t, err)
expectedFileResponse := getExpectedDeleteFileResponse(u)
assert.NotNil(t, fileResponse)
Expand All @@ -92,7 +93,7 @@ func testDeleteRepoFile(t *testing.T, u *url.URL) {
})

t.Run("Verify README.md has been deleted", func(t *testing.T) {
fileResponse, err := files_service.DeleteRepoFile(repo, doer, opts)
fileResponse, err := files_service.DeleteRepoFile(git.DefaultContext, repo, doer, opts)
assert.Nil(t, fileResponse)
expectedError := "repository file does not exist [path: " + opts.TreePath + "]"
assert.EqualError(t, err, expectedError)
Expand Down Expand Up @@ -122,7 +123,7 @@ func testDeleteRepoFileWithoutBranchNames(t *testing.T, u *url.URL) {
opts.NewBranch = ""

t.Run("Delete README.md without Branch Name", func(t *testing.T) {
fileResponse, err := files_service.DeleteRepoFile(repo, doer, opts)
fileResponse, err := files_service.DeleteRepoFile(git.DefaultContext, repo, doer, opts)
assert.NoError(t, err)
expectedFileResponse := getExpectedDeleteFileResponse(u)
assert.NotNil(t, fileResponse)
Expand Down Expand Up @@ -151,7 +152,7 @@ func TestDeleteRepoFileErrors(t *testing.T) {
t.Run("Bad branch", func(t *testing.T) {
opts := getDeleteRepoFileOptions(repo)
opts.OldBranch = "bad_branch"
fileResponse, err := files_service.DeleteRepoFile(repo, doer, opts)
fileResponse, err := files_service.DeleteRepoFile(git.DefaultContext, repo, doer, opts)
assert.Error(t, err)
assert.Nil(t, fileResponse)
expectedError := "branch does not exist [name: " + opts.OldBranch + "]"
Expand All @@ -162,7 +163,7 @@ func TestDeleteRepoFileErrors(t *testing.T) {
opts := getDeleteRepoFileOptions(repo)
origSHA := opts.SHA
opts.SHA = "bad_sha"
fileResponse, err := files_service.DeleteRepoFile(repo, doer, opts)
fileResponse, err := files_service.DeleteRepoFile(git.DefaultContext, repo, doer, opts)
assert.Nil(t, fileResponse)
assert.Error(t, err)
expectedError := "sha does not match [given: " + opts.SHA + ", expected: " + origSHA + "]"
Expand All @@ -172,7 +173,7 @@ func TestDeleteRepoFileErrors(t *testing.T) {
t.Run("New branch already exists", func(t *testing.T) {
opts := getDeleteRepoFileOptions(repo)
opts.NewBranch = "develop"
fileResponse, err := files_service.DeleteRepoFile(repo, doer, opts)
fileResponse, err := files_service.DeleteRepoFile(git.DefaultContext, repo, doer, opts)
assert.Nil(t, fileResponse)
assert.Error(t, err)
expectedError := "branch already exists [name: " + opts.NewBranch + "]"
Expand All @@ -182,7 +183,7 @@ func TestDeleteRepoFileErrors(t *testing.T) {
t.Run("TreePath is empty:", func(t *testing.T) {
opts := getDeleteRepoFileOptions(repo)
opts.TreePath = ""
fileResponse, err := files_service.DeleteRepoFile(repo, doer, opts)
fileResponse, err := files_service.DeleteRepoFile(git.DefaultContext, repo, doer, opts)
assert.Nil(t, fileResponse)
assert.Error(t, err)
expectedError := "path contains a malformed path component [path: ]"
Expand All @@ -192,7 +193,7 @@ func TestDeleteRepoFileErrors(t *testing.T) {
t.Run("TreePath is a git directory:", func(t *testing.T) {
opts := getDeleteRepoFileOptions(repo)
opts.TreePath = ".git"
fileResponse, err := files_service.DeleteRepoFile(repo, doer, opts)
fileResponse, err := files_service.DeleteRepoFile(git.DefaultContext, repo, doer, opts)
assert.Nil(t, fileResponse)
assert.Error(t, err)
expectedError := "path contains a malformed path component [path: " + opts.TreePath + "]"
Expand Down
20 changes: 10 additions & 10 deletions integrations/repofiles_update_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ func TestCreateOrUpdateRepoFileForCreate(t *testing.T) {
opts := getCreateRepoFileOptions(repo)

// test
fileResponse, err := files_service.CreateOrUpdateRepoFile(repo, doer, opts)
fileResponse, err := files_service.CreateOrUpdateRepoFile(git.DefaultContext, repo, doer, opts)

// asserts
assert.NoError(t, err)
Expand Down Expand Up @@ -234,7 +234,7 @@ func TestCreateOrUpdateRepoFileForUpdate(t *testing.T) {
opts := getUpdateRepoFileOptions(repo)

// test
fileResponse, err := files_service.CreateOrUpdateRepoFile(repo, doer, opts)
fileResponse, err := files_service.CreateOrUpdateRepoFile(git.DefaultContext, repo, doer, opts)

// asserts
assert.NoError(t, err)
Expand Down Expand Up @@ -269,7 +269,7 @@ func TestCreateOrUpdateRepoFileForUpdateWithFileMove(t *testing.T) {
opts.TreePath = "README_new.md" // new file name, README_new.md

// test
fileResponse, err := files_service.CreateOrUpdateRepoFile(repo, doer, opts)
fileResponse, err := files_service.CreateOrUpdateRepoFile(git.DefaultContext, repo, doer, opts)

// asserts
assert.NoError(t, err)
Expand Down Expand Up @@ -319,7 +319,7 @@ func TestCreateOrUpdateRepoFileWithoutBranchNames(t *testing.T) {
opts.NewBranch = ""

// test
fileResponse, err := files_service.CreateOrUpdateRepoFile(repo, doer, opts)
fileResponse, err := files_service.CreateOrUpdateRepoFile(git.DefaultContext, repo, doer, opts)

// asserts
assert.NoError(t, err)
Expand Down Expand Up @@ -349,7 +349,7 @@ func TestCreateOrUpdateRepoFileErrors(t *testing.T) {
t.Run("bad branch", func(t *testing.T) {
opts := getUpdateRepoFileOptions(repo)
opts.OldBranch = "bad_branch"
fileResponse, err := files_service.CreateOrUpdateRepoFile(repo, doer, opts)
fileResponse, err := files_service.CreateOrUpdateRepoFile(git.DefaultContext, repo, doer, opts)
assert.Error(t, err)
assert.Nil(t, fileResponse)
expectedError := "branch does not exist [name: " + opts.OldBranch + "]"
Expand All @@ -360,7 +360,7 @@ func TestCreateOrUpdateRepoFileErrors(t *testing.T) {
opts := getUpdateRepoFileOptions(repo)
origSHA := opts.SHA
opts.SHA = "bad_sha"
fileResponse, err := files_service.CreateOrUpdateRepoFile(repo, doer, opts)
fileResponse, err := files_service.CreateOrUpdateRepoFile(git.DefaultContext, repo, doer, opts)
assert.Nil(t, fileResponse)
assert.Error(t, err)
expectedError := "sha does not match [given: " + opts.SHA + ", expected: " + origSHA + "]"
Expand All @@ -370,7 +370,7 @@ func TestCreateOrUpdateRepoFileErrors(t *testing.T) {
t.Run("new branch already exists", func(t *testing.T) {
opts := getUpdateRepoFileOptions(repo)
opts.NewBranch = "develop"
fileResponse, err := files_service.CreateOrUpdateRepoFile(repo, doer, opts)
fileResponse, err := files_service.CreateOrUpdateRepoFile(git.DefaultContext, repo, doer, opts)
assert.Nil(t, fileResponse)
assert.Error(t, err)
expectedError := "branch already exists [name: " + opts.NewBranch + "]"
Expand All @@ -380,7 +380,7 @@ func TestCreateOrUpdateRepoFileErrors(t *testing.T) {
t.Run("treePath is empty:", func(t *testing.T) {
opts := getUpdateRepoFileOptions(repo)
opts.TreePath = ""
fileResponse, err := files_service.CreateOrUpdateRepoFile(repo, doer, opts)
fileResponse, err := files_service.CreateOrUpdateRepoFile(git.DefaultContext, repo, doer, opts)
assert.Nil(t, fileResponse)
assert.Error(t, err)
expectedError := "path contains a malformed path component [path: ]"
Expand All @@ -390,7 +390,7 @@ func TestCreateOrUpdateRepoFileErrors(t *testing.T) {
t.Run("treePath is a git directory:", func(t *testing.T) {
opts := getUpdateRepoFileOptions(repo)
opts.TreePath = ".git"
fileResponse, err := files_service.CreateOrUpdateRepoFile(repo, doer, opts)
fileResponse, err := files_service.CreateOrUpdateRepoFile(git.DefaultContext, repo, doer, opts)
assert.Nil(t, fileResponse)
assert.Error(t, err)
expectedError := "path contains a malformed path component [path: " + opts.TreePath + "]"
Expand All @@ -400,7 +400,7 @@ func TestCreateOrUpdateRepoFileErrors(t *testing.T) {
t.Run("create file that already exists", func(t *testing.T) {
opts := getCreateRepoFileOptions(repo)
opts.TreePath = "README.md" //already exists
fileResponse, err := files_service.CreateOrUpdateRepoFile(repo, doer, opts)
fileResponse, err := files_service.CreateOrUpdateRepoFile(git.DefaultContext, repo, doer, opts)
assert.Nil(t, fileResponse)
assert.Error(t, err)
expectedError := "repository file already exists [path: " + opts.TreePath + "]"
Expand Down
Loading

0 comments on commit d06314a

Please sign in to comment.