diff --git a/modules/gitrepo/compare_test.go b/modules/gitrepo/compare_test.go index 2d2af0934d..3f7cc4f6a7 100644 --- a/modules/gitrepo/compare_test.go +++ b/modules/gitrepo/compare_test.go @@ -4,9 +4,18 @@ package gitrepo import ( + "bytes" + "os" + "path/filepath" + "strings" "testing" + "time" + + "code.gitea.io/gitea/modules/git/gitcmd" + "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) type mockRepository struct { @@ -17,6 +26,61 @@ 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) + assert.Empty(t, mergeBase) + assert.ErrorIs(t, err, util.ErrNotExist) +} + func TestRepoGetDivergingCommits(t *testing.T) { repo := &mockRepository{path: "repo1_bare"} do, err := GetDivergingCommits(t.Context(), repo, "master", "branch2") diff --git a/modules/gitrepo/merge.go b/modules/gitrepo/merge.go index 8d58e21c8d..92803da6fd 100644 --- a/modules/gitrepo/merge.go +++ b/modules/gitrepo/merge.go @@ -9,6 +9,7 @@ import ( "strings" "code.gitea.io/gitea/modules/git/gitcmd" + "code.gitea.io/gitea/modules/util" ) // MergeBase checks and returns merge base of two commits. @@ -16,6 +17,9 @@ func MergeBase(ctx context.Context, repo Repository, baseCommitID, headCommitID mergeBase, _, err := RunCmdString(ctx, repo, gitcmd.NewCommand("merge-base"). AddDashesAndList(baseCommitID, headCommitID)) if err != nil { + if gitcmd.IsErrorExitCode(err, 1) { + return "", util.NewNotExistErrorf("get merge-base of %s and %s failed", baseCommitID, headCommitID) + } return "", fmt.Errorf("get merge-base of %s and %s failed: %w", baseCommitID, headCommitID, err) } return strings.TrimSpace(mergeBase), nil diff --git a/options/locale/locale_en-US.json b/options/locale/locale_en-US.json index d6c733b50b..0d31e045c8 100644 --- a/options/locale/locale_en-US.json +++ b/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: %[2]s#%[3]d", diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index 733f7346f6..3f004cee88 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -413,6 +413,10 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo { compareInfo, err := git_service.GetCompareInfo(ctx, baseRepo, headRepo, headGitRepo, baseRef, headRef, compareReq.DirectComparison(), fileOnly) if err != nil { + if errors.Is(err, util.ErrNotExist) { + ctx.Data["IsNoMergeBase"] = true + return compareInfo + } ctx.ServerError("GetCompareInfo", err) return nil } @@ -603,9 +607,18 @@ 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 + var nothingToCompare bool + noMergeBase := ctx.Data["IsNoMergeBase"] == true + if noMergeBase { + ctx.Flash.Error(ctx.Tr("repo.pulls.no_common_history"), true) + ctx.Data["PageIsComparePull"] = false + ctx.Data["CommitCount"] = 0 + nothingToCompare = true + } 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) @@ -621,16 +634,13 @@ 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) @@ -638,12 +648,10 @@ func CompareDiff(ctx *context.Context) { 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) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 3933da8764..aa3e6b02b1 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1366,6 +1366,10 @@ func CompareAndPullRequestPost(ctx *context.Context) { if ctx.Written() { return } + if ctx.Data["IsNoMergeBase"] == true { + ctx.JSONError(ctx.Tr("repo.pulls.no_common_history")) + return + } validateRet := ValidateRepoMetasForNewIssue(ctx, *form, true) if ctx.Written() { diff --git a/services/git/compare.go b/services/git/compare.go index 251a035058..71bacfb6bd 100644 --- a/services/git/compare.go +++ b/services/git/compare.go @@ -76,7 +76,7 @@ func GetCompareInfo(ctx context.Context, baseRepo, headRepo *repo_model.Reposito if !directComparison { compareInfo.MergeBase, err = gitrepo.MergeBase(ctx, headRepo, compareInfo.BaseCommitID, compareInfo.HeadCommitID) if err != nil { - return nil, fmt.Errorf("MergeBase: %w", err) + return compareInfo, fmt.Errorf("MergeBase: %w", err) } } else { compareInfo.MergeBase = compareInfo.BaseCommitID diff --git a/templates/repo/diff/compare.tmpl b/templates/repo/diff/compare.tmpl index afd44f26a4..9ed4b73174 100644 --- a/templates/repo/diff/compare.tmpl +++ b/templates/repo/diff/compare.tmpl @@ -13,6 +13,7 @@ {{ctx.Locale.Tr "action.compare_commits_general"}} {{end}} + {{template "base/alert" .}} {{$BaseCompareName := $.Repository.FullName -}} {{$HeadCompareName := $.HeadRepo.FullName -}} {{$OwnForkCompareName := "" -}} diff --git a/tests/integration/compare_test.go b/tests/integration/compare_test.go index a3cb538d5b..f926f10696 100644 --- a/tests/integration/compare_test.go +++ b/tests/integration/compare_test.go @@ -4,19 +4,25 @@ package integration import ( + "bytes" "fmt" "net/http" "net/url" + "os" "strings" "testing" + "time" + repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/git/gitcmd" "code.gitea.io/gitea/modules/test" repo_service "code.gitea.io/gitea/services/repository" "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestCompareTag(t *testing.T) { @@ -124,6 +130,76 @@ func TestCompareBranches(t *testing.T) { inspectCompare(t, htmlDoc, diffCount, diffChanges) } +func createUnrelatedBranch(t *testing.T, repo *repo_model.Repository, user *user_model.User, branchName string) { + t.Helper() + + repoPath := repo_model.RepoPath(user.Name, repo.Name) + require.NoError(t, gitcmd.NewCommand("read-tree", "--empty").WithDir(repoPath).Run(t.Context())) + + stdout, _, err := gitcmd.NewCommand("hash-object", "-w", "--stdin"). + WithDir(repoPath). + WithStdinBytes([]byte("Unrelated File")). + RunStdString(t.Context()) + require.NoError(t, err) + blobSHA := strings.TrimSpace(stdout) + + _, _, err = gitcmd.NewCommand("update-index", "--add", "--replace", "--cacheinfo"). + AddDynamicArguments("100644", blobSHA, "unrelated.txt"). + WithDir(repoPath). + RunStdString(t.Context()) + require.NoError(t, err) + + stdout, _, err = gitcmd.NewCommand("write-tree").WithDir(repoPath).RunStdString(t.Context()) + require.NoError(t, err) + treeSHA := strings.TrimSpace(stdout) + + commitTimeStr := time.Now().Format(time.RFC3339) + doerSig := user.NewGitSig() + env := append(os.Environ(), + "GIT_AUTHOR_NAME="+doerSig.Name, + "GIT_AUTHOR_EMAIL="+doerSig.Email, + "GIT_AUTHOR_DATE="+commitTimeStr, + "GIT_COMMITTER_NAME="+doerSig.Name, + "GIT_COMMITTER_EMAIL="+doerSig.Email, + "GIT_COMMITTER_DATE="+commitTimeStr, + ) + + messageBytes := bytes.NewBufferString("Unrelated\n") + stdout, _, err = gitcmd.NewCommand("commit-tree").AddDynamicArguments(treeSHA). + WithEnv(env). + WithDir(repoPath). + WithStdinBytes(messageBytes.Bytes()). + RunStdString(t.Context()) + require.NoError(t, err) + commitSHA := strings.TrimSpace(stdout) + + _, _, err = gitcmd.NewCommand("branch").AddDynamicArguments(branchName, commitSHA). + WithDir(repoPath). + RunStdString(t.Context()) + require.NoError(t, err) +} + +func TestCompareBranchesNoCommonMergeBase(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user2"}) + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerID: user2.ID, Name: "repo1"}) + createUnrelatedBranch(t, repo1, user2, "unrelated-history") + + session := loginUser(t, "user2") + req := NewRequest(t, "GET", "/user2/repo1/compare/master...unrelated-history") + resp := session.MakeRequest(t, req, http.StatusOK) + body := resp.Body.String() + htmlDoc := NewHTMLParser(t, resp.Body) + + selection := htmlDoc.doc.Find(".ui.dropdown.select-branch") + assert.Lenf(t, selection.Nodes, 2, "The template has changed") + assert.Contains(t, body, "These branches do not share a common merge base") + assert.Equal(t, 1, htmlDoc.doc.Find(`a.item[href="/user2/repo1/compare/master...unrelated-history"]`).Length()) + assert.Equal(t, 1, htmlDoc.doc.Find(`a.item[href="/user2/repo1/compare/master...master"]`).Length()) + assert.Equal(t, 0, htmlDoc.doc.Find(".pullrequest-form").Length()) +} + func TestCompareCodeExpand(t *testing.T) { onGiteaRun(t, func(t *testing.T, u *url.URL) { user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})