Performance optimization for git push (#30104)

Agit returned result should be from `ProcReceive` hook but not
`PostReceive` hook. Then for all non-agit pull requests, it will not
check the pull requests for every pushing `refs/pull/%d/head`.
This commit is contained in:
Lunny Xiao 2024-04-09 11:43:17 +08:00 committed by GitHub
parent 72dc75e594
commit 263a716cb5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 110 additions and 74 deletions

View File

@ -448,21 +448,24 @@ Gitea or set your environment appropriately.`, "")
func hookPrintResults(results []private.HookPostReceiveBranchResult) { func hookPrintResults(results []private.HookPostReceiveBranchResult) {
for _, res := range results { for _, res := range results {
if !res.Message { hookPrintResult(res.Message, res.Create, res.Branch, res.URL)
continue
} }
}
func hookPrintResult(output, isCreate bool, branch, url string) {
if !output {
return
}
fmt.Fprintln(os.Stderr, "") fmt.Fprintln(os.Stderr, "")
if res.Create { if isCreate {
fmt.Fprintf(os.Stderr, "Create a new pull request for '%s':\n", res.Branch) fmt.Fprintf(os.Stderr, "Create a new pull request for '%s':\n", branch)
fmt.Fprintf(os.Stderr, " %s\n", res.URL) fmt.Fprintf(os.Stderr, " %s\n", url)
} else { } else {
fmt.Fprint(os.Stderr, "Visit the existing pull request:\n") fmt.Fprint(os.Stderr, "Visit the existing pull request:\n")
fmt.Fprintf(os.Stderr, " %s\n", res.URL) fmt.Fprintf(os.Stderr, " %s\n", url)
} }
fmt.Fprintln(os.Stderr, "") fmt.Fprintln(os.Stderr, "")
os.Stderr.Sync() os.Stderr.Sync()
}
} }
func pushOptions() map[string]string { func pushOptions() map[string]string {
@ -691,6 +694,12 @@ Gitea or set your environment appropriately.`, "")
} }
err = writeFlushPktLine(ctx, os.Stdout) err = writeFlushPktLine(ctx, os.Stdout)
if err == nil {
for _, res := range resp.Results {
hookPrintResult(res.ShouldShowMessage, res.IsCreatePR, res.HeadBranch, res.URL)
}
}
return err return err
} }

View File

@ -11,6 +11,7 @@ import (
"time" "time"
"code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/optional"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
) )
@ -32,13 +33,13 @@ const (
) )
// Bool checks for a key in the map and parses as a boolean // Bool checks for a key in the map and parses as a boolean
func (g GitPushOptions) Bool(key string, def bool) bool { func (g GitPushOptions) Bool(key string) optional.Option[bool] {
if val, ok := g[key]; ok { if val, ok := g[key]; ok {
if b, err := strconv.ParseBool(val); err == nil { if b, err := strconv.ParseBool(val); err == nil {
return b return optional.Some(b)
} }
} }
return def return optional.None[bool]()
} }
// HookOptions represents the options for the Hook calls // HookOptions represents the options for the Hook calls
@ -94,6 +95,10 @@ type HookProcReceiveRefResult struct {
IsForcePush bool IsForcePush bool
IsNotMatched bool IsNotMatched bool
Err string Err string
IsCreatePR bool
URL string
ShouldShowMessage bool
HeadBranch string
} }
// HookPreReceive check whether the provided commits are allowed // HookPreReceive check whether the provided commits are allowed

View File

@ -6,11 +6,12 @@ package private
import ( import (
"fmt" "fmt"
"net/http" "net/http"
"strconv"
git_model "code.gitea.io/gitea/models/git" git_model "code.gitea.io/gitea/models/git"
issues_model "code.gitea.io/gitea/models/issues" issues_model "code.gitea.io/gitea/models/issues"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo" repo_model "code.gitea.io/gitea/models/repo"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/gitrepo"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
@ -159,8 +160,10 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) {
} }
} }
isPrivate := opts.GitPushOptions.Bool(private.GitPushOptionRepoPrivate)
isTemplate := opts.GitPushOptions.Bool(private.GitPushOptionRepoTemplate)
// Handle Push Options // Handle Push Options
if len(opts.GitPushOptions) > 0 { if isPrivate.Has() || isTemplate.Has() {
// load the repository // load the repository
if repo == nil { if repo == nil {
repo = loadRepository(ctx, ownerName, repoName) repo = loadRepository(ctx, ownerName, repoName)
@ -171,13 +174,49 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) {
wasEmpty = repo.IsEmpty wasEmpty = repo.IsEmpty
} }
repo.IsPrivate = opts.GitPushOptions.Bool(private.GitPushOptionRepoPrivate, repo.IsPrivate) pusher, err := user_model.GetUserByID(ctx, opts.UserID)
repo.IsTemplate = opts.GitPushOptions.Bool(private.GitPushOptionRepoTemplate, repo.IsTemplate) if err != nil {
if err := repo_model.UpdateRepositoryCols(ctx, repo, "is_private", "is_template"); err != nil {
log.Error("Failed to Update: %s/%s Error: %v", ownerName, repoName, err) log.Error("Failed to Update: %s/%s Error: %v", ownerName, repoName, err)
ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{ ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{
Err: fmt.Sprintf("Failed to Update: %s/%s Error: %v", ownerName, repoName, err), Err: fmt.Sprintf("Failed to Update: %s/%s Error: %v", ownerName, repoName, err),
}) })
return
}
perm, err := access_model.GetUserRepoPermission(ctx, repo, pusher)
if err != nil {
log.Error("Failed to Update: %s/%s Error: %v", ownerName, repoName, err)
ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{
Err: fmt.Sprintf("Failed to Update: %s/%s Error: %v", ownerName, repoName, err),
})
return
}
if !perm.IsOwner() && !perm.IsAdmin() {
ctx.JSON(http.StatusNotFound, private.HookPostReceiveResult{
Err: "Permissions denied",
})
return
}
cols := make([]string, 0, len(opts.GitPushOptions))
if isPrivate.Has() {
repo.IsPrivate = isPrivate.Value()
cols = append(cols, "is_private")
}
if isTemplate.Has() {
repo.IsTemplate = isTemplate.Value()
cols = append(cols, "is_template")
}
if len(cols) > 0 {
if err := repo_model.UpdateRepositoryCols(ctx, repo, cols...); err != nil {
log.Error("Failed to Update: %s/%s Error: %v", ownerName, repoName, err)
ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{
Err: fmt.Sprintf("Failed to Update: %s/%s Error: %v", ownerName, repoName, err),
})
return
}
} }
} }
@ -192,42 +231,6 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) {
refFullName := opts.RefFullNames[i] refFullName := opts.RefFullNames[i]
newCommitID := opts.NewCommitIDs[i] newCommitID := opts.NewCommitIDs[i]
// post update for agit pull request
// FIXME: use pr.Flow to test whether it's an Agit PR or a GH PR
if git.DefaultFeatures.SupportProcReceive && refFullName.IsPull() {
if repo == nil {
repo = loadRepository(ctx, ownerName, repoName)
if ctx.Written() {
return
}
}
pullIndex, _ := strconv.ParseInt(refFullName.PullName(), 10, 64)
if pullIndex <= 0 {
continue
}
pr, err := issues_model.GetPullRequestByIndex(ctx, repo.ID, pullIndex)
if err != nil && !issues_model.IsErrPullRequestNotExist(err) {
log.Error("Failed to get PR by index %v Error: %v", pullIndex, err)
ctx.JSON(http.StatusInternalServerError, private.Response{
Err: fmt.Sprintf("Failed to get PR by index %v Error: %v", pullIndex, err),
})
return
}
if pr == nil {
continue
}
results = append(results, private.HookPostReceiveBranchResult{
Message: setting.Git.PullRequestPushMessage && repo.AllowsPulls(ctx),
Create: false,
Branch: "",
URL: fmt.Sprintf("%s/pulls/%d", repo.HTMLURL(), pr.Index),
})
continue
}
// If we've pushed a branch (and not deleted it) // If we've pushed a branch (and not deleted it)
if !git.IsEmptyCommitID(newCommitID) && refFullName.IsBranch() { if !git.IsEmptyCommitID(newCommitID) && refFullName.IsBranch() {
// First ensure we have the repository loaded, we're allowed pulls requests and we can get the base repo // First ensure we have the repository loaded, we're allowed pulls requests and we can get the base repo

View File

@ -16,6 +16,7 @@ import (
"code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/private" "code.gitea.io/gitea/modules/private"
"code.gitea.io/gitea/modules/setting"
notify_service "code.gitea.io/gitea/services/notify" notify_service "code.gitea.io/gitea/services/notify"
pull_service "code.gitea.io/gitea/services/pull" pull_service "code.gitea.io/gitea/services/pull"
) )
@ -149,6 +150,10 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git.
OriginalRef: opts.RefFullNames[i], OriginalRef: opts.RefFullNames[i],
OldOID: objectFormat.EmptyObjectID().String(), OldOID: objectFormat.EmptyObjectID().String(),
NewOID: opts.NewCommitIDs[i], NewOID: opts.NewCommitIDs[i],
IsCreatePR: true,
URL: fmt.Sprintf("%s/pulls/%d", repo.HTMLURL(), pr.Index),
ShouldShowMessage: setting.Git.PullRequestPushMessage && repo.AllowsPulls(ctx),
HeadBranch: headBranch,
}) })
continue continue
} }
@ -213,6 +218,9 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git.
Ref: pr.GetGitRefName(), Ref: pr.GetGitRefName(),
OriginalRef: opts.RefFullNames[i], OriginalRef: opts.RefFullNames[i],
IsForcePush: isForcePush, IsForcePush: isForcePush,
IsCreatePR: false,
URL: fmt.Sprintf("%s/pulls/%d", repo.HTMLURL(), pr.Index),
ShouldShowMessage: setting.Git.PullRequestPushMessage && repo.AllowsPulls(ctx),
}) })
} }

View File

@ -49,6 +49,17 @@ func testGitPush(t *testing.T, u *url.URL) {
}) })
}) })
t.Run("Push branch with options", func(t *testing.T) {
runTestGitPush(t, u, func(t *testing.T, gitPath string) (pushed, deleted []string) {
branchName := "branch-with-options"
doGitCreateBranch(gitPath, branchName)(t)
doGitPushTestRepository(gitPath, "origin", branchName, "-o", "repo.private=true", "-o", "repo.template=true")(t)
pushed = append(pushed, branchName)
return pushed, deleted
})
})
t.Run("Delete branches", func(t *testing.T) { t.Run("Delete branches", func(t *testing.T) {
runTestGitPush(t, u, func(t *testing.T, gitPath string) (pushed, deleted []string) { runTestGitPush(t, u, func(t *testing.T, gitPath string) (pushed, deleted []string) {
doGitPushTestRepository(gitPath, "origin", "master")(t) // make sure master is the default branch instead of a branch we are going to delete doGitPushTestRepository(gitPath, "origin", "master")(t) // make sure master is the default branch instead of a branch we are going to delete