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

LFS: Error 500 when merging pull request #732

Closed
2 of 6 tasks
axeloz opened this issue Jan 23, 2017 · 18 comments · Fixed by #7082
Closed
2 of 6 tasks

LFS: Error 500 when merging pull request #732

axeloz opened this issue Jan 23, 2017 · 18 comments · Fixed by #7082
Labels
Milestone

Comments

@axeloz
Copy link

axeloz commented Jan 23, 2017

  • Gitea version (or commit ref): 1.0.0+144-ga8048c1 (master branch)
  • Git version: 2.5.4 (Apple Git-61)
  • Operating system: MacOS
  • Database (use [x]):
    • PostgreSQL
    • MySQL
    • SQLite
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes (provide example URL)
    • No
    • Not relevant
  • Log gist:
2017/01/23 12:12:17 [...routers/repo/pull.go:420 MergePullRequest()] [E] Merge: git clone: Cloning into 'data/tmp/repos/216094242.git'...
done.
Downloading nofilter.mp4 (2.06 MB)
Error downloading object: nofilter.mp4 (8a870a2f4efbc72e3686c2936b24c5c6f7ebd9475e2b076bd583086e71a345cf)

Description

I wanted to test the master version of Gitea in order to check the new features including LFS.
I added a new repo in Gitea, pushed a mp4 file which I did set for LFS. The file was properly pushed and stored using LFS.

Then I branched the "master" branch into a "dev". I push a "test.html" file into the "dev" branch. Then in Gitea I created a pull request. Gitea said there were no conflict and that the branch can be merged automatically. But when merging, I got a 500 error (see log above). It seems related to the LFS.

Dunno if I did something wrong, but seems to be a bug to me.
Thanks

@lunny lunny added this to the 1.1.0 milestone Jan 23, 2017
@lunny lunny added the type/bug label Jan 23, 2017
@lunny
Copy link
Member

lunny commented Jan 23, 2017

@fabian-z could you give some idea?

@fabian-z
Copy link
Contributor

fabian-z commented Jan 23, 2017

The error is caused by running Gitea under an OS user with the LFS client hooks installed - a workaround is to create a new user account and run Gitea using that account.

This was - apparently incompletely - fixed in my PR by disabling the hooks for external git operations using global command arguments in code.gitea.io/git.

Issue here is that the pull request model uses code.gitea.io/gitea/modules/process package to construct a git command-line: https://github.com/go-gitea/gitea/blob/master/models/pull.go#L284
Therefore, the arguments set for code.gitea.io/git don't get applied and the operation errors out.

Unfortunately, said package is used in many files to construct custom git command-lines. I originally assumed the git package would already provide an abstraction layer.

models/git_diff.go
models/release.go
models/repo.go
models/repo_editor.go
models/repo_mirror.go
models/pull.go

We could either instrument these code parts with the necessary arguments, or port the relevant bits for usage with code.gitea.io/git and get global argument support with it.

@lunny
Copy link
Member

lunny commented Feb 23, 2017

Since no PR to be sent, I will move this to v1.2

@lunny lunny modified the milestones: 1.2.0, 1.1.0 Feb 23, 2017
@lunny lunny modified the milestones: 1.3.0, 1.2.0 May 20, 2017
@lunny lunny modified the milestones: 1.3.0, 1.x.x Oct 25, 2017
@iCodeSometime
Copy link

@fabian-z This is still happening with LFS enabled.

I'm unsure how to run gitea as a local user account.
I tried creating a new user account, and set the service to run under this account, and changing RUN_USER in the app.ini to run under that user, but the service will not start under that configuration.

@terokorp
Copy link

terokorp commented Mar 16, 2019

Got same error and here is gitea.log record:

==> gitea.log <==
2019/03/16 12:31:24 [...routers/repo/pull.go:589 MergePullRequest()] [E] Merge: git merge --no-ff --no-commit [/var/lib/gitea/data/tmp/local-repo/merge-251661747.git]: exec(11:PullRequest.Merge (git merge --no-ff --no-commit): /var/lib/gitea/data/tmp/local-repo/merge-251661747.git) failed: exit status 128(<nil>) stdout:  stderr: Downloading Assets/_myproject/Models/humanoid.fbm/tex_diffuse.png (7.1 MB)

hint: The remote resolves to a file:// URL, which can only work with a
hint: standalone transfer agent.  See section "Using a Custom Transfer Type
hint: without the API server" in custom-transfers.md for details.
Error downloading object: Assets/_myproject/Models/humanoid.fbm/tex_diffuse.png (142123c): Smudge error: Error downloading Assets/_myproject/Models/humanoid.fbm/tex_diffuse.png (142123cd5c85098a8f3c2ae5e7ac600509cc07bd9cc1d0b2591ea357512eb45e): batch request: missing protocol: "file:///home/git/gitea-repositories/team/myproject.git/info/lfs"

Errors logged to /var/lib/gitea/data/tmp/local-repo/merge-251661747.git/.git/lfs/logs/20190316T123115.744431669.log
Use 'git lfs logs last' to view the log.
error: external filter 'git-lfs filter-process' failed
fatal: Assets/_myproject/Models/humanoid.fbm/tex_diffuse.png: smudge filter lfs failed
 - Downloading Assets/_myproject/Models/humanoid.fbm/tex_diffuse.png (7.1 MB)

hint: The remote resolves to a file:// URL, which can only work with a
hint: standalone transfer agent.  See section "Using a Custom Transfer Type
hint: without the API server" in custom-transfers.md for details.
Error downloading object: Assets/_myproject/Models/humanoid.fbm/tex_diffuse.png (142123c): Smudge error: Error downloading Assets/_myproject/Models/humanoid.fbm/tex_diffuse.png (142123cd5c85098a8f3c2ae5e7ac600509cc07bd9cc1d0b2591ea357512eb45e): batch request: missing protocol: "file:///home/git/gitea-repositories/team/myproject.git/info/lfs"

Errors logged to /var/lib/gitea/data/tmp/local-repo/merge-251661747.git/.git/lfs/logs/20190316T123115.744431669.log
Use 'git lfs logs last' to view the log.
error: external filter 'git-lfs filter-process' failed
fatal: Assets/_myproject/Models/humanoid.fbm/tex_diffuse.png: smudge filter lfs failed

Application Version: 1.7.0
Installed by folloring this: https://docs.gitea.io/en-us/install-from-binary/

@stale
Copy link

stale bot commented May 15, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 weeks. Thank you for your contributions.

@stale stale bot added issue/stale and removed issue/stale labels May 15, 2019
@zeripath
Copy link
Contributor

Ah! I think I understand what's causing this!

Can anyone still reproduce this on 1.8+?

@zeripath zeripath changed the title Error 500 when merging pull request LFS: Error 500 when merging pull request May 25, 2019
@zeripath
Copy link
Contributor

I think I may finally have found the cause.

I suspect that you have git-lfs version greater than 2.3.4 installed on your server and gitea user has a .gitconfig file containing:

[filter "lfs"]
	clean = git-lfs clean -- %f
	smudge = git-lfs smudge -- %f
	process = git-lfs filter-process
	required = true

If you remove that that should act as a workaround.

@zeripath
Copy link
Contributor

@Begounet are you able to come up with a minimal example for me? I would love to exactly figure out what is the problem here. I know it's due to the lfs smudge filter but I'd like to catch it in the act.

I think #7062 and this workaround won't be the complete solution because we do need to be aware of lfs additions on merging - i.e. What happens if you add an LFS file through a merge and then delete the adding repository - I'm highly suspicious that this file will be deleted from the LFS.

@Begounet
Copy link

I will try to setup a server from my home, using the same LFS and Gitea versions but it will take some times since I am not really used to that kind of stuff. Unfortunately, I cannot provide access to our enterprise private servers.

But I think the most important thing is to at least have once LFS filtered file in the commit you want to merge.

And your workaround indeed worked! Thank you!

What happens if you add an LFS file through a merge and then delete the adding repository - I'm highly suspicious that this file will be deleted from the LFS.

Shouldn't be the expected behavior?

@zeripath
Copy link
Contributor

@Begounet Was there anything special about your filter configuration in .gitconfig? Was it exactly the same as I put in #732 (comment) ?

The other question is what version of git do you have?

It appears that my PR #7062 is unlikely to be required as we should be setting these (except for process) anyway:

// CheckLFSVersion will check lfs version, if not satisfied, then disable it.
func CheckLFSVersion() {
if LFS.StartServer {
//Disable LFS client hooks if installed for the current OS user
//Needs at least git v2.1.2
binVersion, err := git.BinVersion()
if err != nil {
log.Fatal("Error retrieving git version: %v", err)
}
if !version.Compare(binVersion, "2.1.2", ">=") {
LFS.StartServer = false
log.Error("LFS server support needs at least Git v2.1.2")
} else {
git.GlobalCommandArgs = append(git.GlobalCommandArgs, "-c", "filter.lfs.required=",
"-c", "filter.lfs.smudge=", "-c", "filter.lfs.clean=")
}
}
}

@zeripath
Copy link
Contributor

So perhaps you have the lfs process running...

I'll adjust #7062 to ignore the process at this point instead.

zeripath added a commit to zeripath/gitea that referenced this issue May 28, 2019
@zeripath
Copy link
Contributor

In terms of the potential deletion bug: Suppose you have repository A and fork B.

  • Say you add an LFS'd file f with OID o to B on branch to-merge.
  • Then merge to-merge into A
  • Does an LFSMetaObject get created for f in A?
  • What happens if you delete repo B? If there is no LFSMetaObject for f in A and there is no other user of the oid o then f will get deleted from the LFS.

You would need to check the LFS directly to see if it has disappeared.

This problem wouldn't just affect users like you who are likely affected by the lfs filter process but every user of LFS.

I think gitea needs to handle lfs filtering itself.

@Begounet
Copy link

Begounet commented May 28, 2019

  • The version of the git on the server is 2.4.11 (if I've done the check correctly with <repository url>/info/refs?service=git-upload-pack).
  • The version of my git (client) is 2.21.0.windows.1
  • About the configuration file, I don't know (since I am not the admin sys - I am just relaying to him our conversation here) but I think that was these lines correctly.
    He told me that there were these lines when he executed "git lfs env":

git config filter.lfs.process = "git-lfs filter-process"
git config filter.lfs.smudge = "git-lfs smudge -- %f"
git config filter.lfs.clean = "git-lfs clean -- %f"

And then he founds theses lines in the global config (/etc/gitconfig).

@zeripath
Copy link
Contributor

zeripath commented May 29, 2019

FINALLY! I've managed to reproduce this, and it's stupidly easy to do.

Oh dear.

I'm so sorry this has not been fixed - I can't believe that this has been here for so long.

OK, to reproduce:

  • Create a repository

  • git lfs track '*.jpg'

  • git add .gitattributes

  • git push

    • Or just add a .gitattributes file *.jpg filter=lfs diff=lfs merge=lfs -text on the web interface
  • Then with another user, fork the repository

  • Add a jpg over the web interface

  • create a pull-request with that jpg

  • BOOM

@zeripath
Copy link
Contributor

OK I finally have a working and complete bug fix for this in #7082

Now, the problem is that people who have been merging "successfully", i.e. not experiencing #732 won't realise that if they delete their forks their LFS objects will disappear.

zeripath added a commit to zeripath/gitea that referenced this issue Jun 1, 2019
zeripath added a commit to zeripath/gitea that referenced this issue Jun 2, 2019
lunny added a commit to zeripath/gitea that referenced this issue Jun 22, 2019
zeripath added a commit that referenced this issue Jun 22, 2019
On merge we walk the merge history and ensure that all lfs objects pointed to in
the history are added to the base repository. This switches from relying on having git-lfs installed on the server, (and in fact .gitattributes being correctly installed.)
@lunny lunny modified the milestones: 1.x.x, 1.9.0 Jun 23, 2019
@kleszcz
Copy link

kleszcz commented Jul 9, 2019

Hi, unfortunately we're experiencing this old "500" error when merging pull requests with files in lfs through Gitea.

  • Gitea version 1.8.3/go1.12.5
  • git lfs version 2.7.1
  • database type: sqlite3

Also it comes with error log same as reported in #4463. We've checked for the .gitconfig entries mentioned above and there are none.

@mrsdizzie
Copy link
Member

@kleszcz the mentioned fix for this not present in the 1.8.3 release that you are using. You would need to use master branch or wait for the 1.9 release (1.9 release candidate will be available very soon and hopefully final 1.9 release not terribly long after).

jeffliu27 pushed a commit to jeffliu27/gitea that referenced this issue Jul 18, 2019
…itea#7082)

On merge we walk the merge history and ensure that all lfs objects pointed to in
the history are added to the base repository. This switches from relying on having git-lfs installed on the server, (and in fact .gitattributes being correctly installed.)
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
9 participants