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

git clone with --filter=blob:none fails with older git clients #19118

Closed
cboylan opened this issue Mar 17, 2022 · 11 comments · Fixed by #19430
Closed

git clone with --filter=blob:none fails with older git clients #19118

cboylan opened this issue Mar 17, 2022 · 11 comments · Fixed by #19430

Comments

@cboylan
Copy link
Contributor

cboylan commented Mar 17, 2022

Gitea Version

1.16.4

Git Version

git version 2.30.2

Operating System

Debian Bullseye

How are you running Gitea?

We build our own docker images based on Debian Bullseye using Golang 1.17. We then deploy this image using docker and docker-compose.

Database

MySQL

Can you reproduce the bug on the Gitea demo site?

Yes

Log Gist

No response

Description

Gitea 1.16 introduced support for partial clones by default in 4b3bfd7. The git client on Ubuntu Focal version 2.25.1 fails to clone repositories hosted by Gitea 1.16.4 if the --filter=blob:none flag is used. The git client included in Ubuntu Hirsute version 2.30.2 does not fail. This is a problem because some tools like pip set this flag when fetching resources from git repositories.

We are working around this by setting git.DISABLE_PARTIAL_CLONE to true in our app.ini config file.

I am not sure if this is a bug in Gitea, the version of git on our Gitea server (the version comes from Debian Bullseye and is 2.30.2), or the client version of git..

This is what it looks like reproduced against the trygitea.io site:

$ git clone --filter=blob:none https://try.gitea.io/karambaza/testRep
Cloning into 'testRep'...
remote: Enumerating objects: 6, done.
remote: Counting objects: 100% (6/6), done.
remote: Compressing objects: 100% (5/5), done.
remote: Total 6 (delta 0), reused 0 (delta 0), pack-reused 0
Receiving objects: 100% (6/6), 675 bytes | 675.00 KiB/s, done.
fatal: the remote end hung up unexpectedly
fatal: protocol error: bad pack header
warning: https unexpectedly said: '0000'
warning: Clone succeeded, but checkout failed.
You can inspect what was checked out with 'git status'
and retry with 'git restore --source=HEAD :/'

Even if the bug isn't in Gitea itself I think that Gitea should fall back to the old behavior of ignoring the --filter options on clone when talking to a client that cannot support this functionality.

Screenshots

No response

@zeripath
Copy link
Contributor

I think we need some more verbose git client logs here. The bad pack header is coming out git itself. Now it could be that the protocol that gitea is handling over http is incorrect but it's hard to see.

Would you be able to give us more verbose logs from the client?

Does SSH work?

@Gusted
Copy link
Contributor

Gusted commented Mar 20, 2022

^ SSH:

-> % git clone --filter=blob:none git@try.gitea.io:karambaza/testRep.git
Cloning into 'testRep'...
warning: filtering not recognized by server, ignoring <<<<<<<<<<<<<<<<<<
remote: Enumerating objects: 9, done.
remote: Counting objects: 100% (9/9), done.
remote: Compressing objects: 100% (6/6), done.
remote: Total 9 (delta 0), reused 0 (delta 0), pack-reused 0
Receiving objects: 100% (9/9), done.

@Gusted
Copy link
Contributor

Gusted commented Mar 20, 2022

Even if the bug isn't in Gitea itself I think that Gitea should fall back to the old behavior of ignoring the --filter options on clone when talking to a client that cannot support this functionality.

There was not really a old behavior ever, git would ignore it by-default unless the site administrator enabled it per-repo basis, since that commit git will be allowed to act on the filter when passed. It's not a logical situation from how I see it, a older client is passing on options which the client doesn't even support.

@zeripath IIRC when partial cloning is done, git will send some information about it, it seems like because it's a older client git don't expect the data to be their and thus errors.

@Gusted
Copy link
Contributor

Gusted commented Mar 20, 2022

Hmm telling git to don't allow partial clones will result in:

exit status 128 - fatal: git upload-pack: filtering capability not negotiated

https://github.com/git/git/blob/74cc1aa55f30ed76424a0e7226ab519aa6265061/upload-pack.c#L1077

So we would need to read the request body and then filter(no pun intended) away the filter request. That seems a bit overkill to me as we don't have that architecture ATM.

@cboylan
Copy link
Contributor Author

cboylan commented Mar 21, 2022

There was not really a old behavior ever, git would ignore it by-default unless the site administrator enabled it per-repo basis, since that commit git will be allowed to act on the filter when passed. It's not a logical situation from how I see it, a older client is passing on options which the client doesn't even support.

Ah yup I see that the Ubuntu Git 2.25.1 man page does not document the --filter flag. The Ubuntu Git 2.30.2 man page does document the flag. However, gitlab documentation indicates that git 2.22.0 should be sufficiently new to support the --filter option for partial clones. The upstream git documentation for partial clones seems to first show up with version 2.19.0.

Based on this it isn't clear to me if the git client just doesn't support this functionality on those versions, or if the client is buggy, or if the server is doing something wrong.

Hmm telling git to don't allow partial clones will result in:

exit status 128 - fatal: git upload-pack: filtering capability not negotiated

When we set the Gitea option git.DISABLE_PARTIAL_CLONE then all clients running with --filter=blob:none produce:

warning: filtering not recognized by server, ignoring

This works well enough for us for now though we don't get to take advantage of the better performance with newer clients.

@cboylan
Copy link
Contributor Author

cboylan commented Mar 21, 2022

I've just realized that I failed to neglect to mention that this worked file with Gitea v1.15.11. I think that is because the old behavior in Gitea is the same we see now with git.DISABLE_PARTIAL_CLONE set.

@osfrickler
Copy link

Also for comparison, using the same option against the same repo hosted on github works without issues and does indeed seem to make use of partial cloning, as it reports less objects transferred than without that option.

@zeripath
Copy link
Contributor

I think we need some more verbose git client logs here. The bad pack header is coming out git itself. Now it could be that the protocol that gitea is handling over http is incorrect but it's hard to see.

Would you be able to give us more verbose logs from the client?

@cboylan
Copy link
Contributor Author

cboylan commented Mar 28, 2022

Would you be able to give us more verbose logs from the client?

I'm not sure if these are the more verbose logs you are looking for but I did my best: https://gist.github.com/cboylan/ee695bfe439efc54727f93b22a0dd5c2 in particular I think lines 100-104 show where it is having problems.

As a side note I'm able to reproduce this trivially using docker run -it --rm ubuntu:focal bash then in the container apt-get update && apt-get install git then do the git clones as mentioned above. Then you can add whatever GIT_TRACE* flags are appropriate.

@zeripath
Copy link
Contributor

zeripath commented Mar 28, 2022

Thanks. That's the kind of information I was looking for - plus the reproducer info.

Now it looks to me like the response is coming directly from git itself without Gitea being definitely involved. There's a part of Gitea that does some git protocol communication but I have a feeling that that isn't here.

I guess we need to walk through the protocol and see where the failure is.

If you're interested, here are where we handle git protocol:

func serviceRPC(ctx gocontext.Context, h serviceHandler, service string) {

These are the 0000 packets we write:

_, _ = ctx.Write(packetWrite("# service=git-receive-pack\n"))
_, _ = ctx.Write([]byte("0000"))

_, _ = h.w.Write(packetWrite("# service=git-" + service + "\n"))
_, _ = h.w.Write([]byte("0000"))

I would guess adding some checkpoints in a debugger or log statements there and elsewhere around this file might be helpful

zeripath added a commit to zeripath/gitea that referenced this issue Apr 19, 2022
…der git clients

Older git clients need uploadpack.allowAnySHA1InWant if partial cloning is allowed.

Fix go-gitea#19118

Signed-off-by: Andrew Thornton <art27@cantab.net>
6543 pushed a commit that referenced this issue Apr 20, 2022
…der git clients (#19430)

Older git clients need uploadpack.allowAnySHA1InWant if partial cloning is allowed.

Fix #19118

Signed-off-by: Andrew Thornton <art27@cantab.net>
6543 pushed a commit to 6543-forks/gitea that referenced this issue Apr 20, 2022
…der git clients (go-gitea#19430)

Older git clients need uploadpack.allowAnySHA1InWant if partial cloning is allowed.

Fix go-gitea#19118

Signed-off-by: Andrew Thornton <art27@cantab.net>
6543 added a commit that referenced this issue Apr 20, 2022
…der git clients (#19430) (#19438)

Older git clients need uploadpack.allowAnySHA1InWant if partial cloning is allowed.

Fix #19118

Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: zeripath <art27@cantab.net>
@cboylan
Copy link
Contributor Author

cboylan commented Apr 21, 2022

Thank you for fixing this. I had intended on trying to dig into this a bit more but ended up being distracted by other things. We've since deployed the v1.16.6 release which includes this fix, and it seems to be working for us.

AbdulrhmnGhanem pushed a commit to kitspace/gitea that referenced this issue Aug 24, 2022
…der git clients (go-gitea#19430)

Older git clients need uploadpack.allowAnySHA1InWant if partial cloning is allowed.

Fix go-gitea#19118

Signed-off-by: Andrew Thornton <art27@cantab.net>
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants