Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrating repo with LFS causes 500 error #31137

Closed
Shuenhoy opened this issue May 28, 2024 · 1 comment · Fixed by #31702
Closed

Migrating repo with LFS causes 500 error #31137

Shuenhoy opened this issue May 28, 2024 · 1 comment · Fixed by #31702

Comments

@Shuenhoy
Copy link

Description

https://gitea.com/Shuenhoy/vtk-m is migrated from https://gitlab.kitware.com/vtk/vtk-m.
Accessing the data folder will trigger 500 error.

Whether selecting migrate LFS files is the same.
In the demo site, https://demo.gitea.com/Shuenhoy/vtk-m will not trigger 500 error, but it doesn't treat the files as LFS files at all.

Gitea Version

1.22.0

Can you reproduce the bug on the Gitea demo site?

Yes

Log Gist

No response

Screenshots

image

(On my instance, there is an extra message)

An error occurred:

getFileReader, LFS Meta object does not exist

Git Version

No response

Operating System

No response

How are you running Gitea?

Docker

Database

None

@wolfogre
Copy link
Member

wolfogre commented Jul 12, 2024

It was caused by the fact that the original repo has missing LFS objects. I found it but maybe not the only one: oid 137439af2a14dfaab461b07e643fb45dc8e898690f21ca9514cc794dc4afd50a with size 30144. GitLab returned Object does not exist on the server or you don't have permissions to access it.

And you can find it in commit fecea93da490d460709001ce18c389154c36633c, the preview is broken.

image

But what makes it worse is that Gitea doesn't skip the missing objects, it just stops migrating LFS immediately but treats the migration as successful, so you could see most LFS objects are broken. It will be fixed by #31623.

GiteaBot pushed a commit to GiteaBot/gitea that referenced this issue Jul 31, 2024
…ion (go-gitea#31702)

Fix go-gitea#31137.

Replace go-gitea#31623 go-gitea#31697.

When migrating LFS objects, if there's any object that failed (like some
objects are losted, which is not really critical), Gitea will stop
migrating LFS immediately but treat the migration as successful.

This PR checks the error according to the [LFS api
doc](https://github.com/git-lfs/git-lfs/blob/main/docs/api/batch.md#successful-responses).

> LFS object error codes should match HTTP status codes where possible:
> 
> - 404 - The object does not exist on the server.
> - 409 - The specified hash algorithm disagrees with the server's
acceptable options.
> - 410 - The object was removed by the owner.
> - 422 - Validation error.

If the error is `404`, it's safe to ignore it and continue migration.
Otherwise, stop the migration and mark it as failed to ensure data
integrity of LFS objects.

And maybe we should also ignore others errors (maybe `410`? I'm not sure
what's the difference between "does not exist" and "removed by the
owner".), we can add it later when some users report that they have
failed to migrate LFS because of an error which should be ignored.
lunny pushed a commit that referenced this issue Jul 31, 2024
…ion (#31702) (#31745)

Backport #31702 by @wolfogre

Fix #31137.

Replace #31623 #31697.

When migrating LFS objects, if there's any object that failed (like some
objects are losted, which is not really critical), Gitea will stop
migrating LFS immediately but treat the migration as successful.

This PR checks the error according to the [LFS api
doc](https://github.com/git-lfs/git-lfs/blob/main/docs/api/batch.md#successful-responses).

> LFS object error codes should match HTTP status codes where possible:
> 
> - 404 - The object does not exist on the server.
> - 409 - The specified hash algorithm disagrees with the server's
acceptable options.
> - 410 - The object was removed by the owner.
> - 422 - Validation error.

If the error is `404`, it's safe to ignore it and continue migration.
Otherwise, stop the migration and mark it as failed to ensure data
integrity of LFS objects.

And maybe we should also ignore others errors (maybe `410`? I'm not sure
what's the difference between "does not exist" and "removed by the
owner".), we can add it later when some users report that they have
failed to migrate LFS because of an error which should be ignored.

Co-authored-by: Jason Song <[email protected]>
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Oct 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.