Skip to content

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

Open
bircni wants to merge 5 commits intogo-gitea:release/v1.26from
bircni:backport-bugfix-37469
Open

Compare dropdown fails when selecting branch with no common merge-base (#37470)#37472
bircni wants to merge 5 commits intogo-gitea:release/v1.26from
bircni:backport-bugfix-37469

Conversation

@bircni
Copy link
Copy Markdown
Member

@bircni bircni commented Apr 28, 2026

Summary

  • handle compare requests where base and head refs have no common merge base without returning 500
  • keep the compare branch selectors usable and show a clear warning message
  • add regression coverage for unrelated-history compare selection and merge-base error detection
  • refactor legacy code, remove dead code and fix incorrect logic

Fixes #37469

Backport of: #37470

bircni and others added 4 commits April 28, 2026 23:10
@bircni
Implement merge base error handling and UI updates for branch compari…
047933a 
…sons

- Introduced `ErrNoMergeBase` type to handle cases where two branches do not share a common merge base.
- Updated `MergeBase` function to return this error type when applicable.
- Enhanced the compare functionality to display a warning message in the UI when no common merge base is found.
- Added tests to verify behavior when comparing branches with no common history.
- Updated locale file to include a message for the no common history scenario.
@bircni @codex
Address compare no merge base review feedback
10fb4c0 
Co-Authored-By: Codex <codex@openai.com>
@bircni
fixes
af5e488
@bircni
simplify
d9ec35d
@bircni bircni requested a review from wxiaoguang April 28, 2026 21:12
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 28, 2026
@GiteaBot GiteaBot added this to the 1.26.2 milestone Apr 28, 2026
@silverwind
Copy link
Copy Markdown
Member

silverwind commented Apr 28, 2026

PR title should be same as original title on backports (it goes into changelog)

@bircni bircni changed the title Backport bugfix 37469 Compare dropdown fails when selecting branch with no common merge-base (unrelated histories) Apr 28, 2026
@silverwind
Copy link
Copy Markdown
Member

silverwind commented Apr 28, 2026

Not sure what the changeset here is but it seems vastly different than #37470.

@bircni
Copy link
Copy Markdown
Member Author

bircni commented Apr 28, 2026

#37470

@silverwind
Copy link
Copy Markdown
Member

silverwind commented Apr 28, 2026

Must fix

1. Nil-pointer panic in CompareDiff (the core bug this PR is meant to fix).
services/git/compare.go is unchanged on this branch, so GetCompareInfo still returns (nil, error) on the no-merge-base path (compare.go:78-79). ParseCompareInfo then returns (nil, true), and CompareDiff panics on ci.HeadRepo at compare.go:635. The integration test asserts http.StatusOK on exactly this path, so it will fail once CI runs. Either port the services/git/compare.go change from #37470, or build a populated *CompareInfo (BaseRepo/BaseRef/HeadRepo/HeadRef/CommitIDs) before returning when errors.As(err, &noMergeBase).

2. CompareAndPullRequestPost doesn't handle the no-merge-base case.
ci, _ := ParseCompareInfo(ctx) discards the signal, so a POST will panic on the same nil ci, or — if ci were populated — would create a PR with empty MergeBase. Mirror the original: return ctx.JSONError(ctx.Tr(\"repo.pulls.no_common_history\")) when noMergeBase is true.

Should fix

3. Test helper duplication.
commitRootTree (modules/gitrepo/compare_test.go) and createUnrelatedBranch (tests/integration/compare_test.go) are ~40 lines of git plumbing each. The original used a 12-line git fast-import script that works on v1.26 — consider reusing it instead.

Optional

4. Verify CI is green before merge.
The pre-fix state with &compareInfo wouldn't have compiled against v1.26's *CompareInfo signature, and the integration test currently asserts http.StatusOK on the panicking path. A passing CI run is the proof the fix is real.

This comment was written with the help of Claude Opus 4.7.

@silverwind
Copy link
Copy Markdown
Member

silverwind commented Apr 28, 2026

Well, here's Claude's verdict of this PR.

wxiaoguang
wxiaoguang reviewed Apr 29, 2026
View reviewed changes
Comment thread routers/web/repo/compare.go

// 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) {
Copy link
Copy Markdown
Contributor

@wxiaoguang wxiaoguang Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this return value can be reverted, still use template variable to pass values between functions, to keep the changes minimal.

@wxiaoguang wxiaoguang changed the title Compare dropdown fails when selecting branch with no common merge-base (unrelated histories) Compare dropdown fails when selecting branch with no common merge-base (#37470) Apr 29, 2026
@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented Apr 29, 2026

The PR's description is not for this PR either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wxiaoguang wxiaoguang wxiaoguang left review comments

Assignees

No one assigned

Labels

lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging.

Projects

None yet

Milestone

1.26.2

Development

Successfully merging this pull request may close these issues.

4 participants

@bircni @silverwind @wxiaoguang @GiteaBot