Merge 009a1df2e602e242c2459895d43be8767beeffd1 into c2e23d3301b1be2b2ad667184030087f92ad2470

# Conflicts:
#	modules/git/git.go
#	services/agit/agit.go
#	templates/repo/issue/view_content/sidebar.tmpl

cmd/serv.go

@@ -239,7 +239,8 @@ func runServ(c *cli.Context) error {
 		if git.DefaultFeatures().SupportProcReceive {
 			// for AGit Flow
 			if cmd == "ssh_info" {
-				fmt.Print(`{"type":"gitea","version":1}`)
+				data := private.GetSSHInfo(ctx)
+				fmt.Println(data)
 				return nil
 			}
 		}

modules/git/config.go

@@ -40,35 +40,41 @@ func syncGitConfig() (err error) {
 	}
 
 	// Set git some configurations - these must be set to these values for gitea to work correctly
-	if err := configSet("core.quotePath", "false"); err != nil {
+	if err = configSet("core.quotePath", "false"); err != nil {
 		return err
 	}
 
 	if DefaultFeatures().CheckVersionAtLeast("2.10") {
-		if err := configSet("receive.advertisePushOptions", "true"); err != nil {
+		if err = configSet("receive.advertisePushOptions", "true"); err != nil {
 			return err
 		}
 	}
 
 	if DefaultFeatures().CheckVersionAtLeast("2.18") {
-		if err := configSet("core.commitGraph", "true"); err != nil {
+		if err = configSet("core.commitGraph", "true"); err != nil {
 			return err
 		}
-		if err := configSet("gc.writeCommitGraph", "true"); err != nil {
+		if err = configSet("gc.writeCommitGraph", "true"); err != nil {
 			return err
 		}
-		if err := configSet("fetch.writeCommitGraph", "true"); err != nil {
+		if err = configSet("fetch.writeCommitGraph", "true"); err != nil {
 			return err
 		}
 	}
 
 	if DefaultFeatures().SupportProcReceive {
 		// set support for AGit flow
-		if err := configAddNonExist("receive.procReceiveRefs", "refs/for"); err != nil {
+		if err = configAddNonExist("receive.procReceiveRefs", "refs/for"); err != nil {
+			return err
+		}
+		if err = configAddNonExist("receive.procReceiveRefs", "refs/for-review"); err != nil {
 			return err
 		}
 	} else {
-		if err := configUnsetAll("receive.procReceiveRefs", "refs/for"); err != nil {
+		if err = configUnsetAll("receive.procReceiveRefs", "refs/for"); err != nil {
+			return err
+		}
+		if err = configUnsetAll("receive.procReceiveRefs", "refs/for-review"); err != nil {
 			return err
 		}
 	}

modules/git/ref.go

@@ -67,7 +67,8 @@ func (ref *Reference) RefGroup() string {
 // or refs/for/<targe-branch> -o topic='<topic-branch>'
 const ForPrefix = "refs/for/"
 
-// TODO: /refs/for-review for suggest change interface
+// ForReviewPrefix special ref to update a pull request: refs/for-review/<pull index>
+const ForReviewPrefix = "refs/for-review/"
 
 // RefName represents a full git reference name
 type RefName string
@@ -108,6 +109,12 @@ func (ref RefName) IsFor() bool {
 	return strings.HasPrefix(string(ref), ForPrefix)
 }
 
+var forReviewPattern = regexp.MustCompile(ForReviewPrefix + `[1-9]\d*$`)
+
+func (ref RefName) IsForReview() bool {
+	return forReviewPattern.MatchString(string(ref))
+}
+
 func (ref RefName) nameWithoutPrefix(prefix string) string {
 	if strings.HasPrefix(string(ref), prefix) {
 		return strings.TrimPrefix(string(ref), prefix)

modules/git/ref_test.go

@@ -28,6 +28,18 @@ func TestRefName(t *testing.T) {
 	assert.Equal(t, "main", RefName("refs/for/main").ForBranchName())
 	assert.Equal(t, "my/branch", RefName("refs/for/my/branch").ForBranchName())
 
+	// Test for review name
+	assert.False(t, RefName("refs/for-review/").IsForReview())
+	assert.False(t, RefName("refs/for-review/-1").IsForReview())
+	assert.False(t, RefName("refs/for-review/0").IsForReview())
+	assert.False(t, RefName("refs/for-review/01").IsForReview())
+	assert.True(t, RefName("refs/for-review/1").IsForReview())
+	assert.True(t, RefName("refs/for-review/10").IsForReview())
+	assert.True(t, RefName("refs/for-review/10999").IsForReview())
+	assert.False(t, RefName("refs/for-review/a10").IsForReview())
+	assert.False(t, RefName("refs/for-review/10a").IsForReview())
+	assert.False(t, RefName("refs/for-review/abc").IsForReview())
+
 	// Test commit hashes.
 	assert.Equal(t, "c0ffee", RefName("c0ffee").ShortName())
 }

modules/private/manager.go

@@ -12,6 +12,7 @@ import (
 	"strconv"
 	"time"
 
+	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
 )
 
@@ -36,6 +37,32 @@ func ReloadTemplates(ctx context.Context) ResponseExtra {
 	return requestJSONClientMsg(req, "Reloaded")
 }
 
+// Shutdown calls the internal shutdown function
+func GetSSHInfo(ctx context.Context) string {
+	reqURL := setting.LocalURL + "ssh_info"
+	req := newInternalRequest(ctx, reqURL, "GET")
+
+	resp, err := req.Response()
+	if err != nil {
+		log.Error("GetSSHInfo Error: %v", err)
+		return ""
+	}
+	defer resp.Body.Close()
+
+	if resp.StatusCode != http.StatusOK {
+		log.Error("response status code is not OK, code: %d", resp.StatusCode)
+		return ""
+	}
+
+	content, err := io.ReadAll(resp.Body)
+	if err != nil {
+		log.Error("read body error: %v", err)
+		return ""
+	}
+
+	return string(content)
+}
+
 // FlushOptions represents the options for the flush call
 type FlushOptions struct {
 	Timeout     time.Duration

routers/private/hook_pre_receive.go

@@ -4,9 +4,12 @@
 package private
 
 import (
+	"errors"
 	"fmt"
 	"net/http"
 	"os"
+	"strconv"
+	"strings"
 
 	asymkey_model "code.gitea.io/gitea/models/asymkey"
 	git_model "code.gitea.io/gitea/models/git"
@@ -123,6 +126,8 @@ func HookPreReceive(ctx *gitea_context.PrivateContext) {
 			preReceiveTag(ourCtx, refFullName)
 		case git.DefaultFeatures().SupportProcReceive && refFullName.IsFor():
 			preReceiveFor(ourCtx, refFullName)
+		case git.DefaultFeatures().SupportProcReceive && refFullName.IsForReview():
+			preReceiveForReview(ourCtx, refFullName)
 		default:
 			ourCtx.AssertCanWriteCode()
 		}
@@ -468,6 +473,80 @@ func preReceiveFor(ctx *preReceiveContext, refFullName git.RefName) {
 	}
 }
 
+func canUpdateAgitPull(ctx *preReceiveContext, pull *issues_model.PullRequest) error {
+	if pull.Flow != issues_model.PullRequestFlowAGit {
+		return errors.New("Pull request that are not created through agit cannot be updated using agit")
+	}
+
+	if ctx.opts.UserID == pull.Issue.PosterID {
+		return nil
+	}
+
+	if !pull.AllowMaintainerEdit {
+		return fmt.Errorf("The author does not allow maintainers to edit this pull request")
+	}
+
+	if !ctx.loadPusherAndPermission() {
+		return fmt.Errorf("Internal Server Error (no specific error)")
+	}
+
+	if ctx.userPerm.CanWrite(unit.TypeCode) {
+		return errors.New("You have no permission to update this pull request")
+	}
+	return nil
+}
+
+func preReceiveForReview(ctx *preReceiveContext, refFullName git.RefName) {
+	if !ctx.AssertCreatePullRequest() {
+		return
+	}
+
+	if ctx.Repo.Repository.IsEmpty {
+		ctx.JSON(http.StatusForbidden, private.Response{
+			UserMsg: "Can't create pull request for an empty repository.",
+		})
+		return
+	}
+
+	if ctx.opts.IsWiki {
+		ctx.JSON(http.StatusForbidden, private.Response{
+			UserMsg: "Pull requests are not supported on the wiki.",
+		})
+		return
+	}
+
+	pullIndex, err := strconv.ParseInt(strings.TrimPrefix(string(refFullName), git.ForReviewPrefix), 10, 64)
+	if err != nil {
+		ctx.JSON(http.StatusForbidden, private.Response{
+			UserMsg: "Unknow pull request index.",
+		})
+		return
+	}
+	pull, err := issues_model.GetPullRequestByIndex(ctx, ctx.Repo.Repository.ID, pullIndex)
+	if err != nil {
+		log.Error("preReceiveForReview: GetPullRequestByIndex: err: %v", err)
+		ctx.JSON(http.StatusForbidden, private.Response{
+			UserMsg: "Unknow pull request index.",
+		})
+		return
+	}
+	err = pull.LoadIssue(ctx)
+	if err != nil {
+		log.Error("preReceiveForReview: pull.LoadIssue: err: %v", err)
+		ctx.JSON(http.StatusForbidden, private.Response{
+			UserMsg: "Unknow pull request.",
+		})
+		return
+	}
+
+	if err := canUpdateAgitPull(ctx, pull); err != nil {
+		ctx.JSON(http.StatusForbidden, private.Response{
+			UserMsg: err.Error(),
+		})
+		return
+	}
+}
+
 func generateGitEnv(opts *private.HookOptions) (env []string) {
 	env = os.Environ()
 	if opts.GitAlternativeObjectDirectories != "" {

routers/web/misc/misc.go

@@ -20,7 +20,7 @@ func SSHInfo(rw http.ResponseWriter, req *http.Request) {
 		return
 	}
 	rw.Header().Set("content-type", "text/json;charset=UTF-8")
-	_, err := rw.Write([]byte(`{"type":"gitea","version":1}`))
+	_, err := rw.Write([]byte(`{"type":"gitea","version":2}`))
 	if err != nil {
 		log.Error("fail to write result: err: %v", err)
 		rw.WriteHeader(http.StatusInternalServerError)

services/agit/agit.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"strconv"
 	"strings"
 
 	issues_model "code.gitea.io/gitea/models/issues"
@@ -20,6 +21,84 @@ import (
 	pull_service "code.gitea.io/gitea/services/pull"
 )
 
+type updateExistPullOption struct {
+	ctx       context.Context
+	pr        *issues_model.PullRequest
+	gitRepo   *git.Repository
+	repo      *repo_model.Repository
+	forcePush bool
+	pusher    *user_model.User
+
+	RefFullName git.RefName
+	OldCommitID string
+	NewCommitID string
+}
+
+func updateExistPull(opts *updateExistPullOption) (*private.HookProcReceiveRefResult, error) {
+	// update exist pull request
+	if err := opts.pr.LoadBaseRepo(opts.ctx); err != nil {
+		return nil, fmt.Errorf("unable to load base repository for PR[%d] Error: %w", opts.pr.ID, err)
+	}
+
+	oldCommitID, err := opts.gitRepo.GetRefCommitID(opts.pr.GetGitRefName())
+	if err != nil {
+		return nil, fmt.Errorf("unable to get ref commit id in base repository for PR[%d] Error: %w", opts.pr.ID, err)
+	}
+
+	if oldCommitID == opts.NewCommitID {
+		return &private.HookProcReceiveRefResult{
+			OriginalRef: opts.RefFullName,
+			OldOID:      opts.OldCommitID,
+			NewOID:      opts.NewCommitID,
+			Err:         "new commit is same with old commit",
+		}, nil
+	}
+
+	if !opts.forcePush {
+		output, _, err := git.NewCommand(opts.ctx, "rev-list", "--max-count=1").
+			AddDynamicArguments(oldCommitID, "^"+opts.NewCommitID).
+			RunStdString(&git.RunOpts{Dir: opts.repo.RepoPath(), Env: os.Environ()})
+		if err != nil {
+			return nil, fmt.Errorf("failed to detect force push: %w", err)
+		} else if len(output) > 0 {
+			return &private.HookProcReceiveRefResult{
+				OriginalRef: opts.RefFullName,
+				OldOID:      opts.OldCommitID,
+				NewOID:      opts.NewCommitID,
+				Err:         "request `force-push` push option",
+			}, nil
+		}
+	}
+
+	opts.pr.HeadCommitID = opts.NewCommitID
+	if err = pull_service.UpdateRef(opts.ctx, opts.pr); err != nil {
+		return nil, fmt.Errorf("failed to update pull ref. Error: %w", err)
+	}
+
+	pull_service.AddToTaskQueue(opts.ctx, opts.pr)
+	err = opts.pr.LoadIssue(opts.ctx)
+	if err != nil {
+		return nil, fmt.Errorf("failed to load pull issue. Error: %w", err)
+	}
+	comment, err := pull_service.CreatePushPullComment(opts.ctx, opts.pusher, opts.pr, oldCommitID, opts.NewCommitID)
+	if err == nil && comment != nil {
+		notify_service.PullRequestPushCommits(opts.ctx, opts.pusher, opts.pr, comment)
+	}
+	notify_service.PullRequestSynchronized(opts.ctx, opts.pusher, opts.pr)
+	isForcePush := comment != nil && comment.IsForcePush
+
+	return &private.HookProcReceiveRefResult{
+		OldOID:            oldCommitID,
+		NewOID:            opts.NewCommitID,
+		Ref:               opts.pr.GetGitRefName(),
+		OriginalRef:       opts.RefFullName,
+		IsForcePush:       isForcePush,
+		IsCreatePR:        false,
+		URL:               fmt.Sprintf("%s/pulls/%d", opts.repo.HTMLURL(), opts.pr.Index),
+		ShouldShowMessage: setting.Git.PullRequestPushMessage && opts.repo.AllowsPulls(opts.ctx),
+	}, nil
+}
+
 // ProcReceive handle proc receive work
 func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository, opts *private.HookOptions) ([]private.HookProcReceiveRefResult, error) {
 	results := make([]private.HookProcReceiveRefResult, 0, len(opts.OldCommitIDs))
@@ -46,6 +125,49 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git.
 			continue
 		}
 
+		if opts.RefFullNames[i].IsForReview() {
+			// try match refs/for-review/<pull index>
+			pullIndex, err := strconv.ParseInt(strings.TrimPrefix(string(opts.RefFullNames[i]), git.ForReviewPrefix), 10, 64)
+			if err != nil {
+				results = append(results, private.HookProcReceiveRefResult{
+					OriginalRef: opts.RefFullNames[i],
+					OldOID:      opts.OldCommitIDs[i],
+					NewOID:      opts.NewCommitIDs[i],
+					Err:         "Unknow pull request index",
+				})
+				continue
+			}
+			log.Trace("Pull request index: %d", pullIndex)
+			pull, err := issues_model.GetPullRequestByIndex(ctx, repo.ID, pullIndex)
+			if err != nil {
+				results = append(results, private.HookProcReceiveRefResult{
+					OriginalRef: opts.RefFullNames[i],
+					OldOID:      opts.OldCommitIDs[i],
+					NewOID:      opts.NewCommitIDs[i],
+					Err:         "Unknow pull request index",
+				})
+				continue
+			}
+
+			result, err := updateExistPull(&updateExistPullOption{
+				ctx:         ctx,
+				pr:          pull,
+				gitRepo:     gitRepo,
+				repo:        repo,
+				forcePush:   forcePush.Value(),
+				pusher:      pusher,
+				RefFullName: opts.RefFullNames[i],
+				OldCommitID: opts.OldCommitIDs[i],
+				NewCommitID: opts.NewCommitIDs[i],
+			})
+			if err != nil {
+				return nil, err
+			}
+			results = append(results, *result)
+
+			continue
+		}
+
 		if !opts.RefFullNames[i].IsFor() {
 			results = append(results, private.HookProcReceiveRefResult{
 				IsNotMatched: true,
@@ -161,70 +283,21 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git.
 			continue
 		}
 
-		// update exist pull request
-		if err := pr.LoadBaseRepo(ctx); err != nil {
-			return nil, fmt.Errorf("unable to load base repository for PR[%d] Error: %w", pr.ID, err)
-		}
-
-		oldCommitID, err := gitRepo.GetRefCommitID(pr.GetGitRefName())
-		if err != nil {
-			return nil, fmt.Errorf("unable to get ref commit id in base repository for PR[%d] Error: %w", pr.ID, err)
-		}
-
-		if oldCommitID == opts.NewCommitIDs[i] {
-			results = append(results, private.HookProcReceiveRefResult{
-				OriginalRef: opts.RefFullNames[i],
-				OldOID:      opts.OldCommitIDs[i],
-				NewOID:      opts.NewCommitIDs[i],
-				Err:         "new commit is same with old commit",
-			})
-			continue
-		}
-
-		if !forcePush.Value() {
-			output, _, err := git.NewCommand(ctx, "rev-list", "--max-count=1").
-				AddDynamicArguments(oldCommitID, "^"+opts.NewCommitIDs[i]).
-				RunStdString(&git.RunOpts{Dir: repo.RepoPath(), Env: os.Environ()})
-			if err != nil {
-				return nil, fmt.Errorf("failed to detect force push: %w", err)
-			} else if len(output) > 0 {
-				results = append(results, private.HookProcReceiveRefResult{
-					OriginalRef: opts.RefFullNames[i],
-					OldOID:      opts.OldCommitIDs[i],
-					NewOID:      opts.NewCommitIDs[i],
-					Err:         "request `force-push` push option",
-				})
-				continue
-			}
-		}
-
-		pr.HeadCommitID = opts.NewCommitIDs[i]
-		if err = pull_service.UpdateRef(ctx, pr); err != nil {
-			return nil, fmt.Errorf("failed to update pull ref. Error: %w", err)
-		}
-
-		pull_service.AddToTaskQueue(ctx, pr)
-		err = pr.LoadIssue(ctx)
-		if err != nil {
-			return nil, fmt.Errorf("failed to load pull issue. Error: %w", err)
-		}
-		comment, err := pull_service.CreatePushPullComment(ctx, pusher, pr, oldCommitID, opts.NewCommitIDs[i])
-		if err == nil && comment != nil {
-			notify_service.PullRequestPushCommits(ctx, pusher, pr, comment)
-		}
-		notify_service.PullRequestSynchronized(ctx, pusher, pr)
-		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,
-			IsCreatePR:        false,
-			URL:               fmt.Sprintf("%s/pulls/%d", repo.HTMLURL(), pr.Index),
-			ShouldShowMessage: setting.Git.PullRequestPushMessage && repo.AllowsPulls(ctx),
+		result, err := updateExistPull(&updateExistPullOption{
+			ctx:         ctx,
+			pr:          pr,
+			gitRepo:     gitRepo,
+			repo:        repo,
+			forcePush:   forcePush.Value(),
+			pusher:      pusher,
+			RefFullName: opts.RefFullNames[i],
+			OldCommitID: opts.OldCommitIDs[i],
+			NewCommitID: opts.NewCommitIDs[i],
 		})
+		if err != nil {
+			return nil, err
+		}
+		results = append(results, *result)
 	}
 
 	return results, nil

templates/repo/issue/sidebar/allow_maintainer_edit.tmpl

@@ -1,5 +1,5 @@
 {{if and .Issue.IsPull .IsIssuePoster (not .Issue.IsClosed) .Issue.PullRequest.HeadRepo}}
-	{{if and (not (eq .Issue.PullRequest.HeadRepo.FullName .Issue.PullRequest.BaseRepo.FullName)) .CanWriteToHeadRepo}}
+	{{if or (eq .SignedUserID .Issue.PosterID) (and (not (eq .Issue.PullRequest.HeadRepo.FullName .Issue.PullRequest.BaseRepo.FullName)) .CanWriteToHeadRepo)}}
 	<div class="divider"></div>
 	<div class="ui checkbox loading-icon-2px" id="allow-edits-from-maintainers"
 			data-url="{{.Issue.Link}}"

tests/integration/git_general_test.go

@@ -871,6 +871,48 @@ func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, headBranch string
 			assert.False(t, prMsg.HasMerged)
 			assert.Equal(t, commit, prMsg.Head.Sha)
 		})
+
+		t.Run("AddCommit3", func(t *testing.T) {
+			err := os.WriteFile(path.Join(dstPath, "test_file_2"), []byte("## test content \n ## test content 2"), 0o666)
+			if !assert.NoError(t, err) {
+				return
+			}
+
+			err = git.AddChanges(dstPath, true)
+			assert.NoError(t, err)
+
+			err = git.CommitChanges(dstPath, git.CommitChangesOptions{
+				Committer: &git.Signature{
+					Email: "user2@example.com",
+					Name:  "user2",
+					When:  time.Now(),
+				},
+				Author: &git.Signature{
+					Email: "user2@example.com",
+					Name:  "user2",
+					When:  time.Now(),
+				},
+				Message: "Testing commit 3",
+			})
+			assert.NoError(t, err)
+			commit, err = gitRepo.GetRefCommitID("HEAD")
+			assert.NoError(t, err)
+		})
+
+		t.Run("PushByForReview", func(t *testing.T) {
+			err := git.NewCommand(git.DefaultContext, "push", "origin").AddDynamicArguments(fmt.Sprintf("HEAD:refs/for-review/%d", pr1.Index)).Run(&git.RunOpts{Dir: dstPath})
+			if !assert.NoError(t, err) {
+				return
+			}
+
+			prMsg, err := doAPIGetPullRequest(*ctx, ctx.Username, ctx.Reponame, pr1.Index)(t)
+			if !assert.NoError(t, err) {
+				return
+			}
+			assert.False(t, prMsg.HasMerged)
+			assert.Equal(t, commit, prMsg.Head.Sha)
+		})
+
 		t.Run("Merge", doAPIMergePullRequest(*ctx, ctx.Username, ctx.Reponame, pr1.Index))
 		t.Run("CheckoutMasterAgain", doGitCheckoutBranch(dstPath, "master"))
 	}