Skip to content

Commit 263a716

Browse files
lunnylunny
authored
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`.
1 parent 72dc75e commit 263a716

File tree

5 files changed

+109
-73
lines changed

5 files changed

+109
-73
lines changed

‎cmd/hook.go‎

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -448,21 +448,24 @@ Gitea or set your environment appropriately.`, "")
448448

449449
func hookPrintResults(results []private.HookPostReceiveBranchResult) {
450450
for _, res := range results {
451-
if !res.Message {
452-
continue
453-
}
451+
hookPrintResult(res.Message, res.Create, res.Branch, res.URL)
452+
}
453+
}
454454

455-
fmt.Fprintln(os.Stderr, "")
456-
if res.Create {
457-
fmt.Fprintf(os.Stderr, "Create a new pull request for '%s':\n", res.Branch)
458-
fmt.Fprintf(os.Stderr, " %s\n", res.URL)
459-
} else {
460-
fmt.Fprint(os.Stderr, "Visit the existing pull request:\n")
461-
fmt.Fprintf(os.Stderr, " %s\n", res.URL)
462-
}
463-
fmt.Fprintln(os.Stderr, "")
464-
os.Stderr.Sync()
455+
func hookPrintResult(output, isCreate bool, branch, url string) {
456+
if !output {
457+
return
458+
}
459+
fmt.Fprintln(os.Stderr, "")
460+
if isCreate {
461+
fmt.Fprintf(os.Stderr, "Create a new pull request for '%s':\n", branch)
462+
fmt.Fprintf(os.Stderr, " %s\n", url)
463+
} else {
464+
fmt.Fprint(os.Stderr, "Visit the existing pull request:\n")
465+
fmt.Fprintf(os.Stderr, " %s\n", url)
465466
}
467+
fmt.Fprintln(os.Stderr, "")
468+
os.Stderr.Sync()
466469
}
467470

468471
func pushOptions() map[string]string {
@@ -691,6 +694,12 @@ Gitea or set your environment appropriately.`, "")
691694
}
692695
err = writeFlushPktLine(ctx, os.Stdout)
693696

697+
if err == nil {
698+
for _, res := range resp.Results {
699+
hookPrintResult(res.ShouldShowMessage, res.IsCreatePR, res.HeadBranch, res.URL)
700+
}
701+
}
702+
694703
return err
695704
}
696705

‎modules/private/hook.go‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"time"
1212

1313
"code.gitea.io/gitea/modules/git"
14+
"code.gitea.io/gitea/modules/optional"
1415
"code.gitea.io/gitea/modules/setting"
1516
)
1617

@@ -32,13 +33,13 @@ const (
3233
)
3334

3435
// Bool checks for a key in the map and parses as a boolean
35-
func (g GitPushOptions) Bool(key string, def bool) bool {
36+
func (g GitPushOptions) Bool(key string) optional.Option[bool] {
3637
if val, ok := g[key]; ok {
3738
if b, err := strconv.ParseBool(val); err == nil {
38-
return b
39+
return optional.Some(b)
3940
}
4041
}
41-
return def
42+
return optional.None[bool]()
4243
}
4344

4445
// HookOptions represents the options for the Hook calls
@@ -87,13 +88,17 @@ type HookProcReceiveResult struct {
8788

8889
// HookProcReceiveRefResult represents an individual result from ProcReceive
8990
type HookProcReceiveRefResult struct {
90-
OldOID string
91-
NewOID string
92-
Ref string
93-
OriginalRef git.RefName
94-
IsForcePush bool
95-
IsNotMatched bool
96-
Err string
91+
OldOID string
92+
NewOID string
93+
Ref string
94+
OriginalRef git.RefName
95+
IsForcePush bool
96+
IsNotMatched bool
97+
Err string
98+
IsCreatePR bool
99+
URL string
100+
ShouldShowMessage bool
101+
HeadBranch string
97102
}
98103

99104
// HookPreReceive check whether the provided commits are allowed

‎routers/private/hook_post_receive.go‎

Lines changed: 44 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,12 @@ package private
66
import (
77
"fmt"
88
"net/http"
9-
"strconv"
109

1110
git_model "code.gitea.io/gitea/models/git"
1211
issues_model "code.gitea.io/gitea/models/issues"
12+
access_model "code.gitea.io/gitea/models/perm/access"
1313
repo_model "code.gitea.io/gitea/models/repo"
14+
user_model "code.gitea.io/gitea/models/user"
1415
"code.gitea.io/gitea/modules/git"
1516
"code.gitea.io/gitea/modules/gitrepo"
1617
"code.gitea.io/gitea/modules/log"
@@ -159,8 +160,10 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) {
159160
}
160161
}
161162

163+
isPrivate := opts.GitPushOptions.Bool(private.GitPushOptionRepoPrivate)
164+
isTemplate := opts.GitPushOptions.Bool(private.GitPushOptionRepoTemplate)
162165
// Handle Push Options
163-
if len(opts.GitPushOptions) > 0 {
166+
if isPrivate.Has() || isTemplate.Has() {
164167
// load the repository
165168
if repo == nil {
166169
repo = loadRepository(ctx, ownerName, repoName)
@@ -171,13 +174,49 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) {
171174
wasEmpty = repo.IsEmpty
172175
}
173176

174-
repo.IsPrivate = opts.GitPushOptions.Bool(private.GitPushOptionRepoPrivate, repo.IsPrivate)
175-
repo.IsTemplate = opts.GitPushOptions.Bool(private.GitPushOptionRepoTemplate, repo.IsTemplate)
176-
if err := repo_model.UpdateRepositoryCols(ctx, repo, "is_private", "is_template"); err != nil {
177+
pusher, err := user_model.GetUserByID(ctx, opts.UserID)
178+
if err != nil {
177179
log.Error("Failed to Update: %s/%s Error: %v", ownerName, repoName, err)
178180
ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{
179181
Err: fmt.Sprintf("Failed to Update: %s/%s Error: %v", ownerName, repoName, err),
180182
})
183+
return
184+
}
185+
perm, err := access_model.GetUserRepoPermission(ctx, repo, pusher)
186+
if err != nil {
187+
log.Error("Failed to Update: %s/%s Error: %v", ownerName, repoName, err)
188+
ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{
189+
Err: fmt.Sprintf("Failed to Update: %s/%s Error: %v", ownerName, repoName, err),
190+
})
191+
return
192+
}
193+
if !perm.IsOwner() && !perm.IsAdmin() {
194+
ctx.JSON(http.StatusNotFound, private.HookPostReceiveResult{
195+
Err: "Permissions denied",
196+
})
197+
return
198+
}
199+
200+
cols := make([]string, 0, len(opts.GitPushOptions))
201+
202+
if isPrivate.Has() {
203+
repo.IsPrivate = isPrivate.Value()
204+
cols = append(cols, "is_private")
205+
}
206+
207+
if isTemplate.Has() {
208+
repo.IsTemplate = isTemplate.Value()
209+
cols = append(cols, "is_template")
210+
}
211+
212+
if len(cols) > 0 {
213+
if err := repo_model.UpdateRepositoryCols(ctx, repo, cols...); err != nil {
214+
log.Error("Failed to Update: %s/%s Error: %v", ownerName, repoName, err)
215+
ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{
216+
Err: fmt.Sprintf("Failed to Update: %s/%s Error: %v", ownerName, repoName, err),
217+
})
218+
return
219+
}
181220
}
182221
}
183222

@@ -192,42 +231,6 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) {
192231
refFullName := opts.RefFullNames[i]
193232
newCommitID := opts.NewCommitIDs[i]
194233

195-
// post update for agit pull request
196-
// FIXME: use pr.Flow to test whether it's an Agit PR or a GH PR
197-
if git.DefaultFeatures.SupportProcReceive && refFullName.IsPull() {
198-
if repo == nil {
199-
repo = loadRepository(ctx, ownerName, repoName)
200-
if ctx.Written() {
201-
return
202-
}
203-
}
204-
205-
pullIndex, _ := strconv.ParseInt(refFullName.PullName(), 10, 64)
206-
if pullIndex <= 0 {
207-
continue
208-
}
209-
210-
pr, err := issues_model.GetPullRequestByIndex(ctx, repo.ID, pullIndex)
211-
if err != nil && !issues_model.IsErrPullRequestNotExist(err) {
212-
log.Error("Failed to get PR by index %v Error: %v", pullIndex, err)
213-
ctx.JSON(http.StatusInternalServerError, private.Response{
214-
Err: fmt.Sprintf("Failed to get PR by index %v Error: %v", pullIndex, err),
215-
})
216-
return
217-
}
218-
if pr == nil {
219-
continue
220-
}
221-
222-
results = append(results, private.HookPostReceiveBranchResult{
223-
Message: setting.Git.PullRequestPushMessage && repo.AllowsPulls(ctx),
224-
Create: false,
225-
Branch: "",
226-
URL: fmt.Sprintf("%s/pulls/%d", repo.HTMLURL(), pr.Index),
227-
})
228-
continue
229-
}
230-
231234
// If we've pushed a branch (and not deleted it)
232235
if !git.IsEmptyCommitID(newCommitID) && refFullName.IsBranch() {
233236
// First ensure we have the repository loaded, we're allowed pulls requests and we can get the base repo

‎services/agit/agit.go‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"code.gitea.io/gitea/modules/git"
1717
"code.gitea.io/gitea/modules/log"
1818
"code.gitea.io/gitea/modules/private"
19+
"code.gitea.io/gitea/modules/setting"
1920
notify_service "code.gitea.io/gitea/services/notify"
2021
pull_service "code.gitea.io/gitea/services/pull"
2122
)
@@ -145,10 +146,14 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git.
145146
log.Trace("Pull request created: %d/%d", repo.ID, prIssue.ID)
146147

147148
results = append(results, private.HookProcReceiveRefResult{
148-
Ref: pr.GetGitRefName(),
149-
OriginalRef: opts.RefFullNames[i],
150-
OldOID: objectFormat.EmptyObjectID().String(),
151-
NewOID: opts.NewCommitIDs[i],
149+
Ref: pr.GetGitRefName(),
150+
OriginalRef: opts.RefFullNames[i],
151+
OldOID: objectFormat.EmptyObjectID().String(),
152+
NewOID: opts.NewCommitIDs[i],
153+
IsCreatePR: true,
154+
URL: fmt.Sprintf("%s/pulls/%d", repo.HTMLURL(), pr.Index),
155+
ShouldShowMessage: setting.Git.PullRequestPushMessage && repo.AllowsPulls(ctx),
156+
HeadBranch: headBranch,
152157
})
153158
continue
154159
}
@@ -208,11 +213,14 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git.
208213
isForcePush := comment != nil && comment.IsForcePush
209214

210215
results = append(results, private.HookProcReceiveRefResult{
211-
OldOID: oldCommitID,
212-
NewOID: opts.NewCommitIDs[i],
213-
Ref: pr.GetGitRefName(),
214-
OriginalRef: opts.RefFullNames[i],
215-
IsForcePush: isForcePush,
216+
OldOID: oldCommitID,
217+
NewOID: opts.NewCommitIDs[i],
218+
Ref: pr.GetGitRefName(),
219+
OriginalRef: opts.RefFullNames[i],
220+
IsForcePush: isForcePush,
221+
IsCreatePR: false,
222+
URL: fmt.Sprintf("%s/pulls/%d", repo.HTMLURL(), pr.Index),
223+
ShouldShowMessage: setting.Git.PullRequestPushMessage && repo.AllowsPulls(ctx),
216224
})
217225
}
218226

‎tests/integration/git_push_test.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,17 @@ func testGitPush(t *testing.T, u *url.URL) {
4949
})
5050
})
5151

52+
t.Run("Push branch with options", func(t *testing.T) {
53+
runTestGitPush(t, u, func(t *testing.T, gitPath string) (pushed, deleted []string) {
54+
branchName := "branch-with-options"
55+
doGitCreateBranch(gitPath, branchName)(t)
56+
doGitPushTestRepository(gitPath, "origin", branchName, "-o", "repo.private=true", "-o", "repo.template=true")(t)
57+
pushed = append(pushed, branchName)
58+
59+
return pushed, deleted
60+
})
61+
})
62+
5263
t.Run("Delete branches", func(t *testing.T) {
5364
runTestGitPush(t, u, func(t *testing.T, gitPath string) (pushed, deleted []string) {
5465
doGitPushTestRepository(gitPath, "origin", "master")(t) // make sure master is the default branch instead of a branch we are going to delete

0 commit comments

Comments
 (0)