Skip to content

Commit

Permalink
Propagate context and ensure git commands run in request context
Browse files Browse the repository at this point in the history
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 <art27@cantab.net>
  • Loading branch information
zeripath committed Dec 3, 2021
1 parent 4f81c7d commit 6c9c38a
Show file tree
Hide file tree
Showing 44 changed files with 209 additions and 196 deletions.
2 changes: 1 addition & 1 deletion cmd/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,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
2 changes: 1 addition & 1 deletion integrations/api_repo_get_contents_list_test.go
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
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
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))
assert.NoError(t, os.WriteFile(filepath.Join(dstPath, "README.md"), []byte(fmt.Sprintf("# Testing Repository\n\nOriginally created in: %s", dstPath)), 0644))
assert.NoError(t, git.AddChanges(dstPath, true))
signature := git.Signature{
Expand Down
6 changes: 3 additions & 3 deletions integrations/pull_merge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,11 +240,11 @@ func TestCantMergeConflict(t *testing.T) {
gitRepo, err := git.OpenRepository(models.RepoPath(user1.Name, repo1.Name))
assert.NoError(t, err)

err = pull.Merge(pr, user1, gitRepo, models.MergeStyleMerge, "CONFLICT")
err = pull.Merge(git.DefaultContext, pr, user1, gitRepo, models.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, models.MergeStyleRebase, "CONFLICT")
err = pull.Merge(git.DefaultContext, pr, user1, gitRepo, models.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 @@ -328,7 +328,7 @@ func TestCantMergeUnrelated(t *testing.T) {
BaseBranch: "base",
}).(*models.PullRequest)

err = pull.Merge(pr, user1, gitRepo, models.MergeStyleMerge, "UNRELATED")
err = pull.Merge(git.DefaultContext, pr, user1, gitRepo, models.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
11 changes: 6 additions & 5 deletions integrations/pull_update_test.go
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 @@ -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
8 changes: 6 additions & 2 deletions models/admin/notice.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ func CreateNotice(ctx context.Context, tp NoticeType, desc string, args ...inter

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

Expand All @@ -65,7 +66,8 @@ func RemoveAllWithNotice(ctx context.Context, title, path string) {
if err := util.RemoveAll(path); err != nil {
desc := fmt.Sprintf("%s [%s]: %v", title, path, err)
log.Warn(title+" [%s]: %v", path, err)
if err = CreateNotice(ctx, NoticeRepository, desc); err != nil {
// Note we use the db.DefaultContext here rather than passing in a context as the context may be cancelled
if err = CreateNotice(db.DefaultContext, NoticeRepository, desc); err != nil {
log.Error("CreateRepositoryNotice: %v", err)
}
}
Expand All @@ -77,7 +79,9 @@ func RemoveStorageWithNotice(ctx context.Context, bucket storage.ObjectStorage,
if err := bucket.Delete(path); err != nil {
desc := fmt.Sprintf("%s [%s]: %v", title, path, err)
log.Warn(title+" [%s]: %v", path, err)
if err = CreateNotice(ctx, NoticeRepository, desc); err != nil {

// Note we use the db.DefaultContext here rather than passing in a context as the context may be cancelled
if err = CreateNotice(db.DefaultContext, NoticeRepository, desc); err != nil {
log.Error("CreateRepositoryNotice: %v", err)
}
}
Expand Down
2 changes: 1 addition & 1 deletion modules/git/commit_info_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func cloneRepo(url, dir, name string) (string, error) {
if _, err := os.Stat(repoDir); err == nil {
return repoDir, nil
}
return repoDir, Clone(url, repoDir, CloneRepoOptions{
return repoDir, Clone(DefaultContext, url, repoDir, CloneRepoOptions{
Mirror: false,
Bare: false,
Quiet: true,
Expand Down
11 changes: 3 additions & 8 deletions modules/git/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,13 @@ func IsRepoURLAccessible(url string) bool {
}

// InitRepository initializes a new Git repository.
func InitRepository(repoPath string, bare bool) error {
func InitRepository(ctx context.Context, repoPath string, bare bool) error {
err := os.MkdirAll(repoPath, os.ModePerm)
if err != nil {
return err
}

cmd := NewCommand("init")
cmd := NewCommandContext(ctx, "init")
if bare {
cmd.AddArguments("--bare")
}
Expand Down Expand Up @@ -104,12 +104,7 @@ type CloneRepoOptions struct {
}

// Clone clones original repository to target path.
func Clone(from, to string, opts CloneRepoOptions) error {
return CloneWithContext(DefaultContext, from, to, opts)
}

// CloneWithContext clones original repository to target path.
func CloneWithContext(ctx context.Context, from, to string, opts CloneRepoOptions) error {
func Clone(ctx context.Context, from, to string, opts CloneRepoOptions) error {
cargs := make([]string, len(GlobalCommandArgs))
copy(cargs, GlobalCommandArgs)
return CloneWithArgs(ctx, from, to, cargs, opts)
Expand Down
4 changes: 2 additions & 2 deletions modules/git/repo_branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ func (repo *Repository) GetBranch(branch string) (*Branch, error) {

// GetBranchesByPath returns a branch by it's path
// if limit = 0 it will not limit
func GetBranchesByPath(path string, skip, limit int) ([]*Branch, int, error) {
gitRepo, err := OpenRepository(path)
func GetBranchesByPath(ctx context.Context, path string, skip, limit int) ([]*Branch, int, error) {
gitRepo, err := OpenRepositoryCtx(ctx, path)
if err != nil {
return nil, 0, err
}
Expand Down
6 changes: 3 additions & 3 deletions modules/indexer/code/bleve.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package code

import (
"bufio"
"context"
"fmt"
"io"
"os"
Expand Down Expand Up @@ -271,11 +272,10 @@ func (b *BleveIndexer) Close() {
}

// Index indexes the data
func (b *BleveIndexer) Index(repo *models.Repository, sha string, changes *repoChanges) error {
func (b *BleveIndexer) Index(ctx context.Context, repo *models.Repository, sha string, changes *repoChanges) error {
batch := gitea_bleve.NewFlushingBatch(b.indexer, maxBatchSize)
if len(changes.Updates) > 0 {

batchWriter, batchReader, cancel := git.CatFileBatch(git.DefaultContext, repo.RepoPath())
batchWriter, batchReader, cancel := git.CatFileBatch(ctx, repo.RepoPath())
defer cancel()

for _, update := range changes.Updates {
Expand Down
4 changes: 2 additions & 2 deletions modules/indexer/code/elastic_search.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,11 +244,11 @@ func (b *ElasticSearchIndexer) addDelete(filename string, repo *models.Repositor
}

// Index will save the index data
func (b *ElasticSearchIndexer) Index(repo *models.Repository, sha string, changes *repoChanges) error {
func (b *ElasticSearchIndexer) Index(ctx context.Context, repo *models.Repository, sha string, changes *repoChanges) error {
reqs := make([]elastic.BulkableRequest, 0)
if len(changes.Updates) > 0 {

batchWriter, batchReader, cancel := git.CatFileBatch(git.DefaultContext, repo.RepoPath())
batchWriter, batchReader, cancel := git.CatFileBatch(ctx, repo.RepoPath())
defer cancel()

for _, update := range changes.Updates {
Expand Down
8 changes: 4 additions & 4 deletions modules/indexer/code/indexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ type SearchResultLanguages struct {

// Indexer defines an interface to index and search code contents
type Indexer interface {
Index(repo *models.Repository, sha string, changes *repoChanges) error
Index(ctx context.Context, repo *models.Repository, sha string, changes *repoChanges) error
Delete(repoID int64) error
Search(repoIDs []int64, language, keyword string, page, pageSize int, isMatch bool) (int64, []*SearchResult, []*SearchResultLanguages, error)
Close()
Expand Down Expand Up @@ -82,7 +82,7 @@ var (
indexerQueue queue.UniqueQueue
)

func index(indexer Indexer, repoID int64) error {
func index(ctx context.Context, indexer Indexer, repoID int64) error {
repo, err := models.GetRepositoryByID(repoID)
if models.IsErrRepoNotExist(err) {
return indexer.Delete(repoID)
Expand All @@ -102,7 +102,7 @@ func index(indexer Indexer, repoID int64) error {
return nil
}

if err := indexer.Index(repo, sha, changes); err != nil {
if err := indexer.Index(ctx, repo, sha, changes); err != nil {
return err
}

Expand Down Expand Up @@ -150,7 +150,7 @@ func Init() {
}
log.Trace("IndexerData Process Repo: %d", indexerData.RepoID)

if err := index(indexer, indexerData.RepoID); err != nil {
if err := index(ctx, indexer, indexerData.RepoID); err != nil {
log.Error("index: %v", err)
continue
}
Expand Down
3 changes: 2 additions & 1 deletion modules/indexer/code/indexer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"testing"

"code.gitea.io/gitea/models/unittest"
"code.gitea.io/gitea/modules/git"

"github.com/stretchr/testify/assert"
)
Expand All @@ -20,7 +21,7 @@ func TestMain(m *testing.M) {
func testIndexer(name string, t *testing.T, indexer Indexer) {
t.Run(name, func(t *testing.T) {
var repoID int64 = 1
err := index(indexer, repoID)
err := index(git.DefaultContext, indexer, repoID)
assert.NoError(t, err)
var (
keywords = []struct {
Expand Down
5 changes: 3 additions & 2 deletions modules/indexer/code/wrapped.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package code

import (
"context"
"fmt"
"sync"

Expand Down Expand Up @@ -57,12 +58,12 @@ func (w *wrappedIndexer) get() (Indexer, error) {
return w.internal, nil
}

func (w *wrappedIndexer) Index(repo *models.Repository, sha string, changes *repoChanges) error {
func (w *wrappedIndexer) Index(ctx context.Context, repo *models.Repository, sha string, changes *repoChanges) error {
indexer, err := w.get()
if err != nil {
return err
}
return indexer.Index(repo, sha, changes)
return indexer.Index(ctx, repo, sha, changes)
}

func (w *wrappedIndexer) Delete(repoID int64) error {
Expand Down
16 changes: 8 additions & 8 deletions modules/repository/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func checkGiteaTemplate(tmpDir string) (*models.GiteaTemplate, error) {
return gt, nil
}

func generateRepoCommit(repo, templateRepo, generateRepo *models.Repository, tmpDir string) error {
func generateRepoCommit(ctx context.Context, repo, templateRepo, generateRepo *models.Repository, tmpDir string) error {
commitTimeStr := time.Now().Format(time.RFC3339)
authorSig := repo.Owner.NewGitSig()

Expand All @@ -114,7 +114,7 @@ func generateRepoCommit(repo, templateRepo, generateRepo *models.Repository, tmp

// Clone to temporary path and do the init commit.
templateRepoPath := templateRepo.RepoPath()
if err := git.Clone(templateRepoPath, tmpDir, git.CloneRepoOptions{
if err := git.Clone(ctx, templateRepoPath, tmpDir, git.CloneRepoOptions{
Depth: 1,
Branch: templateRepo.DefaultBranch,
}); err != nil {
Expand Down Expand Up @@ -171,19 +171,19 @@ func generateRepoCommit(repo, templateRepo, generateRepo *models.Repository, tmp
}
}

if err := git.InitRepository(tmpDir, false); err != nil {
if err := git.InitRepository(ctx, tmpDir, false); err != nil {
return err
}

repoPath := repo.RepoPath()
if stdout, err := git.NewCommand("remote", "add", "origin", repoPath).
if stdout, err := git.NewCommandContext(ctx, "remote", "add", "origin", repoPath).
SetDescription(fmt.Sprintf("generateRepoCommit (git remote add): %s to %s", templateRepoPath, tmpDir)).
RunInDirWithEnv(tmpDir, env); err != nil {
log.Error("Unable to add %v as remote origin to temporary repo to %s: stdout %s\nError: %v", repo, tmpDir, stdout, err)
return fmt.Errorf("git remote add: %v", err)
}

return initRepoCommit(tmpDir, repo, repo.Owner, templateRepo.DefaultBranch)
return initRepoCommit(ctx, tmpDir, repo, repo.Owner, templateRepo.DefaultBranch)
}

func generateGitContent(ctx context.Context, repo, templateRepo, generateRepo *models.Repository) (err error) {
Expand All @@ -198,7 +198,7 @@ func generateGitContent(ctx context.Context, repo, templateRepo, generateRepo *m
}
}()

if err = generateRepoCommit(repo, templateRepo, generateRepo, tmpDir); err != nil {
if err = generateRepoCommit(ctx, repo, templateRepo, generateRepo, tmpDir); err != nil {
return fmt.Errorf("generateRepoCommit: %v", err)
}

Expand All @@ -208,7 +208,7 @@ func generateGitContent(ctx context.Context, repo, templateRepo, generateRepo *m
}

repo.DefaultBranch = templateRepo.DefaultBranch
gitRepo, err := git.OpenRepository(repo.RepoPath())
gitRepo, err := git.OpenRepositoryCtx(ctx, repo.RepoPath())
if err != nil {
return fmt.Errorf("openRepository: %v", err)
}
Expand Down Expand Up @@ -272,7 +272,7 @@ func GenerateRepository(ctx context.Context, doer, owner *user_model.User, templ
}
}

if err = checkInitRepository(owner.Name, generateRepo.Name); err != nil {
if err = checkInitRepository(ctx, owner.Name, generateRepo.Name); err != nil {
return generateRepo, err
}

Expand Down
Loading

0 comments on commit 6c9c38a

Please sign in to comment.