fix(pull): handle unrelated history in pull requests better - #39312
Open
silverwind wants to merge 16 commits into
Open
silverwind wants to merge 16 commits into
silverwind wants to merge 16 commits into
Conversation
When the base or head branch of a pull request is rewritten so both no longer share history, the files view returned 404, the commits API was empty and updating the branch failed with a generic error. Keep the last known merge base and compare directly against it, or against the base branch, when no merge base exists, so the web view, the API and the diff and patch downloads keep showing the changes. Report unrelated histories when updating by merge or rebase, and refuse creating such pull requests via the API like the web form does. Assisted-by: Claude Code:claude-opus-5
GiteaBot
added
the
lgtm/need 2
Existing tests create pull requests between unrelated branches via the API on purpose, and fixture pull requests store merge bases that are not ancestors of their head, so the creation check and the two-dot range for diff and patch downloads broke them. Neither is needed to show the diff or report the update error. Assisted-by: Claude Code:claude-opus-5
Contributor
|
I don't think the change is right.
|
Contributor
I don't think |
Member
Author
|
I will rework. BTW: GitHub also auto-closes PRs when the base branch moves and produces unrelated histories, but I think that's a overly-invasive move so I'm aiming to keep PRs open, but of course un-mergable/un-updateable. |
Comparing against the current base branch produced misleading diffs, so fall back only to the last known merge base while the head still contains it, and show a notice and empty file lists when no merge base is left. Replace the boolean parameters of GetCompareInfo with CompareOptions, which also removes the DirectComparison helpers. Assisted-by: Claude Code:claude-opus-5
silverwind
changed the title
Member
Author
|
Updated and also removed |
Member
Author
|
Also there's #39228 which is related. I'll check how to resolve this situation. |
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.
With the empty tree diff, pull requests without any merge base render their files like a root commit, so the notice is no longer needed. The files API follows the same base so both views agree, and one fixture based test now covers both the rewritten base and unrelated head cases. Assisted-by: Claude Code:claude-opus-5
Member
Author
Folded that PR into here via original commit cherry-pick. |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Error handling, legacy Git behavior, and unrelated-history file counts remain incorrect.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Improves pull request behavior when base and head histories diverge.
Changes:
- Falls back to the stored merge base or empty tree.
- Reports unrelated histories during branch updates.
- Adds integration coverage and refactors comparison options.
File summaries
| File | Description |
|---|---|
tests/integration/pull_diff_test.go |
Tests unrelated-history diffs and updates. |
templates/repo/diff/compare.tmpl |
Uses comparison separators directly. |
services/pull/update_rebase.go |
Detects unrelated rebase histories. |
services/pull/pull.go |
Adds stored merge-base fallback. |
services/pull/merge_tree.go |
Preserves usable historical merge bases. |
services/gitdiff/gitdiff.go |
Supports empty-tree diff bases. |
services/gitdiff/git_diff_tree.go |
Accepts empty-tree identifiers. |
services/git/compare.go |
Introduces structured comparison options. |
routers/web/repo/pull.go |
Handles empty-tree views and update errors. |
routers/web/repo/compare.go |
Adopts comparison options. |
routers/common/compare.go |
Removes obsolete comparison helper. |
routers/api/v1/repo/pull.go |
Updates pull files, commits, and errors. |
modules/git/repo_compare.go |
Accepts complete comparison arguments. |
models/issues/pull.go |
Centralizes pull comparison-base selection. |
Review details
- Files reviewed: 14/14 changed files
- Comments generated: 4
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ommon history Pull requests without any merge base now compare from the empty tree with all head commits listed, like GitHub, so the commits tab, file count and commit range selection work. The legacy mergeability check for git older than 2.40 keeps the last known merge base like the merge tree check, so both paths behave the same. Git errors while listing the fallback comparison or finding the rebase merge base are now reported, and review comment patches use the empty tree when there is no merge base. Assisted-by: Claude Code:claude-opus-5
wxiaoguang
reviewed
Sep 14, 2026
wxiaoguang
reviewed
Sep 14, 2026
…e template The compare template decided whether a pull request can be created and which separator to switch to, which is logic that belongs in Go. The page now gets both values from the handler, and the pull request creation POST enforces the same rule so direct ".." comparisons can't be submitted. The branch check left in the template could never fail inside the pull request block, so its tag message is removed. Updating a pull request without common history now returns a translatable unprocessable error from the service, so the API and web handlers use the generic error responses. Review comment patches use the same stored merge base rule as the files view. Assisted-by: Claude Code:claude-opus-5
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Merge-base failures can be misclassified as unrelated histories, and submitted empty pull requests bypass the rendered creation rules.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 20/20 changed files
- Comments generated: 3
- Review effort level: Balanced
The pull request creation POST only checked the branch and separator part of the rule shown on the compare page, because whether there is anything to compare and whether empty pull requests are allowed was only computed while rendering the diff. Both are now computed when parsing the comparison, so the page and the POST share one rule. The mergeability checks treated every merge base failure as unrelated histories, marking the pull request empty and possibly dropping its last merge base when git failed for another reason. Only a missing merge base takes that path now, other errors are returned. Assisted-by: Claude Code:claude-opus-5
* origin/main: (21 commits) fix(repo): surface unrelated histories on Sync Fork (go-gitea#39258) refactor: replace AWS SDK with a REST client for CodeCommit migration (go-gitea#39330) perf(frontend): enable vite module preload (go-gitea#39332) [skip ci] Updated translations via Crowdin fix: add default timeout and handle errors for HaveIBeenPwned API (go-gitea#39316) fix(user): unify email validation for registration and settings (go-gitea#39304) refactor: replace Azure Blob SDK with a REST client (go-gitea#39315) build(gogit): disable gogit builds for stable releases (go-gitea#39324) test(e2e): log out to switch users in pr-review test (go-gitea#39328) enhance: support `ETag` on streamed repository archives, support `If-None-Match: *` (go-gitea#39289) fix: match install page update checker setting with app.ini (go-gitea#39317) fix(actions): use gitea's clock for actions durations (go-gitea#39323) [skip ci] Updated translations via Crowdin enhance(notifications): mark current notification page as read (go-gitea#39294) fix(actions): never show negative running durations (go-gitea#39322) fix: classify git failures on stderr, restrict migration failure detail (go-gitea#39010) [skip ci] Updated translations via Crowdin chore: fix various problems (go-gitea#39298) fix: correct stdErr match in isErrBlameNotFoundOrNotEnoughLines (go-gitea#39309) chore(deps): update actionslib to v1.0.0 (go-gitea#39295) ...
… error pull.Update now returns a translatable error for unrelated histories, so the Sync Fork handlers no longer matched it and fell back to a server error. Render it with the auto error helpers instead. Assisted-by: Claude Code:claude-opus-5
Keep the compare separator switch in the template, derive the pull request creation rule once, and restore the base/head signature of GetDiffNumChangedFiles so the change stays close to the existing code. Assisted-by: Claude Code:claude-opus-5
silverwind
changed the title
Reuse the no common merge base fixture instead of rebuilding it, and drop assertions whose paths are already covered by other checks. Assisted-by: Claude Code:claude-opus-5
Treating them as empty let API merges and auto merge go ahead, and a rebase merge would push the unrelated head history onto the base. A dedicated status makes both refuse such pull requests while their views keep working. The squash message helper no longer panics and the diff and patch downloads no longer fail when no merge base is left. Assisted-by: Claude Code:claude-opus-5-5
Listing every head commit instead walks the whole history for co-authors when all authors are collected, and a pull request without a merge base can't be merged anyway. Assisted-by: Claude Code:claude-opus-5-5
Member
Author
|
Cleaned up a bit and ensured this will merge cleanly against #38404. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When head and base of a pull request no longer share history, the files tab returns 404 and "Update branch" fails with a generic error. Broken since #35192.
GetCompareInfotakes the compare separator instead of theDirectComparisonbooleans, and the compare page's pull request creation rules move to Go.Replaces: #39228
Fixes: #36644