From 319c92163f1fbe3554e65df71676d05d4b4fa797 Mon Sep 17 00:00:00 2001 From: zeripath Date: Wed, 8 Jan 2020 01:06:53 +0000 Subject: [PATCH] Branches not at ref commit ID should not be listed as Merged (#9614) (#9639) 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 | 2 +- 10 files changed, 111 insertions(+), 38 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 YcmbHq)$ 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;bq?&$j;0RBU?YvHUw`g>D3FcfeTKS!cWNSy7ccUyjRjk$m=pAtwWasW|-)NSh}**deiZ1VkRYt{nCrfWtr@op{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 0000000000..98593d6537 --- /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 f93257008b..b81f146371 100644 --- a/models/pull.go +++ b/models/pull.go @@ -136,7 +136,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 5d78518491..f57e76d494 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 ( @@ -32,6 +33,7 @@ type Branch struct { CommitsAhead int CommitsBehind int LatestPullRequest *models.PullRequest + MergeMovedOn bool } // Branches render repository branch page @@ -168,6 +170,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() @@ -196,11 +204,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 + } + } + } branches[i] = &Branch{ @@ -210,6 +253,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 40e3b140ec..8cbad47f2d 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -931,7 +931,10 @@ func ViewIssue(ctx *context.Context) { ctx.Data["IsBlockedByApprovals"] = pull.ProtectedBranch.RequiredApprovals > 0 && cnt < pull.ProtectedBranch.RequiredApprovals 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["PullReviewersWithType"], err = models.GetReviewersByPullID(issue.ID) if err != nil { diff --git a/routers/repo/pull.go b/routers/repo/pull.go index f46d093793..d3c17db4e7 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -315,25 +315,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 @@ -343,46 +355,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 9c53f4e67a..7707b3cf1d 100644 --- a/templates/repo/branch/list.tmpl +++ b/templates/repo/branch/list.tmpl @@ -80,6 +80,12 @@ {{end}} + {{else if and .LatestPullRequest.HasMerged .MergeMovedOn}} + {{if and (not .IsDeleted) $.AllowsPulls (gt .CommitsAhead 0)}} + + + + {{end}} {{else}} #{{.LatestPullRequest.Issue.Index}} {{if .LatestPullRequest.HasMerged}} diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl index f5ce8e0886..a5c6712996 100644 --- a/templates/repo/issue/view_content/pull.tmpl +++ b/templates/repo/issue/view_content/pull.tmpl @@ -71,7 +71,7 @@ {{$.i18n.Tr "repo.pulls.reopen_to_merge"}} {{end}} - {{if .IsPullBranchDeletable}} + {{if and .IsPullBranchDeletable ( not .IsPullRequestBroken )}}