Skip to content

Commit 3112efb

Browse files
committed
fix(pull): show changed files of pull requests without merge base
When head and base have no merge base, GetCompareInfo leaves CompareBase empty and viewPullFiles looked up an empty commit ID, so the files tab returned 404 (400 on older releases). Diff against the empty tree instead, and let gitdiff accept the empty tree as a diff base rather than loading it as a commit.
1 parent 1b2479b commit 3112efb

4 files changed

Lines changed: 54 additions & 5 deletions

File tree

routers/web/repo/pull.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -742,6 +742,9 @@ func viewPullFiles(ctx *context.Context, beforeCommitID, afterCommitID string) {
742742
return
743743
}
744744
beforeCommitID = beforeCommit.ID.String()
745+
} else if beforeCommitID == "" && prCompareInfo.CompareBase == "" {
746+
// no merge base (unrelated histories): the pull request brings in the whole head tree
747+
beforeCommitID = afterCommit.ID.Type().EmptyTree().String()
745748
} else {
746749
beforeCommitID = util.IfZero(beforeCommitID, prCompareInfo.CompareBase)
747750
beforeCommit = indexCommit(prCompareInfo.Commits, beforeCommitID)
@@ -753,10 +756,10 @@ func viewPullFiles(ctx *context.Context, beforeCommitID, afterCommitID string) {
753756
return
754757
}
755758
}
756-
}
757-
if beforeCommit == nil {
758-
ctx.NotFound(nil)
759-
return
759+
if beforeCommit == nil {
760+
ctx.NotFound(nil)
761+
return
762+
}
760763
}
761764

762765
ctx.Data["CompareInfo"] = prCompareInfo

services/gitdiff/git_diff_tree.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,10 @@ func validateGitDiffTreeArguments(ctx context.Context, gitRepo *git.Repository,
112112
return useMergeBase, baseCommit.ID.String(), headCommitID, nil
113113
}
114114

115+
if baseSha == headCommit.ID.Type().EmptyTree().String() {
116+
return false, baseSha, headCommitID, nil // the empty tree is not a commit, but a valid diff base
117+
}
118+
115119
// try and get the base commit
116120
baseCommit, err := gitRepo.GetCommit(ctx, baseSha)
117121
// propagate the error if we couldn't get the base commit

services/gitdiff/gitdiff.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1309,7 +1309,7 @@ func guessBeforeCommitForDiff(ctx context.Context, gitRepo *git.Repository, befo
13091309
commitObjectFormat := afterCommit.ID.Type()
13101310
isBeforeCommitIDEmpty := beforeCommitID == "" || beforeCommitID == commitObjectFormat.EmptyObjectID().String()
13111311

1312-
if isBeforeCommitIDEmpty && afterCommit.ParentCount() == 0 {
1312+
if (isBeforeCommitIDEmpty && afterCommit.ParentCount() == 0) || beforeCommitID == commitObjectFormat.EmptyTree().String() {
13131313
actualBeforeCommitID = commitObjectFormat.EmptyTree()
13141314
} else {
13151315
if isBeforeCommitIDEmpty {

tests/integration/pull_diff_test.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,22 @@
44
package integration
55

66
import (
7+
"fmt"
78
"net/http"
9+
"strings"
810
"testing"
911

12+
auth_model "gitea.dev/models/auth"
13+
repo_model "gitea.dev/models/repo"
14+
"gitea.dev/models/unittest"
15+
user_model "gitea.dev/models/user"
16+
"gitea.dev/modules/git/gitcmd"
17+
api "gitea.dev/modules/structs"
1018
"gitea.dev/tests"
1119

1220
"github.com/PuerkitoBio/goquery"
1321
"github.com/stretchr/testify/assert"
22+
"github.com/stretchr/testify/require"
1423
)
1524

1625
func TestPullDiff(t *testing.T) {
@@ -65,3 +74,36 @@ func testPullDiffAssertPage(t *testing.T, prDiffURL string, reviewBtnDisabled bo
6574
// Ensure the review button is enabled for full PR reviews
6675
assert.Equal(t, reviewBtnDisabled, doc.Find(".js-btn-review").HasClass("disabled"))
6776
}
77+
78+
func TestPullDiffNoCommonMergeBase(t *testing.T) {
79+
defer tests.PrepareTestEnv(t)()
80+
81+
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user2"})
82+
repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerID: user2.ID, Name: "repo1"})
83+
_, _, err := gitcmd.NewCommand("fast-import").WithRepo(repo1).WithStdinBytes([]byte(strings.TrimSpace(`
84+
commit refs/heads/unrelated-history
85+
committer User <user@example.com> 1714310400 +0000
86+
data 13
87+
Second commit
88+
M 100644 inline file2.txt
89+
data 12
90+
Hello from 2
91+
`))).RunStdString(t.Context())
92+
require.NoError(t, err)
93+
94+
// the compare page refuses branches without a merge base, but the API (and history rewrites) still produce such pull requests
95+
session := loginUser(t, "user2")
96+
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)
97+
req := NewRequestWithJSON(t, "POST", "/api/v1/repos/user2/repo1/pulls", &api.CreatePullRequestOption{
98+
Head: "unrelated-history",
99+
Base: "master",
100+
Title: "unrelated histories",
101+
}).AddTokenAuth(token)
102+
pr := DecodeJSON(t, MakeRequest(t, req, http.StatusCreated), &api.PullRequest{})
103+
104+
req = NewRequest(t, "GET", fmt.Sprintf("/user2/repo1/pulls/%d/files", pr.Index))
105+
resp := session.MakeRequest(t, req, http.StatusOK)
106+
doc := NewHTMLParser(t, resp.Body)
107+
assert.Equal(t, 1, doc.Find(".diff-file-box").Length())
108+
assert.Equal(t, "file2.txt", doc.Find(".diff-file-box").AttrOr("data-new-filename", ""))
109+
}

0 commit comments

Comments
 (0)