Skip to content

LFS files in forks fail to download via separate remote or PR ref #17715

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

Open
parnic opened this issue Nov 18, 2021 · 22 comments
Open

LFS files in forks fail to download via separate remote or PR ref #17715

parnic opened this issue Nov 18, 2021 · 22 comments
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented issue/critical This issue should be fixed ASAP. If it is a PR, the PR should be merged ASAP issue/workaround it is or has a workaround topic/lfs

Comments

@parnic
Copy link
Contributor

parnic commented Nov 18, 2021

Gitea Version

1.15.6

Git Version

Seen on different servers running 2.25.1 and 2.34.0

Operating System

Ubuntu 20.04.3

How are you running Gitea?

One server is manually deployed from a GitHub release and started with a systemd script. The other uses the apt-installed version of Gitea.

Database

PostgreSQL

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

I can gather a log if that would be helpful. I've included clear repro steps in the description.

Description

LFS files are failing to download from forks. The only reason I can't repro on try.gitea.io is because LFS doesn't seem to be enabled. This is my minimal repro (visible at https://git.pernicious.games/try-gitea-lfs/lfs-fork-test and the fork at https://git.pernicious.games/cpickett/lfs-fork-test/src/branch/fork-1).

Note that we are pretty sure this used to work on our Gitea installation months ago, so it may be caused by a relatively recent Gitea version (maybe 1.15+).

The repro steps are:

  • Create new repo with an LFS file
  • Fork that repo
  • Commit a new rev of the LFS file in a branch on the fork
  • Open a PR for the main repo from the fork's branch

Now clone the base repo somewhere else and try to either pull from the remote or the PR ref:

  • Clone base repo (e.g. git clone https://git.pernicious.games/try-gitea-lfs/lfs-fork-test.git)
  • Add the fork as a remote and fetch it (e.g.: git remote add parnic https://git.pernicious.games/cpickett/lfs-fork-test.git, git fetch parnic)
  • Try switching to the branch that exists in the fork with the modified LFS file (e.g.: git switch fork-1)
    • Receive error, e.g.:
Downloading test.lfs (9 B)
Error downloading object: test.lfs (7ffc8bb): Smudge error: Error downloading test.lfs (7ffc8bb3b0c443a3d170b1f6aa402a66c6de86c49dd2d74e452d6d82b855939f): [7ffc8bb3b0c443a3d170b1f6aa402a66c6de86c49dd2d74e452d6d82b855939f] Not Found: [404] Not Found
  • Restore modified files and try switching to the PR ref instead (e.g.: git restore ., git fetch origin pull/1/head:test, git switch test)
    • Receive same error as above

Note that the LFS file at the specified revision/hash exists in the fork, e.g.: https://git.pernicious.games/cpickett/lfs-fork-test.git/info/lfs/objects/7ffc8bb3b0c443a3d170b1f6aa402a66c6de86c49dd2d74e452d6d82b855939f/direct
but it does not exist in the base repo: https://git.pernicious.games/try-gitea-lfs/lfs-fork-test.git/info/lfs/objects/7ffc8bb3b0c443a3d170b1f6aa402a66c6de86c49dd2d74e452d6d82b855939f/direct

This is important because when running git lfs fetch --all we see that it is trying to pull from the base org, not the fork:

>git lfs fetch --all
fetch: 2 object(s) found, done.
fetch: Fetching all references...
[7ffc8bb3b0c443a3d170b1f6aa402a66c6de86c49dd2d74e452d6d82b855939f] Not Found: [404] Not Found
error: failed to fetch some objects from 'https://git.pernicious.games/try-gitea-lfs/lfs-fork-test.git/info/lfs'

Finally, note that the LFS file does exist on the disk under the generic LFS directory. For this example, that's
/var/lib/gitea/data/lfs/7f/fc/8bb3b0c443a3d170b1f6aa402a66c6de86c49dd2d74e452d6d82b855939f

Screenshots

No response

@wxiaoguang
Copy link
Contributor

Can @zeripath or @KN4CK3R or someone else help to confirm?

IIRC the LFS code gets some changes during 1.14 -> 1.15, I am not sure whether this is a permission problem or a database consistency problem.

@KN4CK3R
Copy link
Member

KN4CK3R commented Nov 19, 2021

Are you sure that worked before? After creating the PR the base repository contains the dangling pointer without the database reference:

grafik
(top = base, bottom = pr branch)

During 1.14 -> 1.15 I changed the LFS API code but this looks like we need to sync the LFS objects when creating a PR. And that functionality was never implemented I think.

@KN4CK3R
Copy link
Member

KN4CK3R commented Nov 19, 2021

Looks like, I was wrong. I tested 1.12 and it works with that version (but is not correct I think). After creating the PR blob list does only show a single pointer (that's wrong because of the new PR branch?). Seems like the download request associated the pointer, so my changes in #16865 or #15523 may be the reason of the problem.

@KN4CK3R
Copy link
Member

KN4CK3R commented Nov 19, 2021

The old code created the database references when downloading files. I changed that in #15523 so that only upload requests are able to do that.

Reasons:

  • The download request should just not do that (?)
  • To associate the file only Read-rights were enforced
  • The user could associate every file from different repositories (private too) to this repo if he knows the pointer data (was fixed in Test if LFS object is accessible #16865)

So the question is how we would like to handle this problem. Personally I don't like to bring back file association in the download request. Auto-sync when creating a PR?

@wxiaoguang wxiaoguang added the issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented label Nov 19, 2021
@lunny
Copy link
Member

lunny commented Nov 19, 2021

The old code created the database references when downloading files. I changed that in #15523 so that only upload requests are able to do that.

Reasons:

  • The download request should just not do that (?)
  • To associate the file only Read-rights were enforced
  • The user could associate every file from different repositories (private too) to this repo if he knows the pointer data (was fixed in Test if LFS object is accessible #16865)

So the question is how we would like to handle this problem. Personally I don't like to bring back file association in the download request. Auto-sync when creating a PR?

I vote later.

@wxiaoguang
Copy link
Contributor

And just FYI, I remember this issue: #17207 , are they related? That issue also said about the behavior differs between Gitea release.

@KN4CK3R
Copy link
Member

KN4CK3R commented Nov 19, 2021

Yes, I think they are related. Just putting the files in the directory does not associate the repo to them.

@stu1811
Copy link

stu1811 commented Nov 19, 2021

I agree they are probably related and I've run into this issue as well. As a workaround you can do a git lfs fetch <UPSTREAM> and then a git lfs checkout to get the files.

parnic-sks added a commit to parnic-sks/gitea that referenced this issue Nov 22, 2021
Temporary measure to work around
go-gitea#17715
@parnic
Copy link
Contributor Author

parnic commented Nov 23, 2021

We are hosting a private Gitea server behind a VPN, so I've temporarily reintroduced the associate-LFS-pointers-on-download code for a hacked-up local build, in case anyone else needs the temporary workaround: https://github.com/go-gitea/gitea/compare/v1.15.6...parnic-sks:associate-lfs-obj-on-download?expand=1

@stu1811
Copy link

stu1811 commented Dec 8, 2021

And just FYI, I remember this issue: #17207 , are they related? That issue also said about the behavior differs between Gitea release.

I built 1.15.6 cherry-picked the changes from parnic-sks@768aaab and it did not fix issue #17207. Detail below.

We got a new bundle with LFS updates. I extracted the new LFS files, fetched the branch, and tried to push to origin. It would not push due hint: Your push was rejected due to missing or corrupt local objects. I also tried git lfs fetch --all and the push failed again.

@parnic
Copy link
Contributor Author

parnic commented Dec 8, 2021

And just FYI, I remember this issue: #17207 , are they related? That issue also said about the behavior differs between Gitea release.

I built 1.15.6 cherry-picked the changes from parnic-sks@768aaab and it did not fix issue #17207. Detail below.

We got a new bundle with LFS updates. I extracted the new LFS files, fetched the branch, and tried to push to origin. It would not push due hint: Your push was rejected due to missing or corrupt local objects. I also tried git lfs fetch --all and the push failed again.

That one commit is not enough, you need the entire branch diff. parnic-sks@3872374 was actually the correct fix, but it needs the commit after that to fix the nil user deref.

@stu1811
Copy link

stu1811 commented Dec 8, 2021

And just FYI, I remember this issue: #17207 , are they related? That issue also said about the behavior differs between Gitea release.

I built 1.15.6 cherry-picked the changes from parnic-sks@768aaab and it did not fix issue #17207. Detail below.
We got a new bundle with LFS updates. I extracted the new LFS files, fetched the branch, and tried to push to origin. It would not push due hint: Your push was rejected due to missing or corrupt local objects. I also tried git lfs fetch --all and the push failed again.

That one commit is not enough, you need the entire branch diff. parnic-sks@3872374 was actually the correct fix, but it needs the commit after that to fix the nil user deref.

Sorry I mean I cherry-picked the whole branch.

@parnic
Copy link
Contributor Author

parnic commented Jan 15, 2022

Any update here? We are continuing to maintain the aforementioned patch on top of each Gitea release, but I'd love if we could find a more permanent solution that didn't require hand-building each version.

(sorry for the double comment, I keep using the wrong account...)

parnic-sks added a commit to parnic-sks/gitea that referenced this issue Jan 31, 2022
Temporary measure to work around
go-gitea#17715
parnic-sks added a commit to parnic-sks/gitea that referenced this issue Jan 31, 2022
Temporary measure to work around
go-gitea#17715
parnic-sks added a commit to parnic-sks/gitea that referenced this issue Jan 31, 2022
Temporary measure to work around
go-gitea#17715
parnic-sks added a commit to parnic-sks/gitea that referenced this issue Jan 31, 2022
Temporary measure to work around
go-gitea#17715
@zeripath
Copy link
Contributor

zeripath commented Feb 4, 2022

OK the PR is a bit of a false flag here.

  1. Create a repo A
  2. Add an lfs file A_file on branch A_branch
  3. Fork the repo B
  4. Add an lfs file B_file on branch B_branch
  5. clone A
  6. Add B as a remote
  7. checkout B_branch

This will fail.


Clearly this breaks any use of forks with LFS and needs to be fixed.


Ah actually looking at the git lfs documentation this appears to be a standard issue
and you're supposed to use

git lfs --all fetch <remote> <branch>...

to get the lfs objects on the remote branch prior to checking out the branch

or...

git -c lfs.url=http://localhost/gitea/testuser/lfs-test.git/info/lfs checkout -f testuser/bar-to-lfs -b bar-to-lfs

@villivateur
Copy link

I have the exactly the same problem here.

Repo B is forked from repo A, if I clone B to local, and add an upstream for repo A, then fetch A, merge A, and a 404 error occurs. Cannot download LFS files from A.

parnic-sks added a commit to parnic-sks/gitea that referenced this issue Apr 27, 2022
Temporary measure to work around
go-gitea#17715
parnic-sks added a commit to parnic-sks/gitea that referenced this issue May 16, 2022
Temporary measure to work around
go-gitea#17715
parnic-sks added a commit to parnic-sks/gitea that referenced this issue Jul 31, 2022
Temporary measure to work around
go-gitea#17715
parnic-sks added a commit to parnic-sks/gitea that referenced this issue Sep 6, 2022
Temporary measure to work around
go-gitea#17715
parnic-sks added a commit to parnic-sks/gitea that referenced this issue Nov 7, 2022
Temporary measure to work around
go-gitea#17715
parnic-sks added a commit to parnic-sks/gitea that referenced this issue Dec 22, 2022
Temporary measure to work around
go-gitea#17715
parnic-sks added a commit to parnic-sks/gitea that referenced this issue Jan 3, 2023
Temporary measure to work around
go-gitea#17715
parnic-sks added a commit to parnic-sks/gitea that referenced this issue Jan 17, 2023
Temporary measure to work around
go-gitea#17715
parnic-sks added a commit to parnic-sks/gitea that referenced this issue Jul 28, 2023
Temporary measure to work around
go-gitea#17715
parnic-sks added a commit to parnic-sks/gitea that referenced this issue Sep 8, 2023
Temporary measure to work around
go-gitea#17715
parnic-sks added a commit to parnic-sks/gitea that referenced this issue Oct 3, 2023
Temporary measure to work around
go-gitea#17715
brechtvl added a commit to blender/gitea that referenced this issue Oct 20, 2023
Patch taken from upstream Gitea issue go-gitea#17715, associating the LFS
pointer when the file is downloaded.
parnic-sks added a commit to parnic-sks/gitea that referenced this issue Nov 14, 2023
Temporary measure to work around
go-gitea#17715
parnic-sks added a commit to parnic-sks/gitea that referenced this issue Nov 27, 2023
Temporary measure to work around
go-gitea#17715
parnic-sks added a commit to parnic-sks/gitea that referenced this issue Dec 12, 2023
Temporary measure to work around
go-gitea#17715
parnic-sks added a commit to parnic-sks/gitea that referenced this issue Dec 21, 2023
Temporary measure to work around
go-gitea#17715
parnic-sks added a commit to parnic-sks/gitea that referenced this issue Feb 23, 2024
Temporary measure to work around
go-gitea#17715
parnic-sks added a commit to parnic-sks/gitea that referenced this issue Feb 23, 2024
Temporary measure to work around
go-gitea#17715
parnic-sks added a commit to parnic-sks/gitea that referenced this issue Feb 23, 2024
Temporary measure to work around
go-gitea#17715
parnic-sks added a commit to parnic-sks/gitea that referenced this issue Feb 26, 2024
Temporary measure to work around
go-gitea#17715
parnic-sks added a commit to parnic-sks/gitea that referenced this issue Mar 13, 2024
Temporary measure to work around
go-gitea#17715
@stevapple
Copy link
Contributor

Personally I don't like to bring back file association in the download request. Auto-sync when creating a PR?

Just want to mention that it seems to break LFS mirror and migration from external sources. They have nothing to do with forks and PRs and at least should download and add file association.

@boydaihungst
Copy link

In case you still have local files:

  • Go to https://git.example.com/user_name/repo_name/settings/lfs/pointers

  • Check lfs file which isn't in the repo or isn't in store. For example OID: 6c8872a754. Click on to Blob SHA: 1935a2af06 -> it will open a website and dispaly oid sha256:6c8872a754e3519067ab69de950cb7a980a73bd9cc565e858121aa6cd95119c6
    image
    image

  • Now use terminal, cd to repo folder.
    Then run git lfs push origin --object-id 6c8872a754e3519067ab69de950cb7a980a73bd9cc565e858121aa6cd95119c61. Repeat the all other files.

image

@lunny lunny added the issue/workaround it is or has a workaround label Mar 17, 2024
parnic-sks added a commit to parnic-sks/gitea that referenced this issue Mar 26, 2024
Temporary measure to work around
go-gitea#17715
parnic-sks added a commit to parnic-sks/gitea that referenced this issue Mar 26, 2024
Temporary measure to work around
go-gitea#17715
parnic-sks added a commit to parnic-sks/gitea that referenced this issue Apr 16, 2024
Temporary measure to work around
go-gitea#17715
brechtvl added a commit to blender/gitea that referenced this issue May 10, 2024
Patch taken from upstream Gitea issue go-gitea#17715, associating the LFS
pointer when the file is downloaded.
brechtvl added a commit to blender/gitea that referenced this issue May 14, 2024
Patch taken from upstream Gitea issue go-gitea#17715, associating the LFS
pointer when the file is downloaded.
parnic-sks added a commit to parnic-sks/gitea that referenced this issue May 28, 2024
Temporary measure to work around
go-gitea#17715
parnic-sks added a commit to parnic-sks/gitea that referenced this issue Jul 26, 2024
Temporary measure to work around
go-gitea#17715
parnic-sks added a commit to parnic-sks/gitea that referenced this issue Jul 26, 2024
Temporary measure to work around
go-gitea#17715
@test-sha256
Copy link

So the question is how we would like to handle this problem. Personally I don't like to bring back file association in the download request. Auto-sync when creating a PR?

While I agree that associate on download is not ideal, there should be an auto-sync that associates all the files in PR. This would allow the commits and its LFS objects can be checked out in target repository.

@gamedevkirk
Copy link

Hoping this gets some attention at some point. This is a brutally frustrating issue.

@StanleySweet
Copy link
Contributor

I believe 0 A.D. has the same, or a similar issue when syncing forks via the web UI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented issue/critical This issue should be fixed ASAP. If it is a PR, the PR should be merged ASAP issue/workaround it is or has a workaround topic/lfs
Projects
None yet
Development

No branches or pull requests