From 93364b9a51a74a5dce5ec279a1c801416f87e982 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 5 Jan 2020 20:52:25 +0000 Subject: [PATCH 01/14] Don't list as merged branches not at mergecommitid Once a branch has been merged if the commit ID no longer equals that of the merge commit id don't offer to delete the branch on the pull screen and don't list it as merged on branches. Fix #9201 --- routers/repo/issue.go | 19 +++++++++++++++++++ templates/repo/branch/list.tmpl | 6 ++++++ 2 files changed, 25 insertions(+) diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 46020acb6dfb..e61eef28f574 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -29,6 +29,7 @@ import ( comment_service "code.gitea.io/gitea/services/comments" issue_service "code.gitea.io/gitea/services/issue" pull_service "code.gitea.io/gitea/services/pull" + "gopkg.in/src-d/go-git.v4/plumbing" "github.com/unknwon/com" ) @@ -966,6 +967,24 @@ func ViewIssue(ctx *context.Context) { ctx.Data["IsBlockedByRejection"] = pull.ProtectedBranch.MergeBlockedByRejectedReview(pull) ctx.Data["GrantedApprovals"] = cnt } + if pull.HasMerged && pull.HeadRepo != nil { + // && git.IsBranchExist(pull.HeadRepo.RepoPath(), pull.HeadBranch) { + headRepo, err := git.OpenRepository(pull.HeadRepo.RepoPath()) + if err != nil { + ctx.ServerError("OpenRepository", err) + return + } + defer headRepo.Close() + commit, err := headRepo.GetBranchCommitID(pull.HeadBranch) + if err != nil && err != plumbing.ErrReferenceNotFound { + ctx.ServerError("GetBranchCommitID", err) + return + } else if err == nil && commit != pull.MergedCommitID { + // the head has moved on from the merge - we shouldn't delete + canDelete = false + } + + } ctx.Data["IsPullBranchDeletable"] = canDelete && pull.HeadRepo != nil && git.IsBranchExist(pull.HeadRepo.RepoPath(), pull.HeadBranch) ctx.Data["PullReviewers"], err = models.GetReviewersByIssueID(issue.ID) diff --git a/templates/repo/branch/list.tmpl b/templates/repo/branch/list.tmpl index 8fb365d8055a..be852b4cc3df 100644 --- a/templates/repo/branch/list.tmpl +++ b/templates/repo/branch/list.tmpl @@ -84,6 +84,12 @@ {{end}} + {{else if and .LatestPullRequest.HasMerged (ne .LatestPullRequest.MergedCommitID .Commit.ID.String)}} + {{if and (not .IsDeleted) $.AllowsPulls (gt .CommitsAhead 0)}} + + + + {{end}} {{else}} #{{.LatestPullRequest.Issue.Index}} {{if .LatestPullRequest.HasMerged}} From 4af63951953f4646f824df79a5f7150cdbbc453b Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 5 Jan 2020 21:38:17 +0000 Subject: [PATCH 02/14] Need to use the refs/pull/x/head --- routers/repo/branch.go | 42 +++++++++++++++++++++++++++++++++ routers/repo/issue.go | 22 +++++++++++++---- templates/repo/branch/list.tmpl | 2 +- 3 files changed, 60 insertions(+), 6 deletions(-) diff --git a/routers/repo/branch.go b/routers/repo/branch.go index b0a1efc5b92b..305eb0ac42be 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 @@ -213,11 +215,50 @@ func loadBranches(ctx *context.Context) []*Branch { ctx.ServerError("GetLatestPullRequestByHeadInfo", err) return nil } + mergeMovedOn := false if pr != nil { if err := pr.LoadIssue(); err != nil { ctx.ServerError("pr.LoadIssue", err) return nil } + if err := pr.LoadBaseRepo(); err != nil { + ctx.ServerError("pr.LoadBaseRepo", err) + return nil + } + if err := pr.LoadHeadRepo(); err != nil { + ctx.ServerError("pr.LoadHeadRepo", err) + return nil + } + if pr.HasMerged && pr.HeadRepo != nil { + headRepo, err := git.OpenRepository(pr.HeadRepo.RepoPath()) + if err != nil { + ctx.ServerError("OpenRepository", err) + return nil + } + defer headRepo.Close() + baseRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath()) + if err != nil { + ctx.ServerError("OpenRepository", err) + return nil + } + defer baseRepo.Close() + pullCommit, err := baseRepo.GetRefCommitID(pr.GetGitRefName()) + if err != nil && err != plumbing.ErrReferenceNotFound { + ctx.ServerError("GetBranchCommitID", err) + return nil + } + if err == nil { + headCommit, err := headRepo.GetBranchCommitID(pr.HeadBranch) + if err != nil && err != plumbing.ErrReferenceNotFound { + ctx.ServerError("GetBranchCommitID", err) + return nil + } else 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 +271,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 e61eef28f574..e7040049a029 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -975,15 +975,27 @@ func ViewIssue(ctx *context.Context) { return } defer headRepo.Close() - commit, err := headRepo.GetBranchCommitID(pull.HeadBranch) + baseRepo, err := git.OpenRepository(pull.BaseRepo.RepoPath()) + if err != nil { + ctx.ServerError("OpenRepository", err) + return + } + defer baseRepo.Close() + pullCommit, err := baseRepo.GetRefCommitID(pull.GetGitRefName()) if err != nil && err != plumbing.ErrReferenceNotFound { ctx.ServerError("GetBranchCommitID", err) return - } else if err == nil && commit != pull.MergedCommitID { - // the head has moved on from the merge - we shouldn't delete - canDelete = false } - + if err == nil { + headCommit, err := headRepo.GetBranchCommitID(pull.HeadBranch) + if err != nil && err != plumbing.ErrReferenceNotFound { + ctx.ServerError("GetBranchCommitID", err) + return + } else if err == nil && headCommit != pullCommit { + // the head has moved on from the merge - we shouldn't delete + canDelete = false + } + } } ctx.Data["IsPullBranchDeletable"] = canDelete && pull.HeadRepo != nil && git.IsBranchExist(pull.HeadRepo.RepoPath(), pull.HeadBranch) diff --git a/templates/repo/branch/list.tmpl b/templates/repo/branch/list.tmpl index be852b4cc3df..073516f25fe6 100644 --- a/templates/repo/branch/list.tmpl +++ b/templates/repo/branch/list.tmpl @@ -84,7 +84,7 @@ {{end}} - {{else if and .LatestPullRequest.HasMerged (ne .LatestPullRequest.MergedCommitID .Commit.ID.String)}} + {{else if and .LatestPullRequest.HasMerged .MergeMovedOn}} {{if and (not .IsDeleted) $.AllowsPulls (gt .CommitsAhead 0)}} From 84decfeb3f5ca7e222cddb5d3c7e6e1a892a0a5b Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 5 Jan 2020 21:45:18 +0000 Subject: [PATCH 03/14] Slight efficiency improvement --- routers/repo/branch.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/routers/repo/branch.go b/routers/repo/branch.go index 305eb0ac42be..4bd4314c3e39 100644 --- a/routers/repo/branch.go +++ b/routers/repo/branch.go @@ -221,11 +221,15 @@ func loadBranches(ctx *context.Context) []*Branch { ctx.ServerError("pr.LoadIssue", err) return nil } - if err := pr.LoadBaseRepo(); err != nil { + if pr.BaseRepoID == ctx.Repo.Repository.ID { + pr.HeadRepo = ctx.Repo.Repository + } else if err := pr.LoadBaseRepo(); err != nil { ctx.ServerError("pr.LoadBaseRepo", err) return nil } - if err := pr.LoadHeadRepo(); err != nil { + if pr.HeadRepoID == ctx.Repo.Repository.ID { + pr.HeadRepo = ctx.Repo.Repository + } else if err := pr.LoadHeadRepo(); err != nil { ctx.ServerError("pr.LoadHeadRepo", err) return nil } From 3db733553eb6a8cc9b25e452c4b8b9abe1476171 Mon Sep 17 00:00:00 2001 From: zeripath Date: Sun, 5 Jan 2020 21:56:18 +0000 Subject: [PATCH 04/14] Update routers/repo/branch.go --- routers/repo/branch.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/repo/branch.go b/routers/repo/branch.go index 4bd4314c3e39..6ef11a38454b 100644 --- a/routers/repo/branch.go +++ b/routers/repo/branch.go @@ -223,7 +223,7 @@ func loadBranches(ctx *context.Context) []*Branch { } if pr.BaseRepoID == ctx.Repo.Repository.ID { pr.HeadRepo = ctx.Repo.Repository - } else if err := pr.LoadBaseRepo(); err != nil { + } else if err := pr.LoadBaseRepo(); err != nil { ctx.ServerError("pr.LoadBaseRepo", err) return nil } From 6f8d95e5e848a42dc5d60da307a8a09b927b0186 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 6 Jan 2020 18:33:32 +0000 Subject: [PATCH 05/14] Slight efficiency improvement --- routers/repo/branch.go | 48 ++++++++++++++++++++++++++++++------------ 1 file changed, 34 insertions(+), 14 deletions(-) diff --git a/routers/repo/branch.go b/routers/repo/branch.go index 6ef11a38454b..e6855d24a346 100644 --- a/routers/repo/branch.go +++ b/routers/repo/branch.go @@ -187,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() @@ -221,31 +227,45 @@ func loadBranches(ctx *context.Context) []*Branch { ctx.ServerError("pr.LoadIssue", err) return nil } - if pr.BaseRepoID == ctx.Repo.Repository.ID { - pr.HeadRepo = ctx.Repo.Repository + if repo, ok := repoIDToRepo[pr.BaseRepoID]; ok { + pr.HeadRepo = repo } else if err := pr.LoadBaseRepo(); err != nil { ctx.ServerError("pr.LoadBaseRepo", err) return nil + } else { + repoIDToRepo[pr.BaseRepoID] = pr.BaseRepo } - if pr.HeadRepoID == ctx.Repo.Repository.ID { - pr.HeadRepo = ctx.Repo.Repository + + if repo, ok := repoIDToRepo[pr.HeadRepoID]; ok { + pr.HeadRepo = repo } else if err := pr.LoadHeadRepo(); err != nil { ctx.ServerError("pr.LoadHeadRepo", err) return nil + } else { + repoIDToRepo[pr.HeadRepoID] = pr.HeadRepo } + if pr.HasMerged && pr.HeadRepo != nil { - headRepo, err := git.OpenRepository(pr.HeadRepo.RepoPath()) - if err != nil { - ctx.ServerError("OpenRepository", err) - return nil + headRepo, ok := repoIDToGitRepo[pr.HeadRepoID] + if !ok { + headRepo, err = git.OpenRepository(pr.HeadRepo.RepoPath()) + if err != nil { + ctx.ServerError("OpenRepository", err) + return nil + } + defer headRepo.Close() + repoIDToGitRepo[pr.HeadRepoID] = headRepo } - defer headRepo.Close() - baseRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath()) - if err != nil { - ctx.ServerError("OpenRepository", err) - return nil + baseRepo, ok := repoIDToGitRepo[pr.BaseRepoID] + if !ok { + baseRepo, err = git.OpenRepository(pr.BaseRepo.RepoPath()) + if err != nil { + ctx.ServerError("OpenRepository", err) + return nil + } + defer baseRepo.Close() + repoIDToGitRepo[pr.BaseRepoID] = baseRepo } - defer baseRepo.Close() pullCommit, err := baseRepo.GetRefCommitID(pr.GetGitRefName()) if err != nil && err != plumbing.ErrReferenceNotFound { ctx.ServerError("GetBranchCommitID", err) From ba13b0f6cb52e40ff8149189008c87c4f3e5005b Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 6 Jan 2020 20:38:27 +0000 Subject: [PATCH 06/14] Also fix #9158 --- routers/repo/issue.go | 1 - routers/repo/pull.go | 74 ++++++++++++--------- templates/repo/issue/view_content/pull.tmpl | 4 +- 3 files changed, 46 insertions(+), 33 deletions(-) diff --git a/routers/repo/issue.go b/routers/repo/issue.go index e7040049a029..b83f1c5865f4 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -968,7 +968,6 @@ func ViewIssue(ctx *context.Context) { ctx.Data["GrantedApprovals"] = cnt } if pull.HasMerged && pull.HeadRepo != nil { - // && git.IsBranchExist(pull.HeadRepo.RepoPath(), pull.HeadBranch) { headRepo, err := git.OpenRepository(pull.HeadRepo.RepoPath()) if err != nil { ctx.ServerError("OpenRepository", err) diff --git a/routers/repo/pull.go b/routers/repo/pull.go index 418c2e9438bd..a0326bbbf7a5 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,48 @@ 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) + } + + 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 + 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 { + 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/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl index cec10a620ea6..2dc76dcf2e38 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}} - {{if .IsPullBranchDeletable}} + {{if and .IsPullBranchDeletable ( not .IsPullRequestBroken )}}
{{$.i18n.Tr "repo.branch.delete" .HeadTarget}} @@ -105,7 +105,7 @@
{{$.i18n.Tr "repo.pulls.blocked_by_rejection"}} -
+
{{else if .Issue.PullRequest.IsChecking}}
From a9d65b830691451f061af4dd39abb4541b9ce215 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 6 Jan 2020 20:55:59 +0000 Subject: [PATCH 07/14] Further slight efficiency improvement --- routers/repo/issue.go | 35 ++++------------------------------- routers/repo/pull.go | 5 +++++ 2 files changed, 9 insertions(+), 31 deletions(-) diff --git a/routers/repo/issue.go b/routers/repo/issue.go index b83f1c5865f4..0a78e06b41ac 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -29,7 +29,6 @@ import ( comment_service "code.gitea.io/gitea/services/comments" issue_service "code.gitea.io/gitea/services/issue" pull_service "code.gitea.io/gitea/services/pull" - "gopkg.in/src-d/go-git.v4/plumbing" "github.com/unknwon/com" ) @@ -967,36 +966,10 @@ func ViewIssue(ctx *context.Context) { ctx.Data["IsBlockedByRejection"] = pull.ProtectedBranch.MergeBlockedByRejectedReview(pull) ctx.Data["GrantedApprovals"] = cnt } - if pull.HasMerged && pull.HeadRepo != nil { - headRepo, err := git.OpenRepository(pull.HeadRepo.RepoPath()) - if err != nil { - ctx.ServerError("OpenRepository", err) - return - } - defer headRepo.Close() - baseRepo, err := git.OpenRepository(pull.BaseRepo.RepoPath()) - if err != nil { - ctx.ServerError("OpenRepository", err) - return - } - defer baseRepo.Close() - pullCommit, err := baseRepo.GetRefCommitID(pull.GetGitRefName()) - if err != nil && err != plumbing.ErrReferenceNotFound { - ctx.ServerError("GetBranchCommitID", err) - return - } - if err == nil { - headCommit, err := headRepo.GetBranchCommitID(pull.HeadBranch) - if err != nil && err != plumbing.ErrReferenceNotFound { - ctx.ServerError("GetBranchCommitID", err) - return - } else if err == nil && headCommit != pullCommit { - // the head has moved on from the merge - we shouldn't delete - canDelete = false - } - } - } - 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 a0326bbbf7a5..1a5c4a036f24 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -381,6 +381,7 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare sha, err := baseGitRepo.GetRefCommitID(pull.GetGitRefName()) if err != nil { ctx.ServerError(fmt.Sprintf("GetRefCommitID(%s)", pull.GetGitRefName()), err) + return nil } commitStatuses, err := models.GetLatestCommitStatus(repo, sha, 0) @@ -405,6 +406,10 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare 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" From 4c40b8e866cfd75f3085f626c209fbd983d1fcdc Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 6 Jan 2020 21:37:03 +0000 Subject: [PATCH 08/14] Add refs/pull/3/head to fix the integration test --- .../gitea-repositories-meta/user2/repo1.git/refs/pull/3/head | 1 + 1 file changed, 1 insertion(+) create mode 100644 integrations/gitea-repositories-meta/user2/repo1.git/refs/pull/3/head 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 000000000000..f98a263be62f --- /dev/null +++ b/integrations/gitea-repositories-meta/user2/repo1.git/refs/pull/3/head @@ -0,0 +1 @@ +65f1bf27bc3bf70f64657658635e66094edbcb4d From d3a5b4b04f7d169e9b1560eed97862b965ebc6cc Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 6 Jan 2020 22:01:07 +0000 Subject: [PATCH 09/14] Alternative head to push --- .../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 | 2 +- 4 files changed, 1 insertion(+), 1 deletion(-) 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 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 index f98a263be62f..98593d653703 100644 --- a/integrations/gitea-repositories-meta/user2/repo1.git/refs/pull/3/head +++ b/integrations/gitea-repositories-meta/user2/repo1.git/refs/pull/3/head @@ -1 +1 @@ -65f1bf27bc3bf70f64657658635e66094edbcb4d +4a357436d925b5c974181ff12a994538ddc5a269 From 6ebbe30f0599728c837ff35c986f3668dc117d67 Mon Sep 17 00:00:00 2001 From: zeripath Date: Tue, 7 Jan 2020 09:17:14 +0000 Subject: [PATCH 10/14] Update routers/repo/branch.go Co-Authored-By: guillep2k <18600385+guillep2k@users.noreply.github.com> --- routers/repo/branch.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/repo/branch.go b/routers/repo/branch.go index e6855d24a346..4e2935143c92 100644 --- a/routers/repo/branch.go +++ b/routers/repo/branch.go @@ -228,7 +228,7 @@ func loadBranches(ctx *context.Context) []*Branch { return nil } if repo, ok := repoIDToRepo[pr.BaseRepoID]; ok { - pr.HeadRepo = repo + pr.BaseRepo = repo } else if err := pr.LoadBaseRepo(); err != nil { ctx.ServerError("pr.LoadBaseRepo", err) return nil From 705420a1afe60533db63edd9e97bcce9d45b9761 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 7 Jan 2020 09:24:11 +0000 Subject: [PATCH 11/14] Allow HeadRepo to be nil --- models/pull.go | 2 +- routers/repo/branch.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/models/pull.go b/models/pull.go index 9a8777aca303..f86892cbfc92 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 4e2935143c92..75fec0b8b32d 100644 --- a/routers/repo/branch.go +++ b/routers/repo/branch.go @@ -238,10 +238,10 @@ func loadBranches(ctx *context.Context) []*Branch { if repo, ok := repoIDToRepo[pr.HeadRepoID]; ok { pr.HeadRepo = repo - } else if err := pr.LoadHeadRepo(); err != nil { + } else if err := pr.LoadHeadRepo(); err != nil && !models.IsErrRepoNotExist(err) { ctx.ServerError("pr.LoadHeadRepo", err) return nil - } else { + } else if pr.HeadRepo != nil { repoIDToRepo[pr.HeadRepoID] = pr.HeadRepo } From d6d2f87c87ea07559dd1ec543a667f89553d3db9 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 7 Jan 2020 09:34:18 +0000 Subject: [PATCH 12/14] More efficiency - in branch view we know that all of these PRs have the same head repo --- routers/repo/branch.go | 36 ++++++++++-------------------------- 1 file changed, 10 insertions(+), 26 deletions(-) diff --git a/routers/repo/branch.go b/routers/repo/branch.go index 75fec0b8b32d..42c2356dc1ee 100644 --- a/routers/repo/branch.go +++ b/routers/repo/branch.go @@ -221,6 +221,9 @@ func loadBranches(ctx *context.Context) []*Branch { ctx.ServerError("GetLatestPullRequestByHeadInfo", err) return nil } + pr.HeadRepo = ctx.Repo.Repository + headGitRepo := ctx.Repo.GitRepo + mergeMovedOn := false if pr != nil { if err := pr.LoadIssue(); err != nil { @@ -236,43 +239,24 @@ func loadBranches(ctx *context.Context) []*Branch { repoIDToRepo[pr.BaseRepoID] = pr.BaseRepo } - if repo, ok := repoIDToRepo[pr.HeadRepoID]; ok { - pr.HeadRepo = repo - } else if err := pr.LoadHeadRepo(); err != nil && !models.IsErrRepoNotExist(err) { - ctx.ServerError("pr.LoadHeadRepo", err) - return nil - } else if pr.HeadRepo != nil { - repoIDToRepo[pr.HeadRepoID] = pr.HeadRepo - } - - if pr.HasMerged && pr.HeadRepo != nil { - headRepo, ok := repoIDToGitRepo[pr.HeadRepoID] - if !ok { - headRepo, err = git.OpenRepository(pr.HeadRepo.RepoPath()) - if err != nil { - ctx.ServerError("OpenRepository", err) - return nil - } - defer headRepo.Close() - repoIDToGitRepo[pr.HeadRepoID] = headRepo - } - baseRepo, ok := repoIDToGitRepo[pr.BaseRepoID] + if pr.HasMerged { + baseGitRepo, ok := repoIDToGitRepo[pr.BaseRepoID] if !ok { - baseRepo, err = git.OpenRepository(pr.BaseRepo.RepoPath()) + baseGitRepo, err = git.OpenRepository(pr.BaseRepo.RepoPath()) if err != nil { ctx.ServerError("OpenRepository", err) return nil } - defer baseRepo.Close() - repoIDToGitRepo[pr.BaseRepoID] = baseRepo + defer baseGitRepo.Close() + repoIDToGitRepo[pr.BaseRepoID] = baseGitRepo } - pullCommit, err := baseRepo.GetRefCommitID(pr.GetGitRefName()) + pullCommit, err := baseGitRepo.GetRefCommitID(pr.GetGitRefName()) if err != nil && err != plumbing.ErrReferenceNotFound { ctx.ServerError("GetBranchCommitID", err) return nil } if err == nil { - headCommit, err := headRepo.GetBranchCommitID(pr.HeadBranch) + headCommit, err := headGitRepo.GetBranchCommitID(pr.HeadBranch) if err != nil && err != plumbing.ErrReferenceNotFound { ctx.ServerError("GetBranchCommitID", err) return nil From cf502acf9141ab9cac9c5c7186ab28a6c3f81ab7 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 7 Jan 2020 09:52:57 +0000 Subject: [PATCH 13/14] Further improvement - we already have the head commit - so use it --- routers/repo/branch.go | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/routers/repo/branch.go b/routers/repo/branch.go index 42c2356dc1ee..dfb25656b84d 100644 --- a/routers/repo/branch.go +++ b/routers/repo/branch.go @@ -222,8 +222,8 @@ func loadBranches(ctx *context.Context) []*Branch { return nil } pr.HeadRepo = ctx.Repo.Repository - headGitRepo := ctx.Repo.GitRepo - + headCommit := commit.ID.String() + mergeMovedOn := false if pr != nil { if err := pr.LoadIssue(); err != nil { @@ -255,15 +255,9 @@ func loadBranches(ctx *context.Context) []*Branch { ctx.ServerError("GetBranchCommitID", err) return nil } - if err == nil { - headCommit, err := headGitRepo.GetBranchCommitID(pr.HeadBranch) - if err != nil && err != plumbing.ErrReferenceNotFound { - ctx.ServerError("GetBranchCommitID", err) - return nil - } else if err == nil && headCommit != pullCommit { - // the head has moved on from the merge - we shouldn't delete - mergeMovedOn = true - } + if err == nil && headCommit != pullCommit { + // the head has moved on from the merge - we shouldn't delete + mergeMovedOn = true } } From d76f586490c26dda0609025b8215df08c5602da2 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 7 Jan 2020 12:01:08 +0000 Subject: [PATCH 14/14] Fix nil pointer dereference --- routers/repo/branch.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/repo/branch.go b/routers/repo/branch.go index dfb25656b84d..df6e0bf21f6c 100644 --- a/routers/repo/branch.go +++ b/routers/repo/branch.go @@ -221,11 +221,11 @@ func loadBranches(ctx *context.Context) []*Branch { ctx.ServerError("GetLatestPullRequestByHeadInfo", err) return nil } - pr.HeadRepo = ctx.Repo.Repository 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