aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLunny Xiao <xiaolunwen@gmail.com>2024-04-09 11:43:17 +0800
committerGitHub <noreply@github.com>2024-04-09 03:43:17 +0000
commit263a716cb52037f3e7e51f014f6c8cdfad6ae03d (patch)
tree2ca921707de7f1d35bacefe0f06a9ea0aa676f32
parent72dc75e594fb5227abfa1cb74cb652cc33bacc93 (diff)
downloadgitea-263a716cb52037f3e7e51f014f6c8cdfad6ae03d.tar.gz
gitea-263a716cb52037f3e7e51f014f6c8cdfad6ae03d.zip
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`.
-rw-r--r--cmd/hook.go35
-rw-r--r--modules/private/hook.go25
-rw-r--r--routers/private/hook_post_receive.go85
-rw-r--r--services/agit/agit.go26
-rw-r--r--tests/integration/git_push_test.go11
5 files changed, 109 insertions, 73 deletions
diff --git a/cmd/hook.go b/cmd/hook.go
index 6a3358853d..c04591d79e 100644
--- a/cmd/hook.go
+++ b/cmd/hook.go
@@ -448,21 +448,24 @@ Gitea or set your environment appropriately.`, "")
func hookPrintResults(results []private.HookPostReceiveBranchResult) {
for _, res := range results {
- if !res.Message {
- continue
- }
+ hookPrintResult(res.Message, res.Create, res.Branch, res.URL)
+ }
+}
- fmt.Fprintln(os.Stderr, "")
- if res.Create {
- fmt.Fprintf(os.Stderr, "Create a new pull request for '%s':\n", res.Branch)
- fmt.Fprintf(os.Stderr, " %s\n", res.URL)
- } else {
- fmt.Fprint(os.Stderr, "Visit the existing pull request:\n")
- fmt.Fprintf(os.Stderr, " %s\n", res.URL)
- }
- fmt.Fprintln(os.Stderr, "")
- os.Stderr.Sync()
+func hookPrintResult(output, isCreate bool, branch, url string) {
+ if !output {
+ return
+ }
+ fmt.Fprintln(os.Stderr, "")
+ if isCreate {
+ fmt.Fprintf(os.Stderr, "Create a new pull request for '%s':\n", branch)
+ fmt.Fprintf(os.Stderr, " %s\n", url)
+ } else {
+ fmt.Fprint(os.Stderr, "Visit the existing pull request:\n")
+ fmt.Fprintf(os.Stderr, " %s\n", url)
}
+ fmt.Fprintln(os.Stderr, "")
+ os.Stderr.Sync()
}
func pushOptions() map[string]string {
@@ -691,6 +694,12 @@ Gitea or set your environment appropriately.`, "")
}
err = writeFlushPktLine(ctx, os.Stdout)
+ if err == nil {
+ for _, res := range resp.Results {
+ hookPrintResult(res.ShouldShowMessage, res.IsCreatePR, res.HeadBranch, res.URL)
+ }
+ }
+
return err
}
diff --git a/modules/private/hook.go b/modules/private/hook.go
index cab8c81224..79c3d48229 100644
--- a/modules/private/hook.go
+++ b/modules/private/hook.go
@@ -11,6 +11,7 @@ import (
"time"
"code.gitea.io/gitea/modules/git"
+ "code.gitea.io/gitea/modules/optional"
"code.gitea.io/gitea/modules/setting"
)
@@ -32,13 +33,13 @@ const (
)
// 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 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
@@ -87,13 +88,17 @@ type HookProcReceiveResult struct {
// HookProcReceiveRefResult represents an individual result from ProcReceive
type HookProcReceiveRefResult struct {
- OldOID string
- NewOID string
- Ref string
- OriginalRef git.RefName
- IsForcePush bool
- IsNotMatched bool
- Err string
+ OldOID string
+ NewOID string
+ Ref string
+ OriginalRef git.RefName
+ IsForcePush bool
+ IsNotMatched bool
+ Err string
+ IsCreatePR bool
+ URL string
+ ShouldShowMessage bool
+ HeadBranch string
}
// HookPreReceive check whether the provided commits are allowed
diff --git a/routers/private/hook_post_receive.go b/routers/private/hook_post_receive.go
index 101ae92302..769a68970d 100644
--- a/routers/private/hook_post_receive.go
+++ b/routers/private/hook_post_receive.go
@@ -6,11 +6,12 @@ package private
import (
"fmt"
"net/http"
- "strconv"
git_model "code.gitea.io/gitea/models/git"
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"
+ user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/gitrepo"
"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
- if len(opts.GitPushOptions) > 0 {
+ if isPrivate.Has() || isTemplate.Has() {
// load the repository
if repo == nil {
repo = loadRepository(ctx, ownerName, repoName)
@@ -171,13 +174,49 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) {
wasEmpty = repo.IsEmpty
}
- repo.IsPrivate = opts.GitPushOptions.Bool(private.GitPushOptionRepoPrivate, repo.IsPrivate)
- repo.IsTemplate = opts.GitPushOptions.Bool(private.GitPushOptionRepoTemplate, repo.IsTemplate)
- if err := repo_model.UpdateRepositoryCols(ctx, repo, "is_private", "is_template"); err != nil {
+ pusher, err := user_model.GetUserByID(ctx, opts.UserID)
+ 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
+ }
+ 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]
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 !git.IsEmptyCommitID(newCommitID) && refFullName.IsBranch() {
// First ensure we have the repository loaded, we're allowed pulls requests and we can get the base repo
diff --git a/services/agit/agit.go b/services/agit/agit.go
index eb3bafa906..52a70469e0 100644
--- a/services/agit/agit.go
+++ b/services/agit/agit.go
@@ -16,6 +16,7 @@ import (
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/private"
+ "code.gitea.io/gitea/modules/setting"
notify_service "code.gitea.io/gitea/services/notify"
pull_service "code.gitea.io/gitea/services/pull"
)
@@ -145,10 +146,14 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git.
log.Trace("Pull request created: %d/%d", repo.ID, prIssue.ID)
results = append(results, private.HookProcReceiveRefResult{
- Ref: pr.GetGitRefName(),
- OriginalRef: opts.RefFullNames[i],
- OldOID: objectFormat.EmptyObjectID().String(),
- NewOID: opts.NewCommitIDs[i],
+ Ref: pr.GetGitRefName(),
+ OriginalRef: opts.RefFullNames[i],
+ OldOID: objectFormat.EmptyObjectID().String(),
+ 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
}
@@ -208,11 +213,14 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git.
isForcePush := comment != nil && comment.IsForcePush
results = append(results, private.HookProcReceiveRefResult{
- OldOID: oldCommitID,
- NewOID: opts.NewCommitIDs[i],
- Ref: pr.GetGitRefName(),
- OriginalRef: opts.RefFullNames[i],
- IsForcePush: isForcePush,
+ OldOID: oldCommitID,
+ NewOID: opts.NewCommitIDs[i],
+ Ref: pr.GetGitRefName(),
+ OriginalRef: opts.RefFullNames[i],
+ IsForcePush: isForcePush,
+ IsCreatePR: false,
+ URL: fmt.Sprintf("%s/pulls/%d", repo.HTMLURL(), pr.Index),
+ ShouldShowMessage: setting.Git.PullRequestPushMessage && repo.AllowsPulls(ctx),
})
}
diff --git a/tests/integration/git_push_test.go b/tests/integration/git_push_test.go
index 0a35724807..b37fb02444 100644
--- a/tests/integration/git_push_test.go
+++ b/tests/integration/git_push_test.go
@@ -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) {
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