Merge remote-tracking branch 'upstream/main' into limit-repo-size

modules/git/command.go

@@ -40,7 +40,7 @@ const DefaultLocale = "C"
 
 // Command represents a command with its subcommands or arguments.
 type Command struct {
-	name             string
+	prog             string
 	args             []string
 	parentContext    context.Context
 	desc             string
@@ -49,10 +49,28 @@ type Command struct {
 }
 
 func (c *Command) String() string {
-	if len(c.args) == 0 {
-		return c.name
+	return c.toString(false)
+}
+
+func (c *Command) toString(sanitizing bool) string {
+	// WARNING: this function is for debugging purposes only. It's much better than old code (which only joins args with space),
+	// It's impossible to make a simple and 100% correct implementation of argument quoting for different platforms.
+	debugQuote := func(s string) string {
+		if strings.ContainsAny(s, " `'\"\t\r\n") {
+			return fmt.Sprintf("%q", s)
+		}
+		return s
 	}
-	return fmt.Sprintf("%s %s", c.name, strings.Join(c.args, " "))
+	a := make([]string, 0, len(c.args)+1)
+	a = append(a, debugQuote(c.prog))
+	for _, arg := range c.args {
+		if sanitizing && (strings.Contains(arg, "://") && strings.Contains(arg, "@")) {
+			a = append(a, debugQuote(util.SanitizeCredentialURLs(arg)))
+		} else {
+			a = append(a, debugQuote(arg))
+		}
+	}
+	return strings.Join(a, " ")
 }
 
 // NewCommand creates and returns a new Git Command based on given command and arguments.
@@ -67,7 +85,7 @@ func NewCommand(ctx context.Context, args ...internal.CmdArg) *Command {
 		cargs = append(cargs, string(arg))
 	}
 	return &Command{
-		name:             GitExecutable,
+		prog:             GitExecutable,
 		args:             cargs,
 		parentContext:    ctx,
 		globalArgsLength: len(globalCommandArgs),
@@ -82,7 +100,7 @@ func NewCommandContextNoGlobals(ctx context.Context, args ...internal.CmdArg) *C
 		cargs = append(cargs, string(arg))
 	}
 	return &Command{
-		name:          GitExecutable,
+		prog:          GitExecutable,
 		args:          cargs,
 		parentContext: ctx,
 	}
@@ -250,28 +268,18 @@ func (c *Command) Run(opts *RunOpts) error {
 	}
 
 	if len(opts.Dir) == 0 {
-		log.Debug("%s", c)
+		log.Debug("git.Command.Run: %s", c)
 	} else {
-		log.Debug("%s: %v", opts.Dir, c)
+		log.Debug("git.Command.RunDir(%s): %s", opts.Dir, c)
 	}
 
 	desc := c.desc
 	if desc == "" {
-		args := c.args[c.globalArgsLength:]
-		var argSensitiveURLIndexes []int
-		for i, arg := range c.args {
-			if strings.Contains(arg, "://") && strings.Contains(arg, "@") {
-				argSensitiveURLIndexes = append(argSensitiveURLIndexes, i)
-			}
+		if opts.Dir == "" {
+			desc = fmt.Sprintf("git: %s", c.toString(true))
+		} else {
+			desc = fmt.Sprintf("git(dir:%s): %s", opts.Dir, c.toString(true))
 		}
-		if len(argSensitiveURLIndexes) > 0 {
-			args = make([]string, len(c.args))
-			copy(args, c.args)
-			for _, urlArgIndex := range argSensitiveURLIndexes {
-				args[urlArgIndex] = util.SanitizeCredentialURLs(args[urlArgIndex])
-			}
-		}
-		desc = fmt.Sprintf("%s %s [repo_path: %s]", c.name, strings.Join(args, " "), opts.Dir)
 	}
 
 	var ctx context.Context
@@ -285,7 +293,7 @@ func (c *Command) Run(opts *RunOpts) error {
 	}
 	defer finished()
 
-	cmd := exec.CommandContext(ctx, c.name, c.args...)
+	cmd := exec.CommandContext(ctx, c.prog, c.args...)
 	if opts.Env == nil {
 		cmd.Env = os.Environ()
 	} else {

modules/git/command_test.go

@@ -52,3 +52,11 @@ func TestGitArgument(t *testing.T) {
 	assert.True(t, isSafeArgumentValue("x"))
 	assert.False(t, isSafeArgumentValue("-x"))
 }
+
+func TestCommandString(t *testing.T) {
+	cmd := NewCommandContextNoGlobals(context.Background(), "a", "-m msg", "it's a test", `say "hello"`)
+	assert.EqualValues(t, cmd.prog+` a "-m msg" "it's a test" "say \"hello\""`, cmd.String())
+
+	cmd = NewCommandContextNoGlobals(context.Background(), "url: https://a:b@c/")
+	assert.EqualValues(t, cmd.prog+` "url: https://sanitized-credential@c/"`, cmd.toString(true))
+}

modules/git/commit.go

@@ -291,7 +291,7 @@ func (c *Commit) SearchCommits(opts SearchCommitsOptions) ([]*Commit, error) {
 
 // GetFilesChangedSinceCommit get all changed file names between pastCommit to current revision
 func (c *Commit) GetFilesChangedSinceCommit(pastCommit string) ([]string, error) {
-	return c.repo.getFilesChanged(pastCommit, c.ID.String())
+	return c.repo.GetFilesChangedBetween(pastCommit, c.ID.String())
 }
 
 // FileChangedSinceCommit Returns true if the file given has changed since the the past commit

modules/git/repo.go

@@ -209,49 +209,22 @@ func Push(ctx context.Context, repoPath string, opts PushOptions) error {
 	} else {
 		cmd.SetDescription(fmt.Sprintf("push branch %s to %s (force: %t, mirror: %t)", opts.Branch, opts.Remote, opts.Force, opts.Mirror))
 	}
-	var outbuf, errbuf strings.Builder
 
-	if opts.Timeout == 0 {
-		opts.Timeout = -1
-	}
-
-	err := cmd.Run(&RunOpts{
-		Env:     opts.Env,
-		Timeout: opts.Timeout,
-		Dir:     repoPath,
-		Stdout:  &outbuf,
-		Stderr:  &errbuf,
-	})
+	stdout, stderr, err := cmd.RunStdString(&RunOpts{Env: opts.Env, Timeout: opts.Timeout, Dir: repoPath})
 	if err != nil {
-		if strings.Contains(errbuf.String(), "non-fast-forward") {
-			return &ErrPushOutOfDate{
-				StdOut: outbuf.String(),
-				StdErr: errbuf.String(),
-				Err:    err,
-			}
-		} else if strings.Contains(errbuf.String(), "! [remote rejected]") {
-			err := &ErrPushRejected{
-				StdOut: outbuf.String(),
-				StdErr: errbuf.String(),
-				Err:    err,
-			}
+		if strings.Contains(stderr, "non-fast-forward") {
+			return &ErrPushOutOfDate{StdOut: stdout, StdErr: stderr, Err: err}
+		} else if strings.Contains(stderr, "! [remote rejected]") {
+			err := &ErrPushRejected{StdOut: stdout, StdErr: stderr, Err: err}
 			err.GenerateMessage()
 			return err
-		} else if strings.Contains(errbuf.String(), "matches more than one") {
-			err := &ErrMoreThanOne{
-				StdOut: outbuf.String(),
-				StdErr: errbuf.String(),
-				Err:    err,
-			}
-			return err
+		} else if strings.Contains(stderr, "matches more than one") {
+			return &ErrMoreThanOne{StdOut: stdout, StdErr: stderr, Err: err}
 		}
+		return fmt.Errorf("push failed: %w - %s\n%s", err, stderr, stdout)
 	}
 
-	if errbuf.Len() > 0 && err != nil {
-		return fmt.Errorf("%w - %s", err, errbuf.String())
-	}
-
-	return err
+	return nil
 }
 
 // CountObject represents repository count objects report

modules/git/repo_commit.go

@@ -182,14 +182,6 @@ func (repo *Repository) searchCommits(id SHA1, opts SearchCommitsOptions) ([]*Co
 	return repo.parsePrettyFormatLogToList(bytes.TrimSuffix(stdout, []byte{'\n'}))
 }
 
-func (repo *Repository) getFilesChanged(id1, id2 string) ([]string, error) {
-	stdout, _, err := NewCommand(repo.Ctx, "diff", "--name-only").AddDynamicArguments(id1, id2).RunStdBytes(&RunOpts{Dir: repo.Path})
-	if err != nil {
-		return nil, err
-	}
-	return strings.Split(string(stdout), "\n"), nil
-}
-
 // 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) {

modules/git/repo_compare.go

@@ -92,8 +92,11 @@ func (repo *Repository) GetCompareInfo(basePath, baseBranch, headBranch string,
 
 		// We have a common base - therefore we know that ... should work
 		if !fileOnly {
+			// avoid: ambiguous argument 'refs/a...refs/b': unknown revision or path not in the working tree. Use '--': 'git <command> [<revision>...] -- [<file>...]'
 			var logs []byte
-			logs, _, err = NewCommand(repo.Ctx, "log").AddDynamicArguments(baseCommitID + separator + headBranch).AddArguments(prettyLogFormat).RunStdBytes(&RunOpts{Dir: repo.Path})
+			logs, _, err = NewCommand(repo.Ctx, "log").AddArguments(prettyLogFormat).
+				AddDynamicArguments(baseCommitID + separator + headBranch).AddArguments("--").
+				RunStdBytes(&RunOpts{Dir: repo.Path})
 			if err != nil {
 				return nil, err
 			}
@@ -146,7 +149,8 @@ func (repo *Repository) GetDiffNumChangedFiles(base, head string, directComparis
 		separator = ".."
 	}
 
-	if err := NewCommand(repo.Ctx, "diff", "-z", "--name-only").AddDynamicArguments(base + separator + head).
+	// avoid: ambiguous argument 'refs/a...refs/b': unknown revision or path not in the working tree. Use '--': 'git <command> [<revision>...] -- [<file>...]'
+	if err := NewCommand(repo.Ctx, "diff", "-z", "--name-only").AddDynamicArguments(base + separator + head).AddArguments("--").
 		Run(&RunOpts{
 			Dir:    repo.Path,
 			Stdout: w,
@@ -157,7 +161,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").AddDynamicArguments(base, head).Run(&RunOpts{
+			if err = NewCommand(repo.Ctx, "diff", "-z", "--name-only").AddDynamicArguments(base, head).AddArguments("--").Run(&RunOpts{
 				Dir:    repo.Path,
 				Stdout: w,
 				Stderr: stderr,