aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorzeripath2020-09-15 11:09:25 +0100
committerGitHub2020-09-15 13:09:25 +0300
commitff9d99f63ddb795f543a82dcf5d148da5a4cf072 (patch)
tree274a60d0ceda5a8a5b9aa45745c476b41c420509
parent37572551d76d69ea89c3ea7a3c0298e89a5cccd8 (diff)
Stop cloning unnecessarily on PR update (#12839) (#12852)
Backport #12839 Fix #12740 Signed-off-by: Andrew Thornton <art27@cantab.net>
-rw-r--r--services/pull/pull.go79
1 files changed, 19 insertions, 60 deletions
diff --git a/services/pull/pull.go b/services/pull/pull.go
index 373962a2d..963535ec7 100644
--- a/services/pull/pull.go
+++ b/services/pull/pull.go
@@ -9,8 +9,6 @@ import (
"bytes"
"context"
"fmt"
- "os"
- "path"
"strings"
"time"
@@ -177,18 +175,6 @@ func checkForInvalidation(requests models.PullRequestList, repoID int64, doer *m
return nil
}
-func addHeadRepoTasks(prs []*models.PullRequest) {
- for _, pr := range prs {
- log.Trace("addHeadRepoTasks[%d]: composing new test task", pr.ID)
- if err := PushToBaseRepo(pr); err != nil {
- log.Error("PushToBaseRepo: %v", err)
- continue
- }
-
- AddToTaskQueue(pr)
- }
-}
-
// AddTestPullRequestTask adds new test tasks by given head/base repository and head/base branch,
// and generate new patch for testing as needed.
func AddTestPullRequestTask(doer *models.User, repoID int64, branch string, isSync bool, oldCommitID, newCommitID string) {
@@ -245,7 +231,15 @@ func AddTestPullRequestTask(doer *models.User, repoID int64, branch string, isSy
}
}
- addHeadRepoTasks(prs)
+ for _, pr := range prs {
+ log.Trace("Updating PR[%d]: composing new test task", pr.ID)
+ if err := PushToBaseRepo(pr); err != nil {
+ log.Error("PushToBaseRepo: %v", err)
+ continue
+ }
+
+ AddToTaskQueue(pr)
+ }
log.Trace("AddTestPullRequestTask [base_repo_id: %d, base_branch: %s]: finding pull requests", repoID, branch)
prs, err = models.GetUnmergedPullRequestsByBaseInfo(repoID, branch)
@@ -345,54 +339,17 @@ func checkIfPRContentChanged(pr *models.PullRequest, oldCommitID, newCommitID st
func PushToBaseRepo(pr *models.PullRequest) (err error) {
log.Trace("PushToBaseRepo[%d]: pushing commits to base repo '%s'", pr.BaseRepoID, pr.GetGitRefName())
- // Clone base repo.
- tmpBasePath, err := models.CreateTemporaryPath("pull")
- if err != nil {
- log.Error("CreateTemporaryPath: %v", err)
- return err
- }
- defer func() {
- err := models.RemoveTemporaryPath(tmpBasePath)
- if err != nil {
- log.Error("Error whilst removing temporary path: %s Error: %v", tmpBasePath, err)
- }
- }()
-
if err := pr.LoadHeadRepo(); err != nil {
log.Error("Unable to load head repository for PR[%d] Error: %v", pr.ID, err)
return err
}
headRepoPath := pr.HeadRepo.RepoPath()
- if err := git.Clone(headRepoPath, tmpBasePath, git.CloneRepoOptions{
- Bare: true,
- Shared: true,
- Branch: pr.HeadBranch,
- Quiet: true,
- }); err != nil {
- log.Error("git clone tmpBasePath: %v", err)
- return err
- }
- gitRepo, err := git.OpenRepository(tmpBasePath)
- if err != nil {
- return fmt.Errorf("OpenRepository: %v", err)
- }
-
if err := pr.LoadBaseRepo(); err != nil {
log.Error("Unable to load base repository for PR[%d] Error: %v", pr.ID, err)
return err
}
- if err := gitRepo.AddRemote("base", pr.BaseRepo.RepoPath(), false); err != nil {
- return fmt.Errorf("tmpGitRepo.AddRemote: %v", err)
- }
- defer gitRepo.Close()
-
- headFile := pr.GetGitRefName()
-
- // Remove head in case there is a conflict.
- file := path.Join(pr.BaseRepo.RepoPath(), headFile)
-
- _ = os.Remove(file)
+ baseRepoPath := pr.BaseRepo.RepoPath()
if err = pr.LoadIssue(); err != nil {
return fmt.Errorf("unable to load issue %d for pr %d: %v", pr.IssueID, pr.ID, err)
@@ -401,24 +358,26 @@ func PushToBaseRepo(pr *models.PullRequest) (err error) {
return fmt.Errorf("unable to load poster %d for pr %d: %v", pr.Issue.PosterID, pr.ID, err)
}
- if err = git.Push(tmpBasePath, git.PushOptions{
- Remote: "base",
- Branch: fmt.Sprintf("%s:%s", pr.HeadBranch, headFile),
+ gitRefName := pr.GetGitRefName()
+
+ if err := git.Push(headRepoPath, git.PushOptions{
+ Remote: baseRepoPath,
+ Branch: pr.HeadBranch + ":" + gitRefName,
Force: true,
// Use InternalPushingEnvironment here because we know that pre-receive and post-receive do not run on a refs/pulls/...
Env: models.InternalPushingEnvironment(pr.Issue.Poster, pr.BaseRepo),
}); err != nil {
if git.IsErrPushOutOfDate(err) {
// This should not happen as we're using force!
- log.Error("Unable to push PR head for %s#%d (%-v:%s) due to ErrPushOfDate: %v", pr.BaseRepo.FullName(), pr.Index, pr.BaseRepo, headFile, err)
+ log.Error("Unable to push PR head for %s#%d (%-v:%s) due to ErrPushOfDate: %v", pr.BaseRepo.FullName(), pr.Index, pr.BaseRepo, gitRefName, err)
return err
} else if git.IsErrPushRejected(err) {
rejectErr := err.(*git.ErrPushRejected)
- log.Info("Unable to push PR head for %s#%d (%-v:%s) due to rejection:\nStdout: %s\nStderr: %s\nError: %v", pr.BaseRepo.FullName(), pr.Index, pr.BaseRepo, headFile, rejectErr.StdOut, rejectErr.StdErr, rejectErr.Err)
+ log.Info("Unable to push PR head for %s#%d (%-v:%s) due to rejection:\nStdout: %s\nStderr: %s\nError: %v", pr.BaseRepo.FullName(), pr.Index, pr.BaseRepo, gitRefName, rejectErr.StdOut, rejectErr.StdErr, rejectErr.Err)
return err
}
- log.Error("Unable to push PR head for %s#%d (%-v:%s) due to Error: %v", pr.BaseRepo.FullName(), pr.Index, pr.BaseRepo, headFile, err)
- return fmt.Errorf("Push: %s:%s %s:%s %v", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), headFile, err)
+ log.Error("Unable to push PR head for %s#%d (%-v:%s) due to Error: %v", pr.BaseRepo.FullName(), pr.Index, pr.BaseRepo, gitRefName, err)
+ return fmt.Errorf("Push: %s:%s %s:%s %v", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), gitRefName, err)
}
return nil