Skip to content

Commit 2c25e75

Browse files
zeripathlafriks
zeripath
and
lafriks
authored
Various Merge Base fixes (#10786)
* Fix broken merge base migration v128 for merged PR * Allow PRs with deleted base branches to still show diff * as per @lunny Co-authored-by: Lauris BH <[email protected]>
1 parent 73cf0e2 commit 2c25e75

File tree

4 files changed

+148
-8
lines changed

4 files changed

+148
-8
lines changed

‎models/migrations/migrations.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,8 @@ var migrations = []Migration{
200200
NewMigration("Add Branch Protection Protected Files Column", addBranchProtectionProtectedFilesColumn),
201201
// v133 -> v134
202202
NewMigration("Add EmailHash Table", addEmailHashTable),
203+
// v134 -> v135
204+
NewMigration("Refix merge base for merged pull requests", refixMergeBase),
203205
}
204206

205207
// Migrate database to current version

‎models/migrations/v128.go

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func fixMergeBase(x *xorm.Engine) error {
5353
break
5454
}
5555

56-
i += 50
56+
i += len(prs)
5757
for _, pr := range prs {
5858
baseRepo := &Repository{ID: pr.BaseRepoID}
5959
has, err := x.Table("repository").Get(baseRepo)
@@ -81,10 +81,22 @@ func fixMergeBase(x *xorm.Engine) error {
8181
}
8282
}
8383
} else {
84-
var err error
85-
pr.MergeBase, err = git.NewCommand("merge-base", "--", pr.MergedCommitID+"^", gitRefName).RunInDir(repoPath)
84+
parentsString, err := git.NewCommand("rev-list", "--parents", "-n", "1", pr.MergedCommitID).RunInDir(repoPath)
85+
if err != nil {
86+
log.Error("Unable to get parents for merged PR ID %d, Index %d in %s/%s. Error: %v", pr.ID, pr.Index, baseRepo.OwnerName, baseRepo.Name, err)
87+
continue
88+
}
89+
parents := strings.Split(strings.TrimSpace(parentsString), " ")
90+
if len(parents) < 2 {
91+
continue
92+
}
93+
94+
args := append([]string{"merge-base", "--"}, parents[1:]...)
95+
args = append(args, gitRefName)
96+
97+
pr.MergeBase, err = git.NewCommand(args...).RunInDir(repoPath)
8698
if err != nil {
87-
log.Error("Unable to get merge base for merged PR ID %d, Index %d in %s/%s. Error: %", pr.ID, pr.Index, baseRepo.OwnerName, baseRepo.Name, err)
99+
log.Error("Unable to get merge base for merged PR ID %d, Index %d in %s/%s. Error: %v", pr.ID, pr.Index, baseRepo.OwnerName, baseRepo.Name, err)
88100
continue
89101
}
90102
}

‎models/migrations/v134.go

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
// Copyright 2020 The Gitea Authors. All rights reserved.
2+
// Use of this source code is governed by a MIT-style
3+
// license that can be found in the LICENSE file.
4+
5+
package migrations
6+
7+
import (
8+
"fmt"
9+
"path/filepath"
10+
"strings"
11+
12+
"code.gitea.io/gitea/modules/git"
13+
"code.gitea.io/gitea/modules/log"
14+
"code.gitea.io/gitea/modules/setting"
15+
16+
"xorm.io/xorm"
17+
)
18+
19+
func refixMergeBase(x *xorm.Engine) error {
20+
type Repository struct {
21+
ID int64 `xorm:"pk autoincr"`
22+
OwnerID int64 `xorm:"UNIQUE(s) index"`
23+
OwnerName string
24+
LowerName string `xorm:"UNIQUE(s) INDEX NOT NULL"`
25+
Name string `xorm:"INDEX NOT NULL"`
26+
}
27+
28+
type PullRequest struct {
29+
ID int64 `xorm:"pk autoincr"`
30+
Index int64
31+
HeadRepoID int64 `xorm:"INDEX"`
32+
BaseRepoID int64 `xorm:"INDEX"`
33+
HeadBranch string
34+
BaseBranch string
35+
MergeBase string `xorm:"VARCHAR(40)"`
36+
37+
HasMerged bool `xorm:"INDEX"`
38+
MergedCommitID string `xorm:"VARCHAR(40)"`
39+
}
40+
41+
var limit = setting.Database.IterateBufferSize
42+
if limit <= 0 {
43+
limit = 50
44+
}
45+
46+
i := 0
47+
for {
48+
prs := make([]PullRequest, 0, 50)
49+
if err := x.Limit(limit, i).Asc("id").Where("has_merged = ?", true).Find(&prs); err != nil {
50+
return fmt.Errorf("Find: %v", err)
51+
}
52+
if len(prs) == 0 {
53+
break
54+
}
55+
56+
i += len(prs)
57+
for _, pr := range prs {
58+
baseRepo := &Repository{ID: pr.BaseRepoID}
59+
has, err := x.Table("repository").Get(baseRepo)
60+
if err != nil {
61+
return fmt.Errorf("Unable to get base repo %d %v", pr.BaseRepoID, err)
62+
}
63+
if !has {
64+
log.Error("Missing base repo with id %d for PR ID %d", pr.BaseRepoID, pr.ID)
65+
continue
66+
}
67+
userPath := filepath.Join(setting.RepoRootPath, strings.ToLower(baseRepo.OwnerName))
68+
repoPath := filepath.Join(userPath, strings.ToLower(baseRepo.Name)+".git")
69+
70+
gitRefName := fmt.Sprintf("refs/pull/%d/head", pr.Index)
71+
72+
parentsString, err := git.NewCommand("rev-list", "--parents", "-n", "1", pr.MergedCommitID).RunInDir(repoPath)
73+
if err != nil {
74+
log.Error("Unable to get parents for merged PR ID %d, Index %d in %s/%s. Error: %v", pr.ID, pr.Index, baseRepo.OwnerName, baseRepo.Name, err)
75+
continue
76+
}
77+
parents := strings.Split(strings.TrimSpace(parentsString), " ")
78+
if len(parents) < 3 {
79+
continue
80+
}
81+
82+
// we should recalculate
83+
args := append([]string{"merge-base", "--"}, parents[1:]...)
84+
args = append(args, gitRefName)
85+
86+
pr.MergeBase, err = git.NewCommand(args...).RunInDir(repoPath)
87+
if err != nil {
88+
log.Error("Unable to get merge base for merged PR ID %d, Index %d in %s/%s. Error: %v", pr.ID, pr.Index, baseRepo.OwnerName, baseRepo.Name, err)
89+
continue
90+
}
91+
pr.MergeBase = strings.TrimSpace(pr.MergeBase)
92+
x.ID(pr.ID).Cols("merge_base").Update(pr)
93+
}
94+
}
95+
return nil
96+
}

‎routers/repo/pull.go

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,6 @@ func PrepareMergedViewPullInfo(ctx *context.Context, issue *models.Issue) *git.C
308308

309309
compareInfo, err := ctx.Repo.GitRepo.GetCompareInfo(ctx.Repo.Repository.RepoPath(),
310310
pull.MergeBase, pull.GetGitRefName())
311-
312311
if err != nil {
313312
if strings.Contains(err.Error(), "fatal: Not a valid object name") {
314313
ctx.Data["IsPullRequestBroken"] = true
@@ -360,9 +359,40 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare
360359
ctx.Data["IsPullRequestBroken"] = true
361360
ctx.Data["BaseTarget"] = pull.BaseBranch
362361
ctx.Data["HeadTarget"] = pull.HeadBranch
363-
ctx.Data["NumCommits"] = 0
364-
ctx.Data["NumFiles"] = 0
365-
return nil
362+
363+
sha, err := baseGitRepo.GetRefCommitID(pull.GetGitRefName())
364+
if err != nil {
365+
ctx.ServerError(fmt.Sprintf("GetRefCommitID(%s)", pull.GetGitRefName()), err)
366+
return nil
367+
}
368+
commitStatuses, err := models.GetLatestCommitStatus(repo, sha, 0)
369+
if err != nil {
370+
ctx.ServerError("GetLatestCommitStatus", err)
371+
return nil
372+
}
373+
if len(commitStatuses) > 0 {
374+
ctx.Data["LatestCommitStatuses"] = commitStatuses
375+
ctx.Data["LatestCommitStatus"] = models.CalcCommitStatus(commitStatuses)
376+
}
377+
378+
compareInfo, err := baseGitRepo.GetCompareInfo(pull.BaseRepo.RepoPath(),
379+
pull.MergeBase, pull.GetGitRefName())
380+
if err != nil {
381+
if strings.Contains(err.Error(), "fatal: Not a valid object name") {
382+
ctx.Data["IsPullRequestBroken"] = true
383+
ctx.Data["BaseTarget"] = pull.BaseBranch
384+
ctx.Data["NumCommits"] = 0
385+
ctx.Data["NumFiles"] = 0
386+
return nil
387+
}
388+
389+
ctx.ServerError("GetCompareInfo", err)
390+
return nil
391+
}
392+
393+
ctx.Data["NumCommits"] = compareInfo.Commits.Len()
394+
ctx.Data["NumFiles"] = compareInfo.NumFiles
395+
return compareInfo
366396
}
367397

368398
var headBranchExist bool

0 commit comments

Comments
 (0)