From cc8a7c934550f4b18ec9f16ebe7342cf6e6e3b3a Mon Sep 17 00:00:00 2001 From: zeripath Date: Wed, 29 Jul 2020 17:42:22 +0100 Subject: [PATCH] Git 2.28 no longer permits diff with ... on unrelated branches (#12370) Backport #12364 Signed-off-by: Andrew Thornton --- modules/git/commit.go | 9 +++++---- modules/git/repo_commit.go | 26 ++++++++++++++++++++++++-- modules/git/repo_compare.go | 28 +++++++++++++++++++++++----- routers/private/hook.go | 2 ++ 4 files changed, 54 insertions(+), 11 deletions(-) diff --git a/modules/git/commit.go b/modules/git/commit.go index 5e492e27ea..5ba8c2c8af 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -271,11 +271,12 @@ func AllCommitsCount(repoPath string) (int64, error) { return strconv.ParseInt(strings.TrimSpace(stdout), 10, 64) } -func commitsCount(repoPath, revision, relpath string) (int64, error) { +func commitsCount(repoPath string, revision, relpath []string) (int64, error) { cmd := NewCommand("rev-list", "--count") - cmd.AddArguments(revision) + cmd.AddArguments(revision...) if len(relpath) > 0 { - cmd.AddArguments("--", relpath) + cmd.AddArguments("--") + cmd.AddArguments(relpath...) } stdout, err := cmd.RunInDir(repoPath) @@ -288,7 +289,7 @@ func commitsCount(repoPath, revision, relpath string) (int64, error) { // CommitsCount returns number of total commits of until given revision. func CommitsCount(repoPath, revision string) (int64, error) { - return commitsCount(repoPath, revision, "") + return commitsCount(repoPath, []string{revision}, []string{}) } // CommitsCount returns number of total commits of until current revision. diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index 18c732c6f2..bab3d8852c 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -293,7 +293,7 @@ func (repo *Repository) FileChangedBetweenCommits(filename, id1, id2 string) (bo // FileCommitsCount return the number of files at a revison func (repo *Repository) FileCommitsCount(revision, file string) (int64, error) { - return commitsCount(repo.Path, revision, file) + return commitsCount(repo.Path, []string{revision}, []string{file}) } // CommitsByFileAndRange return the commits according revison file and the page @@ -319,6 +319,11 @@ func (repo *Repository) CommitsByFileAndRangeNoFollow(revision, file string, pag // FilesCountBetween return the number of files changed between two commits func (repo *Repository) FilesCountBetween(startCommitID, endCommitID string) (int, error) { stdout, err := NewCommand("diff", "--name-only", startCommitID+"..."+endCommitID).RunInDir(repo.Path) + if err != nil && strings.Contains(err.Error(), "no merge base") { + // git >= 2.28 now returns an error if startCommitID and endCommitID have become unrelated. + // previously it would return the results of git diff --name-only startCommitID endCommitID so let's try that... + stdout, err = NewCommand("diff", "--name-only", startCommitID, endCommitID).RunInDir(repo.Path) + } if err != nil { return 0, err } @@ -333,6 +338,11 @@ func (repo *Repository) CommitsBetween(last *Commit, before *Commit) (*list.List stdout, err = NewCommand("rev-list", last.ID.String()).RunInDirBytes(repo.Path) } else { stdout, err = NewCommand("rev-list", before.ID.String()+"..."+last.ID.String()).RunInDirBytes(repo.Path) + if err != nil && strings.Contains(err.Error(), "no merge base") { + // future versions of git >= 2.28 are likely to return an error if before and last have become unrelated. + // previously it would return the results of git rev-list before last so let's try that... + stdout, err = NewCommand("rev-list", before.ID.String(), last.ID.String()).RunInDirBytes(repo.Path) + } } if err != nil { return nil, err @@ -348,6 +358,11 @@ func (repo *Repository) CommitsBetweenLimit(last *Commit, before *Commit, limit, stdout, err = NewCommand("rev-list", "--max-count", strconv.Itoa(limit), "--skip", strconv.Itoa(skip), last.ID.String()).RunInDirBytes(repo.Path) } else { stdout, err = NewCommand("rev-list", "--max-count", strconv.Itoa(limit), "--skip", strconv.Itoa(skip), before.ID.String()+"..."+last.ID.String()).RunInDirBytes(repo.Path) + if err != nil && strings.Contains(err.Error(), "no merge base") { + // future versions of git >= 2.28 are likely to return an error if before and last have become unrelated. + // previously it would return the results of git rev-list --max-count n before last so let's try that... + stdout, err = NewCommand("rev-list", "--max-count", strconv.Itoa(limit), "--skip", strconv.Itoa(skip), before.ID.String(), last.ID.String()).RunInDirBytes(repo.Path) + } } if err != nil { return nil, err @@ -373,7 +388,14 @@ func (repo *Repository) CommitsBetweenIDs(last, before string) (*list.List, erro // CommitsCountBetween return numbers of commits between two commits func (repo *Repository) CommitsCountBetween(start, end string) (int64, error) { - return commitsCount(repo.Path, start+"..."+end, "") + count, err := commitsCount(repo.Path, []string{start + "..." + end}, []string{}) + if err != nil && strings.Contains(err.Error(), "no merge base") { + // future versions of git >= 2.28 are likely to return an error if before and last have become unrelated. + // previously it would return the results of git rev-list before last so let's try that... + return commitsCount(repo.Path, []string{start, end}, []string{}) + } + + return count, err } // commitsBefore the limit is depth, not total number of returned commits. diff --git a/modules/git/repo_compare.go b/modules/git/repo_compare.go index 5bc7f9ca5a..16ea068db2 100644 --- a/modules/git/repo_compare.go +++ b/modules/git/repo_compare.go @@ -6,6 +6,7 @@ package git import ( + "bytes" "container/list" "fmt" "io" @@ -66,7 +67,7 @@ func (repo *Repository) GetCompareInfo(basePath, baseBranch, headBranch string) compareInfo := new(CompareInfo) compareInfo.MergeBase, remoteBranch, err = repo.GetMergeBase(tmpRemote, baseBranch, headBranch) if err == nil { - // We have a common base + // We have a common base - therefore we know that ... should work logs, err := NewCommand("log", compareInfo.MergeBase+"..."+headBranch, prettyLogFormat).RunInDirBytes(repo.Path) if err != nil { return nil, err @@ -85,6 +86,11 @@ func (repo *Repository) GetCompareInfo(basePath, baseBranch, headBranch string) // Count number of changed files. stdout, err := NewCommand("diff", "--name-only", remoteBranch+"..."+headBranch).RunInDir(repo.Path) + if err != nil && strings.Contains(err.Error(), "no merge base") { + // git >= 2.28 now returns an error if base and head have become unrelated. + // previously it would return the results of git diff --name-only base head so let's try that... + stdout, err = NewCommand("diff", "--name-only", remoteBranch, headBranch).RunInDir(repo.Path) + } if err != nil { return nil, err } @@ -108,12 +114,24 @@ func (repo *Repository) GetDiff(base, head string, w io.Writer) error { // GetPatch generates and returns format-patch data between given revisions. func (repo *Repository) GetPatch(base, head string, w io.Writer) error { - return NewCommand("format-patch", "--binary", "--stdout", base+"..."+head). - RunInDirPipeline(repo.Path, w, nil) + stderr := new(bytes.Buffer) + err := NewCommand("format-patch", "--binary", "--stdout", base+"..."+head). + RunInDirPipeline(repo.Path, w, stderr) + if err != nil && bytes.Contains(stderr.Bytes(), []byte("no merge base")) { + return NewCommand("format-patch", "--binary", "--stdout", base, head). + RunInDirPipeline(repo.Path, w, nil) + } + return err } // GetDiffFromMergeBase generates and return patch data from merge base to head func (repo *Repository) GetDiffFromMergeBase(base, head string, w io.Writer) error { - return NewCommand("diff", "-p", "--binary", base+"..."+head). - RunInDirPipeline(repo.Path, w, nil) + stderr := new(bytes.Buffer) + err := NewCommand("diff", "-p", "--binary", base+"..."+head). + RunInDirPipeline(repo.Path, w, stderr) + if err != nil && bytes.Contains(stderr.Bytes(), []byte("no merge base")) { + return NewCommand("diff", "-p", "--binary", base, head). + RunInDirPipeline(repo.Path, w, nil) + } + return err } diff --git a/routers/private/hook.go b/routers/private/hook.go index 4b57aff588..215793c970 100644 --- a/routers/private/hook.go +++ b/routers/private/hook.go @@ -39,6 +39,7 @@ func verifyCommits(oldCommitID, newCommitID string, repo *git.Repository, env [] _ = stdoutWriter.Close() }() + // This is safe as force pushes are already forbidden err = git.NewCommand("rev-list", oldCommitID+"..."+newCommitID). RunInDirTimeoutEnvFullPipelineFunc(env, -1, repo.Path, stdoutWriter, nil, nil, @@ -70,6 +71,7 @@ func checkFileProtection(oldCommitID, newCommitID string, patterns []glob.Glob, _ = stdoutWriter.Close() }() + // This use of ... is safe as force-pushes have already been ruled out. err = git.NewCommand("diff", "--name-only", oldCommitID+"..."+newCommitID). RunInDirTimeoutEnvFullPipelineFunc(env, -1, repo.Path, stdoutWriter, nil, nil,