From 70bd54a21e08730320b452c8112a17e9850c4e66 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 15 Oct 2022 00:40:20 +0200 Subject: [PATCH 01/11] improve code quality --- modules/git/repo_stats.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/git/repo_stats.go b/modules/git/repo_stats.go index cb44c58cec7e5..5e2c1d5d74d11 100644 --- a/modules/git/repo_stats.go +++ b/modules/git/repo_stats.go @@ -65,7 +65,7 @@ func (repo *Repository) GetCodeActivityStats(fromTime time.Time, branch string) if len(branch) == 0 { args = append(args, "--branches=*") } else { - args = append(args, "--first-parent", branch) + args = append(args, "--first-parent", "--", branch) } stderr := new(strings.Builder) From dff2138b27d269c272f2bbe01179d270a0caf850 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 15 Oct 2022 01:12:46 +0200 Subject: [PATCH 02/11] make searchCommits more explicite --- modules/git/repo_commit.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index d3731cb928e30..dddf5fe9557c1 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -106,23 +106,23 @@ func (repo *Repository) searchCommits(id SHA1, opts SearchCommitsOptions) ([]*Co // 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, "--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, "--committer='"+v+"'") } } // add time constraints if present in search query if len(opts.After) > 0 { - args = append(args, "--after="+opts.After) + args = append(args, "--after='"+opts.After+"'") } if len(opts.Before) > 0 { - args = append(args, "--before="+opts.Before) + args = append(args, "--before='"+opts.Before+"'") } // pretend that all refs along with HEAD were listed on command line as @@ -136,7 +136,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("--grep='" + v + "'") } } @@ -161,7 +161,7 @@ func (repo *Repository) searchCommits(id SHA1, opts SearchCommitsOptions) ([]*Co // add previous arguments except for --grep and --all hashCmd.AddArguments(args...) // add keyword as - hashCmd.AddArguments(v) + hashCmd.AddArguments("--end-of-options", "'"+v+"'") // search with given constraints for commit matching sha hash of v hashMatching, _, err := hashCmd.RunStdBytes(&RunOpts{Dir: repo.Path}) From 721e0811f47b6d92d5bf504fad270101f1a37791 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 15 Oct 2022 01:36:18 +0200 Subject: [PATCH 03/11] reorder stuff --- modules/git/repo_commit.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index dddf5fe9557c1..45bdfdfeb3e0d 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -211,9 +211,11 @@ func (repo *Repository) CommitsByFileAndRange(revision, file string, page int) ( }() go func() { stderr := strings.Builder{} - err := NewCommand(repo.Ctx, "rev-list", revision, + err := NewCommand(repo.Ctx, "rev-list", "--max-count="+strconv.Itoa(setting.Git.CommitsRangeSize*page), - "--skip="+strconv.Itoa(skip), "--", file). + "--skip="+strconv.Itoa(skip), + "--end-of-options", revision, + "--", file). Run(&RunOpts{ Dir: repo.Path, Stdout: stdoutWriter, From 38a631896df48f4810c446d34c046f452584496a Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 15 Oct 2022 01:58:19 +0200 Subject: [PATCH 04/11] some more trimming --- modules/git/commit.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/modules/git/commit.go b/modules/git/commit.go index 32589f534980c..3934444d6f0cf 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -253,15 +253,15 @@ func NewSearchCommitsOptions(searchString string, forAllRefs bool) SearchCommits for _, k := range fields { switch { case strings.HasPrefix(k, "author:"): - authors = append(authors, strings.TrimPrefix(k, "author:")) + authors = append(authors, strings.ReplaceAll(strings.TrimSpace(strings.TrimPrefix(k, "author:")), "'", "")) case strings.HasPrefix(k, "committer:"): - committers = append(committers, strings.TrimPrefix(k, "committer:")) + committers = append(committers, strings.ReplaceAll(strings.TrimSpace(strings.TrimPrefix(k, "committer:")), "'", "")) case strings.HasPrefix(k, "after:"): - after = strings.TrimPrefix(k, "after:") + after = strings.ReplaceAll(strings.TrimSpace(strings.TrimPrefix(k, "after:")), "'", "") case strings.HasPrefix(k, "before:"): - before = strings.TrimPrefix(k, "before:") + before = strings.ReplaceAll(strings.TrimSpace(strings.TrimPrefix(k, "before:")), "'", "") default: - keywords = append(keywords, k) + keywords = append(keywords, strings.ReplaceAll(strings.TrimSpace(k), "'", "")) } } From 269ba73fa4f7464b185e056883404716d5f5a821 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 15 Oct 2022 02:03:27 +0200 Subject: [PATCH 05/11] nit --- modules/git/repo_commit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index 45bdfdfeb3e0d..ffe5a7834e8b3 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -214,7 +214,7 @@ func (repo *Repository) CommitsByFileAndRange(revision, file string, page int) ( err := NewCommand(repo.Ctx, "rev-list", "--max-count="+strconv.Itoa(setting.Git.CommitsRangeSize*page), "--skip="+strconv.Itoa(skip), - "--end-of-options", revision, + "--end-of-options", "'"+revision+"'", "--", file). Run(&RunOpts{ Dir: repo.Path, From f5d5a3d3d1220f5ba892559dd7fdd1831895d863 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 15 Oct 2022 02:33:12 +0200 Subject: [PATCH 06/11] cmd-args-respect-args --- modules/git/repo_commit.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index ffe5a7834e8b3..78c33163c336f 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -106,23 +106,23 @@ func (repo *Repository) searchCommits(id SHA1, opts SearchCommitsOptions) ([]*Co // 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, "--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, "--committer="+v) } } // add time constraints if present in search query if len(opts.After) > 0 { - args = append(args, "--after='"+opts.After+"'") + args = append(args, "--after="+opts.After) } if len(opts.Before) > 0 { - args = append(args, "--before='"+opts.Before+"'") + args = append(args, "--before="+opts.Before) } // pretend that all refs along with HEAD were listed on command line as @@ -136,7 +136,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("--grep=" + v) } } @@ -161,7 +161,7 @@ func (repo *Repository) searchCommits(id SHA1, opts SearchCommitsOptions) ([]*Co // add previous arguments except for --grep and --all hashCmd.AddArguments(args...) // add keyword as - hashCmd.AddArguments("--end-of-options", "'"+v+"'") + hashCmd.AddArguments("--end-of-options", ""+v+"") // search with given constraints for commit matching sha hash of v hashMatching, _, err := hashCmd.RunStdBytes(&RunOpts{Dir: repo.Path}) From 3d647a55d46347086d74114ac182a10c355152ec Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 15 Oct 2022 02:34:47 +0200 Subject: [PATCH 07/11] Update modules/git/repo_commit.go --- modules/git/repo_commit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index 78c33163c336f..70d4084c59799 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -161,7 +161,7 @@ func (repo *Repository) searchCommits(id SHA1, opts SearchCommitsOptions) ([]*Co // add previous arguments except for --grep and --all hashCmd.AddArguments(args...) // add keyword as - hashCmd.AddArguments("--end-of-options", ""+v+"") + hashCmd.AddArguments("--end-of-options", v) // search with given constraints for commit matching sha hash of v hashMatching, _, err := hashCmd.RunStdBytes(&RunOpts{Dir: repo.Path}) From 2e39d02490e14342ebeaade1f0fe16e4050ce19a Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 15 Oct 2022 02:35:18 +0200 Subject: [PATCH 08/11] Update modules/git/repo_commit.go --- modules/git/repo_commit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index 70d4084c59799..6604fd02875a9 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -214,7 +214,7 @@ func (repo *Repository) CommitsByFileAndRange(revision, file string, page int) ( err := NewCommand(repo.Ctx, "rev-list", "--max-count="+strconv.Itoa(setting.Git.CommitsRangeSize*page), "--skip="+strconv.Itoa(skip), - "--end-of-options", "'"+revision+"'", + "--end-of-options", revision, "--", file). Run(&RunOpts{ Dir: repo.Path, From d528d665aa1553a06831fa08c45a3dfce060d782 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 15 Oct 2022 04:31:40 +0200 Subject: [PATCH 09/11] rm --- modules/git/commit.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/modules/git/commit.go b/modules/git/commit.go index 3934444d6f0cf..32589f534980c 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -253,15 +253,15 @@ func NewSearchCommitsOptions(searchString string, forAllRefs bool) SearchCommits for _, k := range fields { switch { case strings.HasPrefix(k, "author:"): - authors = append(authors, strings.ReplaceAll(strings.TrimSpace(strings.TrimPrefix(k, "author:")), "'", "")) + authors = append(authors, strings.TrimPrefix(k, "author:")) case strings.HasPrefix(k, "committer:"): - committers = append(committers, strings.ReplaceAll(strings.TrimSpace(strings.TrimPrefix(k, "committer:")), "'", "")) + committers = append(committers, strings.TrimPrefix(k, "committer:")) case strings.HasPrefix(k, "after:"): - after = strings.ReplaceAll(strings.TrimSpace(strings.TrimPrefix(k, "after:")), "'", "") + after = strings.TrimPrefix(k, "after:") case strings.HasPrefix(k, "before:"): - before = strings.ReplaceAll(strings.TrimSpace(strings.TrimPrefix(k, "before:")), "'", "") + before = strings.TrimPrefix(k, "before:") default: - keywords = append(keywords, strings.ReplaceAll(strings.TrimSpace(k), "'", "")) + keywords = append(keywords, k) } } From f21180eb9b0320907afc3dc314f0f417c5d8857a Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 15 Oct 2022 04:43:00 +0200 Subject: [PATCH 10/11] bump git requirement to 2.24 --- modules/git/git.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/modules/git/git.go b/modules/git/git.go index 28899222e7efd..e70bf74dd1bc4 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -24,7 +24,7 @@ import ( ) // RequiredVersion is the minimum Git version required -const RequiredVersion = "2.0.0" +const RequiredVersion = "2.24.0" var ( // GitExecutable is the command name of git @@ -116,8 +116,7 @@ func VersionInfo() string { } format := "%s" args := []interface{}{gitVersion.Original()} - // Since git wire protocol has been released from git v2.18 - if setting.Git.EnableAutoGitWireProtocol && CheckGitVersionAtLeast("2.18") == nil { + if setting.Git.EnableAutoGitWireProtocol { format += ", Wire Protocol %s Enabled" args = append(args, "Version 2") // for focus color } From 5fcc1b00dc2d69626b0a197af1572e1a74f2bc85 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 15 Oct 2022 11:55:04 +0800 Subject: [PATCH 11/11] fix --- modules/git/command.go | 12 ++++++++++++ modules/git/command_test.go | 17 +++++++++++++++++ modules/git/commit.go | 2 +- modules/git/git.go | 5 +++-- modules/git/repo_commit.go | 19 ++++++++++--------- modules/git/repo_stats.go | 8 ++++---- modules/gitgraph/graph.go | 19 +++++++------------ 7 files changed, 54 insertions(+), 28 deletions(-) diff --git a/modules/git/command.go b/modules/git/command.go index b24d32dbe8743..8ebd8898fba55 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -95,6 +95,18 @@ func (c *Command) AddArguments(args ...string) *Command { return c } +// AddDynamicArguments adds new dynamic argument(s) to the command. +// If the argument is invalid (it shouldn't happen in real life), it panics to caller +func (c *Command) AddDynamicArguments(args ...string) *Command { + for _, arg := range args { + if arg != "" && arg[0] == '-' { + panic("invalid argument: " + arg) + } + } + c.args = append(c.args, args...) + return c +} + // RunOpts represents parameters to run the command. If UseContextTimeout is specified, then Timeout is ignored. type RunOpts struct { Env []string diff --git a/modules/git/command_test.go b/modules/git/command_test.go index 67d4ca388e2b8..c965ea230d89c 100644 --- a/modules/git/command_test.go +++ b/modules/git/command_test.go @@ -26,4 +26,21 @@ func TestRunWithContextStd(t *testing.T) { assert.Contains(t, err.Error(), "exit status 129 - unknown option:") assert.Empty(t, stdout) } + + assert.Panics(t, func() { + cmd = NewCommand(context.Background()) + cmd.AddDynamicArguments("-test") + }) + + assert.Panics(t, func() { + cmd = NewCommand(context.Background()) + cmd.AddDynamicArguments("--test") + }) + + subCmd := "version" + cmd = NewCommand(context.Background()).AddDynamicArguments(subCmd) // for test purpose only, the sub-command should never be dynamic for production + stdout, stderr, err = cmd.RunStdString(&RunOpts{}) + assert.NoError(t, err) + assert.Empty(t, stderr) + assert.Contains(t, stdout, "git version") } diff --git a/modules/git/commit.go b/modules/git/commit.go index 32589f534980c..8fac9921b13d6 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -166,7 +166,7 @@ func AllCommitsCount(ctx context.Context, repoPath string, hidePRRefs bool, file // CommitsCountFiles returns number of total commits of until given revision. func CommitsCountFiles(ctx context.Context, repoPath string, revision, relpath []string) (int64, error) { cmd := NewCommand(ctx, "rev-list", "--count") - cmd.AddArguments(revision...) + cmd.AddDynamicArguments(revision...) if len(relpath) > 0 { cmd.AddArguments("--") cmd.AddArguments(relpath...) diff --git a/modules/git/git.go b/modules/git/git.go index e70bf74dd1bc4..28899222e7efd 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -24,7 +24,7 @@ import ( ) // RequiredVersion is the minimum Git version required -const RequiredVersion = "2.24.0" +const RequiredVersion = "2.0.0" var ( // GitExecutable is the command name of git @@ -116,7 +116,8 @@ func VersionInfo() string { } format := "%s" args := []interface{}{gitVersion.Original()} - if setting.Git.EnableAutoGitWireProtocol { + // Since git wire protocol has been released from git v2.18 + if setting.Git.EnableAutoGitWireProtocol && CheckGitVersionAtLeast("2.18") == nil { format += ", Wire Protocol %s Enabled" args = append(args, "Version 2") // for focus color } diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index 6604fd02875a9..78e037511e551 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -161,7 +161,7 @@ func (repo *Repository) searchCommits(id SHA1, opts SearchCommitsOptions) ([]*Co // add previous arguments except for --grep and --all hashCmd.AddArguments(args...) // add keyword as - hashCmd.AddArguments("--end-of-options", v) + hashCmd.AddDynamicArguments(v) // search with given constraints for commit matching sha hash of v hashMatching, _, err := hashCmd.RunStdBytes(&RunOpts{Dir: repo.Path}) @@ -211,16 +211,17 @@ func (repo *Repository) CommitsByFileAndRange(revision, file string, page int) ( }() go func() { stderr := strings.Builder{} - err := NewCommand(repo.Ctx, "rev-list", + gitCmd := NewCommand(repo.Ctx, "rev-list", "--max-count="+strconv.Itoa(setting.Git.CommitsRangeSize*page), "--skip="+strconv.Itoa(skip), - "--end-of-options", revision, - "--", file). - Run(&RunOpts{ - Dir: repo.Path, - Stdout: stdoutWriter, - Stderr: &stderr, - }) + ) + gitCmd.AddDynamicArguments(revision) + gitCmd.AddArguments("--", file) + err := gitCmd.Run(&RunOpts{ + Dir: repo.Path, + Stdout: stdoutWriter, + Stderr: &stderr, + }) if err != nil { _ = stdoutWriter.CloseWithError(ConcatenateError(err, (&stderr).String())) } else { diff --git a/modules/git/repo_stats.go b/modules/git/repo_stats.go index 5e2c1d5d74d11..7eccf9a19043f 100644 --- a/modules/git/repo_stats.go +++ b/modules/git/repo_stats.go @@ -61,15 +61,15 @@ func (repo *Repository) GetCodeActivityStats(fromTime time.Time, branch string) _ = stdoutWriter.Close() }() - args := []string{"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", fmt.Sprintf("--since='%s'", since)) if len(branch) == 0 { - args = append(args, "--branches=*") + gitCmd.AddArguments("--branches=*") } else { - args = append(args, "--first-parent", "--", branch) + gitCmd.AddArguments("--first-parent").AddDynamicArguments(branch) } stderr := new(strings.Builder) - err = NewCommand(repo.Ctx, args...).Run(&RunOpts{ + err = gitCmd.Run(&RunOpts{ Env: []string{}, Dir: repo.Path, Stdout: stdoutWriter, diff --git a/modules/gitgraph/graph.go b/modules/gitgraph/graph.go index 271382525a4a5..0f3c02134482f 100644 --- a/modules/gitgraph/graph.go +++ b/modules/gitgraph/graph.go @@ -24,19 +24,17 @@ func GetCommitGraph(r *git.Repository, page, maxAllowedColors int, hidePRRefs bo page = 1 } - args := make([]string, 0, 12+len(branches)+len(files)) - - args = append(args, "--graph", "--date-order", "--decorate=full") + graphCmd := git.NewCommand(r.Ctx, "log", "--graph", "--date-order", "--decorate=full") if hidePRRefs { - args = append(args, "--exclude="+git.PullPrefix+"*") + graphCmd.AddArguments("--exclude=" + git.PullPrefix + "*") } if len(branches) == 0 { - args = append(args, "--all") + graphCmd.AddArguments("--all") } - args = append(args, + graphCmd.AddArguments( "-C", "-M", fmt.Sprintf("-n %d", setting.UI.GraphMaxCommitNum*page), @@ -44,15 +42,12 @@ func GetCommitGraph(r *git.Repository, page, maxAllowedColors int, hidePRRefs bo fmt.Sprintf("--pretty=format:%s", format)) if len(branches) > 0 { - args = append(args, branches...) + graphCmd.AddDynamicArguments(branches...) } - args = append(args, "--") if len(files) > 0 { - args = append(args, files...) + graphCmd.AddArguments("--") + graphCmd.AddArguments(files...) } - - graphCmd := git.NewCommand(r.Ctx, "log") - graphCmd.AddArguments(args...) graph := NewGraph() stderr := new(strings.Builder)