From 16cbd5b59ccba3e418ba0c4c345eb2778ef1d15a Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 21 Oct 2022 16:39:26 +0800 Subject: [PATCH 01/20] Fix generating compare link (#21519) Fix #6318 Co-authored-by: zeripath --- modules/templates/helper.go | 13 +++++++++++++ templates/repo/home.tmpl | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/modules/templates/helper.go b/modules/templates/helper.go index 7bd2bc0a1c0d..a7232914400a 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -459,6 +459,19 @@ func NewFuncMap() []template.FuncMap { return items }, "HasPrefix": strings.HasPrefix, + "CompareLink": func(baseRepo, repo *repo_model.Repository, branchName string) string { + var curBranch string + if repo.ID != baseRepo.ID { + curBranch += fmt.Sprintf("%s/%s:", url.PathEscape(repo.OwnerName), url.PathEscape(repo.Name)) + } + curBranch += util.PathEscapeSegments(branchName) + + return fmt.Sprintf("%s/compare/%s...%s", + baseRepo.Link(), + util.PathEscapeSegments(baseRepo.DefaultBranch), + curBranch, + ) + }, }} } diff --git a/templates/repo/home.tmpl b/templates/repo/home.tmpl index 39e58d8e0fb6..1e82d97944fd 100644 --- a/templates/repo/home.tmpl +++ b/templates/repo/home.tmpl @@ -70,7 +70,7 @@ {{if eq $n 0}} {{if and .CanCompareOrPull .IsViewBranch (not .Repository.IsArchived)}} - + {{end}} From e828564445ba5856747f17faf2ac6b1a223a911d Mon Sep 17 00:00:00 2001 From: Yarden Shoham Date: Fri, 21 Oct 2022 15:00:53 +0300 Subject: [PATCH 02/20] Add color previews in markdown (#21474) * Resolves #3047 Every time a color code will be in \`backticks`, a cute little color preview will pop up [Inspiration](https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax#supported-color-models) #### Before ![image](https://user-images.githubusercontent.com/20454870/196631524-298afbbf-d2c8-4018-92a5-0393a693d850.png) #### After ![image](https://user-images.githubusercontent.com/20454870/196631397-36c561e4-08f5-465a-a36e-76084e30b08a.png) Signed-off-by: Yarden Shoham Co-authored-by: KN4CK3R Co-authored-by: silverwind Co-authored-by: Lunny Xiao --- modules/markup/markdown/ast.go | 36 ++++++++++++++++ modules/markup/markdown/goldmark.go | 39 +++++++++++++++++ modules/markup/markdown/markdown_test.go | 55 ++++++++++++++++++++++++ modules/markup/sanitizer.go | 7 ++- web_src/less/_base.less | 8 ++++ 5 files changed, 143 insertions(+), 2 deletions(-) diff --git a/modules/markup/markdown/ast.go b/modules/markup/markdown/ast.go index 5191d94cdd85..c82d5e5e7339 100644 --- a/modules/markup/markdown/ast.go +++ b/modules/markup/markdown/ast.go @@ -144,3 +144,39 @@ func IsIcon(node ast.Node) bool { _, ok := node.(*Icon) return ok } + +// ColorPreview is an inline for a color preview +type ColorPreview struct { + ast.BaseInline + Color []byte +} + +// Dump implements Node.Dump. +func (n *ColorPreview) Dump(source []byte, level int) { + m := map[string]string{} + m["Color"] = string(n.Color) + ast.DumpHelper(n, source, level, m, nil) +} + +// KindColorPreview is the NodeKind for ColorPreview +var KindColorPreview = ast.NewNodeKind("ColorPreview") + +// Kind implements Node.Kind. +func (n *ColorPreview) Kind() ast.NodeKind { + return KindColorPreview +} + +// NewColorPreview returns a new Span node. +func NewColorPreview(color []byte) *ColorPreview { + return &ColorPreview{ + BaseInline: ast.BaseInline{}, + Color: color, + } +} + +// IsColorPreview returns true if the given node implements the ColorPreview interface, +// otherwise false. +func IsColorPreview(node ast.Node) bool { + _, ok := node.(*ColorPreview) + return ok +} diff --git a/modules/markup/markdown/goldmark.go b/modules/markup/markdown/goldmark.go index 8417019ddbad..1a3668136619 100644 --- a/modules/markup/markdown/goldmark.go +++ b/modules/markup/markdown/goldmark.go @@ -16,6 +16,7 @@ import ( "code.gitea.io/gitea/modules/setting" giteautil "code.gitea.io/gitea/modules/util" + "github.com/microcosm-cc/bluemonday/css" "github.com/yuin/goldmark/ast" east "github.com/yuin/goldmark/extension/ast" "github.com/yuin/goldmark/parser" @@ -178,6 +179,11 @@ func (g *ASTTransformer) Transform(node *ast.Document, reader text.Reader, pc pa v.SetHardLineBreak(setting.Markdown.EnableHardLineBreakInDocuments) } } + case *ast.CodeSpan: + colorContent := n.Text(reader.Source()) + if css.ColorHandler(strings.ToLower(string(colorContent))) { + v.AppendChild(v, NewColorPreview(colorContent)) + } } return ast.WalkContinue, nil }) @@ -266,10 +272,43 @@ func (r *HTMLRenderer) RegisterFuncs(reg renderer.NodeRendererFuncRegisterer) { reg.Register(KindDetails, r.renderDetails) reg.Register(KindSummary, r.renderSummary) reg.Register(KindIcon, r.renderIcon) + reg.Register(ast.KindCodeSpan, r.renderCodeSpan) reg.Register(KindTaskCheckBoxListItem, r.renderTaskCheckBoxListItem) reg.Register(east.KindTaskCheckBox, r.renderTaskCheckBox) } +// renderCodeSpan renders CodeSpan elements (like goldmark upstream does) but also renders ColorPreview elements. +// See #21474 for reference +func (r *HTMLRenderer) renderCodeSpan(w util.BufWriter, source []byte, n ast.Node, entering bool) (ast.WalkStatus, error) { + if entering { + if n.Attributes() != nil { + _, _ = w.WriteString("') + } else { + _, _ = w.WriteString("") + } + for c := n.FirstChild(); c != nil; c = c.NextSibling() { + switch v := c.(type) { + case *ast.Text: + segment := v.Segment + value := segment.Value(source) + if bytes.HasSuffix(value, []byte("\n")) { + r.Writer.RawWrite(w, value[:len(value)-1]) + r.Writer.RawWrite(w, []byte(" ")) + } else { + r.Writer.RawWrite(w, value) + } + case *ColorPreview: + _, _ = w.WriteString(fmt.Sprintf(``, string(v.Color))) + } + } + return ast.WalkSkipChildren, nil + } + _, _ = w.WriteString("") + return ast.WalkContinue, nil +} + func (r *HTMLRenderer) renderDocument(w util.BufWriter, source []byte, node ast.Node, entering bool) (ast.WalkStatus, error) { n := node.(*ast.Document) diff --git a/modules/markup/markdown/markdown_test.go b/modules/markup/markdown/markdown_test.go index 49ed3d75d6c0..12c6288c24d1 100644 --- a/modules/markup/markdown/markdown_test.go +++ b/modules/markup/markdown/markdown_test.go @@ -429,6 +429,61 @@ func TestRenderEmojiInLinks_Issue12331(t *testing.T) { assert.Equal(t, expected, res) } +func TestColorPreview(t *testing.T) { + const nl = "\n" + positiveTests := []struct { + testcase string + expected string + }{ + { // hex + "`#FF0000`", + `

#FF0000

` + nl, + }, + { // rgb + "`rgb(16, 32, 64)`", + `

rgb(16, 32, 64)

` + nl, + }, + { // short hex + "This is the color white `#000`", + `

This is the color white #000

` + nl, + }, + { // hsl + "HSL stands for hue, saturation, and lightness. An example: `hsl(0, 100%, 50%)`.", + `

HSL stands for hue, saturation, and lightness. An example: hsl(0, 100%, 50%).

` + nl, + }, + { // uppercase hsl + "HSL stands for hue, saturation, and lightness. An example: `HSL(0, 100%, 50%)`.", + `

HSL stands for hue, saturation, and lightness. An example: HSL(0, 100%, 50%).

` + nl, + }, + } + + for _, test := range positiveTests { + res, err := RenderString(&markup.RenderContext{}, test.testcase) + assert.NoError(t, err, "Unexpected error in testcase: %q", test.testcase) + assert.Equal(t, test.expected, res, "Unexpected result in testcase %q", test.testcase) + + } + + negativeTests := []string{ + // not a color code + "`FF0000`", + // inside a code block + "```javascript" + nl + `const red = "#FF0000";` + nl + "```", + // no backticks + "rgb(166, 32, 64)", + // typo + "`hsI(0, 100%, 50%)`", + // looks like a color but not really + "`hsl(40, 60, 80)`", + } + + for _, test := range negativeTests { + res, err := RenderString(&markup.RenderContext{}, test) + assert.NoError(t, err, "Unexpected error in testcase: %q", test) + assert.NotContains(t, res, ` Date: Fri, 21 Oct 2022 18:21:56 +0200 Subject: [PATCH 03/20] Decouple HookTask from Repository (#17940) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit At the moment a repository reference is needed for webhooks. With the upcoming package PR we need to send webhooks without a repository reference. For example a package is uploaded to an organization. In theory this enables the usage of webhooks for future user actions. This PR removes the repository id from `HookTask` and changes how the hooks are processed (see `services/webhook/deliver.go`). In a follow up PR I want to remove the usage of the `UniqueQueue´ and replace it with a normal queue because there is no reason to be unique. Co-authored-by: 6543 <6543@obermui.de> --- models/fixtures/hook_task.yml | 1 - models/repo.go | 6 +- models/webhook/hooktask.go | 88 ++++++----- models/webhook/webhook.go | 10 +- models/webhook/webhook_test.go | 28 ++-- modules/notification/webhook/webhook.go | 190 ++++++++++-------------- routers/api/v1/repo/hook.go | 2 +- routers/api/v1/repo/hook_test.go | 1 - routers/web/repo/webhook.go | 6 +- services/webhook/deliver.go | 46 ++---- services/webhook/webhook.go | 104 ++++++------- services/webhook/webhook_test.go | 13 +- 12 files changed, 216 insertions(+), 279 deletions(-) diff --git a/models/fixtures/hook_task.yml b/models/fixtures/hook_task.yml index bb662345cdff..6dbb10151abf 100644 --- a/models/fixtures/hook_task.yml +++ b/models/fixtures/hook_task.yml @@ -1,6 +1,5 @@ - id: 1 - repo_id: 1 hook_id: 1 uuid: uuid1 is_delivered: true diff --git a/models/repo.go b/models/repo.go index 65159f14af3e..08fbb0abeac2 100644 --- a/models/repo.go +++ b/models/repo.go @@ -123,6 +123,11 @@ func DeleteRepository(doer *user_model.User, uid, repoID int64) error { return err } + if _, err := db.GetEngine(ctx).In("hook_id", builder.Select("id").From("webhook").Where(builder.Eq{"webhook.repo_id": repo.ID})). + Delete(&webhook.HookTask{}); err != nil { + return err + } + if err := db.DeleteBeans(ctx, &access_model.Access{RepoID: repo.ID}, &activities_model.Action{RepoID: repo.ID}, @@ -130,7 +135,6 @@ func DeleteRepository(doer *user_model.User, uid, repoID int64) error { &issues_model.Comment{RefRepoID: repoID}, &git_model.CommitStatus{RepoID: repoID}, &git_model.DeletedBranch{RepoID: repoID}, - &webhook.HookTask{RepoID: repoID}, &git_model.LFSLock{RepoID: repoID}, &repo_model.LanguageStat{RepoID: repoID}, &issues_model.Milestone{RepoID: repoID}, diff --git a/models/webhook/hooktask.go b/models/webhook/hooktask.go index 2adfcaa60dd8..2b9b63c09bf5 100644 --- a/models/webhook/hooktask.go +++ b/models/webhook/hooktask.go @@ -104,7 +104,6 @@ type HookResponse struct { // HookTask represents a hook task. type HookTask struct { ID int64 `xorm:"pk autoincr"` - RepoID int64 `xorm:"INDEX"` HookID int64 UUID string api.Payloader `xorm:"-"` @@ -178,14 +177,29 @@ func HookTasks(hookID int64, page int) ([]*HookTask, error) { // CreateHookTask creates a new hook task, // it handles conversion from Payload to PayloadContent. -func CreateHookTask(t *HookTask) error { +func CreateHookTask(ctx context.Context, t *HookTask) (*HookTask, error) { data, err := t.Payloader.JSONPayload() if err != nil { - return err + return nil, err } t.UUID = gouuid.New().String() t.PayloadContent = string(data) - return db.Insert(db.DefaultContext, t) + return t, db.Insert(ctx, t) +} + +func GetHookTaskByID(ctx context.Context, id int64) (*HookTask, error) { + t := &HookTask{} + + has, err := db.GetEngine(ctx).ID(id).Get(t) + if err != nil { + return nil, err + } + if !has { + return nil, ErrHookTaskNotExist{ + TaskID: id, + } + } + return t, nil } // UpdateHookTask updates information of hook task. @@ -195,53 +209,36 @@ func UpdateHookTask(t *HookTask) error { } // ReplayHookTask copies a hook task to get re-delivered -func ReplayHookTask(hookID int64, uuid string) (*HookTask, error) { - var newTask *HookTask - - err := db.WithTx(func(ctx context.Context) error { - task := &HookTask{ +func ReplayHookTask(ctx context.Context, hookID int64, uuid string) (*HookTask, error) { + task := &HookTask{ + HookID: hookID, + UUID: uuid, + } + has, err := db.GetByBean(ctx, task) + if err != nil { + return nil, err + } else if !has { + return nil, ErrHookTaskNotExist{ HookID: hookID, UUID: uuid, } - has, err := db.GetByBean(ctx, task) - if err != nil { - return err - } else if !has { - return ErrHookTaskNotExist{ - HookID: hookID, - UUID: uuid, - } - } - - newTask = &HookTask{ - UUID: gouuid.New().String(), - RepoID: task.RepoID, - HookID: task.HookID, - PayloadContent: task.PayloadContent, - EventType: task.EventType, - } - return db.Insert(ctx, newTask) - }) + } - return newTask, err + newTask := &HookTask{ + UUID: gouuid.New().String(), + HookID: task.HookID, + PayloadContent: task.PayloadContent, + EventType: task.EventType, + } + return newTask, db.Insert(ctx, newTask) } // FindUndeliveredHookTasks represents find the undelivered hook tasks -func FindUndeliveredHookTasks() ([]*HookTask, error) { +func FindUndeliveredHookTasks(ctx context.Context) ([]*HookTask, error) { tasks := make([]*HookTask, 0, 10) - if err := db.GetEngine(db.DefaultContext).Where("is_delivered=?", false).Find(&tasks); err != nil { - return nil, err - } - return tasks, nil -} - -// FindRepoUndeliveredHookTasks represents find the undelivered hook tasks of one repository -func FindRepoUndeliveredHookTasks(repoID int64) ([]*HookTask, error) { - tasks := make([]*HookTask, 0, 5) - if err := db.GetEngine(db.DefaultContext).Where("repo_id=? AND is_delivered=?", repoID, false).Find(&tasks); err != nil { - return nil, err - } - return tasks, nil + return tasks, db.GetEngine(ctx). + Where("is_delivered=?", false). + Find(&tasks) } // CleanupHookTaskTable deletes rows from hook_task as needed. @@ -250,7 +247,7 @@ func CleanupHookTaskTable(ctx context.Context, cleanupType HookTaskCleanupType, if cleanupType == OlderThan { deleteOlderThan := time.Now().Add(-olderThan).UnixNano() - deletes, err := db.GetEngine(db.DefaultContext). + deletes, err := db.GetEngine(ctx). Where("is_delivered = ? and delivered < ?", true, deleteOlderThan). Delete(new(HookTask)) if err != nil { @@ -259,7 +256,8 @@ func CleanupHookTaskTable(ctx context.Context, cleanupType HookTaskCleanupType, log.Trace("Deleted %d rows from hook_task", deletes) } else if cleanupType == PerWebhook { hookIDs := make([]int64, 0, 10) - err := db.GetEngine(db.DefaultContext).Table("webhook"). + err := db.GetEngine(ctx). + Table("webhook"). Where("id > 0"). Cols("id"). Find(&hookIDs) diff --git a/models/webhook/webhook.go b/models/webhook/webhook.go index 83200a3d1c21..4551dcff5fb1 100644 --- a/models/webhook/webhook.go +++ b/models/webhook/webhook.go @@ -19,13 +19,6 @@ import ( "xorm.io/builder" ) -// __ __ ___. .__ __ -// / \ / \ ____\_ |__ | |__ ____ ____ | | __ -// \ \/\/ // __ \| __ \| | \ / _ \ / _ \| |/ / -// \ /\ ___/| \_\ \ Y ( <_> | <_> ) < -// \__/\ / \___ >___ /___| /\____/ \____/|__|_ \ -// \/ \/ \/ \/ \/ - // ErrWebhookNotExist represents a "WebhookNotExist" kind of error. type ErrWebhookNotExist struct { ID int64 @@ -47,6 +40,7 @@ func (err ErrWebhookNotExist) Unwrap() error { // ErrHookTaskNotExist represents a "HookTaskNotExist" kind of error. type ErrHookTaskNotExist struct { + TaskID int64 HookID int64 UUID string } @@ -58,7 +52,7 @@ func IsErrHookTaskNotExist(err error) bool { } func (err ErrHookTaskNotExist) Error() string { - return fmt.Sprintf("hook task does not exist [hook: %d, uuid: %s]", err.HookID, err.UUID) + return fmt.Sprintf("hook task does not exist [task: %d, hook: %d, uuid: %s]", err.TaskID, err.HookID, err.UUID) } func (err ErrHookTaskNotExist) Unwrap() error { diff --git a/models/webhook/webhook_test.go b/models/webhook/webhook_test.go index 7ec7edc0b77e..8c4838ebdc05 100644 --- a/models/webhook/webhook_test.go +++ b/models/webhook/webhook_test.go @@ -208,12 +208,12 @@ func TestHookTasks(t *testing.T) { func TestCreateHookTask(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) hookTask := &HookTask{ - RepoID: 3, HookID: 3, Payloader: &api.PushPayload{}, } unittest.AssertNotExistsBean(t, hookTask) - assert.NoError(t, CreateHookTask(hookTask)) + _, err := CreateHookTask(db.DefaultContext, hookTask) + assert.NoError(t, err) unittest.AssertExistsAndLoadBean(t, hookTask) } @@ -232,14 +232,14 @@ func TestUpdateHookTask(t *testing.T) { func TestCleanupHookTaskTable_PerWebhook_DeletesDelivered(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) hookTask := &HookTask{ - RepoID: 3, HookID: 3, Payloader: &api.PushPayload{}, IsDelivered: true, Delivered: time.Now().UnixNano(), } unittest.AssertNotExistsBean(t, hookTask) - assert.NoError(t, CreateHookTask(hookTask)) + _, err := CreateHookTask(db.DefaultContext, hookTask) + assert.NoError(t, err) unittest.AssertExistsAndLoadBean(t, hookTask) assert.NoError(t, CleanupHookTaskTable(context.Background(), PerWebhook, 168*time.Hour, 0)) @@ -249,13 +249,13 @@ func TestCleanupHookTaskTable_PerWebhook_DeletesDelivered(t *testing.T) { func TestCleanupHookTaskTable_PerWebhook_LeavesUndelivered(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) hookTask := &HookTask{ - RepoID: 2, HookID: 4, Payloader: &api.PushPayload{}, IsDelivered: false, } unittest.AssertNotExistsBean(t, hookTask) - assert.NoError(t, CreateHookTask(hookTask)) + _, err := CreateHookTask(db.DefaultContext, hookTask) + assert.NoError(t, err) unittest.AssertExistsAndLoadBean(t, hookTask) assert.NoError(t, CleanupHookTaskTable(context.Background(), PerWebhook, 168*time.Hour, 0)) @@ -265,14 +265,14 @@ func TestCleanupHookTaskTable_PerWebhook_LeavesUndelivered(t *testing.T) { func TestCleanupHookTaskTable_PerWebhook_LeavesMostRecentTask(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) hookTask := &HookTask{ - RepoID: 2, HookID: 4, Payloader: &api.PushPayload{}, IsDelivered: true, Delivered: time.Now().UnixNano(), } unittest.AssertNotExistsBean(t, hookTask) - assert.NoError(t, CreateHookTask(hookTask)) + _, err := CreateHookTask(db.DefaultContext, hookTask) + assert.NoError(t, err) unittest.AssertExistsAndLoadBean(t, hookTask) assert.NoError(t, CleanupHookTaskTable(context.Background(), PerWebhook, 168*time.Hour, 1)) @@ -282,14 +282,14 @@ func TestCleanupHookTaskTable_PerWebhook_LeavesMostRecentTask(t *testing.T) { func TestCleanupHookTaskTable_OlderThan_DeletesDelivered(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) hookTask := &HookTask{ - RepoID: 3, HookID: 3, Payloader: &api.PushPayload{}, IsDelivered: true, Delivered: time.Now().AddDate(0, 0, -8).UnixNano(), } unittest.AssertNotExistsBean(t, hookTask) - assert.NoError(t, CreateHookTask(hookTask)) + _, err := CreateHookTask(db.DefaultContext, hookTask) + assert.NoError(t, err) unittest.AssertExistsAndLoadBean(t, hookTask) assert.NoError(t, CleanupHookTaskTable(context.Background(), OlderThan, 168*time.Hour, 0)) @@ -299,13 +299,13 @@ func TestCleanupHookTaskTable_OlderThan_DeletesDelivered(t *testing.T) { func TestCleanupHookTaskTable_OlderThan_LeavesUndelivered(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) hookTask := &HookTask{ - RepoID: 2, HookID: 4, Payloader: &api.PushPayload{}, IsDelivered: false, } unittest.AssertNotExistsBean(t, hookTask) - assert.NoError(t, CreateHookTask(hookTask)) + _, err := CreateHookTask(db.DefaultContext, hookTask) + assert.NoError(t, err) unittest.AssertExistsAndLoadBean(t, hookTask) assert.NoError(t, CleanupHookTaskTable(context.Background(), OlderThan, 168*time.Hour, 0)) @@ -315,14 +315,14 @@ func TestCleanupHookTaskTable_OlderThan_LeavesUndelivered(t *testing.T) { func TestCleanupHookTaskTable_OlderThan_LeavesTaskEarlierThanAgeToDelete(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) hookTask := &HookTask{ - RepoID: 2, HookID: 4, Payloader: &api.PushPayload{}, IsDelivered: true, Delivered: time.Now().AddDate(0, 0, -6).UnixNano(), } unittest.AssertNotExistsBean(t, hookTask) - assert.NoError(t, CreateHookTask(hookTask)) + _, err := CreateHookTask(db.DefaultContext, hookTask) + assert.NoError(t, err) unittest.AssertExistsAndLoadBean(t, hookTask) assert.NoError(t, CleanupHookTaskTable(context.Background(), OlderThan, 168*time.Hour, 0)) diff --git a/modules/notification/webhook/webhook.go b/modules/notification/webhook/webhook.go index b93e90368a17..630b56598464 100644 --- a/modules/notification/webhook/webhook.go +++ b/modules/notification/webhook/webhook.go @@ -61,7 +61,7 @@ func (m *webhookNotifier) NotifyIssueClearLabels(doer *user_model.User, issue *i return } - err = webhook_services.PrepareWebhooks(issue.Repo, webhook.HookEventPullRequestLabel, &api.PullRequestPayload{ + err = webhook_services.PrepareWebhooks(ctx, webhook_services.EventSource{Repository: issue.Repo}, webhook.HookEventPullRequestLabel, &api.PullRequestPayload{ Action: api.HookIssueLabelCleared, Index: issue.Index, PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, nil), @@ -69,7 +69,7 @@ func (m *webhookNotifier) NotifyIssueClearLabels(doer *user_model.User, issue *i Sender: convert.ToUser(doer, nil), }) } else { - err = webhook_services.PrepareWebhooks(issue.Repo, webhook.HookEventIssueLabel, &api.IssuePayload{ + err = webhook_services.PrepareWebhooks(ctx, webhook_services.EventSource{Repository: issue.Repo}, webhook.HookEventIssueLabel, &api.IssuePayload{ Action: api.HookIssueLabelCleared, Index: issue.Index, Issue: convert.ToAPIIssue(issue), @@ -87,7 +87,7 @@ func (m *webhookNotifier) NotifyForkRepository(doer *user_model.User, oldRepo, r mode, _ := access_model.AccessLevel(doer, repo) // forked webhook - if err := webhook_services.PrepareWebhooks(oldRepo, webhook.HookEventFork, &api.ForkPayload{ + if err := webhook_services.PrepareWebhooks(db.DefaultContext, webhook_services.EventSource{Repository: oldRepo}, webhook.HookEventFork, &api.ForkPayload{ Forkee: convert.ToRepo(oldRepo, oldMode), Repo: convert.ToRepo(repo, mode), Sender: convert.ToUser(doer, nil), @@ -99,7 +99,7 @@ func (m *webhookNotifier) NotifyForkRepository(doer *user_model.User, oldRepo, r // Add to hook queue for created repo after session commit. if u.IsOrganization() { - if err := webhook_services.PrepareWebhooks(repo, webhook.HookEventRepository, &api.RepositoryPayload{ + if err := webhook_services.PrepareWebhooks(db.DefaultContext, webhook_services.EventSource{Repository: repo}, webhook.HookEventRepository, &api.RepositoryPayload{ Action: api.HookRepoCreated, Repository: convert.ToRepo(repo, perm.AccessModeOwner), Organization: convert.ToUser(u, nil), @@ -112,7 +112,7 @@ func (m *webhookNotifier) NotifyForkRepository(doer *user_model.User, oldRepo, r func (m *webhookNotifier) NotifyCreateRepository(doer, u *user_model.User, repo *repo_model.Repository) { // Add to hook queue for created repo after session commit. - if err := webhook_services.PrepareWebhooks(repo, webhook.HookEventRepository, &api.RepositoryPayload{ + if err := webhook_services.PrepareWebhooks(db.DefaultContext, webhook_services.EventSource{Repository: repo}, webhook.HookEventRepository, &api.RepositoryPayload{ Action: api.HookRepoCreated, Repository: convert.ToRepo(repo, perm.AccessModeOwner), Organization: convert.ToUser(u, nil), @@ -125,7 +125,7 @@ func (m *webhookNotifier) NotifyCreateRepository(doer, u *user_model.User, repo func (m *webhookNotifier) NotifyDeleteRepository(doer *user_model.User, repo *repo_model.Repository) { u := repo.MustOwner() - if err := webhook_services.PrepareWebhooks(repo, webhook.HookEventRepository, &api.RepositoryPayload{ + if err := webhook_services.PrepareWebhooks(db.DefaultContext, webhook_services.EventSource{Repository: repo}, webhook.HookEventRepository, &api.RepositoryPayload{ Action: api.HookRepoDeleted, Repository: convert.ToRepo(repo, perm.AccessModeOwner), Organization: convert.ToUser(u, nil), @@ -137,7 +137,7 @@ func (m *webhookNotifier) NotifyDeleteRepository(doer *user_model.User, repo *re func (m *webhookNotifier) NotifyMigrateRepository(doer, u *user_model.User, repo *repo_model.Repository) { // Add to hook queue for created repo after session commit. - if err := webhook_services.PrepareWebhooks(repo, webhook.HookEventRepository, &api.RepositoryPayload{ + if err := webhook_services.PrepareWebhooks(db.DefaultContext, webhook_services.EventSource{Repository: repo}, webhook.HookEventRepository, &api.RepositoryPayload{ Action: api.HookRepoCreated, Repository: convert.ToRepo(repo, perm.AccessModeOwner), Organization: convert.ToUser(u, nil), @@ -171,7 +171,7 @@ func (m *webhookNotifier) NotifyIssueChangeAssignee(doer *user_model.User, issue apiPullRequest.Action = api.HookIssueAssigned } // Assignee comment triggers a webhook - if err := webhook_services.PrepareWebhooks(issue.Repo, webhook.HookEventPullRequestAssign, apiPullRequest); err != nil { + if err := webhook_services.PrepareWebhooks(ctx, webhook_services.EventSource{Repository: issue.Repo}, webhook.HookEventPullRequestAssign, apiPullRequest); err != nil { log.Error("PrepareWebhooks [is_pull: %v, remove_assignee: %v]: %v", issue.IsPull, removed, err) return } @@ -189,7 +189,7 @@ func (m *webhookNotifier) NotifyIssueChangeAssignee(doer *user_model.User, issue apiIssue.Action = api.HookIssueAssigned } // Assignee comment triggers a webhook - if err := webhook_services.PrepareWebhooks(issue.Repo, webhook.HookEventIssueAssign, apiIssue); err != nil { + if err := webhook_services.PrepareWebhooks(ctx, webhook_services.EventSource{Repository: issue.Repo}, webhook.HookEventIssueAssign, apiIssue); err != nil { log.Error("PrepareWebhooks [is_pull: %v, remove_assignee: %v]: %v", issue.IsPull, removed, err) return } @@ -208,7 +208,7 @@ func (m *webhookNotifier) NotifyIssueChangeTitle(doer *user_model.User, issue *i return } issue.PullRequest.Issue = issue - err = webhook_services.PrepareWebhooks(issue.Repo, webhook.HookEventPullRequest, &api.PullRequestPayload{ + err = webhook_services.PrepareWebhooks(ctx, webhook_services.EventSource{Repository: issue.Repo}, webhook.HookEventPullRequest, &api.PullRequestPayload{ Action: api.HookIssueEdited, Index: issue.Index, Changes: &api.ChangesPayload{ @@ -221,7 +221,7 @@ func (m *webhookNotifier) NotifyIssueChangeTitle(doer *user_model.User, issue *i Sender: convert.ToUser(doer, nil), }) } else { - err = webhook_services.PrepareWebhooks(issue.Repo, webhook.HookEventIssues, &api.IssuePayload{ + err = webhook_services.PrepareWebhooks(ctx, webhook_services.EventSource{Repository: issue.Repo}, webhook.HookEventIssues, &api.IssuePayload{ Action: api.HookIssueEdited, Index: issue.Index, Changes: &api.ChangesPayload{ @@ -263,7 +263,7 @@ func (m *webhookNotifier) NotifyIssueChangeStatus(doer *user_model.User, issue * } else { apiPullRequest.Action = api.HookIssueReOpened } - err = webhook_services.PrepareWebhooks(issue.Repo, webhook.HookEventPullRequest, apiPullRequest) + err = webhook_services.PrepareWebhooks(ctx, webhook_services.EventSource{Repository: issue.Repo}, webhook.HookEventPullRequest, apiPullRequest) } else { apiIssue := &api.IssuePayload{ Index: issue.Index, @@ -276,7 +276,7 @@ func (m *webhookNotifier) NotifyIssueChangeStatus(doer *user_model.User, issue * } else { apiIssue.Action = api.HookIssueReOpened } - err = webhook_services.PrepareWebhooks(issue.Repo, webhook.HookEventIssues, apiIssue) + err = webhook_services.PrepareWebhooks(ctx, webhook_services.EventSource{Repository: issue.Repo}, webhook.HookEventIssues, apiIssue) } if err != nil { log.Error("PrepareWebhooks [is_pull: %v, is_closed: %v]: %v", issue.IsPull, isClosed, err) @@ -294,7 +294,7 @@ func (m *webhookNotifier) NotifyNewIssue(issue *issues_model.Issue, mentions []* } mode, _ := access_model.AccessLevel(issue.Poster, issue.Repo) - if err := webhook_services.PrepareWebhooks(issue.Repo, webhook.HookEventIssues, &api.IssuePayload{ + if err := webhook_services.PrepareWebhooks(db.DefaultContext, webhook_services.EventSource{Repository: issue.Repo}, webhook.HookEventIssues, &api.IssuePayload{ Action: api.HookIssueOpened, Index: issue.Index, Issue: convert.ToAPIIssue(issue), @@ -323,7 +323,7 @@ func (m *webhookNotifier) NotifyNewPullRequest(pull *issues_model.PullRequest, m } mode, _ := access_model.AccessLevel(pull.Issue.Poster, pull.Issue.Repo) - if err := webhook_services.PrepareWebhooks(pull.Issue.Repo, webhook.HookEventPullRequest, &api.PullRequestPayload{ + if err := webhook_services.PrepareWebhooks(ctx, webhook_services.EventSource{Repository: pull.Issue.Repo}, webhook.HookEventPullRequest, &api.PullRequestPayload{ Action: api.HookIssueOpened, Index: pull.Issue.Index, PullRequest: convert.ToAPIPullRequest(ctx, pull, nil), @@ -342,7 +342,7 @@ func (m *webhookNotifier) NotifyIssueChangeContent(doer *user_model.User, issue var err error if issue.IsPull { issue.PullRequest.Issue = issue - err = webhook_services.PrepareWebhooks(issue.Repo, webhook.HookEventPullRequest, &api.PullRequestPayload{ + err = webhook_services.PrepareWebhooks(ctx, webhook_services.EventSource{Repository: issue.Repo}, webhook.HookEventPullRequest, &api.PullRequestPayload{ Action: api.HookIssueEdited, Index: issue.Index, Changes: &api.ChangesPayload{ @@ -355,7 +355,7 @@ func (m *webhookNotifier) NotifyIssueChangeContent(doer *user_model.User, issue Sender: convert.ToUser(doer, nil), }) } else { - err = webhook_services.PrepareWebhooks(issue.Repo, webhook.HookEventIssues, &api.IssuePayload{ + err = webhook_services.PrepareWebhooks(ctx, webhook_services.EventSource{Repository: issue.Repo}, webhook.HookEventIssues, &api.IssuePayload{ Action: api.HookIssueEdited, Index: issue.Index, Changes: &api.ChangesPayload{ @@ -374,54 +374,41 @@ func (m *webhookNotifier) NotifyIssueChangeContent(doer *user_model.User, issue } func (m *webhookNotifier) NotifyUpdateComment(doer *user_model.User, c *issues_model.Comment, oldContent string) { - var err error - - if err = c.LoadPoster(); err != nil { + if err := c.LoadPoster(); err != nil { log.Error("LoadPoster: %v", err) return } - if err = c.LoadIssue(); err != nil { + if err := c.LoadIssue(); err != nil { log.Error("LoadIssue: %v", err) return } - if err = c.Issue.LoadAttributes(db.DefaultContext); err != nil { + if err := c.Issue.LoadAttributes(db.DefaultContext); err != nil { log.Error("LoadAttributes: %v", err) return } - mode, _ := access_model.AccessLevel(doer, c.Issue.Repo) + var eventType webhook.HookEventType if c.Issue.IsPull { - err = webhook_services.PrepareWebhooks(c.Issue.Repo, webhook.HookEventPullRequestComment, &api.IssueCommentPayload{ - Action: api.HookIssueCommentEdited, - Issue: convert.ToAPIIssue(c.Issue), - Comment: convert.ToComment(c), - Changes: &api.ChangesPayload{ - Body: &api.ChangesFromPayload{ - From: oldContent, - }, - }, - Repository: convert.ToRepo(c.Issue.Repo, mode), - Sender: convert.ToUser(doer, nil), - IsPull: true, - }) + eventType = webhook.HookEventPullRequestComment } else { - err = webhook_services.PrepareWebhooks(c.Issue.Repo, webhook.HookEventIssueComment, &api.IssueCommentPayload{ - Action: api.HookIssueCommentEdited, - Issue: convert.ToAPIIssue(c.Issue), - Comment: convert.ToComment(c), - Changes: &api.ChangesPayload{ - Body: &api.ChangesFromPayload{ - From: oldContent, - }, - }, - Repository: convert.ToRepo(c.Issue.Repo, mode), - Sender: convert.ToUser(doer, nil), - IsPull: false, - }) + eventType = webhook.HookEventIssueComment } - if err != nil { + mode, _ := access_model.AccessLevel(doer, c.Issue.Repo) + if err := webhook_services.PrepareWebhooks(db.DefaultContext, webhook_services.EventSource{Repository: c.Issue.Repo}, eventType, &api.IssueCommentPayload{ + Action: api.HookIssueCommentEdited, + Issue: convert.ToAPIIssue(c.Issue), + Comment: convert.ToComment(c), + Changes: &api.ChangesPayload{ + Body: &api.ChangesFromPayload{ + From: oldContent, + }, + }, + Repository: convert.ToRepo(c.Issue.Repo, mode), + Sender: convert.ToUser(doer, nil), + IsPull: c.Issue.IsPull, + }); err != nil { log.Error("PrepareWebhooks [comment_id: %d]: %v", c.ID, err) } } @@ -429,30 +416,22 @@ func (m *webhookNotifier) NotifyUpdateComment(doer *user_model.User, c *issues_m func (m *webhookNotifier) NotifyCreateIssueComment(doer *user_model.User, repo *repo_model.Repository, issue *issues_model.Issue, comment *issues_model.Comment, mentions []*user_model.User, ) { - mode, _ := access_model.AccessLevel(doer, repo) - - var err error + var eventType webhook.HookEventType if issue.IsPull { - err = webhook_services.PrepareWebhooks(issue.Repo, webhook.HookEventPullRequestComment, &api.IssueCommentPayload{ - Action: api.HookIssueCommentCreated, - Issue: convert.ToAPIIssue(issue), - Comment: convert.ToComment(comment), - Repository: convert.ToRepo(repo, mode), - Sender: convert.ToUser(doer, nil), - IsPull: true, - }) + eventType = webhook.HookEventPullRequestComment } else { - err = webhook_services.PrepareWebhooks(issue.Repo, webhook.HookEventIssueComment, &api.IssueCommentPayload{ - Action: api.HookIssueCommentCreated, - Issue: convert.ToAPIIssue(issue), - Comment: convert.ToComment(comment), - Repository: convert.ToRepo(repo, mode), - Sender: convert.ToUser(doer, nil), - IsPull: false, - }) + eventType = webhook.HookEventIssueComment } - if err != nil { + mode, _ := access_model.AccessLevel(doer, repo) + if err := webhook_services.PrepareWebhooks(db.DefaultContext, webhook_services.EventSource{Repository: issue.Repo}, eventType, &api.IssueCommentPayload{ + Action: api.HookIssueCommentCreated, + Issue: convert.ToAPIIssue(issue), + Comment: convert.ToComment(comment), + Repository: convert.ToRepo(repo, mode), + Sender: convert.ToUser(doer, nil), + IsPull: issue.IsPull, + }); err != nil { log.Error("PrepareWebhooks [comment_id: %d]: %v", comment.ID, err) } } @@ -474,36 +453,29 @@ func (m *webhookNotifier) NotifyDeleteComment(doer *user_model.User, comment *is return } - mode, _ := access_model.AccessLevel(doer, comment.Issue.Repo) - + var eventType webhook.HookEventType if comment.Issue.IsPull { - err = webhook_services.PrepareWebhooks(comment.Issue.Repo, webhook.HookEventPullRequestComment, &api.IssueCommentPayload{ - Action: api.HookIssueCommentDeleted, - Issue: convert.ToAPIIssue(comment.Issue), - Comment: convert.ToComment(comment), - Repository: convert.ToRepo(comment.Issue.Repo, mode), - Sender: convert.ToUser(doer, nil), - IsPull: true, - }) + eventType = webhook.HookEventPullRequestComment } else { - err = webhook_services.PrepareWebhooks(comment.Issue.Repo, webhook.HookEventIssueComment, &api.IssueCommentPayload{ - Action: api.HookIssueCommentDeleted, - Issue: convert.ToAPIIssue(comment.Issue), - Comment: convert.ToComment(comment), - Repository: convert.ToRepo(comment.Issue.Repo, mode), - Sender: convert.ToUser(doer, nil), - IsPull: false, - }) + eventType = webhook.HookEventIssueComment } - if err != nil { + mode, _ := access_model.AccessLevel(doer, comment.Issue.Repo) + if err := webhook_services.PrepareWebhooks(db.DefaultContext, webhook_services.EventSource{Repository: comment.Issue.Repo}, eventType, &api.IssueCommentPayload{ + Action: api.HookIssueCommentDeleted, + Issue: convert.ToAPIIssue(comment.Issue), + Comment: convert.ToComment(comment), + Repository: convert.ToRepo(comment.Issue.Repo, mode), + Sender: convert.ToUser(doer, nil), + IsPull: comment.Issue.IsPull, + }); err != nil { log.Error("PrepareWebhooks [comment_id: %d]: %v", comment.ID, err) } } func (m *webhookNotifier) NotifyNewWikiPage(doer *user_model.User, repo *repo_model.Repository, page, comment string) { // Add to hook queue for created wiki page. - if err := webhook_services.PrepareWebhooks(repo, webhook.HookEventWiki, &api.WikiPayload{ + if err := webhook_services.PrepareWebhooks(db.DefaultContext, webhook_services.EventSource{Repository: repo}, webhook.HookEventWiki, &api.WikiPayload{ Action: api.HookWikiCreated, Repository: convert.ToRepo(repo, perm.AccessModeOwner), Sender: convert.ToUser(doer, nil), @@ -516,7 +488,7 @@ func (m *webhookNotifier) NotifyNewWikiPage(doer *user_model.User, repo *repo_mo func (m *webhookNotifier) NotifyEditWikiPage(doer *user_model.User, repo *repo_model.Repository, page, comment string) { // Add to hook queue for edit wiki page. - if err := webhook_services.PrepareWebhooks(repo, webhook.HookEventWiki, &api.WikiPayload{ + if err := webhook_services.PrepareWebhooks(db.DefaultContext, webhook_services.EventSource{Repository: repo}, webhook.HookEventWiki, &api.WikiPayload{ Action: api.HookWikiEdited, Repository: convert.ToRepo(repo, perm.AccessModeOwner), Sender: convert.ToUser(doer, nil), @@ -529,7 +501,7 @@ func (m *webhookNotifier) NotifyEditWikiPage(doer *user_model.User, repo *repo_m func (m *webhookNotifier) NotifyDeleteWikiPage(doer *user_model.User, repo *repo_model.Repository, page string) { // Add to hook queue for edit wiki page. - if err := webhook_services.PrepareWebhooks(repo, webhook.HookEventWiki, &api.WikiPayload{ + if err := webhook_services.PrepareWebhooks(db.DefaultContext, webhook_services.EventSource{Repository: repo}, webhook.HookEventWiki, &api.WikiPayload{ Action: api.HookWikiDeleted, Repository: convert.ToRepo(repo, perm.AccessModeOwner), Sender: convert.ToUser(doer, nil), @@ -567,7 +539,7 @@ func (m *webhookNotifier) NotifyIssueChangeLabels(doer *user_model.User, issue * log.Error("LoadIssue: %v", err) return } - err = webhook_services.PrepareWebhooks(issue.Repo, webhook.HookEventPullRequestLabel, &api.PullRequestPayload{ + err = webhook_services.PrepareWebhooks(ctx, webhook_services.EventSource{Repository: issue.Repo}, webhook.HookEventPullRequestLabel, &api.PullRequestPayload{ Action: api.HookIssueLabelUpdated, Index: issue.Index, PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, nil), @@ -575,7 +547,7 @@ func (m *webhookNotifier) NotifyIssueChangeLabels(doer *user_model.User, issue * Sender: convert.ToUser(doer, nil), }) } else { - err = webhook_services.PrepareWebhooks(issue.Repo, webhook.HookEventIssueLabel, &api.IssuePayload{ + err = webhook_services.PrepareWebhooks(ctx, webhook_services.EventSource{Repository: issue.Repo}, webhook.HookEventIssueLabel, &api.IssuePayload{ Action: api.HookIssueLabelUpdated, Index: issue.Index, Issue: convert.ToAPIIssue(issue), @@ -612,7 +584,7 @@ func (m *webhookNotifier) NotifyIssueChangeMilestone(doer *user_model.User, issu log.Error("LoadIssue: %v", err) return } - err = webhook_services.PrepareWebhooks(issue.Repo, webhook.HookEventPullRequestMilestone, &api.PullRequestPayload{ + err = webhook_services.PrepareWebhooks(ctx, webhook_services.EventSource{Repository: issue.Repo}, webhook.HookEventPullRequestMilestone, &api.PullRequestPayload{ Action: hookAction, Index: issue.Index, PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, nil), @@ -620,7 +592,7 @@ func (m *webhookNotifier) NotifyIssueChangeMilestone(doer *user_model.User, issu Sender: convert.ToUser(doer, nil), }) } else { - err = webhook_services.PrepareWebhooks(issue.Repo, webhook.HookEventIssueMilestone, &api.IssuePayload{ + err = webhook_services.PrepareWebhooks(ctx, webhook_services.EventSource{Repository: issue.Repo}, webhook.HookEventIssueMilestone, &api.IssuePayload{ Action: hookAction, Index: issue.Index, Issue: convert.ToAPIIssue(issue), @@ -644,7 +616,7 @@ func (m *webhookNotifier) NotifyPushCommits(pusher *user_model.User, repo *repo_ return } - if err := webhook_services.PrepareWebhooks(repo, webhook.HookEventPush, &api.PushPayload{ + if err := webhook_services.PrepareWebhooks(ctx, webhook_services.EventSource{Repository: repo}, webhook.HookEventPush, &api.PushPayload{ Ref: opts.RefFullName, Before: opts.OldCommitID, After: opts.NewCommitID, @@ -695,7 +667,7 @@ func (*webhookNotifier) NotifyMergePullRequest(pr *issues_model.PullRequest, doe Action: api.HookIssueClosed, } - err = webhook_services.PrepareWebhooks(pr.Issue.Repo, webhook.HookEventPullRequest, apiPullRequest) + err = webhook_services.PrepareWebhooks(ctx, webhook_services.EventSource{Repository: pr.Issue.Repo}, webhook.HookEventPullRequest, apiPullRequest) if err != nil { log.Error("PrepareWebhooks: %v", err) } @@ -717,7 +689,7 @@ func (m *webhookNotifier) NotifyPullRequestChangeTargetBranch(doer *user_model.U } issue.PullRequest.Issue = issue mode, _ := access_model.AccessLevel(issue.Poster, issue.Repo) - err = webhook_services.PrepareWebhooks(issue.Repo, webhook.HookEventPullRequest, &api.PullRequestPayload{ + err = webhook_services.PrepareWebhooks(ctx, webhook_services.EventSource{Repository: issue.Repo}, webhook.HookEventPullRequest, &api.PullRequestPayload{ Action: api.HookIssueEdited, Index: issue.Index, Changes: &api.ChangesPayload{ @@ -764,7 +736,7 @@ func (m *webhookNotifier) NotifyPullRequestReview(pr *issues_model.PullRequest, log.Error("models.AccessLevel: %v", err) return } - if err := webhook_services.PrepareWebhooks(review.Issue.Repo, reviewHookType, &api.PullRequestPayload{ + if err := webhook_services.PrepareWebhooks(ctx, webhook_services.EventSource{Repository: review.Issue.Repo}, reviewHookType, &api.PullRequestPayload{ Action: api.HookIssueReviewed, Index: review.Issue.Index, PullRequest: convert.ToAPIPullRequest(ctx, pr, nil), @@ -784,7 +756,7 @@ func (m *webhookNotifier) NotifyCreateRef(pusher *user_model.User, repo *repo_mo apiRepo := convert.ToRepo(repo, perm.AccessModeNone) refName := git.RefEndName(refFullName) - if err := webhook_services.PrepareWebhooks(repo, webhook.HookEventCreate, &api.CreatePayload{ + if err := webhook_services.PrepareWebhooks(db.DefaultContext, webhook_services.EventSource{Repository: repo}, webhook.HookEventCreate, &api.CreatePayload{ Ref: refName, Sha: refID, RefType: refType, @@ -808,7 +780,7 @@ func (m *webhookNotifier) NotifyPullRequestSynchronized(doer *user_model.User, p return } - if err := webhook_services.PrepareWebhooks(pr.Issue.Repo, webhook.HookEventPullRequestSync, &api.PullRequestPayload{ + if err := webhook_services.PrepareWebhooks(ctx, webhook_services.EventSource{Repository: pr.Issue.Repo}, webhook.HookEventPullRequestSync, &api.PullRequestPayload{ Action: api.HookIssueSynchronized, Index: pr.Issue.Index, PullRequest: convert.ToAPIPullRequest(ctx, pr, nil), @@ -824,7 +796,7 @@ func (m *webhookNotifier) NotifyDeleteRef(pusher *user_model.User, repo *repo_mo apiRepo := convert.ToRepo(repo, perm.AccessModeNone) refName := git.RefEndName(refFullName) - if err := webhook_services.PrepareWebhooks(repo, webhook.HookEventDelete, &api.DeletePayload{ + if err := webhook_services.PrepareWebhooks(db.DefaultContext, webhook_services.EventSource{Repository: repo}, webhook.HookEventDelete, &api.DeletePayload{ Ref: refName, RefType: refType, PusherType: api.PusherTypeUser, @@ -842,7 +814,7 @@ func sendReleaseHook(doer *user_model.User, rel *repo_model.Release, action api. } mode, _ := access_model.AccessLevel(doer, rel.Repo) - if err := webhook_services.PrepareWebhooks(rel.Repo, webhook.HookEventRelease, &api.ReleasePayload{ + if err := webhook_services.PrepareWebhooks(db.DefaultContext, webhook_services.EventSource{Repository: rel.Repo}, webhook.HookEventRelease, &api.ReleasePayload{ Action: action, Release: convert.ToRelease(rel), Repository: convert.ToRepo(rel.Repo, mode), @@ -875,7 +847,7 @@ func (m *webhookNotifier) NotifySyncPushCommits(pusher *user_model.User, repo *r return } - if err := webhook_services.PrepareWebhooks(repo, webhook.HookEventPush, &api.PushPayload{ + if err := webhook_services.PrepareWebhooks(ctx, webhook_services.EventSource{Repository: repo}, webhook.HookEventPush, &api.PushPayload{ Ref: opts.RefFullName, Before: opts.OldCommitID, After: opts.NewCommitID, @@ -908,9 +880,9 @@ func (m *webhookNotifier) NotifyPackageDelete(doer *user_model.User, pd *package } func notifyPackage(sender *user_model.User, pd *packages_model.PackageDescriptor, action api.HookPackageAction) { - if pd.Repository == nil { - // TODO https://github.com/go-gitea/gitea/pull/17940 - return + source := webhook_services.EventSource{ + Repository: pd.Repository, + Owner: pd.Owner, } ctx, _, finished := process.GetManager().AddContext(graceful.GetManager().HammerContext(), fmt.Sprintf("webhook.notifyPackage Package: %s[%d]", pd.Package.Name, pd.Package.ID)) @@ -922,7 +894,7 @@ func notifyPackage(sender *user_model.User, pd *packages_model.PackageDescriptor return } - if err := webhook_services.PrepareWebhooks(pd.Repository, webhook.HookEventPackage, &api.PackagePayload{ + if err := webhook_services.PrepareWebhooks(ctx, source, webhook.HookEventPackage, &api.PackagePayload{ Action: action, Package: apiPackage, Sender: convert.ToUser(sender, nil), diff --git a/routers/api/v1/repo/hook.go b/routers/api/v1/repo/hook.go index 86361817cb46..5956fe9da92f 100644 --- a/routers/api/v1/repo/hook.go +++ b/routers/api/v1/repo/hook.go @@ -168,7 +168,7 @@ func TestHook(ctx *context.APIContext) { commit := convert.ToPayloadCommit(ctx.Repo.Repository, ctx.Repo.Commit) commitID := ctx.Repo.Commit.ID.String() - if err := webhook_service.PrepareWebhook(hook, ctx.Repo.Repository, webhook.HookEventPush, &api.PushPayload{ + if err := webhook_service.PrepareWebhook(ctx, hook, webhook.HookEventPush, &api.PushPayload{ Ref: ref, Before: commitID, After: commitID, diff --git a/routers/api/v1/repo/hook_test.go b/routers/api/v1/repo/hook_test.go index 07f1532f82d6..fd9a165bf358 100644 --- a/routers/api/v1/repo/hook_test.go +++ b/routers/api/v1/repo/hook_test.go @@ -28,7 +28,6 @@ func TestTestHook(t *testing.T) { assert.EqualValues(t, http.StatusNoContent, ctx.Resp.Status()) unittest.AssertExistsAndLoadBean(t, &webhook.HookTask{ - RepoID: 1, HookID: 1, }, unittest.Cond("is_delivered=?", false)) } diff --git a/routers/web/repo/webhook.go b/routers/web/repo/webhook.go index 425198ce2442..ee980333b72f 100644 --- a/routers/web/repo/webhook.go +++ b/routers/web/repo/webhook.go @@ -633,7 +633,7 @@ func TestWebhook(ctx *context.Context) { hookID := ctx.ParamsInt64(":id") w, err := webhook.GetWebhookByRepoID(ctx.Repo.Repository.ID, hookID) if err != nil { - ctx.Flash.Error("GetWebhookByID: " + err.Error()) + ctx.Flash.Error("GetWebhookByRepoID: " + err.Error()) ctx.Status(http.StatusInternalServerError) return } @@ -679,7 +679,7 @@ func TestWebhook(ctx *context.Context) { Pusher: apiUser, Sender: apiUser, } - if err := webhook_service.PrepareWebhook(w, ctx.Repo.Repository, webhook.HookEventPush, p); err != nil { + if err := webhook_service.PrepareWebhook(ctx, w, webhook.HookEventPush, p); err != nil { ctx.Flash.Error("PrepareWebhook: " + err.Error()) ctx.Status(http.StatusInternalServerError) } else { @@ -697,7 +697,7 @@ func ReplayWebhook(ctx *context.Context) { return } - if err := webhook_service.ReplayHookTask(w, hookTaskUUID); err != nil { + if err := webhook_service.ReplayHookTask(ctx, w, hookTaskUUID); err != nil { if webhook.IsErrHookTaskNotExist(err) { ctx.NotFound("ReplayHookTask", nil) } else { diff --git a/services/webhook/deliver.go b/services/webhook/deliver.go index 77744473f1ce..74a69c297ca3 100644 --- a/services/webhook/deliver.go +++ b/services/webhook/deliver.go @@ -23,7 +23,6 @@ import ( "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/hostmatcher" "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/process" "code.gitea.io/gitea/modules/proxy" "code.gitea.io/gitea/modules/queue" "code.gitea.io/gitea/modules/setting" @@ -44,7 +43,7 @@ func Deliver(ctx context.Context, t *webhook_model.HookTask) error { return } // There was a panic whilst delivering a hook... - log.Error("PANIC whilst trying to deliver webhook[%d] for repo[%d] to %s Panic: %v\nStacktrace: %s", t.ID, t.RepoID, w.URL, err, log.Stack(2)) + log.Error("PANIC whilst trying to deliver webhook[%d] to %s Panic: %v\nStacktrace: %s", t.ID, w.URL, err, log.Stack(2)) }() t.IsDelivered = true @@ -202,35 +201,6 @@ func Deliver(ctx context.Context, t *webhook_model.HookTask) error { return nil } -// populateDeliverHooks checks and delivers undelivered hooks. -func populateDeliverHooks(ctx context.Context) { - select { - case <-ctx.Done(): - return - default: - } - ctx, _, finished := process.GetManager().AddTypedContext(ctx, "Service: DeliverHooks", process.SystemProcessType, true) - defer finished() - tasks, err := webhook_model.FindUndeliveredHookTasks() - if err != nil { - log.Error("DeliverHooks: %v", err) - return - } - - // Update hook task status. - for _, t := range tasks { - select { - case <-ctx.Done(): - return - default: - } - - if err := addToTask(t.RepoID); err != nil { - log.Error("DeliverHook failed [%d]: %v", t.RepoID, err) - } - } -} - var ( webhookHTTPClient *http.Client once sync.Once @@ -281,13 +251,23 @@ func Init() error { }, } - hookQueue = queue.CreateUniqueQueue("webhook_sender", handle, "") + hookQueue = queue.CreateUniqueQueue("webhook_sender", handle, int64(0)) if hookQueue == nil { return fmt.Errorf("Unable to create webhook_sender Queue") } go graceful.GetManager().RunWithShutdownFns(hookQueue.Run) - populateDeliverHooks(graceful.GetManager().HammerContext()) + tasks, err := webhook_model.FindUndeliveredHookTasks(graceful.GetManager().HammerContext()) + if err != nil { + log.Error("FindUndeliveredHookTasks failed: %v", err) + return err + } + + for _, task := range tasks { + if err := enqueueHookTask(task); err != nil { + log.Error("enqueueHookTask failed: %v", err) + } + } return nil } diff --git a/services/webhook/webhook.go b/services/webhook/webhook.go index 767c3701f37d..e877e16edaa3 100644 --- a/services/webhook/webhook.go +++ b/services/webhook/webhook.go @@ -7,11 +7,10 @@ package webhook import ( "context" "fmt" - "strconv" "strings" - "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" + user_model "code.gitea.io/gitea/models/user" webhook_model "code.gitea.io/gitea/models/webhook" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/graceful" @@ -104,49 +103,38 @@ func getPayloadBranch(p api.Payloader) string { return "" } -// handle passed PR IDs and test the PRs -func handle(data ...queue.Data) []queue.Data { - for _, datum := range data { - repoIDStr := datum.(string) - log.Trace("DeliverHooks [repo_id: %v]", repoIDStr) +// EventSource represents the source of a webhook action. Repository and/or Owner must be set. +type EventSource struct { + Repository *repo_model.Repository + Owner *user_model.User +} - repoID, err := strconv.ParseInt(repoIDStr, 10, 64) - if err != nil { - log.Error("Invalid repo ID: %s", repoIDStr) - continue - } +// handle delivers hook tasks +func handle(data ...queue.Data) []queue.Data { + ctx := graceful.GetManager().HammerContext() - tasks, err := webhook_model.FindRepoUndeliveredHookTasks(repoID) + for _, taskID := range data { + task, err := webhook_model.GetHookTaskByID(ctx, taskID.(int64)) if err != nil { - log.Error("Get repository [%d] hook tasks: %v", repoID, err) - continue - } - for _, t := range tasks { - if err = Deliver(graceful.GetManager().HammerContext(), t); err != nil { - log.Error("deliver: %v", err) + log.Error("GetHookTaskByID failed: %v", err) + } else { + if err := Deliver(ctx, task); err != nil { + log.Error("webhook.Deliver failed: %v", err) } } } + return nil } -func addToTask(repoID int64) error { - err := hookQueue.PushFunc(strconv.FormatInt(repoID, 10), nil) +func enqueueHookTask(task *webhook_model.HookTask) error { + err := hookQueue.PushFunc(task.ID, nil) if err != nil && err != queue.ErrAlreadyInQueue { return err } return nil } -// PrepareWebhook adds special webhook to task queue for given payload. -func PrepareWebhook(w *webhook_model.Webhook, repo *repo_model.Repository, event webhook_model.HookEventType, p api.Payloader) error { - if err := prepareWebhook(w, repo, event, p); err != nil { - return err - } - - return addToTask(repo.ID) -} - func checkBranch(w *webhook_model.Webhook, branch string) bool { if w.BranchFilter == "" || w.BranchFilter == "*" { return true @@ -162,7 +150,8 @@ func checkBranch(w *webhook_model.Webhook, branch string) bool { return g.Match(branch) } -func prepareWebhook(w *webhook_model.Webhook, repo *repo_model.Repository, event webhook_model.HookEventType, p api.Payloader) error { +// PrepareWebhook creates a hook task and enqueues it for processing +func PrepareWebhook(ctx context.Context, w *webhook_model.Webhook, event webhook_model.HookEventType, p api.Payloader) error { // Skip sending if webhooks are disabled. if setting.DisableWebhooks { return nil @@ -207,44 +196,45 @@ func prepareWebhook(w *webhook_model.Webhook, repo *repo_model.Repository, event payloader = p } - if err = webhook_model.CreateHookTask(&webhook_model.HookTask{ - RepoID: repo.ID, + task, err := webhook_model.CreateHookTask(ctx, &webhook_model.HookTask{ HookID: w.ID, Payloader: payloader, EventType: event, - }); err != nil { + }) + if err != nil { return fmt.Errorf("CreateHookTask: %v", err) } - return nil + + return enqueueHookTask(task) } // PrepareWebhooks adds new webhooks to task queue for given payload. -func PrepareWebhooks(repo *repo_model.Repository, event webhook_model.HookEventType, p api.Payloader) error { - if err := prepareWebhooks(db.DefaultContext, repo, event, p); err != nil { - return err - } +func PrepareWebhooks(ctx context.Context, source EventSource, event webhook_model.HookEventType, p api.Payloader) error { + owner := source.Owner - return addToTask(repo.ID) -} + var ws []*webhook_model.Webhook -func prepareWebhooks(ctx context.Context, repo *repo_model.Repository, event webhook_model.HookEventType, p api.Payloader) error { - ws, err := webhook_model.ListWebhooksByOpts(ctx, &webhook_model.ListWebhookOptions{ - RepoID: repo.ID, - IsActive: util.OptionalBoolTrue, - }) - if err != nil { - return fmt.Errorf("GetActiveWebhooksByRepoID: %v", err) + if source.Repository != nil { + repoHooks, err := webhook_model.ListWebhooksByOpts(ctx, &webhook_model.ListWebhookOptions{ + RepoID: source.Repository.ID, + IsActive: util.OptionalBoolTrue, + }) + if err != nil { + return fmt.Errorf("ListWebhooksByOpts: %v", err) + } + ws = append(ws, repoHooks...) + + owner = source.Repository.MustOwner() } - // check if repo belongs to org and append additional webhooks - if repo.MustOwner().IsOrganization() { - // get hooks for org + // check if owner is an org and append additional webhooks + if owner != nil && owner.IsOrganization() { orgHooks, err := webhook_model.ListWebhooksByOpts(ctx, &webhook_model.ListWebhookOptions{ - OrgID: repo.OwnerID, + OrgID: owner.ID, IsActive: util.OptionalBoolTrue, }) if err != nil { - return fmt.Errorf("GetActiveWebhooksByOrgID: %v", err) + return fmt.Errorf("ListWebhooksByOpts: %v", err) } ws = append(ws, orgHooks...) } @@ -261,7 +251,7 @@ func prepareWebhooks(ctx context.Context, repo *repo_model.Repository, event web } for _, w := range ws { - if err = prepareWebhook(w, repo, event, p); err != nil { + if err := PrepareWebhook(ctx, w, event, p); err != nil { return err } } @@ -269,11 +259,11 @@ func prepareWebhooks(ctx context.Context, repo *repo_model.Repository, event web } // ReplayHookTask replays a webhook task -func ReplayHookTask(w *webhook_model.Webhook, uuid string) error { - t, err := webhook_model.ReplayHookTask(w.ID, uuid) +func ReplayHookTask(ctx context.Context, w *webhook_model.Webhook, uuid string) error { + task, err := webhook_model.ReplayHookTask(ctx, w.ID, uuid) if err != nil { return err } - return addToTask(t.RepoID) + return enqueueHookTask(task) } diff --git a/services/webhook/webhook_test.go b/services/webhook/webhook_test.go index 1887cc71fef1..8d44aa504ae2 100644 --- a/services/webhook/webhook_test.go +++ b/services/webhook/webhook_test.go @@ -7,6 +7,7 @@ package webhook import ( "testing" + "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" webhook_model "code.gitea.io/gitea/models/webhook" @@ -32,12 +33,12 @@ func TestPrepareWebhooks(t *testing.T) { repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) hookTasks := []*webhook_model.HookTask{ - {RepoID: repo.ID, HookID: 1, EventType: webhook_model.HookEventPush}, + {HookID: 1, EventType: webhook_model.HookEventPush}, } for _, hookTask := range hookTasks { unittest.AssertNotExistsBean(t, hookTask) } - assert.NoError(t, PrepareWebhooks(repo, webhook_model.HookEventPush, &api.PushPayload{Commits: []*api.PayloadCommit{{}}})) + assert.NoError(t, PrepareWebhooks(db.DefaultContext, EventSource{Repository: repo}, webhook_model.HookEventPush, &api.PushPayload{Commits: []*api.PayloadCommit{{}}})) for _, hookTask := range hookTasks { unittest.AssertExistsAndLoadBean(t, hookTask) } @@ -48,13 +49,13 @@ func TestPrepareWebhooksBranchFilterMatch(t *testing.T) { repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) hookTasks := []*webhook_model.HookTask{ - {RepoID: repo.ID, HookID: 4, EventType: webhook_model.HookEventPush}, + {HookID: 4, EventType: webhook_model.HookEventPush}, } for _, hookTask := range hookTasks { unittest.AssertNotExistsBean(t, hookTask) } // this test also ensures that * doesn't handle / in any special way (like shell would) - assert.NoError(t, PrepareWebhooks(repo, webhook_model.HookEventPush, &api.PushPayload{Ref: "refs/heads/feature/7791", Commits: []*api.PayloadCommit{{}}})) + assert.NoError(t, PrepareWebhooks(db.DefaultContext, EventSource{Repository: repo}, webhook_model.HookEventPush, &api.PushPayload{Ref: "refs/heads/feature/7791", Commits: []*api.PayloadCommit{{}}})) for _, hookTask := range hookTasks { unittest.AssertExistsAndLoadBean(t, hookTask) } @@ -65,12 +66,12 @@ func TestPrepareWebhooksBranchFilterNoMatch(t *testing.T) { repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) hookTasks := []*webhook_model.HookTask{ - {RepoID: repo.ID, HookID: 4, EventType: webhook_model.HookEventPush}, + {HookID: 4, EventType: webhook_model.HookEventPush}, } for _, hookTask := range hookTasks { unittest.AssertNotExistsBean(t, hookTask) } - assert.NoError(t, PrepareWebhooks(repo, webhook_model.HookEventPush, &api.PushPayload{Ref: "refs/heads/fix_weird_bug"})) + assert.NoError(t, PrepareWebhooks(db.DefaultContext, EventSource{Repository: repo}, webhook_model.HookEventPush, &api.PushPayload{Ref: "refs/heads/fix_weird_bug"})) for _, hookTask := range hookTasks { unittest.AssertNotExistsBean(t, hookTask) From cd33979f5a683fd91bd79225e300e41173335926 Mon Sep 17 00:00:00 2001 From: rock2dust Date: Sat, 22 Oct 2022 11:24:09 +0800 Subject: [PATCH 04/20] Added check for disabled Packages (#21540) At the moment, If admin disable Packages, still show the Packages on the admin dashboard This patch added a check to hide the Packages entry Signed-off-by: baronbunny Signed-off-by: baronbunny --- routers/web/web.go | 1 + templates/admin/navbar.tmpl | 8 +++++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/routers/web/web.go b/routers/web/web.go index 62503b3141e8..9b814c3f5424 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -591,6 +591,7 @@ func RegisterRoutes(m *web.Route) { }) }, func(ctx *context.Context) { ctx.Data["EnableOAuth2"] = setting.OAuth2.Enable + ctx.Data["EnablePackages"] = setting.Packages.Enabled }, adminReq) // ***** END: Admin ***** diff --git a/templates/admin/navbar.tmpl b/templates/admin/navbar.tmpl index b138eb79ba4e..1c8b12fc2faf 100644 --- a/templates/admin/navbar.tmpl +++ b/templates/admin/navbar.tmpl @@ -12,9 +12,11 @@ {{.locale.Tr "admin.repositories"}} - - {{.locale.Tr "packages.title"}} - + {{if .EnablePackages}} + + {{.locale.Tr "packages.title"}} + + {{end}} {{if not DisableWebhooks}} {{.locale.Tr "admin.hooks"}} From 2c77d4b19505f8eede4d2cb5d415f6440d6cffc2 Mon Sep 17 00:00:00 2001 From: Yarden Shoham Date: Sat, 22 Oct 2022 10:25:34 +0300 Subject: [PATCH 05/20] Remove unnecessary debug log (#21536) It distractingly shows up on unit tests * Looks like a leftover from #20571 Signed-off-by: Yarden Shoham Co-authored-by: Lunny Xiao --- modules/markup/markdown/meta.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/modules/markup/markdown/meta.go b/modules/markup/markdown/meta.go index 45d79d537a73..fb37236d77eb 100644 --- a/modules/markup/markdown/meta.go +++ b/modules/markup/markdown/meta.go @@ -10,8 +10,6 @@ import ( "unicode" "unicode/utf8" - "code.gitea.io/gitea/modules/log" - "gopkg.in/yaml.v3" ) @@ -99,8 +97,6 @@ func ExtractMetadataBytes(contents []byte, out interface{}) ([]byte, error) { return contents, errors.New("could not determine metadata") } - log.Info("%s", string(front)) - if err := yaml.Unmarshal(front, out); err != nil { return contents, err } From 69fcca2d4564f706fa41280895e3a20d81740598 Mon Sep 17 00:00:00 2001 From: Raymond Date: Sat, 22 Oct 2022 11:23:20 +0200 Subject: [PATCH 06/20] Remove deleted repos from searchresult (#21512) This prevents a 500 response, because null pointer exceptions in rendering the template. This happends bc the repoId is not in the repoMap because it is delete fix #19076 Co-authored-by: Lunny Xiao Co-authored-by: 6543 <6543@obermui.de> Co-authored-by: wxiaoguang --- routers/web/explore/code.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/routers/web/explore/code.go b/routers/web/explore/code.go index 2357b34fd0ca..38474255d19b 100644 --- a/routers/web/explore/code.go +++ b/routers/web/explore/code.go @@ -110,6 +110,18 @@ func Code(ctx *context.Context) { } ctx.Data["RepoMaps"] = repoMaps + + if len(loadRepoIDs) != len(repoMaps) { + // Remove deleted repos from search results + cleanedSearchResults := make([]*code_indexer.Result, 0, len(repoMaps)) + for _, sr := range searchResults { + if _, found := repoMaps[sr.RepoID]; found { + cleanedSearchResults = append(cleanedSearchResults, sr) + } + } + + searchResults = cleanedSearchResults + } } ctx.Data["SearchResults"] = searchResults From 154efa59a5a837d8375c09fb0b18a1b63bea6a3a Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Sat, 22 Oct 2022 15:36:44 +0200 Subject: [PATCH 07/20] Prevent Authorization header for presigned LFS urls (#21531) Fixes #21525 Co-authored-by: Lunny Xiao --- services/lfs/server.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/services/lfs/server.go b/services/lfs/server.go index b868db39dbec..830112fac6e3 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -438,14 +438,21 @@ func buildObjectResponse(rc *requestContext, pointer lfs_module.Pointer, downloa } if download { - rep.Actions["download"] = &lfs_module.Link{Href: rc.DownloadLink(pointer), Header: header} + var link *lfs_module.Link if setting.LFS.ServeDirect { // If we have a signed url (S3, object storage), redirect to this directly. u, err := storage.LFS.URL(pointer.RelativePath(), pointer.Oid) if u != nil && err == nil { - rep.Actions["download"] = &lfs_module.Link{Href: u.String(), Header: header} + // Presigned url does not need the Authorization header + // https://github.com/go-gitea/gitea/issues/21525 + delete(header, "Authorization") + link = &lfs_module.Link{Href: u.String(), Header: header} } } + if link == nil { + link = &lfs_module.Link{Href: rc.DownloadLink(pointer), Header: header} + } + rep.Actions["download"] = link } if upload { rep.Actions["upload"] = &lfs_module.Link{Href: rc.UploadLink(pointer), Header: header} From 82ecd3b19eb1382521b7999bd0e2dd977c3b994d Mon Sep 17 00:00:00 2001 From: Ashley Nelson Date: Sat, 22 Oct 2022 10:08:10 -0500 Subject: [PATCH 08/20] Update milestone counters when issue is deleted (#21459) When actions besides "delete" are performed on issues, the milestone counter is updated. However, since deleting issues goes through a different code path, the associated milestone's count wasn't being updated, resulting in inaccurate counts until another issue in the same milestone had a non-delete action performed on it. I verified this change fixes the inaccurate counts using a local docker build. Fixes #21254 Co-authored-by: Lunny Xiao --- .../expected_milestone.yml | 19 ++++++++ .../Test_updateOpenMilestoneCounts/issue.yml | 25 ++++++++++ .../milestone.yml | 19 ++++++++ models/migrations/migrations.go | 2 + models/migrations/v229.go | 47 +++++++++++++++++++ models/migrations/v229_test.go | 46 ++++++++++++++++++ services/issue/issue.go | 5 ++ 7 files changed, 163 insertions(+) create mode 100644 models/migrations/fixtures/Test_updateOpenMilestoneCounts/expected_milestone.yml create mode 100644 models/migrations/fixtures/Test_updateOpenMilestoneCounts/issue.yml create mode 100644 models/migrations/fixtures/Test_updateOpenMilestoneCounts/milestone.yml create mode 100644 models/migrations/v229.go create mode 100644 models/migrations/v229_test.go diff --git a/models/migrations/fixtures/Test_updateOpenMilestoneCounts/expected_milestone.yml b/models/migrations/fixtures/Test_updateOpenMilestoneCounts/expected_milestone.yml new file mode 100644 index 000000000000..9326fa550b14 --- /dev/null +++ b/models/migrations/fixtures/Test_updateOpenMilestoneCounts/expected_milestone.yml @@ -0,0 +1,19 @@ +# type Milestone struct { +# ID int64 `xorm:"pk autoincr"` +# IsClosed bool +# NumIssues int +# NumClosedIssues int +# Completeness int // Percentage(1-100). +# } +- + id: 1 + is_closed: false + num_issues: 3 + num_closed_issues: 1 + completeness: 33 +- + id: 2 + is_closed: true + num_issues: 5 + num_closed_issues: 5 + completeness: 100 diff --git a/models/migrations/fixtures/Test_updateOpenMilestoneCounts/issue.yml b/models/migrations/fixtures/Test_updateOpenMilestoneCounts/issue.yml new file mode 100644 index 000000000000..fdaacd9f6827 --- /dev/null +++ b/models/migrations/fixtures/Test_updateOpenMilestoneCounts/issue.yml @@ -0,0 +1,25 @@ +# type Issue struct { +# ID int64 `xorm:"pk autoincr"` +# RepoID int64 `xorm:"INDEX UNIQUE(repo_index)"` +# Index int64 `xorm:"UNIQUE(repo_index)"` // Index in one repository. +# MilestoneID int64 `xorm:"INDEX"` +# IsClosed bool `xorm:"INDEX"` +# } +- + id: 1 + repo_id: 1 + index: 1 + milestone_id: 1 + is_closed: false +- + id: 2 + repo_id: 1 + index: 2 + milestone_id: 1 + is_closed: true +- + id: 4 + repo_id: 1 + index: 3 + milestone_id: 1 + is_closed: false diff --git a/models/migrations/fixtures/Test_updateOpenMilestoneCounts/milestone.yml b/models/migrations/fixtures/Test_updateOpenMilestoneCounts/milestone.yml new file mode 100644 index 000000000000..0bcf4cffd3aa --- /dev/null +++ b/models/migrations/fixtures/Test_updateOpenMilestoneCounts/milestone.yml @@ -0,0 +1,19 @@ +# type Milestone struct { +# ID int64 `xorm:"pk autoincr"` +# IsClosed bool +# NumIssues int +# NumClosedIssues int +# Completeness int // Percentage(1-100). +# } +- + id: 1 + is_closed: false + num_issues: 4 + num_closed_issues: 2 + completeness: 50 +- + id: 2 + is_closed: true + num_issues: 5 + num_closed_issues: 5 + completeness: 100 diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 46ef052829b6..cca6c52d429a 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -419,6 +419,8 @@ var migrations = []Migration{ NewMigration("Create key/value table for system settings", createSystemSettingsTable), // v228 -> v229 NewMigration("Add TeamInvite table", addTeamInviteTable), + // v229 -> v230 + NewMigration("Update counts of all open milestones", updateOpenMilestoneCounts), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v229.go b/models/migrations/v229.go new file mode 100644 index 000000000000..42ec2033fee2 --- /dev/null +++ b/models/migrations/v229.go @@ -0,0 +1,47 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "fmt" + + "code.gitea.io/gitea/models/issues" + + "xorm.io/builder" + "xorm.io/xorm" +) + +func updateOpenMilestoneCounts(x *xorm.Engine) error { + var openMilestoneIDs []int64 + err := x.Table("milestone").Select("id").Where(builder.Neq{"is_closed": 1}).Find(&openMilestoneIDs) + if err != nil { + return fmt.Errorf("error selecting open milestone IDs: %w", err) + } + + for _, id := range openMilestoneIDs { + _, err := x.ID(id). + SetExpr("num_issues", builder.Select("count(*)").From("issue").Where( + builder.Eq{"milestone_id": id}, + )). + SetExpr("num_closed_issues", builder.Select("count(*)").From("issue").Where( + builder.Eq{ + "milestone_id": id, + "is_closed": true, + }, + )). + Update(&issues.Milestone{}) + if err != nil { + return fmt.Errorf("error updating issue counts in milestone %d: %w", id, err) + } + _, err = x.Exec("UPDATE `milestone` SET completeness=100*num_closed_issues/(CASE WHEN num_issues > 0 THEN num_issues ELSE 1 END) WHERE id=?", + id, + ) + if err != nil { + return fmt.Errorf("error setting completeness on milestone %d: %w", id, err) + } + } + + return nil +} diff --git a/models/migrations/v229_test.go b/models/migrations/v229_test.go new file mode 100644 index 000000000000..f8a147c9bd69 --- /dev/null +++ b/models/migrations/v229_test.go @@ -0,0 +1,46 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "testing" + + "code.gitea.io/gitea/models/issues" + + "github.com/stretchr/testify/assert" +) + +func Test_updateOpenMilestoneCounts(t *testing.T) { + type ExpectedMilestone issues.Milestone + + // Prepare and load the testing database + x, deferable := prepareTestEnv(t, 0, new(issues.Milestone), new(ExpectedMilestone), new(issues.Issue)) + defer deferable() + if x == nil || t.Failed() { + return + } + + if err := updateOpenMilestoneCounts(x); err != nil { + assert.NoError(t, err) + return + } + + expected := []ExpectedMilestone{} + if err := x.Table("expected_milestone").Asc("id").Find(&expected); !assert.NoError(t, err) { + return + } + + got := []issues.Milestone{} + if err := x.Table("milestone").Asc("id").Find(&got); !assert.NoError(t, err) { + return + } + + for i, e := range expected { + got := got[i] + assert.Equal(t, e.ID, got.ID) + assert.Equal(t, e.NumIssues, got.NumIssues) + assert.Equal(t, e.NumClosedIssues, got.NumClosedIssues) + } +} diff --git a/services/issue/issue.go b/services/issue/issue.go index 69b87686c143..47782e50d36a 100644 --- a/services/issue/issue.go +++ b/services/issue/issue.go @@ -224,6 +224,11 @@ func deleteIssue(issue *issues_model.Issue) error { return err } + if err := issues_model.UpdateMilestoneCounters(ctx, issue.MilestoneID); err != nil { + return fmt.Errorf("error updating counters for milestone id %d: %w", + issue.MilestoneID, err) + } + if err := activities_model.DeleteIssueActions(ctx, issue.RepoID, issue.ID); err != nil { return err } From 63ebb53fd526021666bd9fab1f9d092380f7a2f4 Mon Sep 17 00:00:00 2001 From: Yarden Shoham Date: Sat, 22 Oct 2022 20:15:52 +0300 Subject: [PATCH 09/20] Add link to user profile in markdown mention only if user exists (#21533) Previously mentioning a user would link to its profile, regardless of whether the user existed. This change tests if the user exists and only if it does - a link to its profile is added. * Fixes #3444 Signed-off-by: Yarden Shoham Co-authored-by: wxiaoguang Co-authored-by: Lunny Xiao --- contrib/pr/checkout.go | 3 ++- modules/markup/html.go | 10 ++++++-- modules/markup/markdown/markdown_test.go | 5 ++++ modules/markup/renderer.go | 12 +++++++++- routers/api/v1/misc/markdown_test.go | 7 ++++++ routers/init.go | 3 ++- services/markup/main_test.go | 19 ++++++++++++++++ services/markup/processorhelper.go | 29 ++++++++++++++++++++++++ services/markup/processorhelper_test.go | 20 ++++++++++++++++ 9 files changed, 103 insertions(+), 5 deletions(-) create mode 100644 services/markup/main_test.go create mode 100644 services/markup/processorhelper.go create mode 100644 services/markup/processorhelper_test.go diff --git a/contrib/pr/checkout.go b/contrib/pr/checkout.go index 09510ac2c575..686a3ddffaac 100644 --- a/contrib/pr/checkout.go +++ b/contrib/pr/checkout.go @@ -33,6 +33,7 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/routers" + markup_service "code.gitea.io/gitea/services/markup" "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/config" @@ -112,7 +113,7 @@ func runPR() { log.Printf("[PR] Setting up router\n") // routers.GlobalInit() external.RegisterRenderers() - markup.Init() + markup.Init(markup_service.ProcessorHelper()) c := routers.NormalRoutes(graceful.GetManager().HammerContext()) log.Printf("[PR] Ready for testing !\n") diff --git a/modules/markup/html.go b/modules/markup/html.go index a5606dbb516a..ae00c3905fe8 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -603,8 +603,14 @@ func mentionProcessor(ctx *RenderContext, node *html.Node) { start = loc.End continue } - replaceContent(node, loc.Start, loc.End, createLink(util.URLJoin(setting.AppURL, mention[1:]), mention, "mention")) - node = node.NextSibling.NextSibling + mentionedUsername := mention[1:] + + if processorHelper.IsUsernameMentionable != nil && processorHelper.IsUsernameMentionable(ctx.Ctx, mentionedUsername) { + replaceContent(node, loc.Start, loc.End, createLink(util.URLJoin(setting.AppURL, mentionedUsername), mention, "mention")) + node = node.NextSibling.NextSibling + } else { + node = node.NextSibling + } start = 0 } } diff --git a/modules/markup/markdown/markdown_test.go b/modules/markup/markdown/markdown_test.go index 12c6288c24d1..fbb741d1cd8a 100644 --- a/modules/markup/markdown/markdown_test.go +++ b/modules/markup/markdown/markdown_test.go @@ -38,6 +38,11 @@ func TestMain(m *testing.M) { if err := git.InitSimple(context.Background()); err != nil { log.Fatal("git init failed, err: %v", err) } + markup.Init(&markup.ProcessorHelper{ + IsUsernameMentionable: func(ctx context.Context, username string) bool { + return username == "r-lyeh" + }, + }) os.Exit(m.Run()) } diff --git a/modules/markup/renderer.go b/modules/markup/renderer.go index 5f69dc72354f..b3289cb3c3b0 100644 --- a/modules/markup/renderer.go +++ b/modules/markup/renderer.go @@ -19,8 +19,18 @@ import ( "code.gitea.io/gitea/modules/setting" ) +type ProcessorHelper struct { + IsUsernameMentionable func(ctx context.Context, username string) bool +} + +var processorHelper ProcessorHelper + // Init initialize regexps for markdown parsing -func Init() { +func Init(ph *ProcessorHelper) { + if ph != nil { + processorHelper = *ph + } + NewSanitizer() if len(setting.Markdown.CustomURLSchemes) > 0 { CustomLinkURLSchemes(setting.Markdown.CustomURLSchemes) diff --git a/routers/api/v1/misc/markdown_test.go b/routers/api/v1/misc/markdown_test.go index 7809fa5cc72a..65ce06027802 100644 --- a/routers/api/v1/misc/markdown_test.go +++ b/routers/api/v1/misc/markdown_test.go @@ -5,6 +5,7 @@ package misc import ( + go_context "context" "io" "net/http" "net/http/httptest" @@ -13,6 +14,7 @@ import ( "testing" "code.gitea.io/gitea/modules/context" + "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/templates" @@ -50,6 +52,11 @@ func wrap(ctx *context.Context) *context.APIContext { func TestAPI_RenderGFM(t *testing.T) { setting.AppURL = AppURL + markup.Init(&markup.ProcessorHelper{ + IsUsernameMentionable: func(ctx go_context.Context, username string) bool { + return username == "r-lyeh" + }, + }) options := api.MarkdownOption{ Mode: "gfm", diff --git a/routers/init.go b/routers/init.go index 0f2e993413af..9045437f872b 100644 --- a/routers/init.go +++ b/routers/init.go @@ -41,6 +41,7 @@ import ( "code.gitea.io/gitea/services/automerge" "code.gitea.io/gitea/services/cron" "code.gitea.io/gitea/services/mailer" + markup_service "code.gitea.io/gitea/services/markup" repo_migrations "code.gitea.io/gitea/services/migrations" mirror_service "code.gitea.io/gitea/services/mirror" pull_service "code.gitea.io/gitea/services/pull" @@ -123,7 +124,7 @@ func GlobalInitInstalled(ctx context.Context) { highlight.NewContext() external.RegisterRenderers() - markup.Init() + markup.Init(markup_service.ProcessorHelper()) if setting.EnableSQLite3 { log.Info("SQLite3 support is enabled") diff --git a/services/markup/main_test.go b/services/markup/main_test.go new file mode 100644 index 000000000000..8efd08e69d64 --- /dev/null +++ b/services/markup/main_test.go @@ -0,0 +1,19 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package markup + +import ( + "path/filepath" + "testing" + + "code.gitea.io/gitea/models/unittest" +) + +func TestMain(m *testing.M) { + unittest.MainTest(m, &unittest.TestOptions{ + GiteaRootPath: filepath.Join("..", ".."), + FixtureFiles: []string{"user.yml"}, + }) +} diff --git a/services/markup/processorhelper.go b/services/markup/processorhelper.go new file mode 100644 index 000000000000..2b1cac2a5b88 --- /dev/null +++ b/services/markup/processorhelper.go @@ -0,0 +1,29 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package markup + +import ( + "context" + + "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/markup" +) + +func ProcessorHelper() *markup.ProcessorHelper { + return &markup.ProcessorHelper{ + IsUsernameMentionable: func(ctx context.Context, username string) bool { + // TODO: cast ctx to modules/context.Context and use IsUserVisibleToViewer + + // Only link if the user actually exists + userExists, err := user.IsUserExist(ctx, 0, username) + if err != nil { + log.Error("Failed to validate user in mention %q exists, assuming it does", username) + userExists = true + } + return userExists + }, + } +} diff --git a/services/markup/processorhelper_test.go b/services/markup/processorhelper_test.go new file mode 100644 index 000000000000..386465bc9189 --- /dev/null +++ b/services/markup/processorhelper_test.go @@ -0,0 +1,20 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package markup + +import ( + "context" + "testing" + + "code.gitea.io/gitea/models/unittest" + + "github.com/stretchr/testify/assert" +) + +func TestProcessorHelper(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + assert.True(t, ProcessorHelper().IsUsernameMentionable(context.Background(), "user10")) + assert.False(t, ProcessorHelper().IsUsernameMentionable(context.Background(), "no-such-user")) +} From 876ee8c3cd956025aadda14175f80ce4cccfe1bb Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Sun, 23 Oct 2022 03:18:15 +0200 Subject: [PATCH 10/20] Allow package version sorting (#21453) --- models/packages/container/search.go | 19 +++++++++++++++++-- models/packages/package_version.go | 23 +++++++++++++++++------ options/locale/locale_en-US.ini | 4 ++++ routers/web/user/package.go | 7 ++++++- templates/admin/packages/list.tmpl | 12 ++++++------ templates/package/shared/versionlist.tmpl | 10 ++++++++-- 6 files changed, 58 insertions(+), 17 deletions(-) diff --git a/models/packages/container/search.go b/models/packages/container/search.go index a3409fe74311..e4a5a538488f 100644 --- a/models/packages/container/search.go +++ b/models/packages/container/search.go @@ -165,6 +165,7 @@ type ImageTagsSearchOptions struct { PackageID int64 Query string IsTagged bool + Sort packages.VersionSort db.Paginator } @@ -195,12 +196,26 @@ func (opts *ImageTagsSearchOptions) toConds() builder.Cond { return cond } +func (opts *ImageTagsSearchOptions) configureOrderBy(e db.Engine) { + switch opts.Sort { + case packages.SortVersionDesc: + e.Desc("package_version.version") + case packages.SortVersionAsc: + e.Asc("package_version.version") + case packages.SortCreatedAsc: + e.Asc("package_version.created_unix") + default: + e.Desc("package_version.created_unix") + } +} + // SearchImageTags gets a sorted list of the tags of an image func SearchImageTags(ctx context.Context, opts *ImageTagsSearchOptions) ([]*packages.PackageVersion, int64, error) { sess := db.GetEngine(ctx). Join("INNER", "package", "package.id = package_version.package_id"). - Where(opts.toConds()). - Desc("package_version.created_unix") + Where(opts.toConds()) + + opts.configureOrderBy(sess) if opts.Paginator != nil { sess = db.SetSessionPagination(sess, opts) diff --git a/models/packages/package_version.go b/models/packages/package_version.go index 5479bae1c29f..f9965bcb7489 100644 --- a/models/packages/package_version.go +++ b/models/packages/package_version.go @@ -163,6 +163,17 @@ type SearchValue struct { ExactMatch bool } +type VersionSort = string + +const ( + SortNameAsc VersionSort = "name_asc" + SortNameDesc VersionSort = "name_desc" + SortVersionAsc VersionSort = "version_asc" + SortVersionDesc VersionSort = "version_desc" + SortCreatedAsc VersionSort = "created_asc" + SortCreatedDesc VersionSort = "created_desc" +) + // PackageSearchOptions are options for SearchXXX methods // Besides IsInternal are all fields optional and are not used if they have their default value (nil, "", 0) type PackageSearchOptions struct { @@ -176,7 +187,7 @@ type PackageSearchOptions struct { IsInternal util.OptionalBool HasFileWithName string // only results are found which are associated with a file with the specific name HasFiles util.OptionalBool // only results are found which have associated files - Sort string + Sort VersionSort db.Paginator } @@ -254,15 +265,15 @@ func (opts *PackageSearchOptions) toConds() builder.Cond { func (opts *PackageSearchOptions) configureOrderBy(e db.Engine) { switch opts.Sort { - case "alphabetically": + case SortNameAsc: e.Asc("package.name") - case "reversealphabetically": + case SortNameDesc: e.Desc("package.name") - case "highestversion": + case SortVersionDesc: e.Desc("package_version.version") - case "lowestversion": + case SortVersionAsc: e.Asc("package_version.version") - case "oldest": + case SortCreatedAsc: e.Asc("package_version.created_unix") default: e.Desc("package_version.created_unix") diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index a35c6a668af4..3c2e70187cd7 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -106,6 +106,10 @@ never = Never rss_feed = RSS Feed +[filter] +string.asc = A - Z +string.desc = Z - A + [error] occurred = An error occurred report_message = If you are sure this is a Gitea bug, please search for issues on GitHub or open a new issue if necessary. diff --git a/routers/web/user/package.go b/routers/web/user/package.go index c72592e7284d..7179e2df97a7 100644 --- a/routers/web/user/package.go +++ b/routers/web/user/package.go @@ -233,6 +233,7 @@ func ListPackageVersions(ctx *context.Context) { } query := ctx.FormTrim("q") + sort := ctx.FormTrim("sort") ctx.Data["Title"] = ctx.Tr("packages.title") ctx.Data["IsPackagesPage"] = true @@ -243,9 +244,11 @@ func ListPackageVersions(ctx *context.Context) { Owner: ctx.Package.Owner, } ctx.Data["Query"] = query + ctx.Data["Sort"] = sort pagerParams := map[string]string{ - "q": query, + "q": query, + "sort": sort, } var ( @@ -264,6 +267,7 @@ func ListPackageVersions(ctx *context.Context) { PackageID: p.ID, Query: query, IsTagged: tagged == "" || tagged == "tagged", + Sort: sort, }) if err != nil { ctx.ServerError("SearchImageTags", err) @@ -278,6 +282,7 @@ func ListPackageVersions(ctx *context.Context) { Value: query, }, IsInternal: util.OptionalBoolFalse, + Sort: sort, }) if err != nil { ctx.ServerError("SearchVersions", err) diff --git a/templates/admin/packages/list.tmpl b/templates/admin/packages/list.tmpl index b62e78879915..3aab2873c6b7 100644 --- a/templates/admin/packages/list.tmpl +++ b/templates/admin/packages/list.tmpl @@ -37,20 +37,20 @@ ID {{.locale.Tr "admin.packages.owner"}} {{.locale.Tr "admin.packages.type"}} - + {{.locale.Tr "admin.packages.name"}} - {{SortArrow "alphabetically" "reversealphabetically" .SortType false}} + {{SortArrow "name_asc" "name_desc" .SortType false}} - + {{.locale.Tr "admin.packages.version"}} - {{SortArrow "highestversion" "lowestversion" .SortType false}} + {{SortArrow "version_desc" "version_asc" .SortType false}} {{.locale.Tr "admin.packages.creator"}} {{.locale.Tr "admin.packages.repository"}} {{.locale.Tr "admin.packages.size"}} - + {{.locale.Tr "admin.packages.published"}} - {{SortArrow "oldest" "newest" .SortType true}} + {{SortArrow "created_asc" "created_desc" .SortType true}} {{.locale.Tr "admin.notices.op"}} diff --git a/templates/package/shared/versionlist.tmpl b/templates/package/shared/versionlist.tmpl index 02bda3fb8fed..952634cfbe9f 100644 --- a/templates/package/shared/versionlist.tmpl +++ b/templates/package/shared/versionlist.tmpl @@ -3,11 +3,17 @@
+ {{if eq .PackageDescriptor.Package.Type "container"}} {{end}} From f982a71997e057bce009574d04503177788e5a4e Mon Sep 17 00:00:00 2001 From: silverwind Date: Sun, 23 Oct 2022 06:05:20 +0200 Subject: [PATCH 11/20] CSS color enhancements (#21534) - Add [`accent-color`](https://developer.mozilla.org/en-US/docs/Web/CSS/accent-color) which will change the color of various native HTML elements from OS-color to specified one. Affects unstyled checkbox, radio, range and progress - Change `--color-accent` to `--color-primary-light-1` - Change progress bar color to `--color-accent` - Add new `--color-primary-contrast` meant to contrast over primary - Avoid layout shift on clicking `.viewed-file-form` - Add styles for `input[type=file]` upload button Screen Shot 2022-10-21 at 18 05 35 Screen Shot 2022-10-21 at 19 41 27 image Screen Shot 2022-10-21 at 18 21 24 Screen Shot 2022-10-21 at 18 56 59 Screen Shot 2022-10-21 at 18 57 09 Co-authored-by: delvh Co-authored-by: Lunny Xiao --- web_src/less/_base.less | 27 ++++++++++++++++++------ web_src/less/_review.less | 8 +++---- web_src/less/_tribute.less | 2 +- web_src/less/themes/theme-arc-green.less | 3 ++- 4 files changed, 28 insertions(+), 12 deletions(-) diff --git a/web_src/less/_base.less b/web_src/less/_base.less index b255f81d7a77..1f3230701273 100644 --- a/web_src/less/_base.less +++ b/web_src/less/_base.less @@ -12,6 +12,7 @@ --height-loading: 12rem; /* base colors */ --color-primary: #4183c4; + --color-primary-contrast: #ffffff; --color-primary-dark-1: #3876b3; --color-primary-dark-2: #31699f; --color-primary-dark-3: #2b5c8b; @@ -163,9 +164,11 @@ --color-tooltip-text: #ffffff; --color-header-bar: #ffffff; --color-label-active-bg: #d0d0d0; + --color-accent: var(--color-primary-light-1); --color-small-accent: var(--color-primary-light-6); - --color-accent: var(--color-primary-light-4); --color-active-line: #fffbdd; + + accent-color: var(--color-accent); } :root * { @@ -230,10 +233,10 @@ progress::-webkit-progress-bar { background: var(--color-secondary-dark-1); } progress::-webkit-progress-value { - background-color: var(--color-secondary-dark-4); + background-color: var(--color-accent); } progress::-moz-progress-bar { - background: var(--color-secondary-dark-4); + background-color: var(--color-accent); } * { @@ -260,6 +263,17 @@ progress::-moz-progress-bar { background: transparent; } +::file-selector-button { + border: 1px solid var(--color-light-border); + color: var(--color-text-light); + background: var(--color-light); + border-radius: var(--border-radius); +} +::file-selector-button:hover { + color: var(--color-text); + background: var(--color-hover); +} + ::selection { background: var(--color-primary-light-1) !important; color: var(--color-white) !important; @@ -1598,11 +1612,11 @@ footer { .activity-bar-graph { background-color: var(--color-primary); - color: #fff; + color: var(--color-primary-contrast); } .activity-bar-graph-alt { - color: #fff; + color: var(--color-primary-contrast); } .archived-icon { @@ -1899,6 +1913,7 @@ a.ui.label:hover { .ui.primary.button, .ui.primary.buttons .button { background-color: var(--color-primary) !important; + color: var(--color-primary-contrast) !important; } .ui.primary.button:hover, @@ -1914,7 +1929,7 @@ a.ui.label:hover { .ui.basic.primary.button, .ui.basic.primary.buttons .button { box-shadow: inset 0 0 0 1px var(--color-primary) !important; - color: #fff !important; + color: var(--color-primary-contrast) !important; } .ui.basic.primary.button:hover, diff --git a/web_src/less/_review.less b/web_src/less/_review.less index 33e4003b2c7d..013d6d5aa11f 100644 --- a/web_src/less/_review.less +++ b/web_src/less/_review.less @@ -194,7 +194,7 @@ a.blob-excerpt { a.blob-excerpt:hover { background: var(--color-primary); - color: #fff; + color: var(--color-primary-contrast); } // See the comment of createCommentEasyMDE() for the review editor @@ -244,7 +244,7 @@ a.blob-excerpt:hover { #review-box .review-comments-counter { background-color: var(--color-primary-light-4); - color: #fff; + color: var(--color-primary-contrast); } #review-box:hover .review-comments-counter { @@ -275,7 +275,7 @@ a.blob-excerpt:hover { .viewed-file-form { display: flex; align-items: center; - border: 1px none; + border: 1px solid transparent; padding: 4px 8px; margin: -8px 0; // just like other buttons in the diff box header border-radius: .285rem; // just like .ui.tiny.button @@ -288,7 +288,7 @@ a.blob-excerpt:hover { .viewed-file-checked-form { background-color: var(--color-small-accent); - border: 1px solid var(--color-accent); + border-color: var(--color-accent); } #viewed-files-summary { diff --git a/web_src/less/_tribute.less b/web_src/less/_tribute.less index 1626461c5756..9adf155ffaf4 100644 --- a/web_src/less/_tribute.less +++ b/web_src/less/_tribute.less @@ -23,7 +23,7 @@ .tribute-container li.highlight, .tribute-container li:hover { background: var(--color-primary) !important; - color: #ffffff !important; + color: var(--color-primary-contrast) !important; } .tribute-item { diff --git a/web_src/less/themes/theme-arc-green.less b/web_src/less/themes/theme-arc-green.less index fe83162154fc..1f6da76db9df 100644 --- a/web_src/less/themes/theme-arc-green.less +++ b/web_src/less/themes/theme-arc-green.less @@ -4,6 +4,7 @@ :root { --is-dark-theme: true; --color-primary: #87ab63; + --color-primary-contrast: #ffffff; --color-primary-dark-1: #93b373; --color-primary-dark-2: #9fbc82; --color-primary-dark-3: #abc492; @@ -135,8 +136,8 @@ --color-reaction-active-bg: var(--color-primary-alpha-40); --color-header-bar: #2e323e; --color-label-active-bg: #4c525e; + --color-accent: var(--color-primary-light-1); --color-small-accent: var(--color-primary-light-5); - --color-accent: var(--color-primary-light-3); --color-active-line: #534d1b; } From afebbf29a92b895cd41038a06a68e6f4013df357 Mon Sep 17 00:00:00 2001 From: M Hickford Date: Sun, 23 Oct 2022 07:28:46 +0200 Subject: [PATCH 12/20] Require authentication for OAuth token refresh (#21421) According to the OAuth spec https://datatracker.ietf.org/doc/html/rfc6749#section-6 when "Refreshing an Access Token" > The authorization server MUST ... require client authentication for confidential clients Fixes #21418 Co-authored-by: Gusted Co-authored-by: Lunny Xiao --- routers/web/auth/oauth.go | 29 +++++++++++++++++++ tests/integration/oauth_test.go | 51 +++++++++++++++++++++++++-------- 2 files changed, 68 insertions(+), 12 deletions(-) diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index e0e3c6e59f74..c98385c8f6cd 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -48,6 +48,7 @@ const ( // TODO move error and responses to SDK or models // AuthorizeErrorCode represents an error code specified in RFC 6749 +// https://datatracker.ietf.org/doc/html/rfc6749#section-4.2.2.1 type AuthorizeErrorCode string const ( @@ -68,6 +69,7 @@ const ( ) // AuthorizeError represents an error type specified in RFC 6749 +// https://datatracker.ietf.org/doc/html/rfc6749#section-4.2.2.1 type AuthorizeError struct { ErrorCode AuthorizeErrorCode `json:"error" form:"error"` ErrorDescription string @@ -80,6 +82,7 @@ func (err AuthorizeError) Error() string { } // AccessTokenErrorCode represents an error code specified in RFC 6749 +// https://datatracker.ietf.org/doc/html/rfc6749#section-5.2 type AccessTokenErrorCode string const ( @@ -98,6 +101,7 @@ const ( ) // AccessTokenError represents an error response specified in RFC 6749 +// https://datatracker.ietf.org/doc/html/rfc6749#section-5.2 type AccessTokenError struct { ErrorCode AccessTokenErrorCode `json:"error" form:"error"` ErrorDescription string `json:"error_description"` @@ -129,6 +133,7 @@ const ( ) // AccessTokenResponse represents a successful access token response +// https://datatracker.ietf.org/doc/html/rfc6749#section-4.2.2 type AccessTokenResponse struct { AccessToken string `json:"access_token"` TokenType TokenType `json:"token_type"` @@ -663,6 +668,30 @@ func AccessTokenOAuth(ctx *context.Context) { } func handleRefreshToken(ctx *context.Context, form forms.AccessTokenForm, serverKey, clientKey oauth2.JWTSigningKey) { + app, err := auth.GetOAuth2ApplicationByClientID(ctx, form.ClientID) + if err != nil { + handleAccessTokenError(ctx, AccessTokenError{ + ErrorCode: AccessTokenErrorCodeInvalidClient, + ErrorDescription: fmt.Sprintf("cannot load client with client id: %q", form.ClientID), + }) + return + } + // "The authorization server MUST ... require client authentication for confidential clients" + // https://datatracker.ietf.org/doc/html/rfc6749#section-6 + if !app.ValidateClientSecret([]byte(form.ClientSecret)) { + errorDescription := "invalid client secret" + if form.ClientSecret == "" { + errorDescription = "invalid empty client secret" + } + // "invalid_client ... Client authentication failed" + // https://datatracker.ietf.org/doc/html/rfc6749#section-5.2 + handleAccessTokenError(ctx, AccessTokenError{ + ErrorCode: AccessTokenErrorCodeInvalidClient, + ErrorDescription: errorDescription, + }) + return + } + token, err := oauth2.ParseToken(form.RefreshToken, serverKey) if err != nil { handleAccessTokenError(ctx, AccessTokenError{ diff --git a/tests/integration/oauth_test.go b/tests/integration/oauth_test.go index 9621acbbccad..acd32e3625d7 100644 --- a/tests/integration/oauth_test.go +++ b/tests/integration/oauth_test.go @@ -299,10 +299,11 @@ func TestAccessTokenExchangeWithBasicAuth(t *testing.T) { "client_secret": "inconsistent", }) req.Header.Add("Authorization", "Basic ZGE3ZGEzYmEtOWExMy00MTY3LTg1NmYtMzg5OWRlMGIwMTM4OjRNSzhOYTZSNTVzbWRDWTBXdUNDdW1aNmhqUlBuR1k1c2FXVlJISGpKaUE9") + resp = MakeRequest(t, req, http.StatusBadRequest) parsedError = new(auth.AccessTokenError) assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsedError)) assert.Equal(t, "invalid_request", string(parsedError.ErrorCode)) - assert.Equal(t, "client_id in request body inconsistent with Authorization header", parsedError.ErrorDescription) + assert.Equal(t, "client_secret in request body inconsistent with Authorization header", parsedError.ErrorDescription) } func TestRefreshTokenInvalidation(t *testing.T) { @@ -329,7 +330,33 @@ func TestRefreshTokenInvalidation(t *testing.T) { // test without invalidation setting.OAuth2.InvalidateRefreshTokens = false - refreshReq := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + req = NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "grant_type": "refresh_token", + "client_id": "da7da3ba-9a13-4167-856f-3899de0b0138", + // omit secret + "redirect_uri": "a", + "refresh_token": parsed.RefreshToken, + }) + resp = MakeRequest(t, req, http.StatusBadRequest) + parsedError := new(auth.AccessTokenError) + assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsedError)) + assert.Equal(t, "invalid_client", string(parsedError.ErrorCode)) + assert.Equal(t, "invalid empty client secret", parsedError.ErrorDescription) + + req = NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "grant_type": "refresh_token", + "client_id": "da7da3ba-9a13-4167-856f-3899de0b0138", + "client_secret": "4MK8Na6R55smdCY0WuCCumZ6hjRPnGY5saWVRHHjJiA=", + "redirect_uri": "a", + "refresh_token": "UNEXPECTED", + }) + resp = MakeRequest(t, req, http.StatusBadRequest) + parsedError = new(auth.AccessTokenError) + assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsedError)) + assert.Equal(t, "unauthorized_client", string(parsedError.ErrorCode)) + assert.Equal(t, "unable to parse refresh token", parsedError.ErrorDescription) + + req = NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ "grant_type": "refresh_token", "client_id": "da7da3ba-9a13-4167-856f-3899de0b0138", "client_secret": "4MK8Na6R55smdCY0WuCCumZ6hjRPnGY5saWVRHHjJiA=", @@ -337,24 +364,24 @@ func TestRefreshTokenInvalidation(t *testing.T) { "refresh_token": parsed.RefreshToken, }) - bs, err := io.ReadAll(refreshReq.Body) + bs, err := io.ReadAll(req.Body) assert.NoError(t, err) - refreshReq.Body = io.NopCloser(bytes.NewReader(bs)) - MakeRequest(t, refreshReq, http.StatusOK) + req.Body = io.NopCloser(bytes.NewReader(bs)) + MakeRequest(t, req, http.StatusOK) - refreshReq.Body = io.NopCloser(bytes.NewReader(bs)) - MakeRequest(t, refreshReq, http.StatusOK) + req.Body = io.NopCloser(bytes.NewReader(bs)) + MakeRequest(t, req, http.StatusOK) // test with invalidation setting.OAuth2.InvalidateRefreshTokens = true - refreshReq.Body = io.NopCloser(bytes.NewReader(bs)) - MakeRequest(t, refreshReq, http.StatusOK) + req.Body = io.NopCloser(bytes.NewReader(bs)) + MakeRequest(t, req, http.StatusOK) // repeat request should fail - refreshReq.Body = io.NopCloser(bytes.NewReader(bs)) - resp = MakeRequest(t, refreshReq, http.StatusBadRequest) - parsedError := new(auth.AccessTokenError) + req.Body = io.NopCloser(bytes.NewReader(bs)) + resp = MakeRequest(t, req, http.StatusBadRequest) + parsedError = new(auth.AccessTokenError) assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsedError)) assert.Equal(t, "unauthorized_client", string(parsedError.ErrorCode)) assert.Equal(t, "token was already used", parsedError.ErrorDescription) From 88a03a6133e90337ee17cb0277f306f54877f34b Mon Sep 17 00:00:00 2001 From: Yarden Shoham Date: Sun, 23 Oct 2022 12:13:52 +0300 Subject: [PATCH 13/20] Link mentioned user in markdown only if they are visible to viewer (#21554) We need to make sure a user can't confirm the existence of a user with private visibility * Follow up on #21533 ### Before #### User ![image](https://user-images.githubusercontent.com/20454870/197357580-340911d7-1659-4fc9-a9f6-7ed6bc3476b4.png) #### Admin ![image](https://user-images.githubusercontent.com/20454870/197357676-a8f0ae63-8f80-4221-a9b5-b6311552910a.png) ### After #### User ![image](https://user-images.githubusercontent.com/20454870/197357536-05616edb-7821-469d-8e51-6f8cb84c1362.png) #### Admin ![image](https://user-images.githubusercontent.com/20454870/197357703-071fe984-de79-43aa-a77c-a85b046292a4.png) Signed-off-by: Yarden Shoham Co-authored-by: wxiaoguang Co-authored-by: Lunny Xiao --- services/markup/processorhelper.go | 20 +++++++------ services/markup/processorhelper_test.go | 37 +++++++++++++++++++++++-- 2 files changed, 47 insertions(+), 10 deletions(-) diff --git a/services/markup/processorhelper.go b/services/markup/processorhelper.go index 2b1cac2a5b88..5042884e5ef9 100644 --- a/services/markup/processorhelper.go +++ b/services/markup/processorhelper.go @@ -8,22 +8,26 @@ import ( "context" "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/log" + gitea_context "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/markup" ) func ProcessorHelper() *markup.ProcessorHelper { return &markup.ProcessorHelper{ IsUsernameMentionable: func(ctx context.Context, username string) bool { - // TODO: cast ctx to modules/context.Context and use IsUserVisibleToViewer - - // Only link if the user actually exists - userExists, err := user.IsUserExist(ctx, 0, username) + mentionedUser, err := user.GetUserByName(ctx, username) if err != nil { - log.Error("Failed to validate user in mention %q exists, assuming it does", username) - userExists = true + return false + } + + giteaCtx, ok := ctx.(*gitea_context.Context) + if !ok { + // when using general context, use user's visibility to check + return mentionedUser.Visibility.IsPublic() } - return userExists + + // when using gitea context (web context), use user's visibility and user's permission to check + return user.IsUserVisibleToViewer(giteaCtx, mentionedUser, giteaCtx.Doer) }, } } diff --git a/services/markup/processorhelper_test.go b/services/markup/processorhelper_test.go index 386465bc9189..f7eab3d958b0 100644 --- a/services/markup/processorhelper_test.go +++ b/services/markup/processorhelper_test.go @@ -6,15 +6,48 @@ package markup import ( "context" + "net/http" "testing" + "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/models/user" + gitea_context "code.gitea.io/gitea/modules/context" "github.com/stretchr/testify/assert" ) func TestProcessorHelper(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - assert.True(t, ProcessorHelper().IsUsernameMentionable(context.Background(), "user10")) - assert.False(t, ProcessorHelper().IsUsernameMentionable(context.Background(), "no-such-user")) + + userPublic := "user1" + userPrivate := "user31" + userLimited := "user33" + userNoSuch := "no-such-user" + + unittest.AssertCount(t, &user.User{Name: userPublic}, 1) + unittest.AssertCount(t, &user.User{Name: userPrivate}, 1) + unittest.AssertCount(t, &user.User{Name: userLimited}, 1) + unittest.AssertCount(t, &user.User{Name: userNoSuch}, 0) + + // when using general context, use user's visibility to check + assert.True(t, ProcessorHelper().IsUsernameMentionable(context.Background(), userPublic)) + assert.False(t, ProcessorHelper().IsUsernameMentionable(context.Background(), userLimited)) + assert.False(t, ProcessorHelper().IsUsernameMentionable(context.Background(), userPrivate)) + assert.False(t, ProcessorHelper().IsUsernameMentionable(context.Background(), userNoSuch)) + + // when using web context, use user.IsUserVisibleToViewer to check + var err error + giteaCtx := &gitea_context.Context{} + giteaCtx.Req, err = http.NewRequest("GET", "/", nil) + assert.NoError(t, err) + + giteaCtx.Doer = nil + assert.True(t, ProcessorHelper().IsUsernameMentionable(giteaCtx, userPublic)) + assert.False(t, ProcessorHelper().IsUsernameMentionable(giteaCtx, userPrivate)) + + giteaCtx.Doer, err = user.GetUserByName(db.DefaultContext, userPrivate) + assert.NoError(t, err) + assert.True(t, ProcessorHelper().IsUsernameMentionable(giteaCtx, userPublic)) + assert.True(t, ProcessorHelper().IsUsernameMentionable(giteaCtx, userPrivate)) } From 4eeea7b30ee5d90ed4e9410ec5c7d0252ada3a3b Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 23 Oct 2022 18:50:48 +0800 Subject: [PATCH 14/20] Update binding to fix bugs (#21556) Fix #19698 --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 5bafaf414257..408249880c75 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( code.gitea.io/gitea-vet v0.2.2-0.20220122151748-48ebc902541b code.gitea.io/sdk/gitea v0.15.1 codeberg.org/gusted/mcaptcha v0.0.0-20220723083913-4f3072e1d570 - gitea.com/go-chi/binding v0.0.0-20220309004920-114340dabecb + gitea.com/go-chi/binding v0.0.0-20221013104517-b29891619681 gitea.com/go-chi/cache v0.2.0 gitea.com/go-chi/captcha v0.0.0-20211013065431-70641c1a35d5 gitea.com/go-chi/session v0.0.0-20211218221615-e3605d8b28b8 diff --git a/go.sum b/go.sum index b53732f6a5da..65841b8ec334 100644 --- a/go.sum +++ b/go.sum @@ -81,8 +81,8 @@ contrib.go.opencensus.io/resource v0.1.1/go.mod h1:F361eGI91LCmW1I/Saf+rX0+OFcig dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= git.sr.ht/~mariusor/go-xsd-duration v0.0.0-20220703122237-02e73435a078 h1:cliQ4HHsCo6xi2oWZYKWW4bly/Ory9FuTpFPRxj/mAg= git.sr.ht/~mariusor/go-xsd-duration v0.0.0-20220703122237-02e73435a078/go.mod h1:g/V2Hjas6Z1UHUp4yIx6bATpNzJ7DYtD0FG3+xARWxs= -gitea.com/go-chi/binding v0.0.0-20220309004920-114340dabecb h1:Yy0Bxzc8R2wxiwXoG/rECGplJUSpXqCsog9PuJFgiHs= -gitea.com/go-chi/binding v0.0.0-20220309004920-114340dabecb/go.mod h1:77TZu701zMXWJFvB8gvTbQ92zQ3DQq/H7l5wAEjQRKc= +gitea.com/go-chi/binding v0.0.0-20221013104517-b29891619681 h1:MMSPgnVULVwV9kEBgvyEUhC9v/uviZ55hPJEMjpbNR4= +gitea.com/go-chi/binding v0.0.0-20221013104517-b29891619681/go.mod h1:77TZu701zMXWJFvB8gvTbQ92zQ3DQq/H7l5wAEjQRKc= gitea.com/go-chi/cache v0.0.0-20210110083709-82c4c9ce2d5e/go.mod h1:k2V/gPDEtXGjjMGuBJiapffAXTv76H4snSmlJRLUhH0= gitea.com/go-chi/cache v0.2.0 h1:E0npuTfDW6CT1yD8NMDVc1SK6IeRjfmRL2zlEsCEd7w= gitea.com/go-chi/cache v0.2.0/go.mod h1:iQlVK2aKTZ/rE9UcHyz9pQWGvdP9i1eI2spOpzgCrtE= From dcd9fc7ee894700f702f3847d7d2a41d6a009b7e Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 23 Oct 2022 22:44:45 +0800 Subject: [PATCH 15/20] Refactor git command arguments and make all arguments to be safe to be used (#21535) Follow #21464 Make all git command arguments strictly safe. Most changes are one-to-one replacing, keep all existing logic. --- models/migrations/v128.go | 13 ++-- models/migrations/v134.go | 9 ++- modules/doctor/heads.go | 4 +- modules/doctor/mergebase.go | 14 ++-- modules/git/command.go | 63 +++++++++++---- modules/git/commit.go | 50 +++++------- modules/git/diff.go | 23 +++--- modules/git/git.go | 18 ++--- modules/git/log_name_status.go | 17 +++-- modules/git/pipeline/revlist.go | 2 +- modules/git/remote.go | 4 +- modules/git/repo.go | 28 +++---- modules/git/repo_archive.go | 15 ++-- modules/git/repo_attribute.go | 26 +++---- modules/git/repo_blame.go | 7 +- modules/git/repo_branch.go | 14 ++-- modules/git/repo_branch_nogogit.go | 14 ++-- modules/git/repo_commit.go | 76 +++++++++++-------- modules/git/repo_commit_gogit.go | 2 +- modules/git/repo_commit_nogogit.go | 8 +- modules/git/repo_compare.go | 30 ++++---- modules/git/repo_index.go | 13 +--- modules/git/repo_stats.go | 4 +- modules/git/repo_tag.go | 8 +- modules/git/repo_tag_nogogit.go | 2 +- modules/git/repo_tree.go | 6 +- modules/git/repo_tree_gogit.go | 2 +- modules/git/tree.go | 9 +-- modules/git/tree_nogogit.go | 8 +- modules/gitgraph/graph.go | 7 +- modules/indexer/code/bleve.go | 2 +- modules/indexer/code/elastic_search.go | 2 +- modules/indexer/code/git.go | 10 +-- modules/repository/generate.go | 2 +- modules/repository/init.go | 18 ++--- modules/repository/push.go | 2 +- routers/private/hook_pre_receive.go | 2 +- routers/private/hook_verification.go | 4 +- routers/web/repo/blame.go | 2 +- routers/web/repo/compare.go | 2 +- routers/web/repo/http.go | 6 +- routers/web/repo/lfs.go | 2 +- routers/web/repo/pull.go | 2 +- routers/web/repo/view.go | 2 +- services/agit/agit.go | 2 +- services/cron/tasks_basic.go | 8 +- services/cron/tasks_extended.go | 8 +- services/gitdiff/gitdiff.go | 28 +++---- services/gitdiff/gitdiff_test.go | 2 +- services/migrations/dump.go | 8 +- services/migrations/gitea_uploader.go | 8 +- services/migrations/gitea_uploader_test.go | 8 +- services/mirror/mirror_pull.go | 18 ++--- services/mirror/mirror_push.go | 8 +- services/pull/check.go | 4 +- services/pull/merge.go | 39 +++++----- services/pull/patch.go | 16 ++-- services/pull/pull.go | 2 +- services/pull/temp_repo.go | 10 +-- services/release/release.go | 2 +- services/repository/check.go | 6 +- services/repository/files/patch.go | 2 +- services/repository/files/temp_repo.go | 23 ++---- services/repository/files/update.go | 2 +- services/repository/files/upload.go | 2 +- services/repository/fork.go | 2 +- tests/integration/api_repo_git_tags_test.go | 4 +- tests/integration/git_clone_wiki_test.go | 2 +- .../git_helper_for_declarative_test.go | 24 +++--- tests/integration/git_test.go | 16 ++-- tests/integration/pull_merge_test.go | 6 +- 71 files changed, 424 insertions(+), 390 deletions(-) diff --git a/models/migrations/v128.go b/models/migrations/v128.go index 2aa68f9b6474..aba0ed689768 100644 --- a/models/migrations/v128.go +++ b/models/migrations/v128.go @@ -83,17 +83,17 @@ func fixMergeBase(x *xorm.Engine) error { if !pr.HasMerged { var err error - pr.MergeBase, _, err = git.NewCommand(git.DefaultContext, "merge-base", "--", pr.BaseBranch, gitRefName).RunStdString(&git.RunOpts{Dir: repoPath}) + pr.MergeBase, _, err = git.NewCommand(git.DefaultContext, "merge-base").AddDashesAndList(pr.BaseBranch, gitRefName).RunStdString(&git.RunOpts{Dir: repoPath}) if err != nil { var err2 error - pr.MergeBase, _, err2 = git.NewCommand(git.DefaultContext, "rev-parse", git.BranchPrefix+pr.BaseBranch).RunStdString(&git.RunOpts{Dir: repoPath}) + pr.MergeBase, _, err2 = git.NewCommand(git.DefaultContext, "rev-parse").AddDynamicArguments(git.BranchPrefix + pr.BaseBranch).RunStdString(&git.RunOpts{Dir: repoPath}) if err2 != nil { log.Error("Unable to get merge base for PR ID %d, Index %d in %s/%s. Error: %v & %v", pr.ID, pr.Index, baseRepo.OwnerName, baseRepo.Name, err, err2) continue } } } else { - parentsString, _, err := git.NewCommand(git.DefaultContext, "rev-list", "--parents", "-n", "1", pr.MergedCommitID).RunStdString(&git.RunOpts{Dir: repoPath}) + parentsString, _, err := git.NewCommand(git.DefaultContext, "rev-list", "--parents", "-n", "1").AddDynamicArguments(pr.MergedCommitID).RunStdString(&git.RunOpts{Dir: repoPath}) if err != nil { log.Error("Unable to get parents for merged PR ID %d, Index %d in %s/%s. Error: %v", pr.ID, pr.Index, baseRepo.OwnerName, baseRepo.Name, err) continue @@ -103,10 +103,11 @@ func fixMergeBase(x *xorm.Engine) error { continue } - args := append([]string{"merge-base", "--"}, parents[1:]...) - args = append(args, gitRefName) + refs := append([]string{}, parents[1:]...) + refs = append(refs, gitRefName) + cmd := git.NewCommand(git.DefaultContext, "merge-base").AddDashesAndList(refs...) - pr.MergeBase, _, err = git.NewCommand(git.DefaultContext, args...).RunStdString(&git.RunOpts{Dir: repoPath}) + pr.MergeBase, _, err = cmd.RunStdString(&git.RunOpts{Dir: repoPath}) if err != nil { log.Error("Unable to get merge base for merged PR ID %d, Index %d in %s/%s. Error: %v", pr.ID, pr.Index, baseRepo.OwnerName, baseRepo.Name, err) continue diff --git a/models/migrations/v134.go b/models/migrations/v134.go index 16e8ec8e9425..06d0f8276ac1 100644 --- a/models/migrations/v134.go +++ b/models/migrations/v134.go @@ -80,7 +80,7 @@ func refixMergeBase(x *xorm.Engine) error { gitRefName := fmt.Sprintf("refs/pull/%d/head", pr.Index) - parentsString, _, err := git.NewCommand(git.DefaultContext, "rev-list", "--parents", "-n", "1", pr.MergedCommitID).RunStdString(&git.RunOpts{Dir: repoPath}) + parentsString, _, err := git.NewCommand(git.DefaultContext, "rev-list", "--parents", "-n", "1").AddDynamicArguments(pr.MergedCommitID).RunStdString(&git.RunOpts{Dir: repoPath}) if err != nil { log.Error("Unable to get parents for merged PR ID %d, Index %d in %s/%s. Error: %v", pr.ID, pr.Index, baseRepo.OwnerName, baseRepo.Name, err) continue @@ -91,10 +91,11 @@ func refixMergeBase(x *xorm.Engine) error { } // we should recalculate - args := append([]string{"merge-base", "--"}, parents[1:]...) - args = append(args, gitRefName) + refs := append([]string{}, parents[1:]...) + refs = append(refs, gitRefName) + cmd := git.NewCommand(git.DefaultContext, "merge-base").AddDashesAndList(refs...) - pr.MergeBase, _, err = git.NewCommand(git.DefaultContext, args...).RunStdString(&git.RunOpts{Dir: repoPath}) + pr.MergeBase, _, err = cmd.RunStdString(&git.RunOpts{Dir: repoPath}) if err != nil { log.Error("Unable to get merge base for merged PR ID %d, Index %d in %s/%s. Error: %v", pr.ID, pr.Index, baseRepo.OwnerName, baseRepo.Name, err) continue diff --git a/modules/doctor/heads.go b/modules/doctor/heads.go index ec14aa4eadb1..7f3b2a8a0296 100644 --- a/modules/doctor/heads.go +++ b/modules/doctor/heads.go @@ -21,7 +21,7 @@ func synchronizeRepoHeads(ctx context.Context, logger log.Logger, autofix bool) numRepos++ runOpts := &git.RunOpts{Dir: repo.RepoPath()} - _, _, defaultBranchErr := git.NewCommand(ctx, "rev-parse", "--", repo.DefaultBranch).RunStdString(runOpts) + _, _, defaultBranchErr := git.NewCommand(ctx, "rev-parse").AddDashesAndList(repo.DefaultBranch).RunStdString(runOpts) head, _, headErr := git.NewCommand(ctx, "symbolic-ref", "--short", "HEAD").RunStdString(runOpts) @@ -49,7 +49,7 @@ func synchronizeRepoHeads(ctx context.Context, logger log.Logger, autofix bool) } // otherwise, let's try fixing HEAD - err := git.NewCommand(ctx, "symbolic-ref", "--", "HEAD", repo.DefaultBranch).Run(runOpts) + err := git.NewCommand(ctx, "symbolic-ref").AddDashesAndList("HEAD", repo.DefaultBranch).Run(runOpts) if err != nil { logger.Warn("Failed to fix HEAD for %s/%s: %v", repo.OwnerName, repo.Name, err) return nil diff --git a/modules/doctor/mergebase.go b/modules/doctor/mergebase.go index 46369290a13d..4a10c72e6e79 100644 --- a/modules/doctor/mergebase.go +++ b/modules/doctor/mergebase.go @@ -44,17 +44,17 @@ func checkPRMergeBase(ctx context.Context, logger log.Logger, autofix bool) erro if !pr.HasMerged { var err error - pr.MergeBase, _, err = git.NewCommand(ctx, "merge-base", "--", pr.BaseBranch, pr.GetGitRefName()).RunStdString(&git.RunOpts{Dir: repoPath}) + pr.MergeBase, _, err = git.NewCommand(ctx, "merge-base").AddDashesAndList(pr.BaseBranch, pr.GetGitRefName()).RunStdString(&git.RunOpts{Dir: repoPath}) if err != nil { var err2 error - pr.MergeBase, _, err2 = git.NewCommand(ctx, "rev-parse", git.BranchPrefix+pr.BaseBranch).RunStdString(&git.RunOpts{Dir: repoPath}) + pr.MergeBase, _, err2 = git.NewCommand(ctx, "rev-parse").AddDynamicArguments(git.BranchPrefix + pr.BaseBranch).RunStdString(&git.RunOpts{Dir: repoPath}) if err2 != nil { logger.Warn("Unable to get merge base for PR ID %d, #%d onto %s in %s/%s. Error: %v & %v", pr.ID, pr.Index, pr.BaseBranch, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, err, err2) return nil } } } else { - parentsString, _, err := git.NewCommand(ctx, "rev-list", "--parents", "-n", "1", pr.MergedCommitID).RunStdString(&git.RunOpts{Dir: repoPath}) + parentsString, _, err := git.NewCommand(ctx, "rev-list", "--parents", "-n", "1").AddDynamicArguments(pr.MergedCommitID).RunStdString(&git.RunOpts{Dir: repoPath}) if err != nil { logger.Warn("Unable to get parents for merged PR ID %d, #%d onto %s in %s/%s. Error: %v", pr.ID, pr.Index, pr.BaseBranch, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, err) return nil @@ -64,10 +64,10 @@ func checkPRMergeBase(ctx context.Context, logger log.Logger, autofix bool) erro return nil } - args := append([]string{"merge-base", "--"}, parents[1:]...) - args = append(args, pr.GetGitRefName()) - - pr.MergeBase, _, err = git.NewCommand(ctx, args...).RunStdString(&git.RunOpts{Dir: repoPath}) + refs := append([]string{}, parents[1:]...) + refs = append(refs, pr.GetGitRefName()) + cmd := git.NewCommand(ctx, "merge-base").AddDashesAndList(refs...) + pr.MergeBase, _, err = cmd.RunStdString(&git.RunOpts{Dir: repoPath}) if err != nil { logger.Warn("Unable to get merge base for merged PR ID %d, #%d onto %s in %s/%s. Error: %v", pr.ID, pr.Index, pr.BaseBranch, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, err) return nil diff --git a/modules/git/command.go b/modules/git/command.go index ed06dd9b08de..abf40b0cd76b 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -24,7 +24,7 @@ import ( var ( // globalCommandArgs global command args for external package setting - globalCommandArgs []string + globalCommandArgs []CmdArg // defaultCommandExecutionTimeout default command execution timeout duration defaultCommandExecutionTimeout = 360 * time.Second @@ -43,6 +43,8 @@ type Command struct { brokenArgs []string } +type CmdArg string + func (c *Command) String() string { if len(c.args) == 0 { return c.name @@ -52,13 +54,18 @@ func (c *Command) String() string { // NewCommand creates and returns a new Git Command based on given command and arguments. // Each argument should be safe to be trusted. User-provided arguments should be passed to AddDynamicArguments instead. -func NewCommand(ctx context.Context, args ...string) *Command { +func NewCommand(ctx context.Context, args ...CmdArg) *Command { // Make an explicit copy of globalCommandArgs, otherwise append might overwrite it - cargs := make([]string, len(globalCommandArgs)) - copy(cargs, globalCommandArgs) + cargs := make([]string, 0, len(globalCommandArgs)+len(args)) + for _, arg := range globalCommandArgs { + cargs = append(cargs, string(arg)) + } + for _, arg := range args { + cargs = append(cargs, string(arg)) + } return &Command{ name: GitExecutable, - args: append(cargs, args...), + args: cargs, parentContext: ctx, globalArgsLength: len(globalCommandArgs), } @@ -66,16 +73,20 @@ func NewCommand(ctx context.Context, args ...string) *Command { // NewCommandNoGlobals creates and returns a new Git Command based on given command and arguments only with the specify args and don't care global command args // Each argument should be safe to be trusted. User-provided arguments should be passed to AddDynamicArguments instead. -func NewCommandNoGlobals(args ...string) *Command { +func NewCommandNoGlobals(args ...CmdArg) *Command { return NewCommandContextNoGlobals(DefaultContext, args...) } // NewCommandContextNoGlobals creates and returns a new Git Command based on given command and arguments only with the specify args and don't care global command args // Each argument should be safe to be trusted. User-provided arguments should be passed to AddDynamicArguments instead. -func NewCommandContextNoGlobals(ctx context.Context, args ...string) *Command { +func NewCommandContextNoGlobals(ctx context.Context, args ...CmdArg) *Command { + cargs := make([]string, 0, len(args)) + for _, arg := range args { + cargs = append(cargs, string(arg)) + } return &Command{ name: GitExecutable, - args: args, + args: cargs, parentContext: ctx, } } @@ -93,10 +104,12 @@ func (c *Command) SetDescription(desc string) *Command { return c } -// AddArguments adds new argument(s) to the command. Each argument must be safe to be trusted. +// AddArguments adds new git argument(s) to the command. Each argument must be safe to be trusted. // User-provided arguments should be passed to AddDynamicArguments instead. -func (c *Command) AddArguments(args ...string) *Command { - c.args = append(c.args, args...) +func (c *Command) AddArguments(args ...CmdArg) *Command { + for _, arg := range args { + c.args = append(c.args, string(arg)) + } return c } @@ -115,6 +128,26 @@ func (c *Command) AddDynamicArguments(args ...string) *Command { return c } +// AddDashesAndList adds the "--" and then add the list as arguments, it's usually for adding file list +// At the moment, this function can be only called once, maybe in future it can be refactored to support multiple calls (if necessary) +func (c *Command) AddDashesAndList(list ...string) *Command { + c.args = append(c.args, "--") + // Some old code also checks `arg != ""`, IMO it's not necessary. + // If the check is needed, the list should be prepared before the call to this function + c.args = append(c.args, list...) + return c +} + +// CmdArgCheck checks whether the string is safe to be used as a dynamic argument. +// It panics if the check fails. Usually it should not be used, it's just for refactoring purpose +// deprecated +func CmdArgCheck(s string) CmdArg { + if s != "" && s[0] == '-' { + panic("invalid git cmd argument: " + s) + } + return CmdArg(s) +} + // RunOpts represents parameters to run the command. If UseContextTimeout is specified, then Timeout is ignored. type RunOpts struct { Env []string @@ -153,7 +186,7 @@ func CommonGitCmdEnvs() []string { }...) } -// CommonCmdServEnvs is like CommonGitCmdEnvs but it only returns minimal required environment variables for the "gitea serv" command +// CommonCmdServEnvs is like CommonGitCmdEnvs, but it only returns minimal required environment variables for the "gitea serv" command func CommonCmdServEnvs() []string { return commonBaseEnvs() } @@ -318,12 +351,12 @@ func (c *Command) RunStdBytes(opts *RunOpts) (stdout, stderr []byte, runErr RunS } // AllowLFSFiltersArgs return globalCommandArgs with lfs filter, it should only be used for tests -func AllowLFSFiltersArgs() []string { +func AllowLFSFiltersArgs() []CmdArg { // Now here we should explicitly allow lfs filters to run - filteredLFSGlobalArgs := make([]string, len(globalCommandArgs)) + filteredLFSGlobalArgs := make([]CmdArg, len(globalCommandArgs)) j := 0 for _, arg := range globalCommandArgs { - if strings.Contains(arg, "lfs") { + if strings.Contains(string(arg), "lfs") { j-- } else { filteredLFSGlobalArgs[j] = arg diff --git a/modules/git/commit.go b/modules/git/commit.go index 8fac9921b13d..061adc108235 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -92,13 +92,13 @@ func AddChanges(repoPath string, all bool, files ...string) error { } // AddChangesWithArgs marks local changes to be ready for commit. -func AddChangesWithArgs(repoPath string, globalArgs []string, all bool, files ...string) error { +func AddChangesWithArgs(repoPath string, globalArgs []CmdArg, all bool, files ...string) error { cmd := NewCommandNoGlobals(append(globalArgs, "add")...) if all { cmd.AddArguments("--all") } - cmd.AddArguments("--") - _, _, err := cmd.AddArguments(files...).RunStdString(&RunOpts{Dir: repoPath}) + cmd.AddDashesAndList(files...) + _, _, err := cmd.RunStdString(&RunOpts{Dir: repoPath}) return err } @@ -112,17 +112,17 @@ type CommitChangesOptions struct { // CommitChanges commits local changes with given committer, author and message. // If author is nil, it will be the same as committer. func CommitChanges(repoPath string, opts CommitChangesOptions) error { - cargs := make([]string, len(globalCommandArgs)) + cargs := make([]CmdArg, len(globalCommandArgs)) copy(cargs, globalCommandArgs) return CommitChangesWithArgs(repoPath, cargs, opts) } // CommitChangesWithArgs commits local changes with given committer, author and message. // If author is nil, it will be the same as committer. -func CommitChangesWithArgs(repoPath string, args []string, opts CommitChangesOptions) error { +func CommitChangesWithArgs(repoPath string, args []CmdArg, opts CommitChangesOptions) error { cmd := NewCommandNoGlobals(args...) if opts.Committer != nil { - cmd.AddArguments("-c", "user.name="+opts.Committer.Name, "-c", "user.email="+opts.Committer.Email) + cmd.AddArguments("-c", CmdArg("user.name="+opts.Committer.Name), "-c", CmdArg("user.email="+opts.Committer.Email)) } cmd.AddArguments("commit") @@ -130,9 +130,9 @@ func CommitChangesWithArgs(repoPath string, args []string, opts CommitChangesOpt opts.Author = opts.Committer } if opts.Author != nil { - cmd.AddArguments(fmt.Sprintf("--author='%s <%s>'", opts.Author.Name, opts.Author.Email)) + cmd.AddArguments(CmdArg(fmt.Sprintf("--author='%s <%s>'", opts.Author.Name, opts.Author.Email))) } - cmd.AddArguments("-m", opts.Message) + cmd.AddArguments("-m").AddDynamicArguments(opts.Message) _, _, err := cmd.RunStdString(&RunOpts{Dir: repoPath}) // No stderr but exit status 1 means nothing to commit. @@ -144,15 +144,13 @@ func CommitChangesWithArgs(repoPath string, args []string, opts CommitChangesOpt // AllCommitsCount returns count of all commits in repository func AllCommitsCount(ctx context.Context, repoPath string, hidePRRefs bool, files ...string) (int64, error) { - args := []string{"--all", "--count"} + cmd := NewCommand(ctx, "rev-list") if hidePRRefs { - args = append([]string{"--exclude=" + PullPrefix + "*"}, args...) + cmd.AddArguments("--exclude=" + PullPrefix + "*") } - cmd := NewCommand(ctx, "rev-list") - cmd.AddArguments(args...) + cmd.AddArguments("--all", "--count") if len(files) > 0 { - cmd.AddArguments("--") - cmd.AddArguments(files...) + cmd.AddDashesAndList(files...) } stdout, _, err := cmd.RunStdString(&RunOpts{Dir: repoPath}) @@ -168,8 +166,7 @@ func CommitsCountFiles(ctx context.Context, repoPath string, revision, relpath [ cmd := NewCommand(ctx, "rev-list", "--count") cmd.AddDynamicArguments(revision...) if len(relpath) > 0 { - cmd.AddArguments("--") - cmd.AddArguments(relpath...) + cmd.AddDashesAndList(relpath...) } stdout, _, err := cmd.RunStdString(&RunOpts{Dir: repoPath}) @@ -209,7 +206,7 @@ func (c *Commit) HasPreviousCommit(commitHash SHA1) (bool, error) { return false, nil } - _, _, err := NewCommand(c.repo.Ctx, "merge-base", "--is-ancestor", that, this).RunStdString(&RunOpts{Dir: c.repo.Path}) + _, _, err := NewCommand(c.repo.Ctx, "merge-base", "--is-ancestor").AddDynamicArguments(that, this).RunStdString(&RunOpts{Dir: c.repo.Path}) if err == nil { return true, nil } @@ -392,15 +389,12 @@ func (c *Commit) GetSubModule(entryname string) (*SubModule, error) { // GetBranchName gets the closest branch name (as returned by 'git name-rev --name-only') func (c *Commit) GetBranchName() (string, error) { - args := []string{ - "name-rev", - } + cmd := NewCommand(c.repo.Ctx, "name-rev") if CheckGitVersionAtLeast("2.13.0") == nil { - args = append(args, "--exclude", "refs/tags/*") + cmd.AddArguments("--exclude", "refs/tags/*") } - args = append(args, "--name-only", "--no-undefined", c.ID.String()) - - data, _, err := NewCommand(c.repo.Ctx, args...).RunStdString(&RunOpts{Dir: c.repo.Path}) + cmd.AddArguments("--name-only", "--no-undefined").AddDynamicArguments(c.ID.String()) + data, _, err := cmd.RunStdString(&RunOpts{Dir: c.repo.Path}) if err != nil { // handle special case where git can not describe commit if strings.Contains(err.Error(), "cannot describe") { @@ -426,7 +420,7 @@ func (c *Commit) LoadBranchName() (err error) { // GetTagName gets the current tag name for given commit func (c *Commit) GetTagName() (string, error) { - data, _, err := NewCommand(c.repo.Ctx, "describe", "--exact-match", "--tags", "--always", c.ID.String()).RunStdString(&RunOpts{Dir: c.repo.Path}) + data, _, err := NewCommand(c.repo.Ctx, "describe", "--exact-match", "--tags", "--always").AddDynamicArguments(c.ID.String()).RunStdString(&RunOpts{Dir: c.repo.Path}) if err != nil { // handle special case where there is no tag for this commit if strings.Contains(err.Error(), "no tag exactly matches") { @@ -503,9 +497,7 @@ func GetCommitFileStatus(ctx context.Context, repoPath, commitID string) (*Commi }() stderr := new(bytes.Buffer) - args := []string{"log", "--name-status", "-c", "--pretty=format:", "--parents", "--no-renames", "-z", "-1", commitID} - - err := NewCommand(ctx, args...).Run(&RunOpts{ + err := NewCommand(ctx, "log", "--name-status", "-c", "--pretty=format:", "--parents", "--no-renames", "-z", "-1").AddDynamicArguments(commitID).Run(&RunOpts{ Dir: repoPath, Stdout: w, Stderr: stderr, @@ -521,7 +513,7 @@ func GetCommitFileStatus(ctx context.Context, repoPath, commitID string) (*Commi // GetFullCommitID returns full length (40) of commit ID by given short SHA in a repository. func GetFullCommitID(ctx context.Context, repoPath, shortID string) (string, error) { - commitID, _, err := NewCommand(ctx, "rev-parse", shortID).RunStdString(&RunOpts{Dir: repoPath}) + commitID, _, err := NewCommand(ctx, "rev-parse").AddDynamicArguments(shortID).RunStdString(&RunOpts{Dir: repoPath}) if err != nil { if strings.Contains(err.Error(), "exit status 128") { return "", ErrNotExist{shortID, ""} diff --git a/modules/git/diff.go b/modules/git/diff.go index f75ebd4fd21d..13ff6bd1e6a5 100644 --- a/modules/git/diff.go +++ b/modules/git/diff.go @@ -35,7 +35,7 @@ func GetRawDiff(repo *Repository, commitID string, diffType RawDiffType, writer // GetReverseRawDiff dumps the reverse diff results of repository in given commit ID to io.Writer. func GetReverseRawDiff(ctx context.Context, repoPath, commitID string, writer io.Writer) error { stderr := new(bytes.Buffer) - cmd := NewCommand(ctx, "show", "--pretty=format:revert %H%n", "-R", commitID) + cmd := NewCommand(ctx, "show", "--pretty=format:revert %H%n", "-R").AddDynamicArguments(commitID) if err := cmd.Run(&RunOpts{ Dir: repoPath, Stdout: writer, @@ -52,39 +52,38 @@ func GetRepoRawDiffForFile(repo *Repository, startCommit, endCommit string, diff if err != nil { return err } - fileArgs := make([]string, 0) + var files []string if len(file) > 0 { - fileArgs = append(fileArgs, "--", file) + files = append(files, file) } - var args []string + cmd := NewCommand(repo.Ctx) switch diffType { case RawDiffNormal: if len(startCommit) != 0 { - args = append([]string{"diff", "-M", startCommit, endCommit}, fileArgs...) + cmd.AddArguments("diff", "-M").AddDynamicArguments(startCommit, endCommit).AddDashesAndList(files...) } else if commit.ParentCount() == 0 { - args = append([]string{"show", endCommit}, fileArgs...) + cmd.AddArguments("show").AddDynamicArguments(endCommit).AddDashesAndList(files...) } else { c, _ := commit.Parent(0) - args = append([]string{"diff", "-M", c.ID.String(), endCommit}, fileArgs...) + cmd.AddArguments("diff", "-M").AddDynamicArguments(c.ID.String(), endCommit).AddDashesAndList(files...) } case RawDiffPatch: if len(startCommit) != 0 { query := fmt.Sprintf("%s...%s", endCommit, startCommit) - args = append([]string{"format-patch", "--no-signature", "--stdout", "--root", query}, fileArgs...) + cmd.AddArguments("format-patch", "--no-signature", "--stdout", "--root").AddDynamicArguments(query).AddDashesAndList(files...) } else if commit.ParentCount() == 0 { - args = append([]string{"format-patch", "--no-signature", "--stdout", "--root", endCommit}, fileArgs...) + cmd.AddArguments("format-patch", "--no-signature", "--stdout", "--root").AddDynamicArguments(endCommit).AddDashesAndList(files...) } else { c, _ := commit.Parent(0) query := fmt.Sprintf("%s...%s", endCommit, c.ID.String()) - args = append([]string{"format-patch", "--no-signature", "--stdout", query}, fileArgs...) + cmd.AddArguments("format-patch", "--no-signature", "--stdout").AddDynamicArguments(query).AddDashesAndList(files...) } default: return fmt.Errorf("invalid diffType: %s", diffType) } stderr := new(bytes.Buffer) - cmd := NewCommand(repo.Ctx, args...) if err = cmd.Run(&RunOpts{ Dir: repo.Path, Stdout: writer, @@ -287,7 +286,7 @@ func GetAffectedFiles(repo *Repository, oldCommitID, newCommitID string, env []s affectedFiles := make([]string, 0, 32) // Run `git diff --name-only` to get the names of the changed files - err = NewCommand(repo.Ctx, "diff", "--name-only", oldCommitID, newCommitID). + err = NewCommand(repo.Ctx, "diff", "--name-only").AddDynamicArguments(oldCommitID, newCommitID). Run(&RunOpts{ Env: env, Dir: repo.Path, diff --git a/modules/git/git.go b/modules/git/git.go index 28899222e7ef..18d62838df16 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -313,7 +313,7 @@ func CheckGitVersionAtLeast(atLeast string) error { } func configSet(key, value string) error { - stdout, _, err := NewCommand(DefaultContext, "config", "--get", key).RunStdString(nil) + stdout, _, err := NewCommand(DefaultContext, "config", "--get").AddDynamicArguments(key).RunStdString(nil) if err != nil && !err.IsExitCode(1) { return fmt.Errorf("failed to get git config %s, err: %w", key, err) } @@ -323,7 +323,7 @@ func configSet(key, value string) error { return nil } - _, _, err = NewCommand(DefaultContext, "config", "--global", key, value).RunStdString(nil) + _, _, err = NewCommand(DefaultContext, "config", "--global").AddDynamicArguments(key, value).RunStdString(nil) if err != nil { return fmt.Errorf("failed to set git global config %s, err: %w", key, err) } @@ -332,14 +332,14 @@ func configSet(key, value string) error { } func configSetNonExist(key, value string) error { - _, _, err := NewCommand(DefaultContext, "config", "--get", key).RunStdString(nil) + _, _, err := NewCommand(DefaultContext, "config", "--get").AddDynamicArguments(key).RunStdString(nil) if err == nil { // already exist return nil } if err.IsExitCode(1) { // not exist, set new config - _, _, err = NewCommand(DefaultContext, "config", "--global", key, value).RunStdString(nil) + _, _, err = NewCommand(DefaultContext, "config", "--global").AddDynamicArguments(key, value).RunStdString(nil) if err != nil { return fmt.Errorf("failed to set git global config %s, err: %w", key, err) } @@ -350,14 +350,14 @@ func configSetNonExist(key, value string) error { } func configAddNonExist(key, value string) error { - _, _, err := NewCommand(DefaultContext, "config", "--get", key, regexp.QuoteMeta(value)).RunStdString(nil) + _, _, err := NewCommand(DefaultContext, "config", "--get").AddDynamicArguments(key, regexp.QuoteMeta(value)).RunStdString(nil) if err == nil { // already exist return nil } if err.IsExitCode(1) { // not exist, add new config - _, _, err = NewCommand(DefaultContext, "config", "--global", "--add", key, value).RunStdString(nil) + _, _, err = NewCommand(DefaultContext, "config", "--global", "--add").AddDynamicArguments(key, value).RunStdString(nil) if err != nil { return fmt.Errorf("failed to add git global config %s, err: %w", key, err) } @@ -367,10 +367,10 @@ func configAddNonExist(key, value string) error { } func configUnsetAll(key, value string) error { - _, _, err := NewCommand(DefaultContext, "config", "--get", key).RunStdString(nil) + _, _, err := NewCommand(DefaultContext, "config", "--get").AddDynamicArguments(key).RunStdString(nil) if err == nil { // exist, need to remove - _, _, err = NewCommand(DefaultContext, "config", "--global", "--unset-all", key, regexp.QuoteMeta(value)).RunStdString(nil) + _, _, err = NewCommand(DefaultContext, "config", "--global", "--unset-all").AddDynamicArguments(key, regexp.QuoteMeta(value)).RunStdString(nil) if err != nil { return fmt.Errorf("failed to unset git global config %s, err: %w", key, err) } @@ -384,6 +384,6 @@ func configUnsetAll(key, value string) error { } // Fsck verifies the connectivity and validity of the objects in the database -func Fsck(ctx context.Context, repoPath string, timeout time.Duration, args ...string) error { +func Fsck(ctx context.Context, repoPath string, timeout time.Duration, args ...CmdArg) error { return NewCommand(ctx, "fsck").AddArguments(args...).Run(&RunOpts{Timeout: timeout, Dir: repoPath}) } diff --git a/modules/git/log_name_status.go b/modules/git/log_name_status.go index 469932525fb5..dee4fc226ef2 100644 --- a/modules/git/log_name_status.go +++ b/modules/git/log_name_status.go @@ -35,30 +35,33 @@ func LogNameStatusRepo(ctx context.Context, repository, head, treepath string, p _ = stdoutWriter.Close() } - args := make([]string, 0, 8+len(paths)) - args = append(args, "log", "--name-status", "-c", "--format=commit%x00%H %P%x00", "--parents", "--no-renames", "-t", "-z", head, "--") + cmd := NewCommand(ctx) + cmd.AddArguments("log", "--name-status", "-c", "--format=commit%x00%H %P%x00", "--parents", "--no-renames", "-t", "-z").AddDynamicArguments(head) + + var files []string if len(paths) < 70 { if treepath != "" { - args = append(args, treepath) + files = append(files, treepath) for _, pth := range paths { if pth != "" { - args = append(args, path.Join(treepath, pth)) + files = append(files, path.Join(treepath, pth)) } } } else { for _, pth := range paths { if pth != "" { - args = append(args, pth) + files = append(files, pth) } } } } else if treepath != "" { - args = append(args, treepath) + files = append(files, treepath) } + cmd.AddDashesAndList(files...) go func() { stderr := strings.Builder{} - err := NewCommand(ctx, args...).Run(&RunOpts{ + err := cmd.Run(&RunOpts{ Dir: repository, Stdout: stdoutWriter, Stderr: &stderr, diff --git a/modules/git/pipeline/revlist.go b/modules/git/pipeline/revlist.go index 02619cb58304..5f62e85e7f78 100644 --- a/modules/git/pipeline/revlist.go +++ b/modules/git/pipeline/revlist.go @@ -43,7 +43,7 @@ func RevListObjects(ctx context.Context, revListWriter *io.PipeWriter, wg *sync. defer revListWriter.Close() stderr := new(bytes.Buffer) var errbuf strings.Builder - cmd := git.NewCommand(ctx, "rev-list", "--objects", headSHA, "--not", baseSHA) + cmd := git.NewCommand(ctx, "rev-list", "--objects").AddDynamicArguments(headSHA).AddArguments("--not").AddDynamicArguments(baseSHA) if err := cmd.Run(&git.RunOpts{ Dir: tmpBasePath, Stdout: revListWriter, diff --git a/modules/git/remote.go b/modules/git/remote.go index cbb4ac6126b8..c416eea13685 100644 --- a/modules/git/remote.go +++ b/modules/git/remote.go @@ -14,9 +14,9 @@ import ( func GetRemoteAddress(ctx context.Context, repoPath, remoteName string) (string, error) { var cmd *Command if CheckGitVersionAtLeast("2.7") == nil { - cmd = NewCommand(ctx, "remote", "get-url", remoteName) + cmd = NewCommand(ctx, "remote", "get-url").AddDynamicArguments(remoteName) } else { - cmd = NewCommand(ctx, "config", "--get", "remote."+remoteName+".url") + cmd = NewCommand(ctx, "config", "--get").AddDynamicArguments("remote." + remoteName + ".url") } result, _, err := cmd.RunStdString(&RunOpts{Dir: repoPath}) diff --git a/modules/git/repo.go b/modules/git/repo.go index 3176e276959a..575b686e2967 100644 --- a/modules/git/repo.go +++ b/modules/git/repo.go @@ -59,7 +59,7 @@ func (repo *Repository) parsePrettyFormatLogToList(logs []byte) ([]*Commit, erro // IsRepoURLAccessible checks if given repository URL is accessible. func IsRepoURLAccessible(ctx context.Context, url string) bool { - _, _, err := NewCommand(ctx, "ls-remote", "-q", "-h", url, "HEAD").RunStdString(nil) + _, _, err := NewCommand(ctx, "ls-remote", "-q", "-h").AddDynamicArguments(url, "HEAD").RunStdString(nil) return err == nil } @@ -112,13 +112,11 @@ type CloneRepoOptions struct { // Clone clones original repository to target path. func Clone(ctx context.Context, from, to string, opts CloneRepoOptions) error { - cargs := make([]string, len(globalCommandArgs)) - copy(cargs, globalCommandArgs) - return CloneWithArgs(ctx, from, to, cargs, opts) + return CloneWithArgs(ctx, globalCommandArgs, from, to, opts) } // CloneWithArgs original repository to target path. -func CloneWithArgs(ctx context.Context, from, to string, args []string, opts CloneRepoOptions) (err error) { +func CloneWithArgs(ctx context.Context, args []CmdArg, from, to string, opts CloneRepoOptions) (err error) { toDir := path.Dir(to) if err = os.MkdirAll(toDir, os.ModePerm); err != nil { return err @@ -144,15 +142,15 @@ func CloneWithArgs(ctx context.Context, from, to string, args []string, opts Clo cmd.AddArguments("--no-checkout") } if opts.Depth > 0 { - cmd.AddArguments("--depth", strconv.Itoa(opts.Depth)) + cmd.AddArguments("--depth").AddDynamicArguments(strconv.Itoa(opts.Depth)) } if opts.Filter != "" { - cmd.AddArguments("--filter", opts.Filter) + cmd.AddArguments("--filter").AddDynamicArguments(opts.Filter) } if len(opts.Branch) > 0 { - cmd.AddArguments("-b", opts.Branch) + cmd.AddArguments("-b").AddDynamicArguments(opts.Branch) } - cmd.AddArguments("--", from, to) + cmd.AddDashesAndList(from, to) if strings.Contains(from, "://") && strings.Contains(from, "@") { cmd.SetDescription(fmt.Sprintf("clone branch %s from %s to %s (shared: %t, mirror: %t, depth: %d)", opts.Branch, util.SanitizeCredentialURLs(from), to, opts.Shared, opts.Mirror, opts.Depth)) @@ -203,10 +201,12 @@ func Push(ctx context.Context, repoPath string, opts PushOptions) error { if opts.Mirror { cmd.AddArguments("--mirror") } - cmd.AddArguments("--", opts.Remote) + remoteBranchArgs := []string{opts.Remote} if len(opts.Branch) > 0 { - cmd.AddArguments(opts.Branch) + remoteBranchArgs = append(remoteBranchArgs, opts.Branch) } + cmd.AddDashesAndList(remoteBranchArgs...) + if strings.Contains(opts.Remote, "://") && strings.Contains(opts.Remote, "@") { cmd.SetDescription(fmt.Sprintf("push branch %s to %s (force: %t, mirror: %t)", opts.Branch, util.SanitizeCredentialURLs(opts.Remote), opts.Force, opts.Mirror)) } else { @@ -276,7 +276,7 @@ type DivergeObject struct { func checkDivergence(ctx context.Context, repoPath, baseBranch, targetBranch string) (int, error) { branches := fmt.Sprintf("%s..%s", baseBranch, targetBranch) - cmd := NewCommand(ctx, "rev-list", "--count", branches) + cmd := NewCommand(ctx, "rev-list", "--count").AddDynamicArguments(branches) stdout, _, err := cmd.RunStdString(&RunOpts{Dir: repoPath}) if err != nil { return -1, err @@ -319,7 +319,7 @@ func (repo *Repository) CreateBundle(ctx context.Context, commit string, out io. return err } - _, _, err = NewCommand(ctx, "reset", "--soft", commit).RunStdString(&RunOpts{Dir: tmp, Env: env}) + _, _, err = NewCommand(ctx, "reset", "--soft").AddDynamicArguments(commit).RunStdString(&RunOpts{Dir: tmp, Env: env}) if err != nil { return err } @@ -330,7 +330,7 @@ func (repo *Repository) CreateBundle(ctx context.Context, commit string, out io. } tmpFile := filepath.Join(tmp, "bundle") - _, _, err = NewCommand(ctx, "bundle", "create", tmpFile, "bundle", "HEAD").RunStdString(&RunOpts{Dir: tmp, Env: env}) + _, _, err = NewCommand(ctx, "bundle", "create").AddDynamicArguments(tmpFile, "bundle", "HEAD").RunStdString(&RunOpts{Dir: tmp, Env: env}) if err != nil { return err } diff --git a/modules/git/repo_archive.go b/modules/git/repo_archive.go index 4a97989949f3..a0cbfba5d965 100644 --- a/modules/git/repo_archive.go +++ b/modules/git/repo_archive.go @@ -44,20 +44,15 @@ func (repo *Repository) CreateArchive(ctx context.Context, format ArchiveType, t return fmt.Errorf("unknown format: %v", format) } - args := []string{ - "archive", - } + cmd := NewCommand(ctx, "archive") if usePrefix { - args = append(args, "--prefix="+filepath.Base(strings.TrimSuffix(repo.Path, ".git"))+"/") + cmd.AddArguments(CmdArg("--prefix=" + filepath.Base(strings.TrimSuffix(repo.Path, ".git")) + "/")) } - - args = append(args, - "--format="+format.String(), - commitID, - ) + cmd.AddArguments(CmdArg("--format=" + format.String())) + cmd.AddDynamicArguments(commitID) var stderr strings.Builder - err := NewCommand(ctx, args...).Run(&RunOpts{ + err := cmd.Run(&RunOpts{ Dir: repo.Path, Stdout: target, Stderr: &stderr, diff --git a/modules/git/repo_attribute.go b/modules/git/repo_attribute.go index 21ff93e498f1..60dc993dc207 100644 --- a/modules/git/repo_attribute.go +++ b/modules/git/repo_attribute.go @@ -20,7 +20,7 @@ import ( type CheckAttributeOpts struct { CachedOnly bool AllAttributes bool - Attributes []string + Attributes []CmdArg Filenames []string IndexFile string WorkTree string @@ -44,31 +44,23 @@ func (repo *Repository) CheckAttribute(opts CheckAttributeOpts) (map[string]map[ stdOut := new(bytes.Buffer) stdErr := new(bytes.Buffer) - cmdArgs := []string{"check-attr", "-z"} + cmd := NewCommand(repo.Ctx, "check-attr", "-z") if opts.AllAttributes { - cmdArgs = append(cmdArgs, "-a") + cmd.AddArguments("-a") } else { for _, attribute := range opts.Attributes { if attribute != "" { - cmdArgs = append(cmdArgs, attribute) + cmd.AddArguments(attribute) } } } if opts.CachedOnly { - cmdArgs = append(cmdArgs, "--cached") - } - - cmdArgs = append(cmdArgs, "--") - - for _, arg := range opts.Filenames { - if arg != "" { - cmdArgs = append(cmdArgs, arg) - } + cmd.AddArguments("--cached") } - cmd := NewCommand(repo.Ctx, cmdArgs...) + cmd.AddDashesAndList(opts.Filenames...) if err := cmd.Run(&RunOpts{ Env: env, @@ -106,7 +98,7 @@ func (repo *Repository) CheckAttribute(opts CheckAttributeOpts) (map[string]map[ // CheckAttributeReader provides a reader for check-attribute content that can be long running type CheckAttributeReader struct { // params - Attributes []string + Attributes []CmdArg Repo *Repository IndexFile string WorkTree string @@ -122,7 +114,7 @@ type CheckAttributeReader struct { // Init initializes the CheckAttributeReader func (c *CheckAttributeReader) Init(ctx context.Context) error { - cmdArgs := []string{"check-attr", "--stdin", "-z"} + cmdArgs := []CmdArg{"check-attr", "--stdin", "-z"} if len(c.IndexFile) > 0 { cmdArgs = append(cmdArgs, "--cached") @@ -401,7 +393,7 @@ func (repo *Repository) CheckAttributeReader(commitID string) (*CheckAttributeRe } checker := &CheckAttributeReader{ - Attributes: []string{"linguist-vendored", "linguist-generated", "linguist-language", "gitlab-language"}, + Attributes: []CmdArg{"linguist-vendored", "linguist-generated", "linguist-language", "gitlab-language"}, Repo: repo, IndexFile: indexFilename, WorkTree: worktree, diff --git a/modules/git/repo_blame.go b/modules/git/repo_blame.go index 6fe6d235ba92..8a3707aa0918 100644 --- a/modules/git/repo_blame.go +++ b/modules/git/repo_blame.go @@ -8,13 +8,16 @@ import "fmt" // FileBlame return the Blame object of file func (repo *Repository) FileBlame(revision, path, file string) ([]byte, error) { - stdout, _, err := NewCommand(repo.Ctx, "blame", "--root", "--", file).RunStdBytes(&RunOpts{Dir: path}) + stdout, _, err := NewCommand(repo.Ctx, "blame", "--root").AddDashesAndList(file).RunStdBytes(&RunOpts{Dir: path}) return stdout, err } // LineBlame returns the latest commit at the given line func (repo *Repository) LineBlame(revision, path, file string, line uint) (*Commit, error) { - res, _, err := NewCommand(repo.Ctx, "blame", fmt.Sprintf("-L %d,%d", line, line), "-p", revision, "--", file).RunStdString(&RunOpts{Dir: path}) + res, _, err := NewCommand(repo.Ctx, "blame"). + AddArguments(CmdArg(fmt.Sprintf("-L %d,%d", line, line))). + AddArguments("-p").AddDynamicArguments(revision). + AddDashesAndList(file).RunStdString(&RunOpts{Dir: path}) if err != nil { return nil, err } diff --git a/modules/git/repo_branch.go b/modules/git/repo_branch.go index 17d243808e9c..a3fc7e0c4216 100644 --- a/modules/git/repo_branch.go +++ b/modules/git/repo_branch.go @@ -25,7 +25,7 @@ const PullRequestPrefix = "refs/for/" // IsReferenceExist returns true if given reference exists in the repository. func IsReferenceExist(ctx context.Context, repoPath, name string) bool { - _, _, err := NewCommand(ctx, "show-ref", "--verify", "--", name).RunStdString(&RunOpts{Dir: repoPath}) + _, _, err := NewCommand(ctx, "show-ref", "--verify").AddDashesAndList(name).RunStdString(&RunOpts{Dir: repoPath}) return err == nil } @@ -66,7 +66,7 @@ func (repo *Repository) GetHEADBranch() (*Branch, error) { // SetDefaultBranch sets default branch of repository. func (repo *Repository) SetDefaultBranch(name string) error { - _, _, err := NewCommand(repo.Ctx, "symbolic-ref", "HEAD", BranchPrefix+name).RunStdString(&RunOpts{Dir: repo.Path}) + _, _, err := NewCommand(repo.Ctx, "symbolic-ref", "HEAD").AddDynamicArguments(BranchPrefix + name).RunStdString(&RunOpts{Dir: repo.Path}) return err } @@ -141,7 +141,7 @@ func (repo *Repository) DeleteBranch(name string, opts DeleteBranchOptions) erro cmd.AddArguments("-d") } - cmd.AddArguments("--", name) + cmd.AddDashesAndList(name) _, _, err := cmd.RunStdString(&RunOpts{Dir: repo.Path}) return err @@ -150,7 +150,7 @@ func (repo *Repository) DeleteBranch(name string, opts DeleteBranchOptions) erro // CreateBranch create a new branch func (repo *Repository) CreateBranch(branch, oldbranchOrCommit string) error { cmd := NewCommand(repo.Ctx, "branch") - cmd.AddArguments("--", branch, oldbranchOrCommit) + cmd.AddDashesAndList(branch, oldbranchOrCommit) _, _, err := cmd.RunStdString(&RunOpts{Dir: repo.Path}) @@ -163,7 +163,7 @@ func (repo *Repository) AddRemote(name, url string, fetch bool) error { if fetch { cmd.AddArguments("-f") } - cmd.AddArguments(name, url) + cmd.AddDynamicArguments(name, url) _, _, err := cmd.RunStdString(&RunOpts{Dir: repo.Path}) return err @@ -171,7 +171,7 @@ func (repo *Repository) AddRemote(name, url string, fetch bool) error { // RemoveRemote removes a remote from repository. func (repo *Repository) RemoveRemote(name string) error { - _, _, err := NewCommand(repo.Ctx, "remote", "rm", name).RunStdString(&RunOpts{Dir: repo.Path}) + _, _, err := NewCommand(repo.Ctx, "remote", "rm").AddDynamicArguments(name).RunStdString(&RunOpts{Dir: repo.Path}) return err } @@ -182,6 +182,6 @@ func (branch *Branch) GetCommit() (*Commit, error) { // RenameBranch rename a branch func (repo *Repository) RenameBranch(from, to string) error { - _, _, err := NewCommand(repo.Ctx, "branch", "-m", from, to).RunStdString(&RunOpts{Dir: repo.Path}) + _, _, err := NewCommand(repo.Ctx, "branch", "-m").AddDynamicArguments(from, to).RunStdString(&RunOpts{Dir: repo.Path}) return err } diff --git a/modules/git/repo_branch_nogogit.go b/modules/git/repo_branch_nogogit.go index 91112d0190d9..95c371884199 100644 --- a/modules/git/repo_branch_nogogit.go +++ b/modules/git/repo_branch_nogogit.go @@ -63,7 +63,7 @@ func (repo *Repository) IsBranchExist(name string) bool { // GetBranchNames returns branches from the repository, skipping skip initial branches and // returning at most limit branches, or all branches if limit is 0. func (repo *Repository) GetBranchNames(skip, limit int) ([]string, int, error) { - return callShowRef(repo.Ctx, repo.Path, BranchPrefix, []string{BranchPrefix, "--sort=-committerdate"}, skip, limit) + return callShowRef(repo.Ctx, repo.Path, BranchPrefix, []CmdArg{BranchPrefix, "--sort=-committerdate"}, skip, limit) } // WalkReferences walks all the references from the repository @@ -74,19 +74,19 @@ func WalkReferences(ctx context.Context, repoPath string, walkfn func(sha1, refn // WalkReferences walks all the references from the repository // refType should be empty, ObjectTag or ObjectBranch. All other values are equivalent to empty. func (repo *Repository) WalkReferences(refType ObjectType, skip, limit int, walkfn func(sha1, refname string) error) (int, error) { - var args []string + var args []CmdArg switch refType { case ObjectTag: - args = []string{TagPrefix, "--sort=-taggerdate"} + args = []CmdArg{TagPrefix, "--sort=-taggerdate"} case ObjectBranch: - args = []string{BranchPrefix, "--sort=-committerdate"} + args = []CmdArg{BranchPrefix, "--sort=-committerdate"} } return walkShowRef(repo.Ctx, repo.Path, args, skip, limit, walkfn) } // callShowRef return refs, if limit = 0 it will not limit -func callShowRef(ctx context.Context, repoPath, trimPrefix string, extraArgs []string, skip, limit int) (branchNames []string, countAll int, err error) { +func callShowRef(ctx context.Context, repoPath, trimPrefix string, extraArgs []CmdArg, skip, limit int) (branchNames []string, countAll int, err error) { countAll, err = walkShowRef(ctx, repoPath, extraArgs, skip, limit, func(_, branchName string) error { branchName = strings.TrimPrefix(branchName, trimPrefix) branchNames = append(branchNames, branchName) @@ -96,7 +96,7 @@ func callShowRef(ctx context.Context, repoPath, trimPrefix string, extraArgs []s return branchNames, countAll, err } -func walkShowRef(ctx context.Context, repoPath string, extraArgs []string, skip, limit int, walkfn func(sha1, refname string) error) (countAll int, err error) { +func walkShowRef(ctx context.Context, repoPath string, extraArgs []CmdArg, skip, limit int, walkfn func(sha1, refname string) error) (countAll int, err error) { stdoutReader, stdoutWriter := io.Pipe() defer func() { _ = stdoutReader.Close() @@ -105,7 +105,7 @@ func walkShowRef(ctx context.Context, repoPath string, extraArgs []string, skip, go func() { stderrBuilder := &strings.Builder{} - args := []string{"for-each-ref", "--format=%(objectname) %(refname)"} + args := []CmdArg{"for-each-ref", "--format=%(objectname) %(refname)"} args = append(args, extraArgs...) err := NewCommand(ctx, args...).Run(&RunOpts{ Dir: repoPath, diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index ec72593b80a4..90259fd74664 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -61,7 +61,7 @@ func (repo *Repository) getCommitByPathWithID(id SHA1, relpath string) (*Commit, relpath = `\` + relpath } - stdout, _, runErr := NewCommand(repo.Ctx, "log", "-1", prettyLogFormat, id.String(), "--", relpath).RunStdString(&RunOpts{Dir: repo.Path}) + stdout, _, runErr := NewCommand(repo.Ctx, "log", "-1", prettyLogFormat).AddDynamicArguments(id.String()).AddDashesAndList(relpath).RunStdString(&RunOpts{Dir: repo.Path}) if runErr != nil { return nil, runErr } @@ -76,7 +76,7 @@ func (repo *Repository) getCommitByPathWithID(id SHA1, relpath string) (*Commit, // GetCommitByPath returns the last commit of relative path. func (repo *Repository) GetCommitByPath(relpath string) (*Commit, error) { - stdout, _, runErr := NewCommand(repo.Ctx, "log", "-1", prettyLogFormat, "--", relpath).RunStdBytes(&RunOpts{Dir: repo.Path}) + stdout, _, runErr := NewCommand(repo.Ctx, "log", "-1", prettyLogFormat).AddDashesAndList(relpath).RunStdBytes(&RunOpts{Dir: repo.Path}) if runErr != nil { return nil, runErr } @@ -89,8 +89,10 @@ func (repo *Repository) GetCommitByPath(relpath string) (*Commit, error) { } func (repo *Repository) commitsByRange(id SHA1, page, pageSize int) ([]*Commit, error) { - stdout, _, err := NewCommand(repo.Ctx, "log", id.String(), "--skip="+strconv.Itoa((page-1)*pageSize), - "--max-count="+strconv.Itoa(pageSize), prettyLogFormat).RunStdBytes(&RunOpts{Dir: repo.Path}) + stdout, _, err := NewCommand(repo.Ctx, "log"). + AddArguments(CmdArg("--skip="+strconv.Itoa((page-1)*pageSize)), CmdArg("--max-count="+strconv.Itoa(pageSize)), prettyLogFormat). + AddDynamicArguments(id.String()). + RunStdBytes(&RunOpts{Dir: repo.Path}) if err != nil { return nil, err } @@ -99,30 +101,30 @@ func (repo *Repository) commitsByRange(id SHA1, page, pageSize int) ([]*Commit, func (repo *Repository) searchCommits(id SHA1, opts SearchCommitsOptions) ([]*Commit, error) { // create new git log command with limit of 100 commis - cmd := NewCommand(repo.Ctx, "log", id.String(), "-100", prettyLogFormat) + cmd := NewCommand(repo.Ctx, "log", "-100", prettyLogFormat).AddDynamicArguments(id.String()) // ignore case - args := []string{"-i"} + args := []CmdArg{"-i"} // add authors if present in search query if len(opts.Authors) > 0 { for _, v := range opts.Authors { - args = append(args, "--author="+v) + args = append(args, CmdArg("--author="+v)) } } // add committers if present in search query if len(opts.Committers) > 0 { for _, v := range opts.Committers { - args = append(args, "--committer="+v) + args = append(args, CmdArg("--committer="+v)) } } // add time constraints if present in search query if len(opts.After) > 0 { - args = append(args, "--after="+opts.After) + args = append(args, CmdArg("--after="+opts.After)) } if len(opts.Before) > 0 { - args = append(args, "--before="+opts.Before) + args = append(args, CmdArg("--before="+opts.Before)) } // pretend that all refs along with HEAD were listed on command line as @@ -136,7 +138,7 @@ func (repo *Repository) searchCommits(id SHA1, opts SearchCommitsOptions) ([]*Co // note this is done only for command created above if len(opts.Keywords) > 0 { for _, v := range opts.Keywords { - cmd.AddArguments("--grep=" + v) + cmd.AddArguments(CmdArg("--grep=" + v)) } } @@ -178,7 +180,7 @@ func (repo *Repository) searchCommits(id SHA1, opts SearchCommitsOptions) ([]*Co } func (repo *Repository) getFilesChanged(id1, id2 string) ([]string, error) { - stdout, _, err := NewCommand(repo.Ctx, "diff", "--name-only", id1, id2).RunStdBytes(&RunOpts{Dir: repo.Path}) + stdout, _, err := NewCommand(repo.Ctx, "diff", "--name-only").AddDynamicArguments(id1, id2).RunStdBytes(&RunOpts{Dir: repo.Path}) if err != nil { return nil, err } @@ -188,7 +190,7 @@ func (repo *Repository) getFilesChanged(id1, id2 string) ([]string, error) { // FileChangedBetweenCommits Returns true if the file changed between commit IDs id1 and id2 // You must ensure that id1 and id2 are valid commit ids. func (repo *Repository) FileChangedBetweenCommits(filename, id1, id2 string) (bool, error) { - stdout, _, err := NewCommand(repo.Ctx, "diff", "--name-only", "-z", id1, id2, "--", filename).RunStdBytes(&RunOpts{Dir: repo.Path}) + stdout, _, err := NewCommand(repo.Ctx, "diff", "--name-only", "-z").AddDynamicArguments(id1, id2).AddDashesAndList(filename).RunStdBytes(&RunOpts{Dir: repo.Path}) if err != nil { return false, err } @@ -211,12 +213,11 @@ func (repo *Repository) CommitsByFileAndRange(revision, file string, page int) ( }() go func() { stderr := strings.Builder{} - gitCmd := NewCommand(repo.Ctx, "rev-list", - "--max-count="+strconv.Itoa(setting.Git.CommitsRangeSize*page), - "--skip="+strconv.Itoa(skip), - ) + gitCmd := NewCommand(repo.Ctx, "rev-list"). + AddArguments(CmdArg("--max-count=" + strconv.Itoa(setting.Git.CommitsRangeSize*page))). + AddArguments(CmdArg("--skip=" + strconv.Itoa(skip))) gitCmd.AddDynamicArguments(revision) - gitCmd.AddArguments("--", file) + gitCmd.AddDashesAndList(file) err := gitCmd.Run(&RunOpts{ Dir: repo.Path, Stdout: stdoutWriter, @@ -257,11 +258,11 @@ func (repo *Repository) CommitsByFileAndRange(revision, file string, page int) ( // FilesCountBetween return the number of files changed between two commits func (repo *Repository) FilesCountBetween(startCommitID, endCommitID string) (int, error) { - stdout, _, err := NewCommand(repo.Ctx, "diff", "--name-only", startCommitID+"..."+endCommitID).RunStdString(&RunOpts{Dir: repo.Path}) + stdout, _, err := NewCommand(repo.Ctx, "diff", "--name-only").AddDynamicArguments(startCommitID + "..." + endCommitID).RunStdString(&RunOpts{Dir: repo.Path}) if err != nil && strings.Contains(err.Error(), "no merge base") { // git >= 2.28 now returns an error if startCommitID and endCommitID have become unrelated. // previously it would return the results of git diff --name-only startCommitID endCommitID so let's try that... - stdout, _, err = NewCommand(repo.Ctx, "diff", "--name-only", startCommitID, endCommitID).RunStdString(&RunOpts{Dir: repo.Path}) + stdout, _, err = NewCommand(repo.Ctx, "diff", "--name-only").AddDynamicArguments(startCommitID, endCommitID).RunStdString(&RunOpts{Dir: repo.Path}) } if err != nil { return 0, err @@ -275,13 +276,13 @@ func (repo *Repository) CommitsBetween(last, before *Commit) ([]*Commit, error) var stdout []byte var err error if before == nil { - stdout, _, err = NewCommand(repo.Ctx, "rev-list", last.ID.String()).RunStdBytes(&RunOpts{Dir: repo.Path}) + stdout, _, err = NewCommand(repo.Ctx, "rev-list").AddDynamicArguments(last.ID.String()).RunStdBytes(&RunOpts{Dir: repo.Path}) } else { - stdout, _, err = NewCommand(repo.Ctx, "rev-list", before.ID.String()+".."+last.ID.String()).RunStdBytes(&RunOpts{Dir: repo.Path}) + stdout, _, err = NewCommand(repo.Ctx, "rev-list").AddDynamicArguments(before.ID.String() + ".." + last.ID.String()).RunStdBytes(&RunOpts{Dir: repo.Path}) if err != nil && strings.Contains(err.Error(), "no merge base") { // future versions of git >= 2.28 are likely to return an error if before and last have become unrelated. // previously it would return the results of git rev-list before last so let's try that... - stdout, _, err = NewCommand(repo.Ctx, "rev-list", before.ID.String(), last.ID.String()).RunStdBytes(&RunOpts{Dir: repo.Path}) + stdout, _, err = NewCommand(repo.Ctx, "rev-list").AddDynamicArguments(before.ID.String(), last.ID.String()).RunStdBytes(&RunOpts{Dir: repo.Path}) } } if err != nil { @@ -295,13 +296,22 @@ func (repo *Repository) CommitsBetweenLimit(last, before *Commit, limit, skip in var stdout []byte var err error if before == nil { - stdout, _, err = NewCommand(repo.Ctx, "rev-list", "--max-count", strconv.Itoa(limit), "--skip", strconv.Itoa(skip), last.ID.String()).RunStdBytes(&RunOpts{Dir: repo.Path}) + stdout, _, err = NewCommand(repo.Ctx, "rev-list", + "--max-count", CmdArg(strconv.Itoa(limit)), + "--skip", CmdArg(strconv.Itoa(skip))). + AddDynamicArguments(last.ID.String()).RunStdBytes(&RunOpts{Dir: repo.Path}) } else { - stdout, _, err = NewCommand(repo.Ctx, "rev-list", "--max-count", strconv.Itoa(limit), "--skip", strconv.Itoa(skip), before.ID.String()+".."+last.ID.String()).RunStdBytes(&RunOpts{Dir: repo.Path}) + stdout, _, err = NewCommand(repo.Ctx, "rev-list", + "--max-count", CmdArg(strconv.Itoa(limit)), + "--skip", CmdArg(strconv.Itoa(skip))). + AddDynamicArguments(before.ID.String() + ".." + last.ID.String()).RunStdBytes(&RunOpts{Dir: repo.Path}) if err != nil && strings.Contains(err.Error(), "no merge base") { // future versions of git >= 2.28 are likely to return an error if before and last have become unrelated. // previously it would return the results of git rev-list --max-count n before last so let's try that... - stdout, _, err = NewCommand(repo.Ctx, "rev-list", "--max-count", strconv.Itoa(limit), "--skip", strconv.Itoa(skip), before.ID.String(), last.ID.String()).RunStdBytes(&RunOpts{Dir: repo.Path}) + stdout, _, err = NewCommand(repo.Ctx, "rev-list", + "--max-count", CmdArg(strconv.Itoa(limit)), + "--skip", CmdArg(strconv.Itoa(skip))). + AddDynamicArguments(before.ID.String(), last.ID.String()).RunStdBytes(&RunOpts{Dir: repo.Path}) } } if err != nil { @@ -342,9 +352,9 @@ func (repo *Repository) CommitsCountBetween(start, end string) (int64, error) { func (repo *Repository) commitsBefore(id SHA1, limit int) ([]*Commit, error) { cmd := NewCommand(repo.Ctx, "log") if limit > 0 { - cmd.AddArguments("-"+strconv.Itoa(limit), prettyLogFormat, id.String()) + cmd.AddArguments(CmdArg("-"+strconv.Itoa(limit)), prettyLogFormat).AddDynamicArguments(id.String()) } else { - cmd.AddArguments(prettyLogFormat, id.String()) + cmd.AddArguments(prettyLogFormat).AddDynamicArguments(id.String()) } stdout, _, runErr := cmd.RunStdBytes(&RunOpts{Dir: repo.Path}) @@ -384,7 +394,11 @@ func (repo *Repository) getCommitsBeforeLimit(id SHA1, num int) ([]*Commit, erro func (repo *Repository) getBranches(commit *Commit, limit int) ([]string, error) { if CheckGitVersionAtLeast("2.7.0") == nil { - stdout, _, err := NewCommand(repo.Ctx, "for-each-ref", "--count="+strconv.Itoa(limit), "--format=%(refname:strip=2)", "--contains", commit.ID.String(), BranchPrefix).RunStdString(&RunOpts{Dir: repo.Path}) + stdout, _, err := NewCommand(repo.Ctx, "for-each-ref", + CmdArg("--count="+strconv.Itoa(limit)), + "--format=%(refname:strip=2)", "--contains"). + AddDynamicArguments(commit.ID.String(), BranchPrefix). + RunStdString(&RunOpts{Dir: repo.Path}) if err != nil { return nil, err } @@ -393,7 +407,7 @@ func (repo *Repository) getBranches(commit *Commit, limit int) ([]string, error) return branches, nil } - stdout, _, err := NewCommand(repo.Ctx, "branch", "--contains", commit.ID.String()).RunStdString(&RunOpts{Dir: repo.Path}) + stdout, _, err := NewCommand(repo.Ctx, "branch", "--contains").AddDynamicArguments(commit.ID.String()).RunStdString(&RunOpts{Dir: repo.Path}) if err != nil { return nil, err } @@ -432,7 +446,7 @@ func (repo *Repository) GetCommitsFromIDs(commitIDs []string) []*Commit { // IsCommitInBranch check if the commit is on the branch func (repo *Repository) IsCommitInBranch(commitID, branch string) (r bool, err error) { - stdout, _, err := NewCommand(repo.Ctx, "branch", "--contains", commitID, branch).RunStdString(&RunOpts{Dir: repo.Path}) + stdout, _, err := NewCommand(repo.Ctx, "branch", "--contains").AddDynamicArguments(commitID, branch).RunStdString(&RunOpts{Dir: repo.Path}) if err != nil { return false, err } diff --git a/modules/git/repo_commit_gogit.go b/modules/git/repo_commit_gogit.go index 9333b0d7b756..14fec3f9c6bd 100644 --- a/modules/git/repo_commit_gogit.go +++ b/modules/git/repo_commit_gogit.go @@ -49,7 +49,7 @@ func (repo *Repository) ConvertToSHA1(commitID string) (SHA1, error) { } } - actualCommitID, _, err := NewCommand(repo.Ctx, "rev-parse", "--verify", commitID).RunStdString(&RunOpts{Dir: repo.Path}) + actualCommitID, _, err := NewCommand(repo.Ctx, "rev-parse", "--verify").AddDynamicArguments(commitID).RunStdString(&RunOpts{Dir: repo.Path}) if err != nil { if strings.Contains(err.Error(), "unknown revision or path") || strings.Contains(err.Error(), "fatal: Needed a single revision") { diff --git a/modules/git/repo_commit_nogogit.go b/modules/git/repo_commit_nogogit.go index efea307b37a2..13a7be778f58 100644 --- a/modules/git/repo_commit_nogogit.go +++ b/modules/git/repo_commit_nogogit.go @@ -17,7 +17,7 @@ import ( // ResolveReference resolves a name to a reference func (repo *Repository) ResolveReference(name string) (string, error) { - stdout, _, err := NewCommand(repo.Ctx, "show-ref", "--hash", name).RunStdString(&RunOpts{Dir: repo.Path}) + stdout, _, err := NewCommand(repo.Ctx, "show-ref", "--hash").AddDynamicArguments(name).RunStdString(&RunOpts{Dir: repo.Path}) if err != nil { if strings.Contains(err.Error(), "not a valid ref") { return "", ErrNotExist{name, ""} @@ -50,19 +50,19 @@ func (repo *Repository) GetRefCommitID(name string) (string, error) { // SetReference sets the commit ID string of given reference (e.g. branch or tag). func (repo *Repository) SetReference(name, commitID string) error { - _, _, err := NewCommand(repo.Ctx, "update-ref", name, commitID).RunStdString(&RunOpts{Dir: repo.Path}) + _, _, err := NewCommand(repo.Ctx, "update-ref").AddDynamicArguments(name, commitID).RunStdString(&RunOpts{Dir: repo.Path}) return err } // RemoveReference removes the given reference (e.g. branch or tag). func (repo *Repository) RemoveReference(name string) error { - _, _, err := NewCommand(repo.Ctx, "update-ref", "--no-deref", "-d", name).RunStdString(&RunOpts{Dir: repo.Path}) + _, _, err := NewCommand(repo.Ctx, "update-ref", "--no-deref", "-d").AddDynamicArguments(name).RunStdString(&RunOpts{Dir: repo.Path}) return err } // IsCommitExist returns true if given commit exists in current repository. func (repo *Repository) IsCommitExist(name string) bool { - _, _, err := NewCommand(repo.Ctx, "cat-file", "-e", name).RunStdString(&RunOpts{Dir: repo.Path}) + _, _, err := NewCommand(repo.Ctx, "cat-file", "-e").AddDynamicArguments(name).RunStdString(&RunOpts{Dir: repo.Path}) return err == nil } diff --git a/modules/git/repo_compare.go b/modules/git/repo_compare.go index 589a7eae00bf..aad78370ff21 100644 --- a/modules/git/repo_compare.go +++ b/modules/git/repo_compare.go @@ -40,13 +40,13 @@ func (repo *Repository) GetMergeBase(tmpRemote, base, head string) (string, stri if tmpRemote != "origin" { tmpBaseName := RemotePrefix + tmpRemote + "/tmp_" + base // Fetch commit into a temporary branch in order to be able to handle commits and tags - _, _, err := NewCommand(repo.Ctx, "fetch", "--no-tags", tmpRemote, "--", base+":"+tmpBaseName).RunStdString(&RunOpts{Dir: repo.Path}) + _, _, err := NewCommand(repo.Ctx, "fetch", "--no-tags").AddDynamicArguments(tmpRemote).AddDashesAndList(base + ":" + tmpBaseName).RunStdString(&RunOpts{Dir: repo.Path}) if err == nil { base = tmpBaseName } } - stdout, _, err := NewCommand(repo.Ctx, "merge-base", "--", base, head).RunStdString(&RunOpts{Dir: repo.Path}) + stdout, _, err := NewCommand(repo.Ctx, "merge-base").AddDashesAndList(base, head).RunStdString(&RunOpts{Dir: repo.Path}) return strings.TrimSpace(stdout), base, err } @@ -94,7 +94,7 @@ func (repo *Repository) GetCompareInfo(basePath, baseBranch, headBranch string, // We have a common base - therefore we know that ... should work if !fileOnly { var logs []byte - logs, _, err = NewCommand(repo.Ctx, "log", baseCommitID+separator+headBranch, prettyLogFormat).RunStdBytes(&RunOpts{Dir: repo.Path}) + logs, _, err = NewCommand(repo.Ctx, "log").AddDynamicArguments(baseCommitID + separator + headBranch).AddArguments(prettyLogFormat).RunStdBytes(&RunOpts{Dir: repo.Path}) if err != nil { return nil, err } @@ -147,7 +147,7 @@ func (repo *Repository) GetDiffNumChangedFiles(base, head string, directComparis separator = ".." } - if err := NewCommand(repo.Ctx, "diff", "-z", "--name-only", base+separator+head). + if err := NewCommand(repo.Ctx, "diff", "-z", "--name-only").AddDynamicArguments(base + separator + head). Run(&RunOpts{ Dir: repo.Path, Stdout: w, @@ -158,7 +158,7 @@ func (repo *Repository) GetDiffNumChangedFiles(base, head string, directComparis // previously it would return the results of git diff -z --name-only base head so let's try that... w = &lineCountWriter{} stderr.Reset() - if err = NewCommand(repo.Ctx, "diff", "-z", "--name-only", base, head).Run(&RunOpts{ + if err = NewCommand(repo.Ctx, "diff", "-z", "--name-only").AddDynamicArguments(base, head).Run(&RunOpts{ Dir: repo.Path, Stdout: w, Stderr: stderr, @@ -173,20 +173,20 @@ func (repo *Repository) GetDiffNumChangedFiles(base, head string, directComparis // GetDiffShortStat counts number of changed files, number of additions and deletions func (repo *Repository) GetDiffShortStat(base, head string) (numFiles, totalAdditions, totalDeletions int, err error) { - numFiles, totalAdditions, totalDeletions, err = GetDiffShortStat(repo.Ctx, repo.Path, base+"..."+head) + numFiles, totalAdditions, totalDeletions, err = GetDiffShortStat(repo.Ctx, repo.Path, CmdArgCheck(base+"..."+head)) if err != nil && strings.Contains(err.Error(), "no merge base") { - return GetDiffShortStat(repo.Ctx, repo.Path, base, head) + return GetDiffShortStat(repo.Ctx, repo.Path, CmdArgCheck(base), CmdArgCheck(head)) } return numFiles, totalAdditions, totalDeletions, err } // GetDiffShortStat counts number of changed files, number of additions and deletions -func GetDiffShortStat(ctx context.Context, repoPath string, args ...string) (numFiles, totalAdditions, totalDeletions int, err error) { +func GetDiffShortStat(ctx context.Context, repoPath string, args ...CmdArg) (numFiles, totalAdditions, totalDeletions int, err error) { // Now if we call: // $ git diff --shortstat 1ebb35b98889ff77299f24d82da426b434b0cca0...788b8b1440462d477f45b0088875 // we get: // " 9902 files changed, 2034198 insertions(+), 298800 deletions(-)\n" - args = append([]string{ + args = append([]CmdArg{ "diff", "--shortstat", }, args...) @@ -247,7 +247,7 @@ func (repo *Repository) GetDiffOrPatch(base, head string, w io.Writer, patch, bi // GetDiff generates and returns patch data between given revisions, optimized for human readability func (repo *Repository) GetDiff(base, head string, w io.Writer) error { - return NewCommand(repo.Ctx, "diff", "-p", base, head).Run(&RunOpts{ + return NewCommand(repo.Ctx, "diff", "-p").AddDynamicArguments(base, head).Run(&RunOpts{ Dir: repo.Path, Stdout: w, }) @@ -255,7 +255,7 @@ func (repo *Repository) GetDiff(base, head string, w io.Writer) error { // GetDiffBinary generates and returns patch data between given revisions, including binary diffs. func (repo *Repository) GetDiffBinary(base, head string, w io.Writer) error { - return NewCommand(repo.Ctx, "diff", "-p", "--binary", "--histogram", base, head).Run(&RunOpts{ + return NewCommand(repo.Ctx, "diff", "-p", "--binary", "--histogram").AddDynamicArguments(base, head).Run(&RunOpts{ Dir: repo.Path, Stdout: w, }) @@ -264,14 +264,14 @@ func (repo *Repository) GetDiffBinary(base, head string, w io.Writer) error { // GetPatch generates and returns format-patch data between given revisions, able to be used with `git apply` func (repo *Repository) GetPatch(base, head string, w io.Writer) error { stderr := new(bytes.Buffer) - err := NewCommand(repo.Ctx, "format-patch", "--binary", "--stdout", base+"..."+head). + err := NewCommand(repo.Ctx, "format-patch", "--binary", "--stdout").AddDynamicArguments(base + "..." + head). Run(&RunOpts{ Dir: repo.Path, Stdout: w, Stderr: stderr, }) if err != nil && bytes.Contains(stderr.Bytes(), []byte("no merge base")) { - return NewCommand(repo.Ctx, "format-patch", "--binary", "--stdout", base, head). + return NewCommand(repo.Ctx, "format-patch", "--binary", "--stdout").AddDynamicArguments(base, head). Run(&RunOpts{ Dir: repo.Path, Stdout: w, @@ -282,7 +282,7 @@ func (repo *Repository) GetPatch(base, head string, w io.Writer) error { // GetFilesChangedBetween returns a list of all files that have been changed between the given commits func (repo *Repository) GetFilesChangedBetween(base, head string) ([]string, error) { - stdout, _, err := NewCommand(repo.Ctx, "diff", "--name-only", base+".."+head).RunStdString(&RunOpts{Dir: repo.Path}) + stdout, _, err := NewCommand(repo.Ctx, "diff", "--name-only").AddDynamicArguments(base + ".." + head).RunStdString(&RunOpts{Dir: repo.Path}) if err != nil { return nil, err } @@ -292,7 +292,7 @@ func (repo *Repository) GetFilesChangedBetween(base, head string) ([]string, err // GetDiffFromMergeBase generates and return patch data from merge base to head func (repo *Repository) GetDiffFromMergeBase(base, head string, w io.Writer) error { stderr := new(bytes.Buffer) - err := NewCommand(repo.Ctx, "diff", "-p", "--binary", base+"..."+head). + err := NewCommand(repo.Ctx, "diff", "-p", "--binary").AddDynamicArguments(base + "..." + head). Run(&RunOpts{ Dir: repo.Path, Stdout: w, diff --git a/modules/git/repo_index.go b/modules/git/repo_index.go index 50d82c77d7ed..5542883288c6 100644 --- a/modules/git/repo_index.go +++ b/modules/git/repo_index.go @@ -18,7 +18,7 @@ import ( // ReadTreeToIndex reads a treeish to the index func (repo *Repository) ReadTreeToIndex(treeish string, indexFilename ...string) error { if len(treeish) != 40 { - res, _, err := NewCommand(repo.Ctx, "rev-parse", "--verify", treeish).RunStdString(&RunOpts{Dir: repo.Path}) + res, _, err := NewCommand(repo.Ctx, "rev-parse", "--verify").AddDynamicArguments(treeish).RunStdString(&RunOpts{Dir: repo.Path}) if err != nil { return err } @@ -38,7 +38,7 @@ func (repo *Repository) readTreeToIndex(id SHA1, indexFilename ...string) error if len(indexFilename) > 0 { env = append(os.Environ(), "GIT_INDEX_FILE="+indexFilename[0]) } - _, _, err := NewCommand(repo.Ctx, "read-tree", id.String()).RunStdString(&RunOpts{Dir: repo.Path, Env: env}) + _, _, err := NewCommand(repo.Ctx, "read-tree").AddDynamicArguments(id.String()).RunStdString(&RunOpts{Dir: repo.Path, Env: env}) if err != nil { return err } @@ -75,12 +75,7 @@ func (repo *Repository) EmptyIndex() error { // LsFiles checks if the given filenames are in the index func (repo *Repository) LsFiles(filenames ...string) ([]string, error) { - cmd := NewCommand(repo.Ctx, "ls-files", "-z", "--") - for _, arg := range filenames { - if arg != "" { - cmd.AddArguments(arg) - } - } + cmd := NewCommand(repo.Ctx, "ls-files", "-z").AddDashesAndList(filenames...) res, _, err := cmd.RunStdBytes(&RunOpts{Dir: repo.Path}) if err != nil { return nil, err @@ -116,7 +111,7 @@ func (repo *Repository) RemoveFilesFromIndex(filenames ...string) error { // AddObjectToIndex adds the provided object hash to the index at the provided filename func (repo *Repository) AddObjectToIndex(mode string, object SHA1, filename string) error { - cmd := NewCommand(repo.Ctx, "update-index", "--add", "--replace", "--cacheinfo", mode, object.String(), filename) + cmd := NewCommand(repo.Ctx, "update-index", "--add", "--replace", "--cacheinfo").AddDynamicArguments(mode, object.String(), filename) _, _, err := cmd.RunStdString(&RunOpts{Dir: repo.Path}) return err } diff --git a/modules/git/repo_stats.go b/modules/git/repo_stats.go index 7eccf9a19043..002e2525e2e8 100644 --- a/modules/git/repo_stats.go +++ b/modules/git/repo_stats.go @@ -41,7 +41,7 @@ func (repo *Repository) GetCodeActivityStats(fromTime time.Time, branch string) since := fromTime.Format(time.RFC3339) - stdout, _, runErr := NewCommand(repo.Ctx, "rev-list", "--count", "--no-merges", "--branches=*", "--date=iso", fmt.Sprintf("--since='%s'", since)).RunStdString(&RunOpts{Dir: repo.Path}) + stdout, _, runErr := NewCommand(repo.Ctx, "rev-list", "--count", "--no-merges", "--branches=*", "--date=iso", CmdArg(fmt.Sprintf("--since='%s'", since))).RunStdString(&RunOpts{Dir: repo.Path}) if runErr != nil { return nil, runErr } @@ -61,7 +61,7 @@ func (repo *Repository) GetCodeActivityStats(fromTime time.Time, branch string) _ = stdoutWriter.Close() }() - gitCmd := NewCommand(repo.Ctx, "log", "--numstat", "--no-merges", "--pretty=format:---%n%h%n%aN%n%aE%n", "--date=iso", fmt.Sprintf("--since='%s'", since)) + gitCmd := NewCommand(repo.Ctx, "log", "--numstat", "--no-merges", "--pretty=format:---%n%h%n%aN%n%aE%n", "--date=iso", CmdArg(fmt.Sprintf("--since='%s'", since))) if len(branch) == 0 { gitCmd.AddArguments("--branches=*") } else { diff --git a/modules/git/repo_tag.go b/modules/git/repo_tag.go index 8444e8d035a0..fd0de7b3a1c5 100644 --- a/modules/git/repo_tag.go +++ b/modules/git/repo_tag.go @@ -25,13 +25,13 @@ func IsTagExist(ctx context.Context, repoPath, name string) bool { // CreateTag create one tag in the repository func (repo *Repository) CreateTag(name, revision string) error { - _, _, err := NewCommand(repo.Ctx, "tag", "--", name, revision).RunStdString(&RunOpts{Dir: repo.Path}) + _, _, err := NewCommand(repo.Ctx, "tag").AddDashesAndList(name, revision).RunStdString(&RunOpts{Dir: repo.Path}) return err } // CreateAnnotatedTag create one annotated tag in the repository func (repo *Repository) CreateAnnotatedTag(name, message, revision string) error { - _, _, err := NewCommand(repo.Ctx, "tag", "-a", "-m", message, "--", name, revision).RunStdString(&RunOpts{Dir: repo.Path}) + _, _, err := NewCommand(repo.Ctx, "tag", "-a", "-m").AddDynamicArguments(message).AddDashesAndList(name, revision).RunStdString(&RunOpts{Dir: repo.Path}) return err } @@ -64,7 +64,7 @@ func (repo *Repository) GetTagNameBySHA(sha string) (string, error) { // GetTagID returns the object ID for a tag (annotated tags have both an object SHA AND a commit SHA) func (repo *Repository) GetTagID(name string) (string, error) { - stdout, _, err := NewCommand(repo.Ctx, "show-ref", "--tags", "--", name).RunStdString(&RunOpts{Dir: repo.Path}) + stdout, _, err := NewCommand(repo.Ctx, "show-ref", "--tags").AddDashesAndList(name).RunStdString(&RunOpts{Dir: repo.Path}) if err != nil { return "", err } @@ -122,7 +122,7 @@ func (repo *Repository) GetTagInfos(page, pageSize int) ([]*Tag, int, error) { rc := &RunOpts{Dir: repo.Path, Stdout: stdoutWriter, Stderr: &stderr} go func() { - err := NewCommand(repo.Ctx, "for-each-ref", "--format", forEachRefFmt.Flag(), "--sort", "-*creatordate", "refs/tags").Run(rc) + err := NewCommand(repo.Ctx, "for-each-ref", CmdArg("--format="+forEachRefFmt.Flag()), "--sort", "-*creatordate", "refs/tags").Run(rc) if err != nil { _ = stdoutWriter.CloseWithError(ConcatenateError(err, stderr.String())) } else { diff --git a/modules/git/repo_tag_nogogit.go b/modules/git/repo_tag_nogogit.go index 72c1c979dc40..5d3aace52fdd 100644 --- a/modules/git/repo_tag_nogogit.go +++ b/modules/git/repo_tag_nogogit.go @@ -26,7 +26,7 @@ func (repo *Repository) IsTagExist(name string) bool { // GetTags returns all tags of the repository. // returning at most limit tags, or all if limit is 0. func (repo *Repository) GetTags(skip, limit int) (tags []string, err error) { - tags, _, err = callShowRef(repo.Ctx, repo.Path, TagPrefix, []string{TagPrefix, "--sort=-taggerdate"}, skip, limit) + tags, _, err = callShowRef(repo.Ctx, repo.Path, TagPrefix, []CmdArg{TagPrefix, "--sort=-taggerdate"}, skip, limit) return tags, err } diff --git a/modules/git/repo_tree.go b/modules/git/repo_tree.go index 2ea3f0187a18..ba81bfc6dbbf 100644 --- a/modules/git/repo_tree.go +++ b/modules/git/repo_tree.go @@ -35,10 +35,10 @@ func (repo *Repository) CommitTree(author, committer *Signature, tree *Tree, opt "GIT_COMMITTER_EMAIL="+committer.Email, "GIT_COMMITTER_DATE="+commitTimeStr, ) - cmd := NewCommand(repo.Ctx, "commit-tree", tree.ID.String()) + cmd := NewCommand(repo.Ctx, "commit-tree").AddDynamicArguments(tree.ID.String()) for _, parent := range opts.Parents { - cmd.AddArguments("-p", parent) + cmd.AddArguments("-p").AddDynamicArguments(parent) } messageBytes := new(bytes.Buffer) @@ -46,7 +46,7 @@ func (repo *Repository) CommitTree(author, committer *Signature, tree *Tree, opt _, _ = messageBytes.WriteString("\n") if opts.KeyID != "" || opts.AlwaysSign { - cmd.AddArguments(fmt.Sprintf("-S%s", opts.KeyID)) + cmd.AddArguments(CmdArg(fmt.Sprintf("-S%s", opts.KeyID))) } if opts.NoGPGSign { diff --git a/modules/git/repo_tree_gogit.go b/modules/git/repo_tree_gogit.go index eef09cddd6a5..e72016493622 100644 --- a/modules/git/repo_tree_gogit.go +++ b/modules/git/repo_tree_gogit.go @@ -21,7 +21,7 @@ func (repo *Repository) getTree(id SHA1) (*Tree, error) { // GetTree find the tree object in the repository. func (repo *Repository) GetTree(idStr string) (*Tree, error) { if len(idStr) != 40 { - res, _, err := NewCommand(repo.Ctx, "rev-parse", "--verify", idStr).RunStdString(&RunOpts{Dir: repo.Path}) + res, _, err := NewCommand(repo.Ctx, "rev-parse", "--verify").AddDynamicArguments(idStr).RunStdString(&RunOpts{Dir: repo.Path}) if err != nil { return nil, err } diff --git a/modules/git/tree.go b/modules/git/tree.go index a83336f3dbd3..f5944dd29c21 100644 --- a/modules/git/tree.go +++ b/modules/git/tree.go @@ -49,12 +49,9 @@ func (t *Tree) SubTree(rpath string) (*Tree, error) { // LsTree checks if the given filenames are in the tree func (repo *Repository) LsTree(ref string, filenames ...string) ([]string, error) { - cmd := NewCommand(repo.Ctx, "ls-tree", "-z", "--name-only", "--", ref) - for _, arg := range filenames { - if arg != "" { - cmd.AddArguments(arg) - } - } + cmd := NewCommand(repo.Ctx, "ls-tree", "-z", "--name-only"). + AddDashesAndList(append([]string{ref}, filenames...)...) + res, _, err := cmd.RunStdBytes(&RunOpts{Dir: repo.Path}) if err != nil { return nil, err diff --git a/modules/git/tree_nogogit.go b/modules/git/tree_nogogit.go index d61d19e4c2e4..5cbb5ffc94a8 100644 --- a/modules/git/tree_nogogit.go +++ b/modules/git/tree_nogogit.go @@ -80,7 +80,7 @@ func (t *Tree) ListEntries() (Entries, error) { } } - stdout, _, runErr := NewCommand(t.repo.Ctx, "ls-tree", "-l", t.ID.String()).RunStdBytes(&RunOpts{Dir: t.repo.Path}) + stdout, _, runErr := NewCommand(t.repo.Ctx, "ls-tree", "-l").AddDynamicArguments(t.ID.String()).RunStdBytes(&RunOpts{Dir: t.repo.Path}) if runErr != nil { if strings.Contains(runErr.Error(), "fatal: Not a valid object name") || strings.Contains(runErr.Error(), "fatal: not a tree object") { return nil, ErrNotExist{ @@ -101,13 +101,13 @@ func (t *Tree) ListEntries() (Entries, error) { // listEntriesRecursive returns all entries of current tree recursively including all subtrees // extraArgs could be "-l" to get the size, which is slower -func (t *Tree) listEntriesRecursive(extraArgs ...string) (Entries, error) { +func (t *Tree) listEntriesRecursive(extraArgs ...CmdArg) (Entries, error) { if t.entriesRecursiveParsed { return t.entriesRecursive, nil } - args := append([]string{"ls-tree", "-t", "-r"}, extraArgs...) - args = append(args, t.ID.String()) + args := append([]CmdArg{"ls-tree", "-t", "-r"}, extraArgs...) + args = append(args, CmdArg(t.ID.String())) stdout, _, runErr := NewCommand(t.repo.Ctx, args...).RunStdBytes(&RunOpts{Dir: t.repo.Path}) if runErr != nil { return nil, runErr diff --git a/modules/gitgraph/graph.go b/modules/gitgraph/graph.go index 0f3c02134482..d6342c928006 100644 --- a/modules/gitgraph/graph.go +++ b/modules/gitgraph/graph.go @@ -37,16 +37,15 @@ func GetCommitGraph(r *git.Repository, page, maxAllowedColors int, hidePRRefs bo graphCmd.AddArguments( "-C", "-M", - fmt.Sprintf("-n %d", setting.UI.GraphMaxCommitNum*page), + git.CmdArg(fmt.Sprintf("-n %d", setting.UI.GraphMaxCommitNum*page)), "--date=iso", - fmt.Sprintf("--pretty=format:%s", format)) + git.CmdArg(fmt.Sprintf("--pretty=format:%s", format))) if len(branches) > 0 { graphCmd.AddDynamicArguments(branches...) } if len(files) > 0 { - graphCmd.AddArguments("--") - graphCmd.AddArguments(files...) + graphCmd.AddDashesAndList(files...) } graph := NewGraph() diff --git a/modules/indexer/code/bleve.go b/modules/indexer/code/bleve.go index 0b31f7119c4f..f1298b01ed7e 100644 --- a/modules/indexer/code/bleve.go +++ b/modules/indexer/code/bleve.go @@ -194,7 +194,7 @@ func (b *BleveIndexer) addUpdate(ctx context.Context, batchWriter git.WriteClose var err error if !update.Sized { var stdout string - stdout, _, err = git.NewCommand(ctx, "cat-file", "-s", update.BlobSha).RunStdString(&git.RunOpts{Dir: repo.RepoPath()}) + stdout, _, err = git.NewCommand(ctx, "cat-file", "-s").AddDynamicArguments(update.BlobSha).RunStdString(&git.RunOpts{Dir: repo.RepoPath()}) if err != nil { return err } diff --git a/modules/indexer/code/elastic_search.go b/modules/indexer/code/elastic_search.go index 7727bfacdef1..108a22467565 100644 --- a/modules/indexer/code/elastic_search.go +++ b/modules/indexer/code/elastic_search.go @@ -223,7 +223,7 @@ func (b *ElasticSearchIndexer) addUpdate(ctx context.Context, batchWriter git.Wr var err error if !update.Sized { var stdout string - stdout, _, err = git.NewCommand(ctx, "cat-file", "-s", update.BlobSha).RunStdString(&git.RunOpts{Dir: repo.RepoPath()}) + stdout, _, err = git.NewCommand(ctx, "cat-file", "-s").AddDynamicArguments(update.BlobSha).RunStdString(&git.RunOpts{Dir: repo.RepoPath()}) if err != nil { return nil, err } diff --git a/modules/indexer/code/git.go b/modules/indexer/code/git.go index 66d76377ade7..774dcc814991 100644 --- a/modules/indexer/code/git.go +++ b/modules/indexer/code/git.go @@ -29,7 +29,7 @@ type repoChanges struct { } func getDefaultBranchSha(ctx context.Context, repo *repo_model.Repository) (string, error) { - stdout, _, err := git.NewCommand(ctx, "show-ref", "-s", git.BranchPrefix+repo.DefaultBranch).RunStdString(&git.RunOpts{Dir: repo.RepoPath()}) + stdout, _, err := git.NewCommand(ctx, "show-ref", "-s").AddDynamicArguments(git.BranchPrefix + repo.DefaultBranch).RunStdString(&git.RunOpts{Dir: repo.RepoPath()}) if err != nil { return "", err } @@ -92,7 +92,7 @@ func parseGitLsTreeOutput(stdout []byte) ([]fileUpdate, error) { // genesisChanges get changes to add repo to the indexer for the first time func genesisChanges(ctx context.Context, repo *repo_model.Repository, revision string) (*repoChanges, error) { var changes repoChanges - stdout, _, runErr := git.NewCommand(ctx, "ls-tree", "--full-tree", "-l", "-r", revision).RunStdBytes(&git.RunOpts{Dir: repo.RepoPath()}) + stdout, _, runErr := git.NewCommand(ctx, "ls-tree", "--full-tree", "-l", "-r").AddDynamicArguments(revision).RunStdBytes(&git.RunOpts{Dir: repo.RepoPath()}) if runErr != nil { return nil, runErr } @@ -104,7 +104,7 @@ func genesisChanges(ctx context.Context, repo *repo_model.Repository, revision s // nonGenesisChanges get changes since the previous indexer update func nonGenesisChanges(ctx context.Context, repo *repo_model.Repository, revision string) (*repoChanges, error) { - diffCmd := git.NewCommand(ctx, "diff", "--name-status", repo.CodeIndexerStatus.CommitSha, revision) + diffCmd := git.NewCommand(ctx, "diff", "--name-status").AddDynamicArguments(repo.CodeIndexerStatus.CommitSha, revision) stdout, _, runErr := diffCmd.RunStdString(&git.RunOpts{Dir: repo.RepoPath()}) if runErr != nil { // previous commit sha may have been removed by a force push, so @@ -169,8 +169,8 @@ func nonGenesisChanges(ctx context.Context, repo *repo_model.Repository, revisio } } - cmd := git.NewCommand(ctx, "ls-tree", "--full-tree", "-l", revision, "--") - cmd.AddArguments(updatedFilenames...) + cmd := git.NewCommand(ctx, "ls-tree", "--full-tree", "-l").AddDynamicArguments(revision). + AddDashesAndList(updatedFilenames...) lsTreeStdout, _, err := cmd.RunStdBytes(&git.RunOpts{Dir: repo.RepoPath()}) if err != nil { return nil, err diff --git a/modules/repository/generate.go b/modules/repository/generate.go index 4d76d33993dd..a69dd12a7893 100644 --- a/modules/repository/generate.go +++ b/modules/repository/generate.go @@ -211,7 +211,7 @@ func generateRepoCommit(ctx context.Context, repo, templateRepo, generateRepo *r } repoPath := repo.RepoPath() - if stdout, _, err := git.NewCommand(ctx, "remote", "add", "origin", repoPath). + if stdout, _, err := git.NewCommand(ctx, "remote", "add", "origin").AddDynamicArguments(repoPath). SetDescription(fmt.Sprintf("generateRepoCommit (git remote add): %s to %s", templateRepoPath, tmpDir)). RunStdString(&git.RunOpts{Dir: tmpDir, Env: env}); err != nil { log.Error("Unable to add %v as remote origin to temporary repo to %s: stdout %s\nError: %v", repo, tmpDir, stdout, err) diff --git a/modules/repository/init.go b/modules/repository/init.go index 37ed0748b4d3..473729a5997e 100644 --- a/modules/repository/init.go +++ b/modules/repository/init.go @@ -228,7 +228,7 @@ func prepareRepoCommit(ctx context.Context, repo *repo_model.Repository, tmpDir, ) // Clone to temporary path and do the init commit. - if stdout, _, err := git.NewCommand(ctx, "clone", repoPath, tmpDir). + if stdout, _, err := git.NewCommand(ctx, "clone").AddDynamicArguments(repoPath, tmpDir). SetDescription(fmt.Sprintf("prepareRepoCommit (git clone): %s to %s", repoPath, tmpDir)). RunStdString(&git.RunOpts{Dir: "", Env: env}); err != nil { log.Error("Failed to clone from %v into %s: stdout: %s\nError: %v", repo, tmpDir, stdout, err) @@ -317,14 +317,14 @@ func initRepoCommit(ctx context.Context, tmpPath string, repo *repo_model.Reposi return fmt.Errorf("git add --all: %v", err) } - args := []string{ - "commit", fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email), + cmd := git.NewCommand(ctx, + "commit", git.CmdArg(fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email)), "-m", "Initial commit", - } + ) sign, keyID, signer, _ := asymkey_service.SignInitialCommit(ctx, tmpPath, u) if sign { - args = append(args, "-S"+keyID) + cmd.AddArguments(git.CmdArg("-S" + keyID)) if repo.GetTrustModel() == repo_model.CommitterTrustModel || repo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel { // need to set the committer to the KeyID owner @@ -332,7 +332,7 @@ func initRepoCommit(ctx context.Context, tmpPath string, repo *repo_model.Reposi committerEmail = signer.Email } } else { - args = append(args, "--no-gpg-sign") + cmd.AddArguments("--no-gpg-sign") } env = append(env, @@ -340,10 +340,10 @@ func initRepoCommit(ctx context.Context, tmpPath string, repo *repo_model.Reposi "GIT_COMMITTER_EMAIL="+committerEmail, ) - if stdout, _, err := git.NewCommand(ctx, args...). + if stdout, _, err := cmd. SetDescription(fmt.Sprintf("initRepoCommit (git commit): %s", tmpPath)). RunStdString(&git.RunOpts{Dir: tmpPath, Env: env}); err != nil { - log.Error("Failed to commit: %v: Stdout: %s\nError: %v", args, stdout, err) + log.Error("Failed to commit: %v: Stdout: %s\nError: %v", cmd.String(), stdout, err) return fmt.Errorf("git commit: %v", err) } @@ -351,7 +351,7 @@ func initRepoCommit(ctx context.Context, tmpPath string, repo *repo_model.Reposi defaultBranch = setting.Repository.DefaultBranch } - if stdout, _, err := git.NewCommand(ctx, "push", "origin", "HEAD:"+defaultBranch). + if stdout, _, err := git.NewCommand(ctx, "push", "origin").AddDynamicArguments("HEAD:" + defaultBranch). SetDescription(fmt.Sprintf("initRepoCommit (git push): %s", tmpPath)). RunStdString(&git.RunOpts{Dir: tmpPath, Env: InternalPushingEnvironment(u, repo)}); err != nil { log.Error("Failed to push back to HEAD: Stdout: %s\nError: %v", stdout, err) diff --git a/modules/repository/push.go b/modules/repository/push.go index e1dbd5d2fcfb..4e4b4000b1a8 100644 --- a/modules/repository/push.go +++ b/modules/repository/push.go @@ -104,7 +104,7 @@ func IsForcePush(ctx context.Context, opts *PushUpdateOptions) (bool, error) { return false, nil } - output, _, err := git.NewCommand(ctx, "rev-list", "--max-count=1", opts.OldCommitID, "^"+opts.NewCommitID). + output, _, err := git.NewCommand(ctx, "rev-list", "--max-count=1").AddDynamicArguments(opts.OldCommitID, "^"+opts.NewCommitID). RunStdString(&git.RunOpts{Dir: repo_model.RepoPath(opts.RepoUserName, opts.RepoName)}) if err != nil { return false, err diff --git a/routers/private/hook_pre_receive.go b/routers/private/hook_pre_receive.go index 3e7d1fe9ef69..fdd0a0bc3a04 100644 --- a/routers/private/hook_pre_receive.go +++ b/routers/private/hook_pre_receive.go @@ -186,7 +186,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullN // 2. Disallow force pushes to protected branches if git.EmptySHA != oldCommitID { - output, _, err := git.NewCommand(ctx, "rev-list", "--max-count=1", oldCommitID, "^"+newCommitID).RunStdString(&git.RunOpts{Dir: repo.RepoPath(), Env: ctx.env}) + output, _, err := git.NewCommand(ctx, "rev-list", "--max-count=1").AddDynamicArguments(oldCommitID, "^"+newCommitID).RunStdString(&git.RunOpts{Dir: repo.RepoPath(), Env: ctx.env}) if err != nil { log.Error("Unable to detect force push between: %s and %s in %-v Error: %v", oldCommitID, newCommitID, repo, err) ctx.JSON(http.StatusInternalServerError, private.Response{ diff --git a/routers/private/hook_verification.go b/routers/private/hook_verification.go index dfa6195b1971..8a2d1cf33db3 100644 --- a/routers/private/hook_verification.go +++ b/routers/private/hook_verification.go @@ -44,7 +44,7 @@ func verifyCommits(oldCommitID, newCommitID string, repo *git.Repository, env [] }() // This is safe as force pushes are already forbidden - err = git.NewCommand(repo.Ctx, "rev-list", oldCommitID+"..."+newCommitID). + err = git.NewCommand(repo.Ctx, "rev-list").AddDynamicArguments(oldCommitID + "..." + newCommitID). Run(&git.RunOpts{ Env: env, Dir: repo.Path, @@ -91,7 +91,7 @@ func readAndVerifyCommit(sha string, repo *git.Repository, env []string) error { }() hash := git.MustIDFromString(sha) - return git.NewCommand(repo.Ctx, "cat-file", "commit", sha). + return git.NewCommand(repo.Ctx, "cat-file", "commit").AddDynamicArguments(sha). Run(&git.RunOpts{ Env: env, Dir: repo.Path, diff --git a/routers/web/repo/blame.go b/routers/web/repo/blame.go index c53a53b47193..64a6f0ec5385 100644 --- a/routers/web/repo/blame.go +++ b/routers/web/repo/blame.go @@ -216,7 +216,7 @@ func renderBlame(ctx *context.Context, blameParts []git.BlamePart, commitNames m filename2attribute2info, err := ctx.Repo.GitRepo.CheckAttribute(git.CheckAttributeOpts{ CachedOnly: true, - Attributes: []string{"linguist-language", "gitlab-language"}, + Attributes: []git.CmdArg{"linguist-language", "gitlab-language"}, Filenames: []string{ctx.Repo.TreePath}, IndexFile: indexFilename, WorkTree: worktree, diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index e7e68d3c5e92..db6b59471ff4 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -560,7 +560,7 @@ func ParseCompareInfo(ctx *context.Context) *CompareInfo { func PrepareCompareDiff( ctx *context.Context, ci *CompareInfo, - whitespaceBehavior string, + whitespaceBehavior git.CmdArg, ) bool { var ( repo = ctx.Repo.Repository diff --git a/routers/web/repo/http.go b/routers/web/repo/http.go index 5aa2bcd13471..1ec781bb13d5 100644 --- a/routers/web/repo/http.go +++ b/routers/web/repo/http.go @@ -398,7 +398,7 @@ func (h *serviceHandler) sendFile(contentType, file string) { var safeGitProtocolHeader = regexp.MustCompile(`^[0-9a-zA-Z]+=[0-9a-zA-Z]+(:[0-9a-zA-Z]+=[0-9a-zA-Z]+)*$`) func getGitConfig(ctx gocontext.Context, option, dir string) string { - out, _, err := git.NewCommand(ctx, "config", option).RunStdString(&git.RunOpts{Dir: dir}) + out, _, err := git.NewCommand(ctx, "config").AddDynamicArguments(option).RunStdString(&git.RunOpts{Dir: dir}) if err != nil { log.Error("%v - %s", err, out) } @@ -471,7 +471,7 @@ func serviceRPC(ctx gocontext.Context, h serviceHandler, service string) { } var stderr bytes.Buffer - cmd := git.NewCommand(h.r.Context(), service, "--stateless-rpc", h.dir) + cmd := git.NewCommand(h.r.Context(), git.CmdArgCheck(service), "--stateless-rpc").AddDynamicArguments(h.dir) cmd.SetDescription(fmt.Sprintf("%s %s %s [repo_path: %s]", git.GitExecutable, service, "--stateless-rpc", h.dir)) if err := cmd.Run(&git.RunOpts{ Dir: h.dir, @@ -543,7 +543,7 @@ func GetInfoRefs(ctx *context.Context) { } h.environ = append(os.Environ(), h.environ...) - refs, _, err := git.NewCommand(ctx, service, "--stateless-rpc", "--advertise-refs", ".").RunStdBytes(&git.RunOpts{Env: h.environ, Dir: h.dir}) + refs, _, err := git.NewCommand(ctx, git.CmdArgCheck(service), "--stateless-rpc", "--advertise-refs", ".").RunStdBytes(&git.RunOpts{Env: h.environ, Dir: h.dir}) if err != nil { log.Error(fmt.Sprintf("%v - %s", err, string(refs))) } diff --git a/routers/web/repo/lfs.go b/routers/web/repo/lfs.go index 633b8ab1a513..41639c4603ac 100644 --- a/routers/web/repo/lfs.go +++ b/routers/web/repo/lfs.go @@ -147,7 +147,7 @@ func LFSLocks(ctx *context.Context) { } name2attribute2info, err := gitRepo.CheckAttribute(git.CheckAttributeOpts{ - Attributes: []string{"lockable"}, + Attributes: []git.CmdArg{"lockable"}, Filenames: filenames, CachedOnly: true, }) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index aa2c4cdb53b2..fc95bbf240f0 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -359,7 +359,7 @@ func PrepareMergedViewPullInfo(ctx *context.Context, issue *issues_model.Issue) } if commitSHA != "" { // Get immediate parent of the first commit in the patch, grab history back - parentCommit, _, err = git.NewCommand(ctx, "rev-list", "-1", "--skip=1", commitSHA).RunStdString(&git.RunOpts{Dir: ctx.Repo.GitRepo.Path}) + parentCommit, _, err = git.NewCommand(ctx, "rev-list", "-1", "--skip=1").AddDynamicArguments(commitSHA).RunStdString(&git.RunOpts{Dir: ctx.Repo.GitRepo.Path}) if err == nil { parentCommit = strings.TrimSpace(parentCommit) } diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index 3e869376ee37..8cb45f623dd9 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -551,7 +551,7 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry, treeLink, rawLink st filename2attribute2info, err := ctx.Repo.GitRepo.CheckAttribute(git.CheckAttributeOpts{ CachedOnly: true, - Attributes: []string{"linguist-language", "gitlab-language"}, + Attributes: []git.CmdArg{"linguist-language", "gitlab-language"}, Filenames: []string{ctx.Repo.TreePath}, IndexFile: indexFilename, WorkTree: worktree, diff --git a/services/agit/agit.go b/services/agit/agit.go index 9f0ce7512335..cea2b1f58057 100644 --- a/services/agit/agit.go +++ b/services/agit/agit.go @@ -179,7 +179,7 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git. } if !forcePush { - output, _, err := git.NewCommand(ctx, "rev-list", "--max-count=1", oldCommitID, "^"+opts.NewCommitIDs[i]).RunStdString(&git.RunOpts{Dir: repo.RepoPath(), Env: os.Environ()}) + 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("Fail to detect force push: %v", err) } else if len(output) > 0 { diff --git a/services/cron/tasks_basic.go b/services/cron/tasks_basic.go index 0d7ef4af0308..ca35f5be57d0 100644 --- a/services/cron/tasks_basic.go +++ b/services/cron/tasks_basic.go @@ -12,6 +12,7 @@ import ( git_model "code.gitea.io/gitea/models/git" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/models/webhook" + "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/services/auth" "code.gitea.io/gitea/services/migrations" @@ -58,7 +59,12 @@ func registerRepoHealthCheck() { Args: []string{}, }, func(ctx context.Context, _ *user_model.User, config Config) error { rhcConfig := config.(*RepoHealthCheckConfig) - return repo_service.GitFsck(ctx, rhcConfig.Timeout, rhcConfig.Args) + // the git args are set by config, they can be safe to be trusted + args := make([]git.CmdArg, 0, len(rhcConfig.Args)) + for _, arg := range rhcConfig.Args { + args = append(args, git.CmdArg(arg)) + } + return repo_service.GitFsck(ctx, rhcConfig.Timeout, args) }) } diff --git a/services/cron/tasks_extended.go b/services/cron/tasks_extended.go index 04b9560be35a..c730477cbd06 100644 --- a/services/cron/tasks_extended.go +++ b/services/cron/tasks_extended.go @@ -13,6 +13,7 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/system" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/updatechecker" repo_service "code.gitea.io/gitea/services/repository" @@ -60,7 +61,12 @@ func registerGarbageCollectRepositories() { Args: setting.Git.GCArgs, }, func(ctx context.Context, _ *user_model.User, config Config) error { rhcConfig := config.(*RepoHealthCheckConfig) - return repo_service.GitGcRepos(ctx, rhcConfig.Timeout, rhcConfig.Args...) + // the git args are set by config, they can be safe to be trusted + args := make([]git.CmdArg, 0, len(rhcConfig.Args)) + for _, arg := range rhcConfig.Args { + args = append(args, git.CmdArg(arg)) + } + return repo_service.GitGcRepos(ctx, rhcConfig.Timeout, args...) }) } diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index e687d9ae9163..3c8c5c81a569 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -1056,7 +1056,7 @@ type DiffOptions struct { MaxLines int MaxLineCharacters int MaxFiles int - WhitespaceBehavior string + WhitespaceBehavior git.CmdArg DirectComparison bool } @@ -1082,7 +1082,7 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff argsLength += len(files) + 1 } - diffArgs := make([]string, 0, argsLength) + diffArgs := make([]git.CmdArg, 0, argsLength) if (len(opts.BeforeCommitID) == 0 || opts.BeforeCommitID == git.EmptySHA) && commit.ParentCount() == 0 { diffArgs = append(diffArgs, "diff", "--src-prefix=\\a/", "--dst-prefix=\\b/", "-M") if len(opts.WhitespaceBehavior) != 0 { @@ -1090,7 +1090,7 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff } // append empty tree ref diffArgs = append(diffArgs, "4b825dc642cb6eb9a060e54bf8d69288fbee4904") - diffArgs = append(diffArgs, opts.AfterCommitID) + diffArgs = append(diffArgs, git.CmdArgCheck(opts.AfterCommitID)) } else { actualBeforeCommitID := opts.BeforeCommitID if len(actualBeforeCommitID) == 0 { @@ -1101,8 +1101,8 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff if len(opts.WhitespaceBehavior) != 0 { diffArgs = append(diffArgs, opts.WhitespaceBehavior) } - diffArgs = append(diffArgs, actualBeforeCommitID) - diffArgs = append(diffArgs, opts.AfterCommitID) + diffArgs = append(diffArgs, git.CmdArgCheck(actualBeforeCommitID)) + diffArgs = append(diffArgs, git.CmdArgCheck(opts.AfterCommitID)) opts.BeforeCommitID = actualBeforeCommitID } @@ -1111,13 +1111,15 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff // the skipping for us parsePatchSkipToFile := opts.SkipTo if opts.SkipTo != "" && git.CheckGitVersionAtLeast("2.31") == nil { - diffArgs = append(diffArgs, "--skip-to="+opts.SkipTo) + diffArgs = append(diffArgs, git.CmdArg("--skip-to="+opts.SkipTo)) parsePatchSkipToFile = "" } if len(files) > 0 { diffArgs = append(diffArgs, "--") - diffArgs = append(diffArgs, files...) + for _, file := range files { + diffArgs = append(diffArgs, git.CmdArg(file)) // it's safe to cast it to CmdArg because there is a "--" before + } } reader, writer := io.Pipe() @@ -1126,7 +1128,7 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff _ = writer.Close() }() - go func(ctx context.Context, diffArgs []string, repoPath string, writer *io.PipeWriter) { + go func(ctx context.Context, diffArgs []git.CmdArg, repoPath string, writer *io.PipeWriter) { cmd := git.NewCommand(ctx, diffArgs...) cmd.SetDescription(fmt.Sprintf("GetDiffRange [repo_path: %s]", repoPath)) if err := cmd.Run(&git.RunOpts{ @@ -1199,15 +1201,15 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff separator = ".." } - shortstatArgs := []string{opts.BeforeCommitID + separator + opts.AfterCommitID} + shortstatArgs := []git.CmdArg{git.CmdArgCheck(opts.BeforeCommitID + separator + opts.AfterCommitID)} if len(opts.BeforeCommitID) == 0 || opts.BeforeCommitID == git.EmptySHA { - shortstatArgs = []string{git.EmptyTreeSHA, opts.AfterCommitID} + shortstatArgs = []git.CmdArg{git.EmptyTreeSHA, git.CmdArgCheck(opts.AfterCommitID)} } diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(gitRepo.Ctx, repoPath, shortstatArgs...) if err != nil && strings.Contains(err.Error(), "no merge base") { // git >= 2.28 now returns an error if base and head have become unrelated. // previously it would return the results of git diff --shortstat base head so let's try that... - shortstatArgs = []string{opts.BeforeCommitID, opts.AfterCommitID} + shortstatArgs = []git.CmdArg{git.CmdArgCheck(opts.BeforeCommitID), git.CmdArgCheck(opts.AfterCommitID)} diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(gitRepo.Ctx, repoPath, shortstatArgs...) } if err != nil { @@ -1322,7 +1324,7 @@ func CommentMustAsDiff(c *issues_model.Comment) *Diff { } // GetWhitespaceFlag returns git diff flag for treating whitespaces -func GetWhitespaceFlag(whitespaceBehavior string) string { +func GetWhitespaceFlag(whitespaceBehavior string) git.CmdArg { whitespaceFlags := map[string]string{ "ignore-all": "-w", "ignore-change": "-b", @@ -1331,7 +1333,7 @@ func GetWhitespaceFlag(whitespaceBehavior string) string { } if flag, ok := whitespaceFlags[whitespaceBehavior]; ok { - return flag + return git.CmdArg(flag) } log.Warn("unknown whitespace behavior: %q, default to 'show-all'", whitespaceBehavior) return "" diff --git a/services/gitdiff/gitdiff_test.go b/services/gitdiff/gitdiff_test.go index dfdd4df9c445..a7fd677fef09 100644 --- a/services/gitdiff/gitdiff_test.go +++ b/services/gitdiff/gitdiff_test.go @@ -627,7 +627,7 @@ func TestGetDiffRangeWithWhitespaceBehavior(t *testing.T) { return } defer gitRepo.Close() - for _, behavior := range []string{"-w", "--ignore-space-at-eol", "-b", ""} { + for _, behavior := range []git.CmdArg{"-w", "--ignore-space-at-eol", "-b", ""} { diffs, err := GetDiff(gitRepo, &DiffOptions{ AfterCommitID: "bd7063cc7c04689c4d082183d32a604ed27a24f9", diff --git a/services/migrations/dump.go b/services/migrations/dump.go index 188f2775e07b..31fb1b4cf3ba 100644 --- a/services/migrations/dump.go +++ b/services/migrations/dump.go @@ -491,7 +491,7 @@ func (g *RepositoryDumper) handlePullRequest(pr *base.PullRequest) error { if pr.Head.CloneURL == "" || pr.Head.Ref == "" { // Set head information if pr.Head.SHA is available if pr.Head.SHA != "" { - _, _, err = git.NewCommand(g.ctx, "update-ref", "--no-deref", pr.GetGitRefName(), pr.Head.SHA).RunStdString(&git.RunOpts{Dir: g.gitPath()}) + _, _, err = git.NewCommand(g.ctx, "update-ref", "--no-deref").AddDynamicArguments(pr.GetGitRefName(), pr.Head.SHA).RunStdString(&git.RunOpts{Dir: g.gitPath()}) if err != nil { log.Error("PR #%d in %s/%s unable to update-ref for pr HEAD: %v", pr.Number, g.repoOwner, g.repoName, err) } @@ -521,7 +521,7 @@ func (g *RepositoryDumper) handlePullRequest(pr *base.PullRequest) error { if !ok { // Set head information if pr.Head.SHA is available if pr.Head.SHA != "" { - _, _, err = git.NewCommand(g.ctx, "update-ref", "--no-deref", pr.GetGitRefName(), pr.Head.SHA).RunStdString(&git.RunOpts{Dir: g.gitPath()}) + _, _, err = git.NewCommand(g.ctx, "update-ref", "--no-deref").AddDynamicArguments(pr.GetGitRefName(), pr.Head.SHA).RunStdString(&git.RunOpts{Dir: g.gitPath()}) if err != nil { log.Error("PR #%d in %s/%s unable to update-ref for pr HEAD: %v", pr.Number, g.repoOwner, g.repoName, err) } @@ -556,7 +556,7 @@ func (g *RepositoryDumper) handlePullRequest(pr *base.PullRequest) error { fetchArg = git.BranchPrefix + fetchArg } - _, _, err = git.NewCommand(g.ctx, "fetch", "--no-tags", "--", remote, fetchArg).RunStdString(&git.RunOpts{Dir: g.gitPath()}) + _, _, err = git.NewCommand(g.ctx, "fetch", "--no-tags").AddDashesAndList(remote, fetchArg).RunStdString(&git.RunOpts{Dir: g.gitPath()}) if err != nil { log.Error("Fetch branch from %s failed: %v", pr.Head.CloneURL, err) // We need to continue here so that the Head.Ref is reset and we attempt to set the gitref for the PR @@ -580,7 +580,7 @@ func (g *RepositoryDumper) handlePullRequest(pr *base.PullRequest) error { pr.Head.SHA = headSha } if pr.Head.SHA != "" { - _, _, err = git.NewCommand(g.ctx, "update-ref", "--no-deref", pr.GetGitRefName(), pr.Head.SHA).RunStdString(&git.RunOpts{Dir: g.gitPath()}) + _, _, err = git.NewCommand(g.ctx, "update-ref", "--no-deref").AddDynamicArguments(pr.GetGitRefName(), pr.Head.SHA).RunStdString(&git.RunOpts{Dir: g.gitPath()}) if err != nil { log.Error("unable to set %s as the local head for PR #%d from %s in %s/%s. Error: %v", pr.Head.SHA, pr.Number, pr.Head.Ref, g.repoOwner, g.repoName, err) } diff --git a/services/migrations/gitea_uploader.go b/services/migrations/gitea_uploader.go index 83388391da26..c4cb59f5723b 100644 --- a/services/migrations/gitea_uploader.go +++ b/services/migrations/gitea_uploader.go @@ -626,7 +626,7 @@ func (g *GiteaLocalUploader) updateGitForPullRequest(pr *base.PullRequest) (head fetchArg = git.BranchPrefix + fetchArg } - _, _, err = git.NewCommand(g.ctx, "fetch", "--no-tags", "--", remote, fetchArg).RunStdString(&git.RunOpts{Dir: g.repo.RepoPath()}) + _, _, err = git.NewCommand(g.ctx, "fetch", "--no-tags").AddDashesAndList(remote, fetchArg).RunStdString(&git.RunOpts{Dir: g.repo.RepoPath()}) if err != nil { log.Error("Fetch branch from %s failed: %v", pr.Head.CloneURL, err) return head, nil @@ -645,7 +645,7 @@ func (g *GiteaLocalUploader) updateGitForPullRequest(pr *base.PullRequest) (head pr.Head.SHA = headSha } - _, _, err = git.NewCommand(g.ctx, "update-ref", "--no-deref", pr.GetGitRefName(), pr.Head.SHA).RunStdString(&git.RunOpts{Dir: g.repo.RepoPath()}) + _, _, err = git.NewCommand(g.ctx, "update-ref", "--no-deref").AddDynamicArguments(pr.GetGitRefName(), pr.Head.SHA).RunStdString(&git.RunOpts{Dir: g.repo.RepoPath()}) if err != nil { return "", err } @@ -662,13 +662,13 @@ func (g *GiteaLocalUploader) updateGitForPullRequest(pr *base.PullRequest) (head // The SHA is empty log.Warn("Empty reference, no pull head for PR #%d in %s/%s", pr.Number, g.repoOwner, g.repoName) } else { - _, _, err = git.NewCommand(g.ctx, "rev-list", "--quiet", "-1", pr.Head.SHA).RunStdString(&git.RunOpts{Dir: g.repo.RepoPath()}) + _, _, err = git.NewCommand(g.ctx, "rev-list", "--quiet", "-1").AddDynamicArguments(pr.Head.SHA).RunStdString(&git.RunOpts{Dir: g.repo.RepoPath()}) if err != nil { // Git update-ref remove bad references with a relative path log.Warn("Deprecated local head %s for PR #%d in %s/%s, removing %s", pr.Head.SHA, pr.Number, g.repoOwner, g.repoName, pr.GetGitRefName()) } else { // set head information - _, _, err = git.NewCommand(g.ctx, "update-ref", "--no-deref", pr.GetGitRefName(), pr.Head.SHA).RunStdString(&git.RunOpts{Dir: g.repo.RepoPath()}) + _, _, err = git.NewCommand(g.ctx, "update-ref", "--no-deref").AddDynamicArguments(pr.GetGitRefName(), pr.Head.SHA).RunStdString(&git.RunOpts{Dir: g.repo.RepoPath()}) if err != nil { log.Error("unable to set %s as the local head for PR #%d from %s in %s/%s. Error: %v", pr.Head.SHA, pr.Number, pr.Head.Ref, g.repoOwner, g.repoName, err) } diff --git a/services/migrations/gitea_uploader_test.go b/services/migrations/gitea_uploader_test.go index af6230decb70..68a7182b07e0 100644 --- a/services/migrations/gitea_uploader_test.go +++ b/services/migrations/gitea_uploader_test.go @@ -234,7 +234,7 @@ func TestGiteaUploadUpdateGitForPullRequest(t *testing.T) { fromRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) baseRef := "master" assert.NoError(t, git.InitRepository(git.DefaultContext, fromRepo.RepoPath(), false)) - err := git.NewCommand(git.DefaultContext, "symbolic-ref", "HEAD", git.BranchPrefix+baseRef).Run(&git.RunOpts{Dir: fromRepo.RepoPath()}) + err := git.NewCommand(git.DefaultContext, "symbolic-ref").AddDynamicArguments("HEAD", git.BranchPrefix+baseRef).Run(&git.RunOpts{Dir: fromRepo.RepoPath()}) assert.NoError(t, err) assert.NoError(t, os.WriteFile(filepath.Join(fromRepo.RepoPath(), "README.md"), []byte(fmt.Sprintf("# Testing Repository\n\nOriginally created in: %s", fromRepo.RepoPath())), 0o644)) assert.NoError(t, git.AddChanges(fromRepo.RepoPath(), true)) @@ -258,7 +258,7 @@ func TestGiteaUploadUpdateGitForPullRequest(t *testing.T) { // fromRepo branch1 // headRef := "branch1" - _, _, err = git.NewCommand(git.DefaultContext, "checkout", "-b", headRef).RunStdString(&git.RunOpts{Dir: fromRepo.RepoPath()}) + _, _, err = git.NewCommand(git.DefaultContext, "checkout", "-b").AddDynamicArguments(headRef).RunStdString(&git.RunOpts{Dir: fromRepo.RepoPath()}) assert.NoError(t, err) assert.NoError(t, os.WriteFile(filepath.Join(fromRepo.RepoPath(), "README.md"), []byte("SOMETHING"), 0o644)) assert.NoError(t, git.AddChanges(fromRepo.RepoPath(), true)) @@ -279,10 +279,10 @@ func TestGiteaUploadUpdateGitForPullRequest(t *testing.T) { // forkHeadRef := "branch2" forkRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 8}) - assert.NoError(t, git.CloneWithArgs(git.DefaultContext, fromRepo.RepoPath(), forkRepo.RepoPath(), []string{}, git.CloneRepoOptions{ + assert.NoError(t, git.CloneWithArgs(git.DefaultContext, nil, fromRepo.RepoPath(), forkRepo.RepoPath(), git.CloneRepoOptions{ Branch: headRef, })) - _, _, err = git.NewCommand(git.DefaultContext, "checkout", "-b", forkHeadRef).RunStdString(&git.RunOpts{Dir: forkRepo.RepoPath()}) + _, _, err = git.NewCommand(git.DefaultContext, "checkout", "-b").AddDynamicArguments(forkHeadRef).RunStdString(&git.RunOpts{Dir: forkRepo.RepoPath()}) assert.NoError(t, err) assert.NoError(t, os.WriteFile(filepath.Join(forkRepo.RepoPath(), "README.md"), []byte(fmt.Sprintf("# branch2 %s", forkRepo.RepoPath())), 0o644)) assert.NoError(t, git.AddChanges(forkRepo.RepoPath(), true)) diff --git a/services/mirror/mirror_pull.go b/services/mirror/mirror_pull.go index a72e7f72eb8b..6002e6b8edb2 100644 --- a/services/mirror/mirror_pull.go +++ b/services/mirror/mirror_pull.go @@ -33,12 +33,12 @@ func UpdateAddress(ctx context.Context, m *repo_model.Mirror, addr string) error remoteName := m.GetRemoteName() repoPath := m.GetRepository().RepoPath() // Remove old remote - _, _, err := git.NewCommand(ctx, "remote", "rm", remoteName).RunStdString(&git.RunOpts{Dir: repoPath}) + _, _, err := git.NewCommand(ctx, "remote", "rm").AddDynamicArguments(remoteName).RunStdString(&git.RunOpts{Dir: repoPath}) if err != nil && !strings.HasPrefix(err.Error(), "exit status 128 - fatal: No such remote ") { return err } - cmd := git.NewCommand(ctx, "remote", "add", remoteName, "--mirror=fetch", addr) + cmd := git.NewCommand(ctx, "remote", "add").AddDynamicArguments(remoteName).AddArguments("--mirror=fetch").AddDynamicArguments(addr) if strings.Contains(addr, "://") && strings.Contains(addr, "@") { cmd.SetDescription(fmt.Sprintf("remote add %s --mirror=fetch %s [repo_path: %s]", remoteName, util.SanitizeCredentialURLs(addr), repoPath)) } else { @@ -53,12 +53,12 @@ func UpdateAddress(ctx context.Context, m *repo_model.Mirror, addr string) error wikiPath := m.Repo.WikiPath() wikiRemotePath := repo_module.WikiRemoteURL(ctx, addr) // Remove old remote of wiki - _, _, err = git.NewCommand(ctx, "remote", "rm", remoteName).RunStdString(&git.RunOpts{Dir: wikiPath}) + _, _, err = git.NewCommand(ctx, "remote", "rm").AddDynamicArguments(remoteName).RunStdString(&git.RunOpts{Dir: wikiPath}) if err != nil && !strings.HasPrefix(err.Error(), "exit status 128 - fatal: No such remote ") { return err } - cmd = git.NewCommand(ctx, "remote", "add", remoteName, "--mirror=fetch", wikiRemotePath) + cmd = git.NewCommand(ctx, "remote", "add").AddDynamicArguments(remoteName).AddArguments("--mirror=fetch").AddDynamicArguments(wikiRemotePath) if strings.Contains(wikiRemotePath, "://") && strings.Contains(wikiRemotePath, "@") { cmd.SetDescription(fmt.Sprintf("remote add %s --mirror=fetch %s [repo_path: %s]", remoteName, util.SanitizeCredentialURLs(wikiRemotePath), wikiPath)) } else { @@ -169,7 +169,7 @@ func pruneBrokenReferences(ctx context.Context, stderrBuilder.Reset() stdoutBuilder.Reset() - pruneErr := git.NewCommand(ctx, "remote", "prune", m.GetRemoteName()). + pruneErr := git.NewCommand(ctx, "remote", "prune").AddDynamicArguments(m.GetRemoteName()). SetDescription(fmt.Sprintf("Mirror.runSync %ssPrune references: %s ", wiki, m.Repo.FullName())). Run(&git.RunOpts{ Timeout: timeout, @@ -204,11 +204,11 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo log.Trace("SyncMirrors [repo: %-v]: running git remote update...", m.Repo) - gitArgs := []string{"remote", "update"} + gitArgs := []git.CmdArg{"remote", "update"} if m.EnablePrune { gitArgs = append(gitArgs, "--prune") } - gitArgs = append(gitArgs, m.GetRemoteName()) + gitArgs = append(gitArgs, git.CmdArgCheck(m.GetRemoteName())) remoteURL, remoteErr := git.GetRemoteURL(ctx, repoPath, m.GetRemoteName()) if remoteErr != nil { @@ -309,7 +309,7 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo log.Trace("SyncMirrors [repo: %-v Wiki]: running git remote update...", m.Repo) stderrBuilder.Reset() stdoutBuilder.Reset() - if err := git.NewCommand(ctx, "remote", "update", "--prune", m.GetRemoteName()). + if err := git.NewCommand(ctx, "remote", "update", "--prune").AddDynamicArguments(m.GetRemoteName()). SetDescription(fmt.Sprintf("Mirror.runSync Wiki: %s ", m.Repo.FullName())). Run(&git.RunOpts{ Timeout: timeout, @@ -336,7 +336,7 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo stderrBuilder.Reset() stdoutBuilder.Reset() - if err = git.NewCommand(ctx, "remote", "update", "--prune", m.GetRemoteName()). + if err = git.NewCommand(ctx, "remote", "update", "--prune").AddDynamicArguments(m.GetRemoteName()). SetDescription(fmt.Sprintf("Mirror.runSync Wiki: %s ", m.Repo.FullName())). Run(&git.RunOpts{ Timeout: timeout, diff --git a/services/mirror/mirror_push.go b/services/mirror/mirror_push.go index 0c8960d78bf2..60611130ba8c 100644 --- a/services/mirror/mirror_push.go +++ b/services/mirror/mirror_push.go @@ -29,7 +29,7 @@ var stripExitStatus = regexp.MustCompile(`exit status \d+ - `) // AddPushMirrorRemote registers the push mirror remote. func AddPushMirrorRemote(ctx context.Context, m *repo_model.PushMirror, addr string) error { addRemoteAndConfig := func(addr, path string) error { - cmd := git.NewCommand(ctx, "remote", "add", "--mirror=push", m.RemoteName, addr) + cmd := git.NewCommand(ctx, "remote", "add", "--mirror=push").AddDynamicArguments(m.RemoteName, addr) if strings.Contains(addr, "://") && strings.Contains(addr, "@") { cmd.SetDescription(fmt.Sprintf("remote add %s --mirror=push %s [repo_path: %s]", m.RemoteName, util.SanitizeCredentialURLs(addr), path)) } else { @@ -38,10 +38,10 @@ func AddPushMirrorRemote(ctx context.Context, m *repo_model.PushMirror, addr str if _, _, err := cmd.RunStdString(&git.RunOpts{Dir: path}); err != nil { return err } - if _, _, err := git.NewCommand(ctx, "config", "--add", "remote."+m.RemoteName+".push", "+refs/heads/*:refs/heads/*").RunStdString(&git.RunOpts{Dir: path}); err != nil { + if _, _, err := git.NewCommand(ctx, "config", "--add", git.CmdArg("remote."+m.RemoteName+".push"), "+refs/heads/*:refs/heads/*").RunStdString(&git.RunOpts{Dir: path}); err != nil { return err } - if _, _, err := git.NewCommand(ctx, "config", "--add", "remote."+m.RemoteName+".push", "+refs/tags/*:refs/tags/*").RunStdString(&git.RunOpts{Dir: path}); err != nil { + if _, _, err := git.NewCommand(ctx, "config", "--add", git.CmdArg("remote."+m.RemoteName+".push"), "+refs/tags/*:refs/tags/*").RunStdString(&git.RunOpts{Dir: path}); err != nil { return err } return nil @@ -65,7 +65,7 @@ func AddPushMirrorRemote(ctx context.Context, m *repo_model.PushMirror, addr str // RemovePushMirrorRemote removes the push mirror remote. func RemovePushMirrorRemote(ctx context.Context, m *repo_model.PushMirror) error { - cmd := git.NewCommand(ctx, "remote", "rm", m.RemoteName) + cmd := git.NewCommand(ctx, "remote", "rm").AddDynamicArguments(m.RemoteName) _ = m.GetRepository() if _, _, err := cmd.RunStdString(&git.RunOpts{Dir: m.Repo.RepoPath()}); err != nil { diff --git a/services/pull/check.go b/services/pull/check.go index 288f4dc0b73b..765672190dc4 100644 --- a/services/pull/check.go +++ b/services/pull/check.go @@ -185,7 +185,7 @@ func getMergeCommit(ctx context.Context, pr *issues_model.PullRequest) (*git.Com headFile := pr.GetGitRefName() // Check if a pull request is merged into BaseBranch - _, _, err = git.NewCommand(ctx, "merge-base", "--is-ancestor", headFile, pr.BaseBranch). + _, _, err = git.NewCommand(ctx, "merge-base", "--is-ancestor").AddDynamicArguments(headFile, pr.BaseBranch). RunStdString(&git.RunOpts{Dir: pr.BaseRepo.RepoPath(), Env: []string{"GIT_INDEX_FILE=" + indexTmpPath, "GIT_DIR=" + pr.BaseRepo.RepoPath()}}) if err != nil { // Errors are signaled by a non-zero status that is not 1 @@ -206,7 +206,7 @@ func getMergeCommit(ctx context.Context, pr *issues_model.PullRequest) (*git.Com cmd := commitID[:40] + ".." + pr.BaseBranch // Get the commit from BaseBranch where the pull request got merged - mergeCommit, _, err := git.NewCommand(ctx, "rev-list", "--ancestry-path", "--merges", "--reverse", cmd). + mergeCommit, _, err := git.NewCommand(ctx, "rev-list", "--ancestry-path", "--merges", "--reverse").AddDynamicArguments(cmd). RunStdString(&git.RunOpts{Dir: "", Env: []string{"GIT_INDEX_FILE=" + indexTmpPath, "GIT_DIR=" + pr.BaseRepo.RepoPath()}}) if err != nil { return nil, fmt.Errorf("git rev-list --ancestry-path --merges --reverse: %v", err) diff --git a/services/pull/merge.go b/services/pull/merge.go index 6f3df6ab2a5a..f9a4e6705f40 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -244,7 +244,7 @@ func rawMerge(ctx context.Context, pr *issues_model.PullRequest, doer *user_mode stagingBranch := "staging" if expectedHeadCommitID != "" { - trackingCommitID, _, err := git.NewCommand(ctx, "show-ref", "--hash", git.BranchPrefix+trackingBranch).RunStdString(&git.RunOpts{Dir: tmpBasePath}) + trackingCommitID, _, err := git.NewCommand(ctx, "show-ref", "--hash").AddDynamicArguments(git.BranchPrefix + trackingBranch).RunStdString(&git.RunOpts{Dir: tmpBasePath}) if err != nil { log.Error("show-ref[%s] --hash refs/heads/trackingn: %v", tmpBasePath, git.BranchPrefix+trackingBranch, err) return "", fmt.Errorf("getDiffTree: %v", err) @@ -360,15 +360,15 @@ func rawMerge(ctx context.Context, pr *issues_model.PullRequest, doer *user_mode committer := sig // Determine if we should sign - var signArg string + var signArg git.CmdArg sign, keyID, signer, _ := asymkey_service.SignMerge(ctx, pr, doer, tmpBasePath, "HEAD", trackingBranch) if sign { - signArg = "-S" + keyID + signArg = git.CmdArg("-S" + keyID) if pr.BaseRepo.GetTrustModel() == repo_model.CommitterTrustModel || pr.BaseRepo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel { committer = signer } } else { - signArg = "--no-gpg-sign" + signArg = git.CmdArg("--no-gpg-sign") } commitTimeStr := time.Now().Format(time.RFC3339) @@ -386,7 +386,7 @@ func rawMerge(ctx context.Context, pr *issues_model.PullRequest, doer *user_mode // Merge commits. switch mergeStyle { case repo_model.MergeStyleMerge: - cmd := git.NewCommand(ctx, "merge", "--no-ff", "--no-commit", trackingBranch) + cmd := git.NewCommand(ctx, "merge", "--no-ff", "--no-commit").AddDynamicArguments(trackingBranch) if err := runMergeCommand(pr, mergeStyle, cmd, tmpBasePath); err != nil { log.Error("Unable to merge tracking into base: %v", err) return "", err @@ -402,7 +402,7 @@ func rawMerge(ctx context.Context, pr *issues_model.PullRequest, doer *user_mode fallthrough case repo_model.MergeStyleRebaseMerge: // Checkout head branch - if err := git.NewCommand(ctx, "checkout", "-b", stagingBranch, trackingBranch). + if err := git.NewCommand(ctx, "checkout", "-b").AddDynamicArguments(stagingBranch, trackingBranch). Run(&git.RunOpts{ Dir: tmpBasePath, Stdout: &outbuf, @@ -415,7 +415,7 @@ func rawMerge(ctx context.Context, pr *issues_model.PullRequest, doer *user_mode errbuf.Reset() // Rebase before merging - if err := git.NewCommand(ctx, "rebase", baseBranch). + if err := git.NewCommand(ctx, "rebase").AddDynamicArguments(baseBranch). Run(&git.RunOpts{ Dir: tmpBasePath, Stdout: &outbuf, @@ -468,7 +468,7 @@ func rawMerge(ctx context.Context, pr *issues_model.PullRequest, doer *user_mode } // Checkout base branch again - if err := git.NewCommand(ctx, "checkout", baseBranch). + if err := git.NewCommand(ctx, "checkout").AddDynamicArguments(baseBranch). Run(&git.RunOpts{ Dir: tmpBasePath, Stdout: &outbuf, @@ -486,7 +486,7 @@ func rawMerge(ctx context.Context, pr *issues_model.PullRequest, doer *user_mode } else { cmd.AddArguments("--no-ff", "--no-commit") } - cmd.AddArguments(stagingBranch) + cmd.AddDynamicArguments(stagingBranch) // Prepare merge with commit if err := runMergeCommand(pr, mergeStyle, cmd, tmpBasePath); err != nil { @@ -501,7 +501,7 @@ func rawMerge(ctx context.Context, pr *issues_model.PullRequest, doer *user_mode } case repo_model.MergeStyleSquash: // Merge with squash - cmd := git.NewCommand(ctx, "merge", "--squash", trackingBranch) + cmd := git.NewCommand(ctx, "merge", "--squash").AddDynamicArguments(trackingBranch) if err := runMergeCommand(pr, mergeStyle, cmd, tmpBasePath); err != nil { log.Error("Unable to merge --squash tracking into base: %v", err) return "", err @@ -513,7 +513,7 @@ func rawMerge(ctx context.Context, pr *issues_model.PullRequest, doer *user_mode } sig := pr.Issue.Poster.NewGitSig() if signArg == "" { - if err := git.NewCommand(ctx, "commit", fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email), "-m", message). + if err := git.NewCommand(ctx, "commit", git.CmdArg(fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email)), "-m").AddDynamicArguments(message). Run(&git.RunOpts{ Env: env, Dir: tmpBasePath, @@ -528,7 +528,10 @@ func rawMerge(ctx context.Context, pr *issues_model.PullRequest, doer *user_mode // add trailer message += fmt.Sprintf("\nCo-authored-by: %s\nCo-committed-by: %s\n", sig.String(), sig.String()) } - if err := git.NewCommand(ctx, "commit", signArg, fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email), "-m", message). + if err := git.NewCommand(ctx, "commit"). + AddArguments(signArg). + AddArguments(git.CmdArg(fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email))). + AddArguments("-m").AddDynamicArguments(message). Run(&git.RunOpts{ Env: env, Dir: tmpBasePath, @@ -592,9 +595,9 @@ func rawMerge(ctx context.Context, pr *issues_model.PullRequest, doer *user_mode var pushCmd *git.Command if mergeStyle == repo_model.MergeStyleRebaseUpdate { // force push the rebase result to head branch - pushCmd = git.NewCommand(ctx, "push", "-f", "head_repo", stagingBranch+":"+git.BranchPrefix+pr.HeadBranch) + pushCmd = git.NewCommand(ctx, "push", "-f", "head_repo").AddDynamicArguments(stagingBranch + ":" + git.BranchPrefix + pr.HeadBranch) } else { - pushCmd = git.NewCommand(ctx, "push", "origin", baseBranch+":"+git.BranchPrefix+pr.BaseBranch) + pushCmd = git.NewCommand(ctx, "push", "origin").AddDynamicArguments(baseBranch + ":" + git.BranchPrefix + pr.BaseBranch) } // Push back to upstream. @@ -629,10 +632,10 @@ func rawMerge(ctx context.Context, pr *issues_model.PullRequest, doer *user_mode return mergeCommitID, nil } -func commitAndSignNoAuthor(ctx context.Context, pr *issues_model.PullRequest, message, signArg, tmpBasePath string, env []string) error { +func commitAndSignNoAuthor(ctx context.Context, pr *issues_model.PullRequest, message string, signArg git.CmdArg, tmpBasePath string, env []string) error { var outbuf, errbuf strings.Builder if signArg == "" { - if err := git.NewCommand(ctx, "commit", "-m", message). + if err := git.NewCommand(ctx, "commit", "-m").AddDynamicArguments(message). Run(&git.RunOpts{ Env: env, Dir: tmpBasePath, @@ -643,7 +646,7 @@ func commitAndSignNoAuthor(ctx context.Context, pr *issues_model.PullRequest, me return fmt.Errorf("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) } } else { - if err := git.NewCommand(ctx, "commit", signArg, "-m", message). + if err := git.NewCommand(ctx, "commit").AddArguments(signArg).AddArguments("-m").AddDynamicArguments(message). Run(&git.RunOpts{ Env: env, Dir: tmpBasePath, @@ -696,7 +699,7 @@ func getDiffTree(ctx context.Context, repoPath, baseBranch, headBranch string) ( getDiffTreeFromBranch := func(repoPath, baseBranch, headBranch string) (string, error) { var outbuf, errbuf strings.Builder // Compute the diff-tree for sparse-checkout - if err := git.NewCommand(ctx, "diff-tree", "--no-commit-id", "--name-only", "-r", "-z", "--root", baseBranch, headBranch, "--"). + if err := git.NewCommand(ctx, "diff-tree", "--no-commit-id", "--name-only", "-r", "-z", "--root").AddDynamicArguments(baseBranch, headBranch). Run(&git.RunOpts{ Dir: repoPath, Stdout: &outbuf, diff --git a/services/pull/patch.go b/services/pull/patch.go index dafd57706915..b9f190826a49 100644 --- a/services/pull/patch.go +++ b/services/pull/patch.go @@ -178,7 +178,7 @@ func attemptMerge(ctx context.Context, file *unmergedFile, tmpBasePath string, g } // Need to get the objects from the object db to attempt to merge - root, _, err := git.NewCommand(ctx, "unpack-file", file.stage1.sha).RunStdString(&git.RunOpts{Dir: tmpBasePath}) + root, _, err := git.NewCommand(ctx, "unpack-file").AddDynamicArguments(file.stage1.sha).RunStdString(&git.RunOpts{Dir: tmpBasePath}) if err != nil { return fmt.Errorf("unable to get root object: %s at path: %s for merging. Error: %w", file.stage1.sha, file.stage1.path, err) } @@ -187,7 +187,7 @@ func attemptMerge(ctx context.Context, file *unmergedFile, tmpBasePath string, g _ = util.Remove(filepath.Join(tmpBasePath, root)) }() - base, _, err := git.NewCommand(ctx, "unpack-file", file.stage2.sha).RunStdString(&git.RunOpts{Dir: tmpBasePath}) + base, _, err := git.NewCommand(ctx, "unpack-file").AddDynamicArguments(file.stage2.sha).RunStdString(&git.RunOpts{Dir: tmpBasePath}) if err != nil { return fmt.Errorf("unable to get base object: %s at path: %s for merging. Error: %w", file.stage2.sha, file.stage2.path, err) } @@ -195,7 +195,7 @@ func attemptMerge(ctx context.Context, file *unmergedFile, tmpBasePath string, g defer func() { _ = util.Remove(base) }() - head, _, err := git.NewCommand(ctx, "unpack-file", file.stage3.sha).RunStdString(&git.RunOpts{Dir: tmpBasePath}) + head, _, err := git.NewCommand(ctx, "unpack-file").AddDynamicArguments(file.stage3.sha).RunStdString(&git.RunOpts{Dir: tmpBasePath}) if err != nil { return fmt.Errorf("unable to get head object:%s at path: %s for merging. Error: %w", file.stage3.sha, file.stage3.path, err) } @@ -205,13 +205,13 @@ func attemptMerge(ctx context.Context, file *unmergedFile, tmpBasePath string, g }() // now git merge-file annoyingly takes a different order to the merge-tree ... - _, _, conflictErr := git.NewCommand(ctx, "merge-file", base, root, head).RunStdString(&git.RunOpts{Dir: tmpBasePath}) + _, _, conflictErr := git.NewCommand(ctx, "merge-file").AddDynamicArguments(base, root, head).RunStdString(&git.RunOpts{Dir: tmpBasePath}) if conflictErr != nil { return &errMergeConflict{file.stage2.path} } // base now contains the merged data - hash, _, err := git.NewCommand(ctx, "hash-object", "-w", "--path", file.stage2.path, base).RunStdString(&git.RunOpts{Dir: tmpBasePath}) + hash, _, err := git.NewCommand(ctx, "hash-object", "-w", "--path").AddDynamicArguments(file.stage2.path, base).RunStdString(&git.RunOpts{Dir: tmpBasePath}) if err != nil { return err } @@ -235,7 +235,7 @@ func AttemptThreeWayMerge(ctx context.Context, gitPath string, gitRepo *git.Repo defer cancel() // First we use read-tree to do a simple three-way merge - if _, _, err := git.NewCommand(ctx, "read-tree", "-m", base, ours, theirs).RunStdString(&git.RunOpts{Dir: gitPath}); err != nil { + if _, _, err := git.NewCommand(ctx, "read-tree", "-m").AddDynamicArguments(base, ours, theirs).RunStdString(&git.RunOpts{Dir: gitPath}); err != nil { log.Error("Unable to run read-tree -m! Error: %v", err) return false, nil, fmt.Errorf("unable to run read-tree -m! Error: %v", err) } @@ -361,7 +361,7 @@ func checkConflicts(ctx context.Context, pr *issues_model.PullRequest, gitRepo * prConfig := prUnit.PullRequestsConfig() // 6. Prepare the arguments to apply the patch against the index - args := []string{"apply", "--check", "--cached"} + args := []git.CmdArg{"apply", "--check", "--cached"} if prConfig.IgnoreWhitespaceConflicts { args = append(args, "--ignore-whitespace") } @@ -370,7 +370,7 @@ func checkConflicts(ctx context.Context, pr *issues_model.PullRequest, gitRepo * args = append(args, "--3way") is3way = true } - args = append(args, patchPath) + args = append(args, git.CmdArgCheck(patchPath)) // 7. Prep the pipe: // - Here we could do the equivalent of: diff --git a/services/pull/pull.go b/services/pull/pull.go index 9de7cb5d4f8e..3b58c675a37f 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -490,7 +490,7 @@ func UpdateRef(ctx context.Context, pr *issues_model.PullRequest) (err error) { return err } - _, _, err = git.NewCommand(ctx, "update-ref", pr.GetGitRefName(), pr.HeadCommitID).RunStdString(&git.RunOpts{Dir: pr.BaseRepo.RepoPath()}) + _, _, err = git.NewCommand(ctx, "update-ref").AddDynamicArguments(pr.GetGitRefName(), pr.HeadCommitID).RunStdString(&git.RunOpts{Dir: pr.BaseRepo.RepoPath()}) if err != nil { log.Error("Unable to update ref in base repository for PR[%d] Error: %v", pr.ID, err) } diff --git a/services/pull/temp_repo.go b/services/pull/temp_repo.go index c1456ef0a99e..6624f5659b60 100644 --- a/services/pull/temp_repo.go +++ b/services/pull/temp_repo.go @@ -94,7 +94,7 @@ func createTemporaryRepo(ctx context.Context, pr *issues_model.PullRequest) (str } var outbuf, errbuf strings.Builder - if err := git.NewCommand(ctx, "remote", "add", "-t", pr.BaseBranch, "-m", pr.BaseBranch, "origin", baseRepoPath). + if err := git.NewCommand(ctx, "remote", "add", "-t").AddDynamicArguments(pr.BaseBranch).AddArguments("-m").AddDynamicArguments(pr.BaseBranch).AddDynamicArguments("origin", baseRepoPath). Run(&git.RunOpts{ Dir: tmpBasePath, Stdout: &outbuf, @@ -109,7 +109,7 @@ func createTemporaryRepo(ctx context.Context, pr *issues_model.PullRequest) (str outbuf.Reset() errbuf.Reset() - if err := git.NewCommand(ctx, "fetch", "origin", "--no-tags", "--", pr.BaseBranch+":"+baseBranch, pr.BaseBranch+":original_"+baseBranch). + if err := git.NewCommand(ctx, "fetch", "origin", "--no-tags").AddDashesAndList(pr.BaseBranch+":"+baseBranch, pr.BaseBranch+":original_"+baseBranch). Run(&git.RunOpts{ Dir: tmpBasePath, Stdout: &outbuf, @@ -124,7 +124,7 @@ func createTemporaryRepo(ctx context.Context, pr *issues_model.PullRequest) (str outbuf.Reset() errbuf.Reset() - if err := git.NewCommand(ctx, "symbolic-ref", "HEAD", git.BranchPrefix+baseBranch). + if err := git.NewCommand(ctx, "symbolic-ref").AddDynamicArguments("HEAD", git.BranchPrefix+baseBranch). Run(&git.RunOpts{ Dir: tmpBasePath, Stdout: &outbuf, @@ -147,7 +147,7 @@ func createTemporaryRepo(ctx context.Context, pr *issues_model.PullRequest) (str return "", fmt.Errorf("Unable to head base repository to temporary repo [%s -> tmpBasePath]: %v", pr.HeadRepo.FullName(), err) } - if err := git.NewCommand(ctx, "remote", "add", remoteRepoName, headRepoPath). + if err := git.NewCommand(ctx, "remote", "add").AddDynamicArguments(remoteRepoName, headRepoPath). Run(&git.RunOpts{ Dir: tmpBasePath, Stdout: &outbuf, @@ -172,7 +172,7 @@ func createTemporaryRepo(ctx context.Context, pr *issues_model.PullRequest) (str } else { headBranch = pr.GetGitRefName() } - if err := git.NewCommand(ctx, "fetch", "--no-tags", remoteRepoName, headBranch+":"+trackingBranch). + if err := git.NewCommand(ctx, "fetch", "--no-tags").AddDynamicArguments(remoteRepoName, headBranch+":"+trackingBranch). Run(&git.RunOpts{ Dir: tmpBasePath, Stdout: &outbuf, diff --git a/services/release/release.go b/services/release/release.go index af1b07523208..25b89e1a0b6d 100644 --- a/services/release/release.go +++ b/services/release/release.go @@ -309,7 +309,7 @@ func DeleteReleaseByID(ctx context.Context, id int64, doer *user_model.User, del } } - if stdout, _, err := git.NewCommand(ctx, "tag", "-d", "--", rel.TagName). + if stdout, _, err := git.NewCommand(ctx, "tag", "-d").AddDashesAndList(rel.TagName). SetDescription(fmt.Sprintf("DeleteReleaseByID (git tag -d): %d", rel.ID)). RunStdString(&git.RunOpts{Dir: repo.RepoPath()}); err != nil && !strings.Contains(err.Error(), "not found") { log.Error("DeleteReleaseByID (git tag -d): %d in %v Failed:\nStdout: %s\nError: %v", rel.ID, repo, stdout, err) diff --git a/services/repository/check.go b/services/repository/check.go index 2417db6a271d..1b2da4cc0823 100644 --- a/services/repository/check.go +++ b/services/repository/check.go @@ -24,7 +24,7 @@ import ( ) // GitFsck calls 'git fsck' to check repository health. -func GitFsck(ctx context.Context, timeout time.Duration, args []string) error { +func GitFsck(ctx context.Context, timeout time.Duration, args []git.CmdArg) error { log.Trace("Doing: GitFsck") if err := db.Iterate( @@ -58,9 +58,9 @@ func GitFsck(ctx context.Context, timeout time.Duration, args []string) error { } // GitGcRepos calls 'git gc' to remove unnecessary files and optimize the local repository -func GitGcRepos(ctx context.Context, timeout time.Duration, args ...string) error { +func GitGcRepos(ctx context.Context, timeout time.Duration, args ...git.CmdArg) error { log.Trace("Doing: GitGcRepos") - args = append([]string{"gc"}, args...) + args = append([]git.CmdArg{"gc"}, args...) if err := db.Iterate( ctx, diff --git a/services/repository/files/patch.go b/services/repository/files/patch.go index d55d793f28cd..437890c488c6 100644 --- a/services/repository/files/patch.go +++ b/services/repository/files/patch.go @@ -139,7 +139,7 @@ func ApplyDiffPatch(ctx context.Context, repo *repo_model.Repository, doer *user stdout := &strings.Builder{} stderr := &strings.Builder{} - args := []string{"apply", "--index", "--recount", "--cached", "--ignore-whitespace", "--whitespace=fix", "--binary"} + args := []git.CmdArg{"apply", "--index", "--recount", "--cached", "--ignore-whitespace", "--whitespace=fix", "--binary"} if git.CheckGitVersionAtLeast("2.32") == nil { args = append(args, "-3") diff --git a/services/repository/files/temp_repo.go b/services/repository/files/temp_repo.go index 1e60d55613fa..9513f2341e15 100644 --- a/services/repository/files/temp_repo.go +++ b/services/repository/files/temp_repo.go @@ -53,7 +53,7 @@ func (t *TemporaryUploadRepository) Close() { // Clone the base repository to our path and set branch as the HEAD func (t *TemporaryUploadRepository) Clone(branch string) error { - if _, _, err := git.NewCommand(t.ctx, "clone", "-s", "--bare", "-b", branch, t.repo.RepoPath(), t.basePath).RunStdString(nil); err != nil { + if _, _, err := git.NewCommand(t.ctx, "clone", "-s", "--bare", "-b").AddDynamicArguments(branch, t.repo.RepoPath(), t.basePath).RunStdString(nil); err != nil { stderr := err.Error() if matched, _ := regexp.MatchString(".*Remote branch .* not found in upstream origin.*", stderr); matched { return git.ErrBranchNotExist{ @@ -104,14 +104,7 @@ func (t *TemporaryUploadRepository) LsFiles(filenames ...string) ([]string, erro stdOut := new(bytes.Buffer) stdErr := new(bytes.Buffer) - cmdArgs := []string{"ls-files", "-z", "--"} - for _, arg := range filenames { - if arg != "" { - cmdArgs = append(cmdArgs, arg) - } - } - - if err := git.NewCommand(t.ctx, cmdArgs...). + if err := git.NewCommand(t.ctx, "ls-files", "-z").AddDashesAndList(filenames...). Run(&git.RunOpts{ Dir: t.basePath, Stdout: stdOut, @@ -177,7 +170,7 @@ func (t *TemporaryUploadRepository) HashObject(content io.Reader) (string, error // AddObjectToIndex adds the provided object hash to the index with the provided mode and path func (t *TemporaryUploadRepository) AddObjectToIndex(mode, objectHash, objectPath string) error { - if _, _, err := git.NewCommand(t.ctx, "update-index", "--add", "--replace", "--cacheinfo", mode, objectHash, objectPath).RunStdString(&git.RunOpts{Dir: t.basePath}); err != nil { + if _, _, err := git.NewCommand(t.ctx, "update-index", "--add", "--replace", "--cacheinfo").AddDynamicArguments(mode, objectHash, objectPath).RunStdString(&git.RunOpts{Dir: t.basePath}); err != nil { stderr := err.Error() if matched, _ := regexp.MatchString(".*Invalid path '.*", stderr); matched { return models.ErrFilePathInvalid{ @@ -211,7 +204,7 @@ func (t *TemporaryUploadRepository) GetLastCommitByRef(ref string) (string, erro if ref == "" { ref = "HEAD" } - stdout, _, err := git.NewCommand(t.ctx, "rev-parse", ref).RunStdString(&git.RunOpts{Dir: t.basePath}) + stdout, _, err := git.NewCommand(t.ctx, "rev-parse").AddDynamicArguments(ref).RunStdString(&git.RunOpts{Dir: t.basePath}) if err != nil { log.Error("Unable to get last ref for %s in temporary repo: %s(%s): Error: %v", ref, t.repo.FullName(), t.basePath, err) return "", fmt.Errorf("Unable to rev-parse %s in temporary repo for: %s Error: %v", ref, t.repo.FullName(), err) @@ -241,11 +234,11 @@ func (t *TemporaryUploadRepository) CommitTreeWithDate(parent string, author, co _, _ = messageBytes.WriteString(message) _, _ = messageBytes.WriteString("\n") - var args []string + var args []git.CmdArg if parent != "" { - args = []string{"commit-tree", treeHash, "-p", parent} + args = []git.CmdArg{"commit-tree", git.CmdArgCheck(treeHash), "-p", git.CmdArgCheck(parent)} } else { - args = []string{"commit-tree", treeHash} + args = []git.CmdArg{"commit-tree", git.CmdArgCheck(treeHash)} } var sign bool @@ -257,7 +250,7 @@ func (t *TemporaryUploadRepository) CommitTreeWithDate(parent string, author, co sign, keyID, signer, _ = asymkey_service.SignInitialCommit(t.ctx, t.repo.RepoPath(), author) } if sign { - args = append(args, "-S"+keyID) + args = append(args, git.CmdArg("-S"+keyID)) if t.repo.GetTrustModel() == repo_model.CommitterTrustModel || t.repo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel { if committerSig.Name != authorSig.Name || committerSig.Email != authorSig.Email { // Add trailers diff --git a/services/repository/files/update.go b/services/repository/files/update.go index 4615a9153a68..87c622a9be3c 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -370,7 +370,7 @@ func CreateOrUpdateRepoFile(ctx context.Context, repo *repo_model.Repository, do if setting.LFS.StartServer && hasOldBranch { // Check there is no way this can return multiple infos filename2attribute2info, err := t.gitRepo.CheckAttribute(git.CheckAttributeOpts{ - Attributes: []string{"filter"}, + Attributes: []git.CmdArg{"filter"}, Filenames: []string{treePath}, CachedOnly: true, }) diff --git a/services/repository/files/upload.go b/services/repository/files/upload.go index 327a2e121ca5..b2dadb6497c5 100644 --- a/services/repository/files/upload.go +++ b/services/repository/files/upload.go @@ -96,7 +96,7 @@ func UploadRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use var filename2attribute2info map[string]map[string]string if setting.LFS.StartServer { filename2attribute2info, err = t.gitRepo.CheckAttribute(git.CheckAttributeOpts{ - Attributes: []string{"filter"}, + Attributes: []git.CmdArg{"filter"}, Filenames: names, CachedOnly: true, }) diff --git a/services/repository/fork.go b/services/repository/fork.go index 32a516b79f99..af0ade99a947 100644 --- a/services/repository/fork.go +++ b/services/repository/fork.go @@ -130,7 +130,7 @@ func ForkRepository(ctx context.Context, doer, owner *user_model.User, opts Fork repoPath := repo_model.RepoPath(owner.Name, repo.Name) if stdout, _, err := git.NewCommand(txCtx, - "clone", "--bare", oldRepoPath, repoPath). + "clone", "--bare").AddDynamicArguments(oldRepoPath, repoPath). SetDescription(fmt.Sprintf("ForkRepository(git clone): %s to %s", opts.BaseRepo.FullName(), repo.FullName())). RunStdBytes(&git.RunOpts{Timeout: 10 * time.Minute}); err != nil { log.Error("Fork Repository (git clone) Failed for %v (from %v):\nStdout: %s\nError: %v", repo, opts.BaseRepo, stdout, err) diff --git a/tests/integration/api_repo_git_tags_test.go b/tests/integration/api_repo_git_tags_test.go index 855eb2451e80..3357f9568dbd 100644 --- a/tests/integration/api_repo_git_tags_test.go +++ b/tests/integration/api_repo_git_tags_test.go @@ -29,8 +29,8 @@ func TestAPIGitTags(t *testing.T) { token := getTokenForLoggedInUser(t, session) // Set up git config for the tagger - _ = git.NewCommand(git.DefaultContext, "config", "user.name", user.Name).Run(&git.RunOpts{Dir: repo.RepoPath()}) - _ = git.NewCommand(git.DefaultContext, "config", "user.email", user.Email).Run(&git.RunOpts{Dir: repo.RepoPath()}) + _ = git.NewCommand(git.DefaultContext, "config", "user.name").AddDynamicArguments(user.Name).Run(&git.RunOpts{Dir: repo.RepoPath()}) + _ = git.NewCommand(git.DefaultContext, "config", "user.email").AddDynamicArguments(user.Email).Run(&git.RunOpts{Dir: repo.RepoPath()}) gitRepo, _ := git.OpenRepository(git.DefaultContext, repo.RepoPath()) defer gitRepo.Close() diff --git a/tests/integration/git_clone_wiki_test.go b/tests/integration/git_clone_wiki_test.go index 3e10b17d40be..e8764e6bd9f5 100644 --- a/tests/integration/git_clone_wiki_test.go +++ b/tests/integration/git_clone_wiki_test.go @@ -41,7 +41,7 @@ func TestRepoCloneWiki(t *testing.T) { u, _ = url.Parse(r) u.User = url.UserPassword("user2", userPassword) t.Run("Clone", func(t *testing.T) { - assert.NoError(t, git.CloneWithArgs(context.Background(), u.String(), dstPath, git.AllowLFSFiltersArgs(), git.CloneRepoOptions{})) + assert.NoError(t, git.CloneWithArgs(context.Background(), git.AllowLFSFiltersArgs(), u.String(), dstPath, git.CloneRepoOptions{})) assertFileEqual(t, filepath.Join(dstPath, "Home.md"), []byte("# Home page\n\nThis is the home page!\n")) assertFileExist(t, filepath.Join(dstPath, "Page-With-Image.md")) assertFileExist(t, filepath.Join(dstPath, "Page-With-Spaced-Name.md")) diff --git a/tests/integration/git_helper_for_declarative_test.go b/tests/integration/git_helper_for_declarative_test.go index 4f2e540883a0..45b4f4447510 100644 --- a/tests/integration/git_helper_for_declarative_test.go +++ b/tests/integration/git_helper_for_declarative_test.go @@ -98,7 +98,7 @@ func onGiteaRun(t *testing.T, callback func(*testing.T, *url.URL), prepare ...bo func doGitClone(dstLocalPath string, u *url.URL) func(*testing.T) { return func(t *testing.T) { - assert.NoError(t, git.CloneWithArgs(context.Background(), u.String(), dstLocalPath, git.AllowLFSFiltersArgs(), git.CloneRepoOptions{})) + assert.NoError(t, git.CloneWithArgs(context.Background(), git.AllowLFSFiltersArgs(), u.String(), dstLocalPath, git.CloneRepoOptions{})) exist, err := util.IsExist(filepath.Join(dstLocalPath, "README.md")) assert.NoError(t, err) assert.True(t, exist) @@ -107,7 +107,7 @@ func doGitClone(dstLocalPath string, u *url.URL) func(*testing.T) { func doPartialGitClone(dstLocalPath string, u *url.URL) func(*testing.T) { return func(t *testing.T) { - assert.NoError(t, git.CloneWithArgs(context.Background(), u.String(), dstLocalPath, git.AllowLFSFiltersArgs(), git.CloneRepoOptions{ + assert.NoError(t, git.CloneWithArgs(context.Background(), git.AllowLFSFiltersArgs(), u.String(), dstLocalPath, git.CloneRepoOptions{ Filter: "blob:none", })) exist, err := util.IsExist(filepath.Join(dstLocalPath, "README.md")) @@ -150,47 +150,47 @@ func doGitInitTestRepository(dstPath string) func(*testing.T) { func doGitAddRemote(dstPath, remoteName string, u *url.URL) func(*testing.T) { return func(t *testing.T) { - _, _, err := git.NewCommand(git.DefaultContext, "remote", "add", remoteName, u.String()).RunStdString(&git.RunOpts{Dir: dstPath}) + _, _, err := git.NewCommand(git.DefaultContext, "remote", "add").AddDynamicArguments(remoteName, u.String()).RunStdString(&git.RunOpts{Dir: dstPath}) assert.NoError(t, err) } } -func doGitPushTestRepository(dstPath string, args ...string) func(*testing.T) { +func doGitPushTestRepository(dstPath string, args ...git.CmdArg) func(*testing.T) { return func(t *testing.T) { - _, _, err := git.NewCommand(git.DefaultContext, append([]string{"push", "-u"}, args...)...).RunStdString(&git.RunOpts{Dir: dstPath}) + _, _, err := git.NewCommand(git.DefaultContext, append([]git.CmdArg{"push", "-u"}, args...)...).RunStdString(&git.RunOpts{Dir: dstPath}) assert.NoError(t, err) } } -func doGitPushTestRepositoryFail(dstPath string, args ...string) func(*testing.T) { +func doGitPushTestRepositoryFail(dstPath string, args ...git.CmdArg) func(*testing.T) { return func(t *testing.T) { - _, _, err := git.NewCommand(git.DefaultContext, append([]string{"push"}, args...)...).RunStdString(&git.RunOpts{Dir: dstPath}) + _, _, err := git.NewCommand(git.DefaultContext, append([]git.CmdArg{"push"}, args...)...).RunStdString(&git.RunOpts{Dir: dstPath}) assert.Error(t, err) } } func doGitCreateBranch(dstPath, branch string) func(*testing.T) { return func(t *testing.T) { - _, _, err := git.NewCommand(git.DefaultContext, "checkout", "-b", branch).RunStdString(&git.RunOpts{Dir: dstPath}) + _, _, err := git.NewCommand(git.DefaultContext, "checkout", "-b").AddDynamicArguments(branch).RunStdString(&git.RunOpts{Dir: dstPath}) assert.NoError(t, err) } } -func doGitCheckoutBranch(dstPath string, args ...string) func(*testing.T) { +func doGitCheckoutBranch(dstPath string, args ...git.CmdArg) func(*testing.T) { return func(t *testing.T) { _, _, err := git.NewCommandNoGlobals(append(append(git.AllowLFSFiltersArgs(), "checkout"), args...)...).RunStdString(&git.RunOpts{Dir: dstPath}) assert.NoError(t, err) } } -func doGitMerge(dstPath string, args ...string) func(*testing.T) { +func doGitMerge(dstPath string, args ...git.CmdArg) func(*testing.T) { return func(t *testing.T) { - _, _, err := git.NewCommand(git.DefaultContext, append([]string{"merge"}, args...)...).RunStdString(&git.RunOpts{Dir: dstPath}) + _, _, err := git.NewCommand(git.DefaultContext, append([]git.CmdArg{"merge"}, args...)...).RunStdString(&git.RunOpts{Dir: dstPath}) assert.NoError(t, err) } } -func doGitPull(dstPath string, args ...string) func(*testing.T) { +func doGitPull(dstPath string, args ...git.CmdArg) func(*testing.T) { return func(t *testing.T) { _, _, err := git.NewCommandNoGlobals(append(append(git.AllowLFSFiltersArgs(), "pull"), args...)...).RunStdString(&git.RunOpts{Dir: dstPath}) assert.NoError(t, err) diff --git a/tests/integration/git_test.go b/tests/integration/git_test.go index bf5e809dab59..6f656ef2ce6d 100644 --- a/tests/integration/git_test.go +++ b/tests/integration/git_test.go @@ -151,7 +151,7 @@ func lfsCommitAndPushTest(t *testing.T, dstPath string) (littleLFS, bigLFS strin prefix := "lfs-data-file-" err := git.NewCommand(git.DefaultContext, "lfs").AddArguments("install").Run(&git.RunOpts{Dir: dstPath}) assert.NoError(t, err) - _, _, err = git.NewCommand(git.DefaultContext, "lfs").AddArguments("track", prefix+"*").RunStdString(&git.RunOpts{Dir: dstPath}) + _, _, err = git.NewCommand(git.DefaultContext, "lfs").AddArguments("track").AddDynamicArguments(prefix + "*").RunStdString(&git.RunOpts{Dir: dstPath}) assert.NoError(t, err) err = git.AddChanges(dstPath, false, ".gitattributes") assert.NoError(t, err) @@ -279,11 +279,11 @@ func lockTest(t *testing.T, repoPath string) { func lockFileTest(t *testing.T, filename, repoPath string) { _, _, err := git.NewCommand(git.DefaultContext, "lfs").AddArguments("locks").RunStdString(&git.RunOpts{Dir: repoPath}) assert.NoError(t, err) - _, _, err = git.NewCommand(git.DefaultContext, "lfs").AddArguments("lock", filename).RunStdString(&git.RunOpts{Dir: repoPath}) + _, _, err = git.NewCommand(git.DefaultContext, "lfs").AddArguments("lock").AddDynamicArguments(filename).RunStdString(&git.RunOpts{Dir: repoPath}) assert.NoError(t, err) _, _, err = git.NewCommand(git.DefaultContext, "lfs").AddArguments("locks").RunStdString(&git.RunOpts{Dir: repoPath}) assert.NoError(t, err) - _, _, err = git.NewCommand(git.DefaultContext, "lfs").AddArguments("unlock", filename).RunStdString(&git.RunOpts{Dir: repoPath}) + _, _, err = git.NewCommand(git.DefaultContext, "lfs").AddArguments("unlock").AddDynamicArguments(filename).RunStdString(&git.RunOpts{Dir: repoPath}) assert.NoError(t, err) } @@ -509,7 +509,7 @@ func doCreatePRAndSetManuallyMerged(ctx, baseCtx APITestContext, dstPath, baseBr })) t.Run("CreateHeadBranch", doGitCreateBranch(dstPath, headBranch)) - t.Run("PushToHeadBranch", doGitPushTestRepository(dstPath, "origin", headBranch)) + t.Run("PushToHeadBranch", doGitPushTestRepository(dstPath, "origin", git.CmdArgCheck(headBranch))) t.Run("CreateEmptyPullRequest", func(t *testing.T) { pr, err = doAPICreatePullRequest(ctx, baseCtx.Username, baseCtx.Reponame, baseBranch, headBranch)(t) assert.NoError(t, err) @@ -736,7 +736,7 @@ func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, baseBranch, headB }) t.Run("Push", func(t *testing.T) { - err := git.NewCommand(git.DefaultContext, "push", "origin", "HEAD:refs/for/master", "-o", "topic="+headBranch).Run(&git.RunOpts{Dir: dstPath}) + err := git.NewCommand(git.DefaultContext, "push", "origin", "HEAD:refs/for/master", "-o").AddDynamicArguments("topic=" + headBranch).Run(&git.RunOpts{Dir: dstPath}) if !assert.NoError(t, err) { return } @@ -757,7 +757,7 @@ func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, baseBranch, headB assert.Contains(t, "Testing commit 1", prMsg.Body) assert.Equal(t, commit, prMsg.Head.Sha) - _, _, err = git.NewCommand(git.DefaultContext, "push", "origin", "HEAD:refs/for/master/test/"+headBranch).RunStdString(&git.RunOpts{Dir: dstPath}) + _, _, err = git.NewCommand(git.DefaultContext, "push", "origin").AddDynamicArguments("HEAD:refs/for/master/test/" + headBranch).RunStdString(&git.RunOpts{Dir: dstPath}) if !assert.NoError(t, err) { return } @@ -810,7 +810,7 @@ func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, baseBranch, headB }) t.Run("Push2", func(t *testing.T) { - err := git.NewCommand(git.DefaultContext, "push", "origin", "HEAD:refs/for/master", "-o", "topic="+headBranch).Run(&git.RunOpts{Dir: dstPath}) + err := git.NewCommand(git.DefaultContext, "push", "origin", "HEAD:refs/for/master", "-o").AddDynamicArguments("topic=" + headBranch).Run(&git.RunOpts{Dir: dstPath}) if !assert.NoError(t, err) { return } @@ -822,7 +822,7 @@ func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, baseBranch, headB assert.Equal(t, false, prMsg.HasMerged) assert.Equal(t, commit, prMsg.Head.Sha) - _, _, err = git.NewCommand(git.DefaultContext, "push", "origin", "HEAD:refs/for/master/test/"+headBranch).RunStdString(&git.RunOpts{Dir: dstPath}) + _, _, err = git.NewCommand(git.DefaultContext, "push", "origin").AddDynamicArguments("HEAD:refs/for/master/test/" + headBranch).RunStdString(&git.RunOpts{Dir: dstPath}) if !assert.NoError(t, err) { return } diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index 335dae4b38ad..9bd430084dcc 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -287,7 +287,7 @@ func TestCantMergeUnrelated(t *testing.T) { assert.NoError(t, err) sha := strings.TrimSpace(stdout.String()) - _, _, err = git.NewCommand(git.DefaultContext, "update-index", "--add", "--replace", "--cacheinfo", "100644", sha, "somewher-over-the-rainbow").RunStdString(&git.RunOpts{Dir: path}) + _, _, err = git.NewCommand(git.DefaultContext, "update-index", "--add", "--replace", "--cacheinfo", "100644", git.CmdArgCheck(sha), "somewher-over-the-rainbow").RunStdString(&git.RunOpts{Dir: path}) assert.NoError(t, err) treeSha, _, err := git.NewCommand(git.DefaultContext, "write-tree").RunStdString(&git.RunOpts{Dir: path}) @@ -310,7 +310,7 @@ func TestCantMergeUnrelated(t *testing.T) { _, _ = messageBytes.WriteString("\n") stdout.Reset() - err = git.NewCommand(git.DefaultContext, "commit-tree", treeSha). + err = git.NewCommand(git.DefaultContext, "commit-tree").AddDynamicArguments(treeSha). Run(&git.RunOpts{ Env: env, Dir: path, @@ -320,7 +320,7 @@ func TestCantMergeUnrelated(t *testing.T) { assert.NoError(t, err) commitSha := strings.TrimSpace(stdout.String()) - _, _, err = git.NewCommand(git.DefaultContext, "branch", "unrelated", commitSha).RunStdString(&git.RunOpts{Dir: path}) + _, _, err = git.NewCommand(git.DefaultContext, "branch", "unrelated").AddDynamicArguments(commitSha).RunStdString(&git.RunOpts{Dir: path}) assert.NoError(t, err) testEditFileToNewBranch(t, session, "user1", "repo1", "master", "conflict", "README.md", "Hello, World (Edited Once)\n") From 578b43ddb5c7fc686b4cbd52034b623d7337ab4e Mon Sep 17 00:00:00 2001 From: Yarden Shoham Date: Sun, 23 Oct 2022 19:09:21 +0300 Subject: [PATCH 16/20] Add yardenshoham to maintainers (#21566) [List of merged PRs](https://github.com/go-gitea/gitea/pulls?q=is%3Apr+author%3Ayardenshoham+is%3Amerged) (with many more on the way!) Co-authored-by: Lauris BH --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 8c4af7432355..660f35607c32 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -49,3 +49,4 @@ silentcode (@silentcodeg) Wim (@42wim) xinyu (@penlinux) Jason Song (@wolfogre) +Yarden Shoham (@yardenshoham) From bf2a72d1269a598a8f50c796bb6e4946200436c3 Mon Sep 17 00:00:00 2001 From: silverwind Date: Mon, 24 Oct 2022 03:23:04 +0200 Subject: [PATCH 17/20] Expand "Go to File" button again, fix 'Add File' margin (#21543) With https://github.com/go-gitea/gitea/pull/21428 we gained some space so we are again able to show the "Go to File" button as text instead of icon-only (the old icon was not particularily fitting anyways). Before: image After: Screen Shot 2022-10-22 at 12 28 01 Screen Shot 2022-10-22 at 12 28 16 --- templates/repo/home.tmpl | 2 +- web_src/less/_repository.less | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/templates/repo/home.tmpl b/templates/repo/home.tmpl index 1e82d97944fd..69eaf17429d0 100644 --- a/templates/repo/home.tmpl +++ b/templates/repo/home.tmpl @@ -74,7 +74,7 @@ {{end}} - {{svg "octicon-file-moved" 15}} + {{.locale.Tr "repo.find_file.go_to_file"}} {{end}} {{if or .CanAddFile .CanUploadFile}}