-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Enable cloning via Git Wire Protocol v2 over HTTP #12170
Conversation
modules/git/command.go
Outdated
@@ -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) { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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. 🤷♂️
There was a problem hiding this comment.
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.
Is there some central location where one could setup Even if it's experimental a central flag would be great |
@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 |
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 |
Ah. I figured out why 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 I'm thinking the most reliable thing to do would be to run cmd.Env = append(os.Environ(), h.environ...) |
Damn missed that you had done that... |
Woohoo! Thanks y'all! |
* Enable Git Wire Protocol v2 over HTTP * use RunInDirTimeoutEnv * pass $HOME environment variable to git upload-pack
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
andgetInfoRefs
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.)