From ca306985d35f295fb2a2f8a54661731462426281 Mon Sep 17 00:00:00 2001 From: Lauris BH Date: Fri, 19 Jan 2018 08:18:51 +0200 Subject: [PATCH] Change how merged PR commit info are prepared (#3368) * Change how merged PR commits and diff are made * Update code.gitea.io/git dependency * Fix typo * Remove unneeded local variable --- models/pull.go | 11 ++- routers/repo/pull.go | 90 ++++++++++--------------- vendor/code.gitea.io/git/repo_commit.go | 8 +-- vendor/vendor.json | 6 +- 4 files changed, 49 insertions(+), 66 deletions(-) diff --git a/models/pull.go b/models/pull.go index 38312e4e4..5c54a2268 100644 --- a/models/pull.go +++ b/models/pull.go @@ -132,6 +132,11 @@ func (pr *PullRequest) GetDefaultSquashMessage() string { return fmt.Sprintf("%s (#%d)", pr.Issue.Title, pr.Issue.Index) } +// GetGitRefName returns git ref for hidden pull request branch +func (pr *PullRequest) GetGitRefName() string { + return fmt.Sprintf("refs/pull/%d/head", pr.Index) +} + // APIFormat assumes following fields have been assigned with valid values: // Required - Issue // Optional - Merger @@ -562,7 +567,7 @@ func (pr *PullRequest) getMergeCommit() (*git.Commit, error) { indexTmpPath := filepath.Join(os.TempDir(), "gitea-"+pr.BaseRepo.Name+"-"+strconv.Itoa(time.Now().Nanosecond())) defer os.Remove(indexTmpPath) - headFile := fmt.Sprintf("refs/pull/%d/head", pr.Index) + headFile := pr.GetGitRefName() // Check if a pull request is merged into BaseBranch _, stderr, err := process.GetManager().ExecDirEnv(-1, "", fmt.Sprintf("isMerged (git merge-base --is-ancestor): %d", pr.BaseRepo.ID), @@ -980,7 +985,7 @@ func (pr *PullRequest) UpdatePatch() (err error) { // corresponding branches of base repository. // FIXME: Only push branches that are actually updates? func (pr *PullRequest) PushToBaseRepo() (err error) { - log.Trace("PushToBaseRepo[%d]: pushing commits to base repo 'refs/pull/%d/head'", pr.BaseRepoID, pr.Index) + log.Trace("PushToBaseRepo[%d]: pushing commits to base repo '%s'", pr.BaseRepoID, pr.GetGitRefName()) headRepoPath := pr.HeadRepo.RepoPath() headGitRepo, err := git.OpenRepository(headRepoPath) @@ -995,7 +1000,7 @@ func (pr *PullRequest) PushToBaseRepo() (err error) { // Make sure to remove the remote even if the push fails defer headGitRepo.RemoveRemote(tmpRemoteName) - headFile := fmt.Sprintf("refs/pull/%d/head", pr.Index) + headFile := pr.GetGitRefName() // Remove head in case there is a conflict. file := path.Join(pr.BaseRepo.RepoPath(), headFile) diff --git a/routers/repo/pull.go b/routers/repo/pull.go index c06c67552..1db1e6327 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -253,40 +253,30 @@ func setMergeTarget(ctx *context.Context, pull *models.PullRequest) { } // PrepareMergedViewPullInfo show meta information for a merged pull request view page -func PrepareMergedViewPullInfo(ctx *context.Context, issue *models.Issue) { +func PrepareMergedViewPullInfo(ctx *context.Context, issue *models.Issue) *git.PullRequestInfo { pull := issue.PullRequest - var err error - if err = pull.GetHeadRepo(); err != nil { - ctx.ServerError("GetHeadRepo", err) - return - } - setMergeTarget(ctx, pull) ctx.Data["HasMerged"] = true - mergedCommit, err := ctx.Repo.GitRepo.GetCommit(pull.MergedCommitID) - if err != nil { - ctx.ServerError("GetCommit", err) - return - } - // the ID of the last commit in the PR (not including the merge commit) - endCommitID, err := mergedCommit.ParentID(mergedCommit.ParentCount() - 1) - if err != nil { - ctx.ServerError("ParentID", err) - return - } + prInfo, err := ctx.Repo.GitRepo.GetPullRequestInfo(ctx.Repo.Repository.RepoPath(), + pull.MergeBase, pull.GetGitRefName()) - ctx.Data["NumCommits"], err = ctx.Repo.GitRepo.CommitsCountBetween(pull.MergeBase, endCommitID.String()) if err != nil { - ctx.ServerError("Repo.GitRepo.CommitsCountBetween", err) - return - } - ctx.Data["NumFiles"], err = ctx.Repo.GitRepo.FilesCountBetween(pull.MergeBase, endCommitID.String()) - if err != nil { - ctx.ServerError("Repo.GitRepo.FilesCountBetween", err) - return + if strings.Contains(err.Error(), "fatal: Not a valid object name") { + ctx.Data["IsPullReuqestBroken"] = true + ctx.Data["BaseTarget"] = "deleted" + ctx.Data["NumCommits"] = 0 + ctx.Data["NumFiles"] = 0 + return nil + } + + ctx.ServerError("GetPullRequestInfo", err) + return nil } + ctx.Data["NumCommits"] = prInfo.Commits.Len() + ctx.Data["NumFiles"] = prInfo.NumFiles + return prInfo } // PrepareViewPullInfo show meta information for a pull request preview page @@ -351,28 +341,16 @@ func ViewPullCommits(ctx *context.Context) { var commits *list.List if pull.HasMerged { - PrepareMergedViewPullInfo(ctx, issue) + prInfo := PrepareMergedViewPullInfo(ctx, issue) if ctx.Written() { return + } else if prInfo == nil { + ctx.NotFound("ViewPullCommits", nil) + return } ctx.Data["Username"] = ctx.Repo.Owner.Name ctx.Data["Reponame"] = ctx.Repo.Repository.Name - - mergedCommit, err := ctx.Repo.GitRepo.GetCommit(pull.MergedCommitID) - if err != nil { - ctx.ServerError("Repo.GitRepo.GetCommit", err) - return - } - endCommitID, err := mergedCommit.ParentID(mergedCommit.ParentCount() - 1) - if err != nil { - ctx.ServerError("ParentID", err) - return - } - commits, err = ctx.Repo.GitRepo.CommitsBetweenIDs(endCommitID.String(), pull.MergeBase) - if err != nil { - ctx.ServerError("Repo.GitRepo.CommitsBetweenIDs", err) - return - } + commits = prInfo.Commits } else { prInfo := PrepareViewPullInfo(ctx, issue) if ctx.Written() { @@ -415,26 +393,26 @@ func ViewPullFiles(ctx *context.Context) { var headTarget string if pull.HasMerged { - PrepareMergedViewPullInfo(ctx, issue) + prInfo := PrepareMergedViewPullInfo(ctx, issue) if ctx.Written() { return + } else if prInfo == nil { + ctx.NotFound("ViewPullFiles", nil) + return } diffRepoPath = ctx.Repo.GitRepo.Path - startCommitID = pull.MergeBase - mergedCommit, err := ctx.Repo.GitRepo.GetCommit(pull.MergedCommitID) - if err != nil { - ctx.ServerError("GetCommit", err) - return - } - endCommitSha, err := mergedCommit.ParentID(mergedCommit.ParentCount() - 1) - if err != nil { - ctx.ServerError("ParentID", err) - return - } - endCommitID = endCommitSha.String() gitRepo = ctx.Repo.GitRepo + headCommitID, err := gitRepo.GetRefCommitID(pull.GetGitRefName()) + if err != nil { + ctx.ServerError("GetRefCommitID", err) + return + } + + startCommitID = prInfo.MergeBase + endCommitID = headCommitID + headTarget = path.Join(ctx.Repo.Owner.Name, ctx.Repo.Repository.Name) ctx.Data["Username"] = ctx.Repo.Owner.Name ctx.Data["Reponame"] = ctx.Repo.Repository.Name diff --git a/vendor/code.gitea.io/git/repo_commit.go b/vendor/code.gitea.io/git/repo_commit.go index 44ad8450f..56bebd7a3 100644 --- a/vendor/code.gitea.io/git/repo_commit.go +++ b/vendor/code.gitea.io/git/repo_commit.go @@ -11,8 +11,8 @@ import ( "strings" ) -// getRefCommitID returns the last commit ID string of given reference (branch or tag). -func (repo *Repository) getRefCommitID(name string) (string, error) { +// GetRefCommitID returns the last commit ID string of given reference (branch or tag). +func (repo *Repository) GetRefCommitID(name string) (string, error) { stdout, err := NewCommand("show-ref", "--verify", name).RunInDir(repo.Path) if err != nil { if strings.Contains(err.Error(), "not a valid ref") { @@ -25,12 +25,12 @@ func (repo *Repository) getRefCommitID(name string) (string, error) { // GetBranchCommitID returns last commit ID string of given branch. func (repo *Repository) GetBranchCommitID(name string) (string, error) { - return repo.getRefCommitID(BranchPrefix + name) + return repo.GetRefCommitID(BranchPrefix + name) } // GetTagCommitID returns last commit ID string of given tag. func (repo *Repository) GetTagCommitID(name string) (string, error) { - return repo.getRefCommitID(TagPrefix + name) + return repo.GetRefCommitID(TagPrefix + name) } // parseCommitData parses commit information from the (uncompressed) raw diff --git a/vendor/vendor.json b/vendor/vendor.json index 51f758fd2..e00816926 100644 --- a/vendor/vendor.json +++ b/vendor/vendor.json @@ -3,10 +3,10 @@ "ignore": "test appengine", "package": [ { - "checksumSHA1": "1WHdGmDRsFRTD5N69l+MEbZr+nM=", + "checksumSHA1": "Gz+a5Qo4PCiB/Gf2f02v8HEAxDM=", "path": "code.gitea.io/git", - "revision": "f4a91053671bee69f1995e456c1541668717c19d", - "revisionTime": "2018-01-07T06:11:05Z" + "revision": "6798d0f202cdc7187c00a467b586a4bdee27e8c9", + "revisionTime": "2018-01-14T14:37:32Z" }, { "checksumSHA1": "Qtq0kW+BnpYMOriaoCjMa86WGG8=",