fix

Author: wxiaoguang

modules/git/gitcmd/command.go

@@ -453,18 +453,16 @@ func IsErrorExitCode(err error, code int) bool {
 
 // RunStdString runs the command and returns stdout/stderr as string. and store stderr to returned error (err combined with stderr).
 func (c *Command) RunStdString(ctx context.Context) (stdout, stderr string, runErr RunStdError) {
-	stdoutBytes, stderrBytes, err := c.WithParentCallerInfo().RunStdBytes(ctx)
-	stdout = util.UnsafeBytesToString(stdoutBytes)
-	stderr = util.UnsafeBytesToString(stderrBytes)
-	if err != nil {
-		return stdout, stderr, &runStdError{err: err, stderr: stderr}
-	}
-	// even if there is no err, there could still be some stderr output, so we just return stdout/stderr as they are
-	return stdout, stderr, nil
+	stdoutBytes, stderrBytes, runErr := c.WithParentCallerInfo().runStdBytes(ctx)
+	return util.UnsafeBytesToString(stdoutBytes), util.UnsafeBytesToString(stderrBytes), runErr
 }
 
 // RunStdBytes runs the command and returns stdout/stderr as bytes. and store stderr to returned error (err combined with stderr).
-func (c *Command) RunStdBytes(ctx context.Context) (stdout, stderr []byte, runErr RunStdError) {
+func (c *Command) RunStdBytes(ctx context.Context) ( /*stdout*/ []byte /*stderr*/, []byte /*runErr*/, RunStdError) {
+	return c.WithParentCallerInfo().runStdBytes(ctx)
+}
+
+func (c *Command) runStdBytes(ctx context.Context) ( /*stdout*/ []byte /*stderr*/, []byte /*runErr*/, RunStdError) {
 	if c.opts.Stdout != nil || c.opts.Stderr != nil {
 		// we must panic here, otherwise there would be bugs if developers set Stdin/Stderr by mistake, and it would be very difficult to debug
 		panic("stdout and stderr field must be nil when using RunStdBytes")

modules/git/gitcmd/command_test.go

@@ -23,38 +23,58 @@ func TestMain(m *testing.M) {
 	defer cleanup()
 
 	setting.Git.HomePath = gitHomePath
+	os.Exit(m.Run())
 }
 
 func TestRunWithContextStd(t *testing.T) {
-	cmd := NewCommand("--version")
-	stdout, stderr, err := cmd.RunStdString(t.Context())
-	assert.NoError(t, err)
-	assert.Empty(t, stderr)
-	assert.Contains(t, stdout, "git version")
-
-	cmd = NewCommand("--no-such-arg")
-	stdout, stderr, err = cmd.RunStdString(t.Context())
-	if assert.Error(t, err) {
-		assert.Equal(t, stderr, err.Stderr())
-		assert.Contains(t, err.Stderr(), "unknown option:")
-		assert.Contains(t, err.Error(), "exit status 129 - unknown option:")
-		assert.Empty(t, stdout)
+	{
+		cmd := NewCommand("--version")
+		stdout, stderr, err := cmd.RunStdString(t.Context())
+		assert.NoError(t, err)
+		assert.Empty(t, stderr)
+		assert.Contains(t, stdout, "git version")
 	}
 
-	cmd = NewCommand()
-	cmd.AddDynamicArguments("-test")
-	assert.ErrorIs(t, cmd.Run(t.Context()), ErrBrokenCommand)
+	{
+		cmd := NewCommand("ls-tree", "no-such")
+		stdout, stderr, err := cmd.RunStdString(t.Context())
+		if assert.Error(t, err) {
+			assert.Equal(t, stderr, err.Stderr())
+			assert.Equal(t, "fatal: Not a valid object name no-such\n", err.Stderr())
+			assert.Equal(t, "exit status 128 - fatal: Not a valid object name no-such\n", err.Error())
+			assert.Empty(t, stdout)
+		}
+	}
+
+	{
+		cmd := NewCommand("ls-tree", "no-such")
+		stdout, stderr, err := cmd.RunStdBytes(t.Context())
+		if assert.Error(t, err) {
+			assert.Equal(t, string(stderr), err.Stderr())
+			assert.Equal(t, "fatal: Not a valid object name no-such\n", err.Stderr())
+			assert.Equal(t, "exit status 128 - fatal: Not a valid object name no-such\n", err.Error())
+			assert.Empty(t, stdout)
+		}
+	}
 
-	cmd = NewCommand()
-	cmd.AddDynamicArguments("--test")
-	assert.ErrorIs(t, cmd.Run(t.Context()), ErrBrokenCommand)
+	{
+		cmd := NewCommand()
+		cmd.AddDynamicArguments("-test")
+		assert.ErrorIs(t, cmd.Run(t.Context()), ErrBrokenCommand)
 
-	subCmd := "version"
-	cmd = NewCommand().AddDynamicArguments(subCmd) // for test purpose only, the sub-command should never be dynamic for production
-	stdout, stderr, err = cmd.RunStdString(t.Context())
-	assert.NoError(t, err)
-	assert.Empty(t, stderr)
-	assert.Contains(t, stdout, "git version")
+		cmd = NewCommand()
+		cmd.AddDynamicArguments("--test")
+		assert.ErrorIs(t, cmd.Run(t.Context()), ErrBrokenCommand)
+	}
+
+	{
+		subCmd := "version"
+		cmd := NewCommand().AddDynamicArguments(subCmd) // for test purpose only, the sub-command should never be dynamic for production
+		stdout, stderr, err := cmd.RunStdString(t.Context())
+		assert.NoError(t, err)
+		assert.Empty(t, stderr)
+		assert.Contains(t, stdout, "git version")
+	}
 }
 
 func TestGitArgument(t *testing.T) {