Server error on user-specific PR diff and a pruned commit #9671

New Issue

GiteaMirror · 2025-11-02T08:46:12-06:00

GiteaMirror commented

2025-11-02 08:46:12 -06:00

Originally created by @zakjan on GitHub (Oct 10, 2022).

Description

This is an edge case happening with the following conditions:

an open PR with an assigned reviewer
the reviewer interacted with the PR, Gitea stores user details about the interaction linked to a particular commit
the author force-pushed to the PR
git GC has run, causing a commit from the time of the interaction to be pruned

Observed behavior:

the reviewer who interacted with the PR can't open PR Files Changed tab
all other users can open it fine

Steps to replicate:

In terminal
- Run Gitea with basic docker-compose from https://docs.gitea.io/en-us/install-with-docker/#basics
In browser
- Install Gitea with SQLite database, admin user author
In terminal
- Shutdown Gitea
- Configure git GC to prune unreachable objects immediately, append to gitea/conf/app.ini:
```
[git]
GC_ARGS = "--prune=now"
```
- Run Gitea again
In browser, logged as author
- Add SSH key
- Create repo repo with initialized main branch, user reviewer, add him to repo collaborators
In terminal
- Clone repo
- Checkout branch feature, edit a file, commit, push
In browser, logged as author
- Create a PR for feature branch, assign reviewer as reviewer
In browser, logged as reviewer
- Open the PR Files Changed tab
- Mark the changed file as Viewed
In terminal
- Edit the file, amend the first commit, force-push
In browser, logged as author
- Site Administration > Maintenance Operations > Garbage collect all repositories > Run
- Confirm that the first commit was pruned - opening it returns 404, this is expected

In browser, logged as reviewer

Open the PR, Files Changed tab - opening returns 500
The following error is logged:

gitea  | 2022/10/07 10:15:40 ...ers/web/repo/pull.go:715:ViewPullFiles() [E] [633fe02b] SyncAndGetUserSpecificDiff: exit status 128 - fatal: Invalid revision range 722a5b2dcdb1560ecfa0306c2492d1c5c7fd2a4e..47703d0b1e66ddf52827ff9596dc80f4de860089
gitea  | 	 - fatal: Invalid revision range 722a5b2dcdb1560ecfa0306c2492d1c5c7fd2a4e..47703d0b1e66ddf52827ff9596dc80f4de860089

722a5b… is the pruned commit, 47703d… is the amended commit

Expected behavior:

PR Files Changed tab can be opened by anyone

Workaround:

close the PR and reopen as a new PR

Gitea Version

1.17.2

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

https://gist.github.com/zakjan/3e3c3574ac9df73d2e2bf0e717cd685d

Screenshots

Git Version

2.36.2

Operating System

macOS

How are you running Gitea?

Docker

Database

SQLite

Originally created by @zakjan on GitHub (Oct 10, 2022). ### Description This is an edge case happening with the following conditions: * an open PR with an assigned reviewer * the reviewer interacted with the PR, Gitea stores user details about the interaction linked to a particular commit * the author force-pushed to the PR * git GC has run, causing a commit from the time of the interaction to be pruned Observed behavior: * the reviewer who interacted with the PR can't open PR Files Changed tab * all other users can open it fine Steps to replicate: * In terminal * Run Gitea with basic docker-compose from https://docs.gitea.io/en-us/install-with-docker/#basics * In browser * Install Gitea with SQLite database, admin user `author` * In terminal * Shutdown Gitea * Configure git GC to prune unreachable objects immediately, append to `gitea/conf/app.ini`: ``` [git] GC_ARGS = "--prune=now" ``` * Run Gitea again * In browser, logged as `author` * Add SSH key * Create repo `repo` with initialized `main` branch, user `reviewer`, add him to `repo` collaborators * In terminal * Clone `repo` * Checkout branch `feature`, edit a file, commit, push * In browser, logged as `author` * Create a PR for `feature` branch, assign `reviewer` as reviewer * In browser, logged as `reviewer` * Open the PR Files Changed tab * Mark the changed file as Viewed * In terminal * Edit the file, amend the first commit, force-push * In browser, logged as `author` * Site Administration > Maintenance Operations > Garbage collect all repositories > Run * Confirm that the first commit was pruned - opening it returns 404, this is expected * In browser, logged as `reviewer` * Open the PR, Files Changed tab - opening returns 500 * The following error is logged: ``` gitea | 2022/10/07 10:15:40 ...ers/web/repo/pull.go:715:ViewPullFiles() [E] [633fe02b] SyncAndGetUserSpecificDiff: exit status 128 - fatal: Invalid revision range 722a5b2dcdb1560ecfa0306c2492d1c5c7fd2a4e..47703d0b1e66ddf52827ff9596dc80f4de860089 gitea | - fatal: Invalid revision range 722a5b2dcdb1560ecfa0306c2492d1c5c7fd2a4e..47703d0b1e66ddf52827ff9596dc80f4de860089 ``` * `722a5b…` is the pruned commit, `47703d…` is the amended commit Expected behavior: * PR Files Changed tab can be opened by anyone Workaround: * close the PR and reopen as a new PR ### Gitea Version 1.17.2 ### Can you reproduce the bug on the Gitea demo site? No ### Log Gist https://gist.github.com/zakjan/3e3c3574ac9df73d2e2bf0e717cd685d ### Screenshots ![Screenshot 2022-10-10 at 9 47 23](https://user-images.githubusercontent.com/173585/194819559-63cc03b7-4b9e-444e-9183-5f809a4dab4d.png) ### Git Version 2.36.2 ### Operating System macOS ### How are you running Gitea? Docker ### Database SQLite

GiteaMirror added the type/bug label 2025-11-02 08:46:12 -06:00

GiteaMirror closed this issue

2025-11-02 08:46:12 -06:00

GiteaMirror commented

2025-11-02 08:46:13 -06:00

@delvh commented on GitHub (Oct 10, 2022):

Ok, yeah, I have a pretty good guess of what's going wrong:

services/gitdiff/gitdiff.go:1239:   changedFiles, err := gitRepo.GetFilesChangedBetween(review.CommitSHA, latestCommit)

probably throws this error simply because Git is asked to display the changes between the two commits, and can't find the first one.
The solution will be to catch the error and assume all files changed (makes the most sense, since it can't find the first commit, so you don't know what the original state was).
Does our implementation throw a specific error for that case, or do I have to compare error messages?

@delvh commented on GitHub (Oct 10, 2022): Ok, yeah, I have a pretty good guess of what's going wrong: ```go services/gitdiff/gitdiff.go:1239: changedFiles, err := gitRepo.GetFilesChangedBetween(review.CommitSHA, latestCommit) ``` probably throws this error simply because Git is asked to display the changes between the two commits, and can't find the first one. The solution will be to catch the error and assume all files **changed** (makes the most sense, since it can't find the first commit, so you don't know what the original state was). Does our implementation throw a specific error for that case, or do I have to compare error messages?

GiteaMirror commented

2025-11-02 08:46:13 -06:00

@zakjan commented on GitHub (Oct 11, 2022):

How about to pre-emptively check if review.CommitSHA exists here? b59b0cad0a/services/gitdiff/gitdiff.go (L1227-L1230)

Then we'd not need to check for a specific error later.

@zakjan commented on GitHub (Oct 11, 2022): How about to pre-emptively check if `review.CommitSHA` exists here? https://github.com/go-gitea/gitea/blob/b59b0cad0a550223f74add109ff13c0d2f4309f3/services/gitdiff/gitdiff.go#L1227-L1230 Then we'd not need to check for a specific error later.

GiteaMirror commented

2025-11-02 08:46:13 -06:00

@delvh commented on GitHub (Oct 11, 2022):

The problem is: That's impossible*.
All we know is the current commit SHA when you made your review, which is simply a string.
We don't care/ know if it points to an actual commit, or if the SHA is an explicit Not a commit SHA and even more text. Lorem Ipsum… for some reason.
It is simply passed down to git, and git then complains.
The only way to check it beforehand would be to explicitly ask git if that commit exists, which is way overkill as that will spawn a complete process before returning a simple boolean.

@delvh commented on GitHub (Oct 11, 2022): The problem is: That's impossible\*. All we know is the current commit SHA when you made your review, which is simply a string. We don't care/ know if it points to an actual commit, or if the SHA is an explicit `Not a commit SHA and even more text. Lorem Ipsum…` for some reason. It is simply passed down to git, and git then complains. The only way to check it beforehand would be to explicitly ask git if that commit exists, which is way overkill as that will spawn a complete process before returning a simple boolean.

GiteaMirror commented

2025-11-02 08:46:13 -06:00

@zakjan commented on GitHub (Oct 11, 2022):

Right, I understand there is an effort to limit calls to git. I'm not sure what kind of git errors does Gitea recognize, so that it would need to differentiate between them. Possibly all errors from the user-specific diff could be catched, so that opening a user-independent diff works, and all files are considered as changed?

@zakjan commented on GitHub (Oct 11, 2022): Right, I understand there is an effort to limit calls to git. I'm not sure what kind of git errors does Gitea recognize, so that it would need to differentiate between them. Possibly all errors from the user-specific diff could be catched, so that opening a user-independent diff works, and all files are considered as changed?

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/gitea#9671