From e5d8e2d10c6798e57ac855f71b025b087b664799 Mon Sep 17 00:00:00 2001
From: zeripath <art27@cantab.net>
Date: Tue, 7 Jan 2020 17:06:14 +0000
Subject: [PATCH] Branches not at ref commit ID should not be listed as Merged
(#9614)
Once a branch has been merged if the commit ID no longer equals that of
the pulls ref commit id don't offer to delete the branch on the pull screen
and don't list it as merged on branches.
Fix #9201
When looking at the pull page we should also get the commits from the refs/pulls/x/head
Fix #9158
---
.../00/750edc07d6415dcc07ae0351e9397b0222b7ba | Bin 0 -> 17 bytes
.../4a/357436d925b5c974181ff12a994538ddc5a269 | Bin 0 -> 840 bytes
.../dc/7a8ba127fee870dd683310ce660dfe59333a1b | Bin 0 -> 78 bytes
.../user2/repo1.git/refs/pull/3/head | 1 +
models/pull.go | 2 +-
routers/repo/branch.go | 44 +++++++++
routers/repo/issue.go | 5 +-
routers/repo/pull.go | 89 +++++++++++-------
templates/repo/branch/list.tmpl | 6 ++
templates/repo/issue/view_content/pull.tmpl | 4 +-
10 files changed, 112 insertions(+), 39 deletions(-)
create mode 100644 integrations/gitea-repositories-meta/user2/repo1.git/objects/00/750edc07d6415dcc07ae0351e9397b0222b7ba
create mode 100644 integrations/gitea-repositories-meta/user2/repo1.git/objects/4a/357436d925b5c974181ff12a994538ddc5a269
create mode 100644 integrations/gitea-repositories-meta/user2/repo1.git/objects/dc/7a8ba127fee870dd683310ce660dfe59333a1b
create mode 100644 integrations/gitea-repositories-meta/user2/repo1.git/refs/pull/3/head
diff --git a/integrations/gitea-repositories-meta/user2/repo1.git/objects/00/750edc07d6415dcc07ae0351e9397b0222b7ba b/integrations/gitea-repositories-meta/user2/repo1.git/objects/00/750edc07d6415dcc07ae0351e9397b0222b7ba
new file mode 100644
index 0000000000000000000000000000000000000000..d3c45d51ea5d755dabd6e35ce0322533b264abdd
GIT binary patch
literal 17
Ycmb<m^geacKgb}-;29GG9|w~@05+rq>Hq)$
literal 0
HcmV?d00001
diff --git a/integrations/gitea-repositories-meta/user2/repo1.git/objects/4a/357436d925b5c974181ff12a994538ddc5a269 b/integrations/gitea-repositories-meta/user2/repo1.git/objects/4a/357436d925b5c974181ff12a994538ddc5a269
new file mode 100644
index 0000000000000000000000000000000000000000..bf97d00fd85d30d1ffb0ee22437b8abcb917b27b
GIT binary patch
literal 840
zcmV-O1GoHm0d138ucAl*g!h?W(eGr3hHh@j&SVg2M3Do?QSpsS(*laf?I6E?o!!ja
z)MF)IrBX>{kNdqGfCyFe*U(W4@=Q&%G!Z4Wpj1;~o+}zcBFw0wz`UTcju1-3lxvfY
zHUm)PLQD%uO*51hDl8PN$f_c13X*>jI;MG=r8wu3akxG@F!r<)!9Pi!ceL-tpL9;{
z?TvoR9`_$WlvNF3RmTYM@Gb7`zUvLN14i=(zCiTOXog4gPUr?n{h1}rkfh%lI{blV
zE$d4L{{E$vWjh}5Z66#Q+cToi(E88k00+vzSyqOzGMSN+(z0N$OHzA&8T1~IXxI6k
z4C7ggTF8iT!%>%HhRN#Sx6gqVUdbg8gmlkuE_JfgmHqyBw1RPWIGxU?#k~j+@0^Dn
z*3|~j)0y-Uo~*~!HkLNhj~qyEnR77L%ERUfNw=66HCsSv)~2s%&Eu?P(MP0&nS<(t
zecI<+rMpEL`m*H6jFlk=qL&xMv181{sha@8cD;<ZRgz#Lx4bx1bFlU9BeeUIq2J9a
zgI~GTtkc%E(^`L>bq?&$j;0RBU?YvHUw`g>D3FcfeTKS!cWNSy7ccUyjRjk$m<qjr
zE(YFh4C#=Ek2u{d9PldTRP(L+0bdLYu|;;LzRKlfO<voT{N?iSUFpi6P@k^~%9dki
zobVi1kGQIzDE;7oT9@=<MiL*lxT6=Qvn@IZ-gsYWA$3=VRO(qN7q6FfORZx#TdFJH
z<3-k+xYtYT%7GlM`EkH;oetFy7L{=hF6eqt2=5*6<n5|)j)b{CKIdUr@ZqGgb!Y9X
zU>=pAtwWasW|-)NSh}**deiZ1VkRYt{nCrfWtr@op{<ZK?O9MArLjIgAFbqWr6wMe
zvNL^m%YoR<m{kw)yxUAqkGUQq%xFm;?1s$ss#C5SAz2H}i_`-Qc8<3>9`(xH?=NGy
zMPdEkq0G$6!FRDN6(#N?A>F8P77v$;G%M|E&wSU$%<-sF)T#GS1*npoN~SPn3t|%D
zKxXRO&#U6OH+0L?UhEh)H;Ho{UD_zvb)FM_1-&Mm;c#(zo1z{2&VldK>H~8Bf5!6G
Se|ii@-aqz3#Qh6B2Tz^`QMG>n
literal 0
HcmV?d00001
diff --git a/integrations/gitea-repositories-meta/user2/repo1.git/objects/dc/7a8ba127fee870dd683310ce660dfe59333a1b b/integrations/gitea-repositories-meta/user2/repo1.git/objects/dc/7a8ba127fee870dd683310ce660dfe59333a1b
new file mode 100644
index 0000000000000000000000000000000000000000..7678d6754dca80b05f1036065276db3f29e77b01
GIT binary patch
literal 78
zcmV-U0I~mg0V^p=O;s>6V=y!@Ff%bxFlJyV<-5av%`x^2`#R>pmzLE`O51lqC4*cY
kU3^{ja#I+*Jp$JT-p{I?uX?i5B(n0=>u8ht079G@Q{japW&i*H
literal 0
HcmV?d00001
diff --git a/integrations/gitea-repositories-meta/user2/repo1.git/refs/pull/3/head b/integrations/gitea-repositories-meta/user2/repo1.git/refs/pull/3/head
new file mode 100644
index 000000000..98593d653
--- /dev/null
+++ b/integrations/gitea-repositories-meta/user2/repo1.git/refs/pull/3/head
@@ -0,0 +1 @@
+4a357436d925b5c974181ff12a994538ddc5a269
diff --git a/models/pull.go b/models/pull.go
index 9a8777aca..f86892cbf 100644
--- a/models/pull.go
+++ b/models/pull.go
@@ -122,7 +122,7 @@ func (pr *PullRequest) LoadHeadRepo() error {
if has, err := x.ID(pr.HeadRepoID).Get(&repo); err != nil {
return err
} else if !has {
- return ErrRepoNotExist{ID: pr.BaseRepoID}
+ return ErrRepoNotExist{ID: pr.HeadRepoID}
}
pr.HeadRepo = &repo
}
diff --git a/routers/repo/branch.go b/routers/repo/branch.go
index b0a1efc5b..df6e0bf21 100644
--- a/routers/repo/branch.go
+++ b/routers/repo/branch.go
@@ -16,6 +16,7 @@ import (
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/repofiles"
"code.gitea.io/gitea/modules/util"
+ "gopkg.in/src-d/go-git.v4/plumbing"
)
const (
@@ -33,6 +34,7 @@ type Branch struct {
CommitsAhead int
CommitsBehind int
LatestPullRequest *models.PullRequest
+ MergeMovedOn bool
}
// Branches render repository branch page
@@ -185,6 +187,12 @@ func loadBranches(ctx *context.Context) []*Branch {
return nil
}
+ repoIDToRepo := map[int64]*models.Repository{}
+ repoIDToRepo[ctx.Repo.Repository.ID] = ctx.Repo.Repository
+
+ repoIDToGitRepo := map[int64]*git.Repository{}
+ repoIDToGitRepo[ctx.Repo.Repository.ID] = ctx.Repo.GitRepo
+
branches := make([]*Branch, len(rawBranches))
for i := range rawBranches {
commit, err := rawBranches[i].GetCommit()
@@ -213,11 +221,46 @@ func loadBranches(ctx *context.Context) []*Branch {
ctx.ServerError("GetLatestPullRequestByHeadInfo", err)
return nil
}
+ headCommit := commit.ID.String()
+
+ mergeMovedOn := false
if pr != nil {
+ pr.HeadRepo = ctx.Repo.Repository
if err := pr.LoadIssue(); err != nil {
ctx.ServerError("pr.LoadIssue", err)
return nil
}
+ if repo, ok := repoIDToRepo[pr.BaseRepoID]; ok {
+ pr.BaseRepo = repo
+ } else if err := pr.LoadBaseRepo(); err != nil {
+ ctx.ServerError("pr.LoadBaseRepo", err)
+ return nil
+ } else {
+ repoIDToRepo[pr.BaseRepoID] = pr.BaseRepo
+ }
+
+ if pr.HasMerged {
+ baseGitRepo, ok := repoIDToGitRepo[pr.BaseRepoID]
+ if !ok {
+ baseGitRepo, err = git.OpenRepository(pr.BaseRepo.RepoPath())
+ if err != nil {
+ ctx.ServerError("OpenRepository", err)
+ return nil
+ }
+ defer baseGitRepo.Close()
+ repoIDToGitRepo[pr.BaseRepoID] = baseGitRepo
+ }
+ pullCommit, err := baseGitRepo.GetRefCommitID(pr.GetGitRefName())
+ if err != nil && err != plumbing.ErrReferenceNotFound {
+ ctx.ServerError("GetBranchCommitID", err)
+ return nil
+ }
+ if err == nil && headCommit != pullCommit {
+ // the head has moved on from the merge - we shouldn't delete
+ mergeMovedOn = true
+ }
+ }
+
}
isIncluded := divergence.Ahead == 0 && ctx.Repo.Repository.DefaultBranch != branchName
@@ -230,6 +273,7 @@ func loadBranches(ctx *context.Context) []*Branch {
CommitsAhead: divergence.Ahead,
CommitsBehind: divergence.Behind,
LatestPullRequest: pr,
+ MergeMovedOn: mergeMovedOn,
}
}
diff --git a/routers/repo/issue.go b/routers/repo/issue.go
index 46020acb6..0a78e06b4 100644
--- a/routers/repo/issue.go
+++ b/routers/repo/issue.go
@@ -966,7 +966,10 @@ func ViewIssue(ctx *context.Context) {
ctx.Data["IsBlockedByRejection"] = pull.ProtectedBranch.MergeBlockedByRejectedReview(pull)
ctx.Data["GrantedApprovals"] = cnt
}
- ctx.Data["IsPullBranchDeletable"] = canDelete && pull.HeadRepo != nil && git.IsBranchExist(pull.HeadRepo.RepoPath(), pull.HeadBranch)
+ ctx.Data["IsPullBranchDeletable"] = canDelete &&
+ pull.HeadRepo != nil &&
+ git.IsBranchExist(pull.HeadRepo.RepoPath(), pull.HeadBranch) &&
+ (!pull.HasMerged || ctx.Data["HeadBranchCommitID"] == ctx.Data["PullHeadCommitID"])
ctx.Data["PullReviewers"], err = models.GetReviewersByIssueID(issue.ID)
if err != nil {
diff --git a/routers/repo/pull.go b/routers/repo/pull.go
index 418c2e943..1a5c4a036 100644
--- a/routers/repo/pull.go
+++ b/routers/repo/pull.go
@@ -330,25 +330,37 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare
repo := ctx.Repo.Repository
pull := issue.PullRequest
- var err error
- if err = pull.GetHeadRepo(); err != nil {
+ if err := pull.GetHeadRepo(); err != nil {
ctx.ServerError("GetHeadRepo", err)
return nil
}
+ if err := pull.GetBaseRepo(); err != nil {
+ ctx.ServerError("GetBaseRepo", err)
+ return nil
+ }
+
setMergeTarget(ctx, pull)
- if err = pull.LoadProtectedBranch(); err != nil {
+ if err := pull.LoadProtectedBranch(); err != nil {
ctx.ServerError("GetLatestCommitStatus", err)
return nil
}
ctx.Data["EnableStatusCheck"] = pull.ProtectedBranch != nil && pull.ProtectedBranch.EnableStatusCheck
- var headGitRepo *git.Repository
+ baseGitRepo, err := git.OpenRepository(pull.BaseRepo.RepoPath())
+ if err != nil {
+ ctx.ServerError("OpenRepository", err)
+ return nil
+ }
+ defer baseGitRepo.Close()
var headBranchExist bool
+ var headBranchSha string
// HeadRepo may be missing
if pull.HeadRepo != nil {
- headGitRepo, err = git.OpenRepository(pull.HeadRepo.RepoPath())
+ var err error
+
+ headGitRepo, err := git.OpenRepository(pull.HeadRepo.RepoPath())
if err != nil {
ctx.ServerError("OpenRepository", err)
return nil
@@ -358,46 +370,53 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare
headBranchExist = headGitRepo.IsBranchExist(pull.HeadBranch)
if headBranchExist {
- sha, err := headGitRepo.GetBranchCommitID(pull.HeadBranch)
+ headBranchSha, err = headGitRepo.GetBranchCommitID(pull.HeadBranch)
if err != nil {
ctx.ServerError("GetBranchCommitID", err)
return nil
}
-
- commitStatuses, err := models.GetLatestCommitStatus(repo, sha, 0)
- if err != nil {
- ctx.ServerError("GetLatestCommitStatus", err)
- return nil
- }
- if len(commitStatuses) > 0 {
- ctx.Data["LatestCommitStatuses"] = commitStatuses
- ctx.Data["LatestCommitStatus"] = models.CalcCommitStatus(commitStatuses)
- }
-
- if pull.ProtectedBranch != nil && pull.ProtectedBranch.EnableStatusCheck {
- ctx.Data["is_context_required"] = func(context string) bool {
- for _, c := range pull.ProtectedBranch.StatusCheckContexts {
- if c == context {
- return true
- }
- }
- return false
- }
- ctx.Data["IsRequiredStatusCheckSuccess"] = pull_service.IsCommitStatusContextSuccess(commitStatuses, pull.ProtectedBranch.StatusCheckContexts)
- }
}
}
- if pull.HeadRepo == nil || !headBranchExist {
- ctx.Data["IsPullRequestBroken"] = true
- ctx.Data["HeadTarget"] = "deleted"
- ctx.Data["NumCommits"] = 0
- ctx.Data["NumFiles"] = 0
+ sha, err := baseGitRepo.GetRefCommitID(pull.GetGitRefName())
+ if err != nil {
+ ctx.ServerError(fmt.Sprintf("GetRefCommitID(%s)", pull.GetGitRefName()), err)
return nil
}
- compareInfo, err := headGitRepo.GetCompareInfo(models.RepoPath(repo.Owner.Name, repo.Name),
- pull.BaseBranch, pull.HeadBranch)
+ commitStatuses, err := models.GetLatestCommitStatus(repo, sha, 0)
+ if err != nil {
+ ctx.ServerError("GetLatestCommitStatus", err)
+ return nil
+ }
+ if len(commitStatuses) > 0 {
+ ctx.Data["LatestCommitStatuses"] = commitStatuses
+ ctx.Data["LatestCommitStatus"] = models.CalcCommitStatus(commitStatuses)
+ }
+
+ if pull.ProtectedBranch != nil && pull.ProtectedBranch.EnableStatusCheck {
+ ctx.Data["is_context_required"] = func(context string) bool {
+ for _, c := range pull.ProtectedBranch.StatusCheckContexts {
+ if c == context {
+ return true
+ }
+ }
+ return false
+ }
+ ctx.Data["IsRequiredStatusCheckSuccess"] = pull_service.IsCommitStatusContextSuccess(commitStatuses, pull.ProtectedBranch.StatusCheckContexts)
+ }
+
+ ctx.Data["HeadBranchMovedOn"] = headBranchSha != sha
+ ctx.Data["HeadBranchCommitID"] = headBranchSha
+ ctx.Data["PullHeadCommitID"] = sha
+
+ if pull.HeadRepo == nil || !headBranchExist || headBranchSha != sha {
+ ctx.Data["IsPullRequestBroken"] = true
+ ctx.Data["HeadTarget"] = "deleted"
+ }
+
+ compareInfo, err := baseGitRepo.GetCompareInfo(pull.BaseRepo.RepoPath(),
+ pull.BaseBranch, pull.GetGitRefName())
if err != nil {
if strings.Contains(err.Error(), "fatal: Not a valid object name") {
ctx.Data["IsPullRequestBroken"] = true
diff --git a/templates/repo/branch/list.tmpl b/templates/repo/branch/list.tmpl
index 8fb365d80..073516f25 100644
--- a/templates/repo/branch/list.tmpl
+++ b/templates/repo/branch/list.tmpl
@@ -84,6 +84,12 @@
<button id="new-pull-request" class="ui compact basic button">{{$.i18n.Tr "repo.pulls.compare_changes"}}</button>
</a>
{{end}}
+ {{else if and .LatestPullRequest.HasMerged .MergeMovedOn}}
+ {{if and (not .IsDeleted) $.AllowsPulls (gt .CommitsAhead 0)}}
+ <a href="{{$.RepoLink}}/compare/{{$.DefaultBranch | EscapePound}}...{{if ne $.Repository.Owner.Name $.Owner.Name}}{{$.Owner.Name}}:{{end}}{{.Name | EscapePound}}">
+ <button id="new-pull-request" class="ui compact basic button">{{$.i18n.Tr "repo.pulls.compare_changes"}}</button>
+ </a>
+ {{end}}
{{else}}
<a href="{{$.RepoLink}}/pulls/{{.LatestPullRequest.Issue.Index}}">#{{.LatestPullRequest.Issue.Index}}</a>
{{if .LatestPullRequest.HasMerged}}
diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl
index cec10a620..2dc76dcf2 100644
--- a/templates/repo/issue/view_content/pull.tmpl
+++ b/templates/repo/issue/view_content/pull.tmpl
@@ -72,7 +72,7 @@
{{$.i18n.Tr "repo.pulls.reopen_to_merge"}}
{{end}}
</div>
- {{if .IsPullBranchDeletable}}
+ {{if and .IsPullBranchDeletable ( not .IsPullRequestBroken )}}
<div class="ui divider"></div>
<div>
<a class="delete-button ui red button" href="" data-url="{{.DeleteBranchLink}}">{{$.i18n.Tr "repo.branch.delete" .HeadTarget}}</a>
@@ -105,7 +105,7 @@
<div class="item text red">
<span class="octicon octicon-x"></span>
{{$.i18n.Tr "repo.pulls.blocked_by_rejection"}}
- </div>
+ </div>
{{else if .Issue.PullRequest.IsChecking}}
<div class="item text yellow">
<span class="octicon octicon-sync"></span>