Fix compare dropdown for branches without common history by bircni · Pull Request #37470 · go-gitea/gitea

bircni · 2026-04-28T15:39:24Z

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

…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.

Co-Authored-By: Codex <codex@openai.com>

wxiaoguang · 2026-04-28T17:08:55Z

Took a deep look, I found that the legacy code is not quite right.

For example: after GetHeadOwnerAndRepo gets introduced, has := headRepo != nil and related logic doesn't make sense anymore.

Will take a deeper look and maybe fix the legacy problems together.

bircni · 2026-04-28T17:21:34Z

Took a deep look, I found that the legacy code is not quite right.

For example: after GetHeadOwnerAndRepo gets introduced, has := headRepo != nil and related logic doesn't make sense anymore.

Will take a deeper look and maybe fix the legacy problems together.

Thank you !!!

wxiaoguang · 2026-04-28T18:02:02Z

Much more changes than I thought (more problems in legacy code than it looks ....). I think it's worth to do the deep clean up changes.

So I guess no need to backport it (the problem has been there for many years, and it doesn't really block daily work, just bad user experience).

Copilot

Pull request overview

Fixes compare UI/server handling when base and head refs have no common merge base (unrelated histories), avoiding 500s and keeping branch selectors usable with a clear warning.

Changes:

Treat “no merge base” as a non-fatal condition during compare info generation and render a warning instead of erroring.
Refactor compare info parsing and compare diff preparation, and wire flash alerts into the compare template.
Add regression tests for merge-base “unrelated history” detection and compare page behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
tests/integration/compare_test.go	Adds integration coverage for compare selection with unrelated histories.
templates/repo/diff/compare.tmpl	Renders flash alerts on compare page (for the new warning).
services/git/compare.go	Updates compare info generation to tolerate missing merge base.
routers/web/repo/pull.go	Switches PR-creation handler to new compare parsing and error handling.
routers/web/repo/compare.go	Refactors compare parsing/diff prep and adds no-merge-base warning behavior.
options/locale/locale_en-US.json	Adds localized warning text for no common merge base.
modules/gitrepo/merge.go	Classifies merge-base “no common history” as not-exist.
modules/gitrepo/compare_test.go	Adds unit test for MergeBase with unrelated histories.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

bircni · 2026-04-28T18:30:32Z

@wxiaoguang would really Like to backport as this bothered us very much today and will still do in the next few weeks/months

wxiaoguang · 2026-04-28T18:31:07Z

OK, maybe you can use the early commits as backport.

The permission-denied path in parseCompareInfo returns util.NewNotExistErrorf("") so the empty message wouldn't leak why the 404 happened. CompareAndPullRequestPost was passing err.Error() into ctx.JSONError, which surfaced an empty errorMessage to the client. Route ErrNotExist through ctx.JSONErrorNotFound() so it sends a generic localized 404 message regardless of the underlying error, matching the GET path's ctx.NotFound(nil) behavior. ErrInvalidArgument keeps its descriptive message via JSONError. Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>

silverwind · 2026-04-28T20:22:43Z

Small error-message fixup added in 109fef5.

* main: Add DEFAULT_TITLE_SOURCE setting for pull request title default behavior (go-gitea#37465) Fix compare dropdown for branches without common history (go-gitea#37470) FIX: URL sanitization to handle schemeless credentials (go-gitea#37440) Refactor pull request view (4) (go-gitea#37451) Fix scheduled action panic with null event payload (go-gitea#37459) Fix attachment Content-Security-Policy (go-gitea#37455) [skip ci] Updated translations via Crowdin Rename CurrentRefPath to CurrentRefSubURL (go-gitea#37453) Clean up org pages layout (go-gitea#37445) Fix script error alert (go-gitea#37458) Fix inconsistent disabled styling on logged-out repo header buttons (go-gitea#37406) Add API endpoint to reply to pull request review comments (go-gitea#36683) Add CurrentURL template variable back (go-gitea#37444)

wxiaoguang · 2026-04-29T09:29:24Z

-> Refactor compare diff/pull page (1) #37481

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 28, 2026

wxiaoguang reviewed Apr 28, 2026

View reviewed changes

Comment thread modules/gitrepo/merge.go Outdated

wxiaoguang reviewed Apr 28, 2026

View reviewed changes

Comment thread modules/gitrepo/merge.go Outdated

wxiaoguang reviewed Apr 28, 2026

View reviewed changes

Comment thread templates/repo/diff/compare.tmpl Outdated

wxiaoguang reviewed Apr 28, 2026

View reviewed changes

Comment thread routers/web/repo/compare.go Outdated

wxiaoguang reviewed Apr 28, 2026

View reviewed changes

Comment thread routers/web/repo/compare.go Outdated

Address compare no merge base review feedback

57995e9

Co-Authored-By: Codex <codex@openai.com>

wxiaoguang reviewed Apr 28, 2026

View reviewed changes

Comment thread templates/repo/diff/compare.tmpl Outdated

wxiaoguang reviewed Apr 28, 2026

View reviewed changes

Comment thread routers/web/repo/compare.go Outdated

bircni added 2 commits April 28, 2026 18:32

fixes

044a3de

simplify

6d6ba9a

bircni requested a review from wxiaoguang April 28, 2026 16:39

bircni added the backport/v1.26 This PR should be backported to Gitea 1.26 label Apr 28, 2026

wxiaoguang self-assigned this Apr 28, 2026

wxiaoguang added 3 commits April 29, 2026 01:40

refactor

2dd8637

use fast-import

5e2934e

fix

1fa183f

wxiaoguang reviewed Apr 28, 2026

View reviewed changes

Comment thread routers/web/repo/compare.go

wxiaoguang reviewed Apr 28, 2026

View reviewed changes

Comment thread routers/web/repo/pull.go

wxiaoguang requested a review from Copilot April 28, 2026 18:05

Copilot started reviewing on behalf of wxiaoguang April 28, 2026 18:06 View session

fix lint

bebdb51

Copilot AI reviewed Apr 28, 2026

View reviewed changes

Comment thread modules/gitrepo/compare_test.go

Comment thread services/git/compare.go

Comment thread routers/web/repo/compare.go Outdated

Comment thread routers/web/repo/compare.go

Comment thread routers/web/repo/pull.go

Comment thread tests/integration/compare_test.go

wxiaoguang added 2 commits April 29, 2026 02:15

fix logic

d26da79

fine tune

9a27615

wxiaoguang added the backport/manual No power to the bots! Create your backport yourself! label Apr 28, 2026

wxiaoguang marked this pull request as ready for review April 28, 2026 18:36

wxiaoguang added 2 commits April 29, 2026 02:47

if no merge base, do not do further "compare"

8e5ef57

fix test

c5a027e

wxiaoguang approved these changes Apr 28, 2026

View reviewed changes

GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 28, 2026

silverwind approved these changes Apr 28, 2026

View reviewed changes

GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 28, 2026

bircni merged commit deec2b0 into go-gitea:main Apr 28, 2026
26 checks passed

GiteaBot added this to the 1.27.0 milestone Apr 28, 2026

bircni mentioned this pull request Apr 28, 2026

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

Open

bircni deleted the bugfix-37469 branch April 28, 2026 21:34

Uh oh!

Conversation

bircni commented Apr 28, 2026 • edited by wxiaoguang Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wxiaoguang commented Apr 28, 2026

Uh oh!

bircni commented Apr 28, 2026

Uh oh!

wxiaoguang commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bircni commented Apr 28, 2026

Uh oh!

wxiaoguang commented Apr 28, 2026

Uh oh!

silverwind commented Apr 28, 2026

Uh oh!

Uh oh!

wxiaoguang commented Apr 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bircni commented Apr 28, 2026 •

edited by wxiaoguang

Loading

wxiaoguang commented Apr 28, 2026 •

edited

Loading