Compare dropdown fails when selecting branch with no common merge-base (#37470)

modules/gitrepo/compare_test.go

@@ -4,9 +4,17 @@
 package gitrepo
 
 import (
+	"bytes"
+	"os"
+	"path/filepath"
+	"strings"
 	"testing"
+	"time"
+
+	"code.gitea.io/gitea/modules/git/gitcmd"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 type mockRepository struct {
@@ -17,6 +25,62 @@ func (r *mockRepository) RelativePath() string {
 	return r.path
 }
 
+func commitRootTree(t *testing.T, repoDir, fileName, content, message string) string {
+	t.Helper()
+
+	require.NoError(t, gitcmd.NewCommand("read-tree", "--empty").WithDir(repoDir).Run(t.Context()))
+
+	stdout, _, err := gitcmd.NewCommand("hash-object", "-w", "--stdin").
+		WithDir(repoDir).
+		WithStdinBytes([]byte(content)).
+		RunStdString(t.Context())
+	require.NoError(t, err)
+	blobSHA := strings.TrimSpace(stdout)
+
+	_, _, err = gitcmd.NewCommand("update-index", "--add", "--replace", "--cacheinfo").
+		AddDynamicArguments("100644", blobSHA, fileName).
+		WithDir(repoDir).
+		RunStdString(t.Context())
+	require.NoError(t, err)
+
+	stdout, _, err = gitcmd.NewCommand("write-tree").WithDir(repoDir).RunStdString(t.Context())
+	require.NoError(t, err)
+	treeSHA := strings.TrimSpace(stdout)
+
+	commitTimeStr := time.Now().Format(time.RFC3339)
+	env := append(os.Environ(),
+		"GIT_AUTHOR_NAME=Test",
+		"GIT_AUTHOR_EMAIL=test@example.com",
+		"GIT_AUTHOR_DATE="+commitTimeStr,
+		"GIT_COMMITTER_NAME=Test",
+		"GIT_COMMITTER_EMAIL=test@example.com",
+		"GIT_COMMITTER_DATE="+commitTimeStr,
+	)
+
+	messageBytes := bytes.NewBufferString(message + "\n")
+	stdout, _, err = gitcmd.NewCommand("commit-tree").AddDynamicArguments(treeSHA).
+		WithEnv(env).
+		WithDir(repoDir).
+		WithStdinBytes(messageBytes.Bytes()).
+		RunStdString(t.Context())
+	require.NoError(t, err)
+
+	return strings.TrimSpace(stdout)
+}
+
+func TestMergeBaseNoCommonHistory(t *testing.T) {
+	repoDir := filepath.Join(t.TempDir(), "repo.git")
+	require.NoError(t, gitcmd.NewCommand("init").AddDynamicArguments(repoDir).Run(t.Context()))
+
+	baseCommit := commitRootTree(t, repoDir, "base.txt", "base", "base")
+	headCommit := commitRootTree(t, repoDir, "head.txt", "head", "head")
+
+	mergeBase, err := MergeBase(t.Context(), &mockRepository{path: repoDir}, baseCommit, headCommit)
+	var noMergeBase ErrNoMergeBase
+	assert.Empty(t, mergeBase)
+	assert.ErrorAs(t, err, &noMergeBase)
+}
+
 func TestRepoGetDivergingCommits(t *testing.T) {
 	repo := &mockRepository{path: "repo1_bare"}
 	do, err := GetDivergingCommits(t.Context(), repo, "master", "branch2")

modules/gitrepo/merge.go

@@ -11,11 +11,32 @@ import (
 	"code.gitea.io/gitea/modules/git/gitcmd"
 )
 
+type ErrNoMergeBase struct {
+	BaseCommitID string
+	HeadCommitID string
+	Err          error
+}
+
+func (err ErrNoMergeBase) Error() string {
+	return fmt.Sprintf("get merge-base of %s and %s failed: %v", err.BaseCommitID, err.HeadCommitID, err.Err)
+}
+
+func (err ErrNoMergeBase) Unwrap() error {
+	return err.Err
+}
+
 // MergeBase checks and returns merge base of two commits.
 func MergeBase(ctx context.Context, repo Repository, baseCommitID, headCommitID string) (string, error) {
 	mergeBase, _, err := RunCmdString(ctx, repo, gitcmd.NewCommand("merge-base").
 		AddDashesAndList(baseCommitID, headCommitID))
 	if err != nil {
+		if gitcmd.IsErrorExitCode(err, 1) {
+			return "", ErrNoMergeBase{
+				BaseCommitID: baseCommitID,
+				HeadCommitID: headCommitID,
+				Err:          err,
+			}
+		}
 		return "", fmt.Errorf("get merge-base of %s and %s failed: %w", baseCommitID, headCommitID, err)
 	}
 	return strings.TrimSpace(mergeBase), nil

options/locale/locale_en-US.json

@@ -1781,6 +1781,7 @@
   "repo.pulls.review_only_possible_for_full_diff": "Review is only possible when viewing the full diff",
   "repo.pulls.filter_changes_by_commit": "Filter by commit",
   "repo.pulls.nothing_to_compare": "These branches are equal. There is no need to create a pull request.",
+  "repo.pulls.no_common_history": "These branches do not share a common merge base. Select a different base or compare branch.",
   "repo.pulls.nothing_to_compare_have_tag": "The selected branches/tags are equal.",
   "repo.pulls.nothing_to_compare_and_allow_empty_pr": "These branches are equal. This PR will be empty.",
   "repo.pulls.has_pull_request": "A pull request between these branches already exists: <a href=\"%[1]s\">%[2]s#%[3]d</a>",

routers/web/repo/compare.go

@@ -190,7 +190,7 @@ func setCsvCompareContext(ctx *context.Context) {
 }
 
 // ParseCompareInfo parse compare info between two commit for preparing comparing references
-func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo {
+func ParseCompareInfo(ctx *context.Context) (*git_service.CompareInfo, bool) {
 	baseRepo := ctx.Repo.Repository
 	fileOnly := ctx.FormBool("file-only")
 
@@ -200,21 +200,21 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo {
 	// remove the check when we support compare with carets
 	if compareReq.BaseOriRefSuffix != "" {
 		ctx.HTTPError(http.StatusBadRequest, "Unsupported comparison syntax: ref with suffix")
-		return nil
+		return nil, false
 	}
 
 	// 2 get repository and owner for head
 	headOwner, headRepo, err := common.GetHeadOwnerAndRepo(ctx, baseRepo, compareReq)
 	switch {
 	case errors.Is(err, util.ErrInvalidArgument):
 		ctx.HTTPError(http.StatusBadRequest, err.Error())
-		return nil
+		return nil, false
 	case errors.Is(err, util.ErrNotExist):
 		ctx.NotFound(nil)
-		return nil
+		return nil, false
 	case err != nil:
 		ctx.ServerError("GetHeadOwnerAndRepo", err)
-		return nil
+		return nil, false
 	}
 
 	isSameRepo := baseRepo.ID == headRepo.ID
@@ -229,7 +229,7 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo {
 		permHead, err := access_model.GetDoerRepoPermission(ctx, headRepo, ctx.Doer)
 		if err != nil {
 			ctx.ServerError("GetDoerRepoPermission", err)
-			return nil
+			return nil, false
 		}
 		if !permHead.CanRead(unit.TypeCode) {
 			if log.IsTrace() {
@@ -239,7 +239,7 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo {
 					permHead)
 			}
 			ctx.NotFound(nil)
-			return nil
+			return nil, false
 		}
 		ctx.Data["CanWriteToHeadRepo"] = permHead.CanWrite(unit.TypeCode)
 	}
@@ -251,7 +251,7 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo {
 	baseRef := ctx.Repo.GitRepo.UnstableGuessRefByShortName(baseRefName)
 	if baseRef == "" {
 		ctx.NotFound(nil)
-		return nil
+		return nil, false
 	}
 	var headGitRepo *git.Repository
 	if isSameRepo {
@@ -260,14 +260,14 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo {
 		headGitRepo, err = gitrepo.OpenRepository(ctx, headRepo)
 		if err != nil {
 			ctx.ServerError("OpenRepository", err)
-			return nil
+			return nil, false
 		}
 		defer headGitRepo.Close()
 	}
 	headRef := headGitRepo.UnstableGuessRefByShortName(headRefName)
 	if headRef == "" {
 		ctx.NotFound(nil)
-		return nil
+		return nil, false
 	}
 
 	ctx.Data["BaseName"] = baseRepo.OwnerName
@@ -294,7 +294,7 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo {
 		if err != nil {
 			if !repo_model.IsErrRepoNotExist(err) {
 				ctx.ServerError("Unable to find root repo", err)
-				return nil
+				return nil, false
 			}
 		} else {
 			rootRepo = baseRepo.BaseRepo
@@ -362,7 +362,7 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo {
 				branches, tags, err := getBranchesAndTagsForRepo(ctx, rootRepo)
 				if err != nil {
 					ctx.ServerError("GetBranchesForRepo", err)
-					return nil
+					return nil, false
 				}
 
 				ctx.Data["RootRepoBranches"] = branches
@@ -387,7 +387,7 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo {
 				branches, tags, err := getBranchesAndTagsForRepo(ctx, ownForkRepo)
 				if err != nil {
 					ctx.ServerError("GetBranchesForRepo", err)
-					return nil
+					return nil, false
 				}
 				ctx.Data["OwnForkRepoBranches"] = branches
 				ctx.Data["OwnForkRepoTags"] = tags
@@ -408,21 +408,24 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo {
 				permBase)
 		}
 		ctx.NotFound(nil)
-		return nil
+		return nil, false
 	}
 
 	compareInfo, err := git_service.GetCompareInfo(ctx, baseRepo, headRepo, headGitRepo, baseRef, headRef, compareReq.DirectComparison(), fileOnly)
 	if err != nil {
+		var noMergeBase gitrepo.ErrNoMergeBase
+		if errors.As(err, &noMergeBase) {
+			return compareInfo, true
+		}
 		ctx.ServerError("GetCompareInfo", err)
-		return nil
+		return nil, false
 	}
 	if compareReq.DirectComparison() {
 		ctx.Data["BeforeCommitID"] = compareInfo.BaseCommitID
 	} else {
 		ctx.Data["BeforeCommitID"] = compareInfo.MergeBase
 	}
-
-	return compareInfo
+	return compareInfo, false
 }
 
 func prepareNewPullRequestTitleContent(ci *git_service.CompareInfo, commits []*git_model.SignCommitWithStatuses) (title, content string) {
@@ -595,7 +598,7 @@ func getBranchesAndTagsForRepo(ctx gocontext.Context, repo *repo_model.Repositor
 
 // CompareDiff show different from one commit to another commit
 func CompareDiff(ctx *context.Context) {
-	ci := ParseCompareInfo(ctx)
+	ci, noMergeBase := ParseCompareInfo(ctx)
 	if ctx.Written() {
 		return
 	}
@@ -604,9 +607,16 @@ func CompareDiff(ctx *context.Context) {
 	ctx.Data["PullRequestWorkInProgressPrefixes"] = setting.Repository.PullRequest.WorkInProgressPrefixes
 	ctx.Data["CompareInfo"] = ci
 
-	nothingToCompare := PrepareCompareDiff(ctx, ci, gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)))
-	if ctx.Written() {
-		return
+	nothingToCompare := true
+	if noMergeBase {
+		ctx.Flash.Error(ctx.Tr("repo.pulls.no_common_history"), true)
+		ctx.Data["PageIsComparePull"] = false
+		ctx.Data["CommitCount"] = 0
+	} else {
+		nothingToCompare = PrepareCompareDiff(ctx, ci, gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)))
+		if ctx.Written() {
+			return
+		}
 	}
 
 	baseTags, err := repo_model.GetTagNamesByRepoID(ctx, ctx.Repo.Repository.ID)
@@ -622,29 +632,24 @@ func CompareDiff(ctx *context.Context) {
 		return
 	}
 
-	headBranches, err := git_model.FindBranchNames(ctx, git_model.FindBranchOptions{
-		RepoID:          ci.HeadRepo.ID,
-		ListOptions:     db.ListOptionsAll,
-		IsDeletedBranch: optional.Some(false),
-	})
+	headBranches, headTags, err := getBranchesAndTagsForRepo(ctx, ci.HeadRepo)
 	if err != nil {
-		ctx.ServerError("GetBranches", err)
+		ctx.ServerError("GetBranchesAndTagsForRepo", err)
 		return
 	}
 	ctx.Data["HeadBranches"] = headBranches
+	ctx.Data["HeadTags"] = headTags
 
 	// For compare repo branches
 	PrepareBranchList(ctx)
 	if ctx.Written() {
 		return
 	}
 
-	headTags, err := repo_model.GetTagNamesByRepoID(ctx, ci.HeadRepo.ID)
-	if err != nil {
-		ctx.ServerError("GetTagNamesByRepoID", err)
+	if noMergeBase {
+		ctx.HTML(http.StatusOK, tplCompare)
 		return
 	}
-	ctx.Data["HeadTags"] = headTags
 
 	if ctx.Data["PageIsComparePull"] == true {
 		pr, err := issues_model.GetUnmergedPullRequest(ctx, ci.HeadRepo.ID, ctx.Repo.Repository.ID, ci.HeadRef.ShortName(), ci.BaseRef.ShortName(), issues_model.PullRequestFlowGithub)

routers/web/repo/pull.go

@@ -1362,7 +1362,7 @@ func CompareAndPullRequestPost(ctx *context.Context) {
 		attachments []string
 	)
 
-	ci := ParseCompareInfo(ctx)
+	ci, _ := ParseCompareInfo(ctx)
 	if ctx.Written() {
 		return
 	}

templates/repo/diff/compare.tmpl

@@ -13,6 +13,7 @@
 				{{ctx.Locale.Tr "action.compare_commits_general"}}
 			{{end}}
 		</h2>
+		{{template "base/alert" .}}
 		{{$BaseCompareName := $.Repository.FullName -}}
 		{{$HeadCompareName := $.HeadRepo.FullName -}}
 		{{$OwnForkCompareName := "" -}}