simplify code add fix some nits

cmd/hook.go

@@ -487,12 +487,6 @@ Gitea or set your environment appropriately.`, "")
 		}
 	}
 
-	lf, err := os.OpenFile("test.log", os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0777)
-	if err != nil {
-		fail("Internal Server Error", "open log file failed: %v", err)
-	}
-	defer lf.Close()
-
 	if git.CheckGitVersionAtLeast("2.29") != nil {
 		fail("Internal Server Error", "git not support proc-receive.")
 	}

models/migrations/v169.go

@@ -14,8 +14,7 @@ func addAgitStylePullRequest(x *xorm.Engine) error {
 	type PullRequestStyle int
 
 	type PullRequest struct {
-		TopicBranch string
-		Style       PullRequestStyle
+		Style PullRequestStyle
 	}
 
 	if err := x.Sync2(new(PullRequest)); err != nil {

models/pull.go

@@ -68,8 +68,8 @@ type PullRequest struct {
 	BaseRepoID      int64       `xorm:"INDEX"`
 	BaseRepo        *Repository `xorm:"-"`
 	HeadBranch      string
+	HeadCommitID    string `xorm:"-"`
 	BaseBranch      string
-	TopicBranch     string           // use for agit style pr
 	ProtectedBranch *ProtectedBranch `xorm:"-"`
 	MergeBase       string           `xorm:"VARCHAR(40)"`
 
@@ -479,11 +479,11 @@ func newPullRequestAttempt(repo *Repository, pull *Issue, labelIDs []int64, uuid
 
 // GetUnmergedPullRequest returns a pull request that is open and has not been merged
 // by given head/base and repo/branch.
-func GetUnmergedPullRequest(headRepoID, baseRepoID int64, headBranch, baseBranch string) (*PullRequest, error) {
+func GetUnmergedPullRequest(headRepoID, baseRepoID int64, headBranch, baseBranch string, style PullRequestStyle) (*PullRequest, error) {
 	pr := new(PullRequest)
 	has, err := x.
 		Where("head_repo_id=? AND head_branch=? AND base_repo_id=? AND base_branch=? AND has_merged=? AND style = ? AND issue.is_closed=?",
-			headRepoID, headBranch, baseRepoID, baseBranch, false, PullRequestStyleGithub, false).
+			headRepoID, headBranch, baseRepoID, baseBranch, false, style, false).
 		Join("INNER", "issue", "issue.id=pull_request.issue_id").
 		Get(pr)
 	if err != nil {
@@ -495,30 +495,6 @@ func GetUnmergedPullRequest(headRepoID, baseRepoID int64, headBranch, baseBranch
 	return pr, nil
 }
 
-// GetUnmergedAGitStylePullRequest get unmerged agit style pull request
-func GetUnmergedAGitStylePullRequest(repoID int64, baseBranch, userName, topicBranch string) (*PullRequest, error) {
-	pr := new(PullRequest)
-
-	headBranch := topicBranch
-	userName = strings.ToLower(userName)
-	if !strings.HasPrefix(topicBranch, userName+"/") {
-		headBranch = userName + "/" + topicBranch
-	}
-
-	has, err := x.
-		Where("head_repo_id=? AND topic_branch=? AND base_repo_id=? AND base_branch=? AND has_merged=? AND style = ? AND issue.is_closed=?",
-			repoID, headBranch, repoID, baseBranch, false, PullRequestStyleAGit, false).
-		Join("INNER", "issue", "issue.id=pull_request.issue_id").
-		Get(pr)
-	if err != nil {
-		return nil, err
-	} else if !has {
-		return nil, ErrPullRequestNotExist{0, 0, repoID, repoID, headBranch, baseBranch}
-	}
-
-	return pr, nil
-}
-
 // GetLatestPullRequestByHeadInfo returns the latest pull request (regardless of its status)
 // by given head information (repo and branch).
 func GetLatestPullRequestByHeadInfo(repoID int64, branch string) (*PullRequest, error) {

models/pull_test.go

@@ -92,11 +92,11 @@ func TestPullRequestsOldest(t *testing.T) {
 
 func TestGetUnmergedPullRequest(t *testing.T) {
 	assert.NoError(t, PrepareTestDatabase())
-	pr, err := GetUnmergedPullRequest(1, 1, "branch2", "master")
+	pr, err := GetUnmergedPullRequest(1, 1, "branch2", "master", PullRequestStyleGithub)
 	assert.NoError(t, err)
 	assert.Equal(t, int64(2), pr.ID)
 
-	_, err = GetUnmergedPullRequest(1, 9223372036854775807, "branch1", "master")
+	_, err = GetUnmergedPullRequest(1, 9223372036854775807, "branch1", "master", PullRequestStyleGithub)
 	assert.Error(t, err)
 	assert.True(t, IsErrPullRequestNotExist(err))
 }

modules/git/repo_branch.go

@@ -40,6 +40,19 @@ type Branch struct {
 	gitRepo *Repository
 }
 
+// GetRefCommitID returns the last commit ID string of given reference (branch or tag).
+func GetRefCommitID(repoPath, name string) (string, error) {
+	stdout, err := NewCommand("show-ref", "--verify", "--hash", name).RunInDir(repoPath)
+	if err != nil {
+		if strings.Contains(err.Error(), "not a valid ref") {
+			return "", ErrNotExist{name, ""}
+		}
+		return "", err
+	}
+
+	return strings.TrimSpace(stdout), nil
+}
+
 // GetHEADBranch returns corresponding branch of HEAD.
 func (repo *Repository) GetHEADBranch() (*Branch, error) {
 	if repo == nil {

routers/api/v1/repo/pull.go

@@ -307,7 +307,7 @@ func CreatePullRequest(ctx *context.APIContext) {
 	defer headGitRepo.Close()
 
 	// Check if another PR exists with the same targets
-	existingPr, err := models.GetUnmergedPullRequest(headRepo.ID, ctx.Repo.Repository.ID, headBranch, baseBranch)
+	existingPr, err := models.GetUnmergedPullRequest(headRepo.ID, ctx.Repo.Repository.ID, headBranch, baseBranch, models.PullRequestStyleGithub)
 	if err != nil {
 		if !models.IsErrPullRequestNotExist(err) {
 			ctx.Error(http.StatusInternalServerError, "GetUnmergedPullRequest", err)

routers/private/hook.go

@@ -541,7 +541,7 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) {
 				continue
 			}
 
-			pr, err := models.GetUnmergedPullRequest(repo.ID, baseRepo.ID, branch, baseRepo.DefaultBranch)
+			pr, err := models.GetUnmergedPullRequest(repo.ID, baseRepo.ID, branch, baseRepo.DefaultBranch, models.PullRequestStyleGithub)
 			if err != nil && !models.IsErrPullRequestNotExist(err) {
 				log.Error("Failed to get active PR in: %-v Branch: %s to: %-v Branch: %s Error: %v", repo, branch, baseRepo, baseRepo.DefaultBranch, err)
 				ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{
@@ -646,7 +646,15 @@ func HookProcReceive(ctx *gitea_context.PrivateContext) {
 			}
 		}
 
-		pr, err := models.GetUnmergedAGitStylePullRequest(repo.ID, baseBranchName, opts.UserName, topicBranch)
+		headBranch := ""
+		userName := strings.ToLower(opts.UserName)
+		if !strings.HasPrefix(topicBranch, userName+"/") {
+			headBranch = userName + "/" + topicBranch
+		} else {
+			headBranch = topicBranch
+		}
+
+		pr, err := models.GetUnmergedPullRequest(repo.ID, repo.ID, headBranch, baseBranchName, models.PullRequestStyleAGit)
 		if err != nil {
 			if !models.IsErrPullRequestNotExist(err) {
 				log.Error("Failed to get unmerged agit style pull request in repository: %s/%s Error: %v", ownerName, repoName, err)
@@ -692,25 +700,17 @@ func HookProcReceive(ctx *gitea_context.PrivateContext) {
 				Content:  description,
 			}
 
-			headBranch := ""
-			userName := strings.ToLower(opts.UserName)
-			if !strings.HasPrefix(topicBranch, userName+"/") {
-				headBranch = userName + "/" + topicBranch
-			} else {
-				headBranch = topicBranch
-			}
-
 			pr := &models.PullRequest{
-				HeadRepoID:  repo.ID,
-				BaseRepoID:  repo.ID,
-				HeadBranch:  opts.NewCommitIDs[i],
-				BaseBranch:  baseBranchName,
-				TopicBranch: headBranch,
-				HeadRepo:    repo,
-				BaseRepo:    repo,
-				MergeBase:   "",
-				Type:        models.PullRequestGitea,
-				Style:       models.PullRequestStyleAGit,
+				HeadRepoID:   repo.ID,
+				BaseRepoID:   repo.ID,
+				HeadBranch:   headBranch,
+				HeadCommitID: opts.NewCommitIDs[i],
+				BaseBranch:   baseBranchName,
+				HeadRepo:     repo,
+				BaseRepo:     repo,
+				MergeBase:    "",
+				Type:         models.PullRequestGitea,
+				Style:        models.PullRequestStyleAGit,
 			}
 
 			if err := pull_service.NewPullRequest(repo, prIssue, []int64{}, []string{}, pr, []int64{}); err != nil {
@@ -734,24 +734,32 @@ func HookProcReceive(ctx *gitea_context.PrivateContext) {
 		}
 
 		// update exist pull request
-		old := pr.HeadBranch
-		pr.HeadBranch = opts.NewCommitIDs[i]
-		if err = pull_service.UpdateRef(pr); err != nil {
-			log.Error("Failed to update pull ref. Error: %v", err)
+		if err := pr.LoadBaseRepo(); err != nil {
+			log.Error("Unable to load base repository for PR[%d] Error: %v", pr.ID, err)
 			ctx.JSON(http.StatusInternalServerError, map[string]interface{}{
-				"Err": fmt.Sprintf("Failed to update pull ref. Error: %v", err),
+				"Err": fmt.Sprintf("Unable to load base repository for PR[%d] Error: %v", pr.ID, err),
 			})
 			return
 		}
 
-		err = pr.UpdateCols("head_branch")
+		old, err := git.GetRefCommitID(pr.BaseRepo.RepoPath(), pr.GetGitRefName())
 		if err != nil {
-			log.Error("Failed to update head commit. Error: %v", err)
+			log.Error("Unable to get ref commit id in base repository for PR[%d] Error: %v", pr.ID, err)
 			ctx.JSON(http.StatusInternalServerError, map[string]interface{}{
-				"Err": fmt.Sprintf("Failed to update head commit. Error: %v", err),
+				"Err": fmt.Sprintf("Unable to get ref commit id in base repository for PR[%d] Error: %v", pr.ID, err),
 			})
 			return
 		}
+
+		pr.HeadCommitID = opts.NewCommitIDs[i]
+		if err = pull_service.UpdateRef(pr); err != nil {
+			log.Error("Failed to update pull ref. Error: %v", err)
+			ctx.JSON(http.StatusInternalServerError, map[string]interface{}{
+				"Err": fmt.Sprintf("Failed to update pull ref. Error: %v", err),
+			})
+			return
+		}
+
 		pull_service.AddToTaskQueue(pr)
 		pusher, err := models.GetUserByID(opts.UserID)
 		if err != nil {

routers/repo/compare.go

@@ -548,7 +548,7 @@ func CompareDiff(ctx *context.Context) {
 		}
 		ctx.Data["HeadBranches"] = headBranches
 
-		pr, err := models.GetUnmergedPullRequest(headRepo.ID, ctx.Repo.Repository.ID, headBranch, baseBranch)
+		pr, err := models.GetUnmergedPullRequest(headRepo.ID, ctx.Repo.Repository.ID, headBranch, baseBranch, models.PullRequestStyleGithub)
 		if err != nil {
 			if !models.IsErrPullRequestNotExist(err) {
 				ctx.ServerError("GetUnmergedPullRequest", err)

routers/repo/issue.go

@@ -1987,7 +1987,7 @@ func NewComment(ctx *context.Context) {
 			if form.Status == "reopen" && issue.IsPull {
 				pull := issue.PullRequest
 				var err error
-				pr, err = models.GetUnmergedPullRequest(pull.HeadRepoID, pull.BaseRepoID, pull.HeadBranch, pull.BaseBranch)
+				pr, err = models.GetUnmergedPullRequest(pull.HeadRepoID, pull.BaseRepoID, pull.HeadBranch, pull.BaseBranch, pull.Style)
 				if err != nil {
 					if !models.IsErrPullRequestNotExist(err) {
 						ctx.ServerError("GetUnmergedPullRequest", err)
@@ -1997,6 +1997,7 @@ func NewComment(ctx *context.Context) {
 
 				// Regenerate patch and test conflict.
 				if pr == nil {
+					issue.PullRequest.HeadCommitID = ""
 					pull_service.AddToTaskQueue(issue.PullRequest)
 				}
 			}

routers/repo/pull.go

@@ -427,18 +427,18 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare
 		if pull.Style == models.PullRequestStyleGithub {
 			headBranchExist = headGitRepo.IsBranchExist(pull.HeadBranch)
 		} else {
-			headBranchExist = headGitRepo.IsCommitExist(pull.HeadBranch)
+			headBranchExist = git.IsReferenceExist(baseGitRepo.Path, pull.GetGitRefName())
 		}
 
 		if headBranchExist {
 			if pull.Style != models.PullRequestStyleGithub {
-				headBranchSha = pull.HeadBranch
+				headBranchSha, err = baseGitRepo.GetRefCommitID(pull.GetGitRefName())
 			} else {
 				headBranchSha, err = headGitRepo.GetBranchCommitID(pull.HeadBranch)
-				if err != nil {
-					ctx.ServerError("GetBranchCommitID", err)
-					return nil
-				}
+			}
+			if err != nil {
+				ctx.ServerError("GetBranchCommitID", err)
+				return nil
 			}
 		}
 	}

services/pull/commit_status.go

@@ -111,16 +111,18 @@ func GetPullRequestCommitStatusState(pr *models.PullRequest) (structs.CommitStat
 	if pr.Style == models.PullRequestStyleGithub && !headGitRepo.IsBranchExist(pr.HeadBranch) {
 		return "", errors.New("Head branch does not exist, can not merge")
 	}
-	if pr.Style != models.PullRequestStyleGithub && !headGitRepo.IsCommitExist(pr.HeadBranch) {
+	if pr.Style != models.PullRequestStyleGithub && !git.IsReferenceExist(headGitRepo.Path, pr.GetGitRefName()) {
 		return "", errors.New("Head branch does not exist, can not merge")
 	}
 
-	sha := pr.HeadBranch
+	sha := ""
 	if pr.Style == models.PullRequestStyleGithub {
 		sha, err = headGitRepo.GetBranchCommitID(pr.HeadBranch)
-		if err != nil {
-			return "", errors.Wrap(err, "GetBranchCommitID")
-		}
+	} else {
+		sha, err = headGitRepo.GetRefCommitID(pr.GetGitRefName())
+	}
+	if err != nil {
+		return "", err
 	}
 
 	if err := pr.LoadBaseRepo(); err != nil {

services/pull/pull.go

@@ -150,7 +150,7 @@ func ChangeTargetBranch(pr *models.PullRequest, doer *models.User, targetBranch
 	}
 
 	// Check if pull request for the new target branch already exists
-	existingPr, err := models.GetUnmergedPullRequest(pr.HeadRepoID, pr.BaseRepoID, pr.HeadBranch, targetBranch)
+	existingPr, err := models.GetUnmergedPullRequest(pr.HeadRepoID, pr.BaseRepoID, pr.HeadBranch, targetBranch, models.PullRequestStyleGithub)
 	if existingPr != nil {
 		return models.ErrPullRequestAlreadyExists{
 			ID:         existingPr.ID,
@@ -452,7 +452,7 @@ func UpdateRef(pr *models.PullRequest) (err error) {
 		return err
 	}
 
-	_, err = git.NewCommand("update-ref", pr.GetGitRefName(), pr.HeadBranch).RunInDir(pr.BaseRepo.RepoPath())
+	_, err = git.NewCommand("update-ref", pr.GetGitRefName(), pr.HeadCommitID).RunInDir(pr.BaseRepo.RepoPath())
 	if err != nil {
 		log.Error("Unable to update ref in base repository for PR[%d] Error: %v", pr.ID, err)
 	}
@@ -568,7 +568,12 @@ func GetSquashMergeCommitMessages(pr *models.PullRequest) string {
 	if pr.Style == models.PullRequestStyleGithub {
 		headCommit, err = gitRepo.GetBranchCommit(pr.HeadBranch)
 	} else {
-		headCommit, err = gitRepo.GetCommit(pr.HeadBranch)
+		pr.HeadCommitID, err = git.GetRefCommitID(gitRepo.Path, pr.GetGitRefName())
+		if err != nil {
+			log.Error("Unable to get head commit: %s Error: %v", pr.GetGitRefName(), err)
+			return ""
+		}
+		headCommit, err = gitRepo.GetCommit(pr.HeadCommitID)
 	}
 	if err != nil {
 		log.Error("Unable to get head commit: %s Error: %v", pr.HeadBranch, err)

services/pull/temp_repo.go

@@ -140,17 +140,20 @@ func createTemporaryRepo(pr *models.PullRequest) (string, error) {
 
 	trackingBranch := "tracking"
 	// Fetch head branch
-	// Note: for agit style pull request, the head branch is an commit id
-	headbanch := pr.HeadBranch
+	var headBranch string
 	if pr.Style == models.PullRequestStyleGithub {
-		headbanch = git.BranchPrefix + headbanch
+		headBranch = git.BranchPrefix + pr.HeadBranch
+	} else if len(pr.HeadCommitID) == 40 { // for not created pull request
+		headBranch = pr.HeadCommitID
+	} else {
+		headBranch = pr.GetGitRefName()
 	}
-	if err := git.NewCommand("fetch", "--no-tags", remoteRepoName, headbanch+":"+trackingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil {
-		log.Error("Unable to fetch head_repo head branch [%s:%s -> tracking in %s]: %v:\n%s\n%s", pr.HeadRepo.FullName(), headbanch, tmpBasePath, err, outbuf.String(), errbuf.String())
+	if err := git.NewCommand("fetch", "--no-tags", remoteRepoName, headBranch+":"+trackingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil {
+		log.Error("Unable to fetch head_repo head branch [%s:%s -> tracking in %s]: %v:\n%s\n%s", pr.HeadRepo.FullName(), headBranch, tmpBasePath, err, outbuf.String(), errbuf.String())
 		if err := models.RemoveTemporaryPath(tmpBasePath); err != nil {
 			log.Error("CreateTempRepo: RemoveTemporaryPath: %s", err)
 		}
-		return "", fmt.Errorf("Unable to fetch head_repo head branch [%s:%s -> tracking in tmpBasePath]: %v\n%s\n%s", pr.HeadRepo.FullName(), headbanch, err, outbuf.String(), errbuf.String())
+		return "", fmt.Errorf("Unable to fetch head_repo head branch [%s:%s -> tracking in tmpBasePath]: %v\n%s\n%s", pr.HeadRepo.FullName(), headBranch, err, outbuf.String(), errbuf.String())
 	}
 	outbuf.Reset()
 	errbuf.Reset()

services/pull/update.go

@@ -22,6 +22,11 @@ func Update(pull *models.PullRequest, doer *models.User, message string) error {
 		BaseBranch: pull.HeadBranch,
 	}
 
+	if pull.Style == models.PullRequestStyleAGit {
+		// TODO: Not support update agit style pull request's head branch
+		return fmt.Errorf("Not support update agit style pull request's head branch")
+	}
+
 	if err := pr.LoadHeadRepo(); err != nil {
 		log.Error("LoadHeadRepo: %v", err)
 		return fmt.Errorf("LoadHeadRepo: %v", err)
@@ -48,11 +53,8 @@ func Update(pull *models.PullRequest, doer *models.User, message string) error {
 
 // IsUserAllowedToUpdate check if user is allowed to update PR with given permissions and branch protections
 func IsUserAllowedToUpdate(pull *models.PullRequest, user *models.User) (bool, error) {
-	if pull.Style != models.PullRequestStyleGithub {
-		if err := pull.LoadIssue(); err != nil {
-			return false, err
-		}
-		return user != nil && user.ID == pull.Issue.PosterID, nil
+	if pull.Style == models.PullRequestStyleAGit {
+		return false, nil
 	}
 
 	headRepoPerm, err := models.GetUserRepoPermission(pull.HeadRepo, user)