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

Enable cloning via Git Wire Protocol v2 over HTTP #12170

Merged
merged 3 commits into from Jul 7, 2020

Conversation

billiegoose
Copy link
Contributor

This change makes it possible to clone Gitea repositories via HTTP(S) using Git Wire Protocol Version 2, which is not currently possible. Original discussion started with my comment here.

TL;DR - we need to run the serviceRPC and getInfoRefs with a new environment variable, GIT_PROTOCOL.

I compiled and ran it locally using the sqlite backend and successfully got partial clone working! (It requires running git config uploadpack.allowfilter 1 on each repository in ~/gitea-repositories, but partial clone is considered experimental so some extra work to enable it seems fair.)

2020-07-06 23 45 11

@techknowlogick techknowlogick added this to the 1.13.0 milestone Jul 7, 2020
@techknowlogick techknowlogick added the type/enhancement An improvement of existing functionality label Jul 7, 2020
@@ -227,6 +227,12 @@ func (c *Command) RunInDirWithEnv(dir string, env []string) (string, error) {
return string(stdout), nil
}

// RunInDirWithEnvBytes executes the command in given directory
// and returns stdout in []byte and error (combined with stderr).
func (c *Command) RunInDirWithEnvBytes(dir string, env []string) ([]byte, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The wrap function seems unnecessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, this file scares me lol. They all look like mostly unnecessary wrapper functions to me. But I was trying to go along with the style. 🤷‍♂️

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just use RunInDirTimeoutEnv(env, -1, dir) directly instead. At some point we'll hopefully destroy all of these and replace them with a single RunOptions{} that can have things set.

Honestly, usually what happens is I end up escalating all the way up to FullPipeline to use Readers because a lot of functions misuse storing unbounded results and/or the error munging with these downstream functions ends up being unhelpful.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 7, 2020
@willstott101
Copy link

Is there some central location where one could setup uploadpack.allowfilter 1? If we have to run it via SSH on all repos then new repos won't benefit from partial clone?

Even if it's experimental a central flag would be great

@zeripath
Copy link
Contributor

zeripath commented Jul 7, 2020

@willstott101 is that a git config flag? You can just add it to the global or Gitea user .gitconfig

@billiegoose
Copy link
Contributor Author

@willstott101 is that a git config flag? You can just add it to the global or Gitea user .gitconfig

🤔 Yeah.... that sounds like it should work... I haven't had success with that approach yet for some reason. Does gitea use a different ~/.gitconfig file? I thought it was running as my normal user.

modules/git/command.go Outdated Show resolved Hide resolved
routers/repo/http.go Outdated Show resolved Hide resolved
@billiegoose
Copy link
Contributor Author

billiegoose commented Jul 7, 2020

So setting the value in ~/.gitconfig seems to work locally:

> git config --global uploadpack.allowfilter true
> GIT_PROTOCOL=version=2 git upload-pack --stateless-rpc --advertise-refs .
000eversion 2
0015agent=git/2.27.0
000cls-refs
0019fetch=shallow filter
0012server-option
0000
> git config --global uploadpack.allowfilter false
> GIT_PROTOCOL=version=2 git upload-pack --stateless-rpc --advertise-refs .
000eversion 2
0015agent=git/2.27.0
000cls-refs
0012fetch=shallow
0012server-option
0000

But when gitea runs git upload-pack it seems like it isn't picking up that setting, and always returns fetch=shallow and not fetch=shallow filter unless I specifically run git config uploadpack.allowfilter true in the repo. :/ Maybe it's a shell environment setting or something...

@billiegoose
Copy link
Contributor Author

Ah. I figured out why ~/.gitconfig isn't respected. getInfoRefs runs without any of the host environment variables, crucially the $HOME environment variable. If I add:

		if protocol := h.r.Header.Get("Git-Protocol"); protocol != "" && safeGitProtocolHeader.MatchString(protocol) {
			h.environ = append(h.environ, "GIT_PROTOCOL="+protocol)
		}
+		h.environ = append(h.environ, "HOME="+os.Getenv("HOME"))

		refs, err := git.NewCommand(service, "--stateless-rpc", "--advertise-refs", ".").RunInDirTimeoutEnv(h.environ, -1, h.dir)

then it picks up that uploadpack.allowfilter is true and advertises fetch=shallow filter as desired.

I'm thinking the most reliable thing to do would be to run getInfoRefs with all the host environment variables like we do for serviceRPC:

cmd.Env = append(os.Environ(), h.environ...)

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jul 7, 2020
routers/repo/http.go Outdated Show resolved Hide resolved
@zeripath
Copy link
Contributor

zeripath commented Jul 7, 2020

Damn missed that you had done that... os.Environ() is used if you don't pass a different environment. You need to apply my suggestion

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jul 7, 2020
@lafriks lafriks merged commit 3a46d1e into go-gitea:master Jul 7, 2020
@billiegoose
Copy link
Contributor Author

Woohoo! Thanks y'all!

@billiegoose billiegoose deleted the feat/http-git-protocol-v2 branch July 8, 2020 00:59
ydelafollye pushed a commit to ydelafollye/gitea that referenced this pull request Jul 31, 2020
* Enable Git Wire Protocol v2 over HTTP

* use RunInDirTimeoutEnv

* pass $HOME environment variable to git upload-pack
@lunny lunny added the type/changelog Adds the changelog for a new Gitea version label Sep 17, 2020
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
@delvh delvh removed the type/changelog Adds the changelog for a new Gitea version label Oct 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants