Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support direct comparison (git diff a..b) as well merge comparison (a…b) #16635

Merged
merged 4 commits into from
Sep 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions modules/git/repo_compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (repo *Repository) GetMergeBase(tmpRemote string, base, head string) (strin
}

// GetCompareInfo generates and returns compare information between base and head branches of repositories.
func (repo *Repository) GetCompareInfo(basePath, baseBranch, headBranch string) (_ *CompareInfo, err error) {
func (repo *Repository) GetCompareInfo(basePath, baseBranch, headBranch string, directComparison bool) (_ *CompareInfo, err error) {
var (
remoteBranch string
tmpRemote string
Expand Down Expand Up @@ -79,8 +79,15 @@ func (repo *Repository) GetCompareInfo(basePath, baseBranch, headBranch string)
if err != nil {
compareInfo.BaseCommitID = remoteBranch
}
separator := "..."
baseCommitID := compareInfo.MergeBase
if directComparison {
separator = ".."
baseCommitID = compareInfo.BaseCommitID
}

// We have a common base - therefore we know that ... should work
logs, err := NewCommand("log", compareInfo.MergeBase+"..."+headBranch, prettyLogFormat).RunInDirBytes(repo.Path)
logs, err := NewCommand("log", baseCommitID+separator+headBranch, prettyLogFormat).RunInDirBytes(repo.Path)
if err != nil {
return nil, err
}
Expand All @@ -100,7 +107,7 @@ func (repo *Repository) GetCompareInfo(basePath, baseBranch, headBranch string)
// Count number of changed files.
// This probably should be removed as we need to use shortstat elsewhere
// Now there is git diff --shortstat but this appears to be slower than simply iterating with --nameonly
compareInfo.NumFiles, err = repo.GetDiffNumChangedFiles(remoteBranch, headBranch)
compareInfo.NumFiles, err = repo.GetDiffNumChangedFiles(remoteBranch, headBranch, directComparison)
if err != nil {
return nil, err
}
Expand All @@ -120,12 +127,17 @@ func (l *lineCountWriter) Write(p []byte) (n int, err error) {

// GetDiffNumChangedFiles counts the number of changed files
// This is substantially quicker than shortstat but...
func (repo *Repository) GetDiffNumChangedFiles(base, head string) (int, error) {
func (repo *Repository) GetDiffNumChangedFiles(base, head string, directComparison bool) (int, error) {
// Now there is git diff --shortstat but this appears to be slower than simply iterating with --nameonly
w := &lineCountWriter{}
stderr := new(bytes.Buffer)

if err := NewCommand("diff", "-z", "--name-only", base+"..."+head).
separator := "..."
if directComparison {
separator = ".."
}

if err := NewCommand("diff", "-z", "--name-only", base+separator+head).
RunInDirPipeline(repo.Path, w, stderr); err != nil {
if strings.Contains(stderr.String(), "no merge base") {
// git >= 2.28 now returns an error if base and head have become unrelated.
Expand Down
2 changes: 2 additions & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -1386,6 +1386,8 @@ pulls.compare_changes = New Pull Request
pulls.compare_changes_desc = Select the branch to merge into and the branch to pull from.
pulls.compare_base = merge into
pulls.compare_compare = pull from
pulls.switch_comparison_type = Switch comparison type
pulls.switch_head_and_base = Switch head and base
pulls.filter_branch = Filter branch
pulls.no_results = No results found.
pulls.nothing_to_compare = These branches are equal. There is no need to create a pull request.
Expand Down
6 changes: 3 additions & 3 deletions routers/api/v1/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -1006,7 +1006,7 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption)
return nil, nil, nil, nil, "", ""
}

compareInfo, err := headGitRepo.GetCompareInfo(models.RepoPath(baseRepo.Owner.Name, baseRepo.Name), baseBranch, headBranch)
compareInfo, err := headGitRepo.GetCompareInfo(models.RepoPath(baseRepo.Owner.Name, baseRepo.Name), baseBranch, headBranch, true)
if err != nil {
headGitRepo.Close()
ctx.Error(http.StatusInternalServerError, "GetCompareInfo", err)
Expand Down Expand Up @@ -1183,9 +1183,9 @@ func GetPullRequestCommits(ctx *context.APIContext) {
}
defer baseGitRepo.Close()
if pr.HasMerged {
prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.MergeBase, pr.GetGitRefName())
prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.MergeBase, pr.GetGitRefName(), true)
} else {
prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.GetGitRefName())
prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.GetGitRefName(), true)
}
if err != nil {
ctx.ServerError("GetCompareInfo", err)
Expand Down
3 changes: 2 additions & 1 deletion routers/web/repo/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,8 @@ func Diff(ctx *context.Context) {
diff, err := gitdiff.GetDiffCommitWithWhitespaceBehavior(gitRepo,
commitID, setting.Git.MaxGitDiffLines,
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles,
gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)))
gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)),
false)
if err != nil {
ctx.NotFound("GetDiffCommitWithWhitespaceBehavior", err)
return
Expand Down
Loading