diff options
author | zeripath <art27@cantab.net> | 2020-01-07 17:06:14 +0000 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-01-07 17:06:14 +0000 |
commit | e5d8e2d10c6798e57ac855f71b025b087b664799 (patch) | |
tree | e00324f545d553aa1ef7ccf693b95577c5dd67a1 | |
parent | 940636863370855f39688ddf0aae41667ac8935c (diff) | |
download | gitea-e5d8e2d10c6798e57ac855f71b025b087b664799.tar.gz gitea-e5d8e2d10c6798e57ac855f71b025b087b664799.zip |
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
-rw-r--r-- | integrations/gitea-repositories-meta/user2/repo1.git/objects/00/750edc07d6415dcc07ae0351e9397b0222b7ba | bin | 0 -> 17 bytes | |||
-rw-r--r-- | integrations/gitea-repositories-meta/user2/repo1.git/objects/4a/357436d925b5c974181ff12a994538ddc5a269 | bin | 0 -> 840 bytes | |||
-rw-r--r-- | integrations/gitea-repositories-meta/user2/repo1.git/objects/dc/7a8ba127fee870dd683310ce660dfe59333a1b | bin | 0 -> 78 bytes | |||
-rw-r--r-- | integrations/gitea-repositories-meta/user2/repo1.git/refs/pull/3/head | 1 | ||||
-rw-r--r-- | models/pull.go | 2 | ||||
-rw-r--r-- | routers/repo/branch.go | 44 | ||||
-rw-r--r-- | routers/repo/issue.go | 5 | ||||
-rw-r--r-- | routers/repo/pull.go | 79 | ||||
-rw-r--r-- | templates/repo/branch/list.tmpl | 6 | ||||
-rw-r--r-- | templates/repo/issue/view_content/pull.tmpl | 4 |
10 files changed, 107 insertions, 34 deletions
diff --git a/integrations/gitea-repositories-meta/user2/repo1.git/objects/00/750edc07d6415dcc07ae0351e9397b0222b7ba b/integrations/gitea-repositories-meta/user2/repo1.git/objects/00/750edc07d6415dcc07ae0351e9397b0222b7ba Binary files differnew file mode 100644 index 0000000000..d3c45d51ea --- /dev/null +++ b/integrations/gitea-repositories-meta/user2/repo1.git/objects/00/750edc07d6415dcc07ae0351e9397b0222b7ba diff --git a/integrations/gitea-repositories-meta/user2/repo1.git/objects/4a/357436d925b5c974181ff12a994538ddc5a269 b/integrations/gitea-repositories-meta/user2/repo1.git/objects/4a/357436d925b5c974181ff12a994538ddc5a269 Binary files differnew file mode 100644 index 0000000000..bf97d00fd8 --- /dev/null +++ b/integrations/gitea-repositories-meta/user2/repo1.git/objects/4a/357436d925b5c974181ff12a994538ddc5a269 diff --git a/integrations/gitea-repositories-meta/user2/repo1.git/objects/dc/7a8ba127fee870dd683310ce660dfe59333a1b b/integrations/gitea-repositories-meta/user2/repo1.git/objects/dc/7a8ba127fee870dd683310ce660dfe59333a1b Binary files differnew file mode 100644 index 0000000000..7678d6754d --- /dev/null +++ b/integrations/gitea-repositories-meta/user2/repo1.git/objects/dc/7a8ba127fee870dd683310ce660dfe59333a1b 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 9a8777aca3..f86892cbfc 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 b0a1efc5b9..df6e0bf21f 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 46020acb6d..0a78e06b41 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 418c2e9438..1a5c4a036f 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) - } + sha, err := baseGitRepo.GetRefCommitID(pull.GetGitRefName()) + if err != nil { + ctx.ServerError(fmt.Sprintf("GetRefCommitID(%s)", pull.GetGitRefName()), err) + return nil + } - 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 + 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 } - ctx.Data["IsRequiredStatusCheckSuccess"] = pull_service.IsCommitStatusContextSuccess(commitStatuses, pull.ProtectedBranch.StatusCheckContexts) } + return false } + ctx.Data["IsRequiredStatusCheckSuccess"] = pull_service.IsCommitStatusContextSuccess(commitStatuses, pull.ProtectedBranch.StatusCheckContexts) } - if pull.HeadRepo == nil || !headBranchExist { + 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" - ctx.Data["NumCommits"] = 0 - ctx.Data["NumFiles"] = 0 - return nil } - compareInfo, err := headGitRepo.GetCompareInfo(models.RepoPath(repo.Owner.Name, repo.Name), - pull.BaseBranch, pull.HeadBranch) + 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 8fb365d805..073516f25f 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 cec10a620e..2dc76dcf2e 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> |