Skip to content
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

Agit flow add refs/for-review/<pull index> support #33179

Open
wants to merge 29 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
70d2872
agit flow add refs/for-review/<pull index> support
a1012112796 Jun 4, 2024
a5b38fc
fix lint
a1012112796 Jun 5, 2024
695bafe
Merge remote-tracking branch 'origin/main' into zzc/dev/agit_2
a1012112796 Jun 5, 2024
09daf03
Merge branch 'main' into zzc/dev/agit_2
silverwind Jun 7, 2024
1685a35
Merge branch 'main' into zzc/dev/agit_2
silverwind Jun 17, 2024
125e7ae
Merge remote-tracking branch 'origin/main' into zzc/dev/agit_2
a1012112796 Oct 9, 2024
e371a21
split updateExistPull
a1012112796 Oct 9, 2024
bd501a6
Update forReiewPattern
hiifong Jan 9, 2025
06c5f30
Merge branch 'main' into agit/update
hiifong Jan 9, 2025
cab9bbc
fix
hiifong Jan 9, 2025
35a2b9b
Update
hiifong Jan 9, 2025
b99fe1c
Update
hiifong Jan 10, 2025
930d8cb
Merge branch 'main' into agit/update
hiifong Jan 10, 2025
cdea357
Merge branch 'main' into agit/update
hiifong Jan 10, 2025
3e0c530
Merge branch 'main' into agit/update
hiifong Jan 10, 2025
1660737
Fix
hiifong Jan 11, 2025
013d437
Merge remote-tracking branch 'refs/remotes/origin/agit/update' into a…
hiifong Jan 11, 2025
30713a6
Fix
hiifong Jan 11, 2025
a041057
Fix
hiifong Jan 11, 2025
3b2aef2
Merge branch 'main' into agit/update
hiifong Jan 12, 2025
e875cba
Merge branch 'main' into agit/update
hiifong Jan 12, 2025
56b1351
Remove
hiifong Jan 12, 2025
4c8ea3d
Merge branch 'main' into agit/update
hiifong Jan 12, 2025
14c7ebe
Merge branch 'main' into agit/update
hiifong Jan 12, 2025
fec0b43
Merge branch 'main' into agit/update
hiifong Jan 12, 2025
2242557
Merge branch 'main' into agit/update
hiifong Jan 13, 2025
d4f2379
Merge branch 'main' into agit/update
hiifong Jan 13, 2025
59f37ce
Merge branch 'main' into agit/update
hiifong Jan 14, 2025
009a1df
Merge branch 'main' into agit/update
hiifong Jan 15, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion cmd/serv.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "data" is always hard-coded string {"type":"gitea","version":2}? Why it needs a http request

return nil
}
}
Expand Down
20 changes: 13 additions & 7 deletions modules/git/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
9 changes: 8 additions & 1 deletion modules/git/ref.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
12 changes: 12 additions & 0 deletions modules/git/ref_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down
27 changes: 27 additions & 0 deletions modules/private/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"strconv"
"time"

"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
)

Expand All @@ -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
Expand Down
79 changes: 79 additions & 0 deletions routers/private/hook_pre_receive.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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()
}
Expand Down Expand Up @@ -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)")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message seems not informative.

}

if ctx.userPerm.CanWrite(unit.TypeCode) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

user with read permission to the code should be allowed to create/update pull requests?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating pull requests requires write permission.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating pull requests needs write permission of the head repository.

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(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should avoid putting err.Error() to UserMsag. At the moment, these messages are written by you, but in the future, developers might fill server-side sensitive information into it.

})
return
}
}

func generateGitEnv(opts *private.HookOptions) (env []string) {
env = os.Environ()
if opts.GitAlternativeObjectDirectories != "" {
Expand Down
2 changes: 1 addition & 1 deletion routers/web/misc/misc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading
Loading