fix(ci): check command out for error when git fails (#63993)

Closes [#1110](https://github.com/sourcegraph/devx-support/issues/1110)
Closes DINF-96

We don't print the stdErr when a command fails … in particular when git
fails. Therefore we see very little in the panic of what went wrong.

Explanation:
> There's a weird behavior that occurs where an error isn't accessible
in the err variable
// from a *Cmd executing a git command after calling CombinedOutput().
// This occurs due to how Git handles errors and how the exec package in
Go interprets the command's output.
// Git often writes error messages to stderr, but it might still exit
with a status code of 0 (indicating success).
// In this case, CombinedOutput() won't return an error, but the error
message will be in the out variable.

## Test plan

Manual testing

```go
func main() {
	ctx := context.Background()
	cmd := exec.CommandContext(ctx, "git", "rev-parse", "--is-inside-work-tree")
	out, err := handleGitCommandExec(cmd)
	if err != nil {
		// er := errors.Wrap(err, fmt.Sprintf("idsdsd: %s", string(out)))
		panic(err)
	}
	fmt.Println("hello", string(out))
}
```

## Changelog

<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->

dev/ci/gitops/BUILD.bazel

@@ -7,6 +7,7 @@ go_library(
     tags = [TAG_INFRA_DEVINFRA],
     visibility = ["//visibility:public"],
     deps = [
+        "//internal/execute",
         "//internal/oobmigration",
         "//lib/errors",
     ],

dev/ci/gitops/git_ops.go

@@ -1,10 +1,12 @@
 package gitops
 
 import (
+	"context"
 	"fmt"
 	"os/exec"
 	"strings"
 
+	"github.com/sourcegraph/sourcegraph/internal/execute"
 	"github.com/sourcegraph/sourcegraph/internal/oobmigration"
 	"github.com/sourcegraph/sourcegraph/lib/errors"
 )
@@ -32,7 +34,7 @@ func determineDiffArgs(baseBranch, commit string) (string, error) {
 }
 
 func GetHEADChangedFiles() ([]string, error) {
-	output, err := exec.Command("git", "diff", "--name-only", "@^").CombinedOutput()
+	output, err := execute.Git(context.Background(), "diff", "--name-only", "@^")
 	if err != nil {
 		return nil, err
 	}
@@ -46,7 +48,7 @@ func GetBranchChangedFiles(baseBranch, commit string) ([]string, error) {
 		return nil, err
 	}
 
-	output, err := exec.Command("git", "diff", "--name-only", diffArgs).CombinedOutput()
+	output, err := execute.Git(context.Background(), "diff", "--name-only", diffArgs)
 	if err != nil {
 		return nil, err
 	}
@@ -55,7 +57,7 @@ func GetBranchChangedFiles(baseBranch, commit string) ([]string, error) {
 }
 
 func GetLatestTag() (string, error) {
-	output, err := exec.Command("git", "tag", "--list", "v*").CombinedOutput()
+	output, err := execute.Git(context.Background(), "tag", "--list", "v*")
 	if err != nil {
 		return "", err
 	}
@@ -84,7 +86,7 @@ func HasIncludedCommit(commits ...string) (bool, error) {
 	found := false
 	var errs error
 	for _, mustIncludeCommit := range commits {
-		output, err := exec.Command("git", "merge-base", "--is-ancestor", mustIncludeCommit, "HEAD").CombinedOutput()
+		output, err := execute.Git(context.Background(), "merge-base", "--is-ancestor", mustIncludeCommit, "HEAD")
 		if err == nil {
 			found = true
 			break

dev/ci/internal/ci/BUILD.bazel

@@ -33,6 +33,7 @@ go_library(
         "//dev/ci/internal/ci/operations",
         "//dev/ci/runtype",
         "//dev/sg/root",
+        "//internal/execute",
         "//internal/lazyregexp",
         "//lib/errors",
         "@com_github_masterminds_semver//:semver",

dev/ci/internal/ci/config.go

@@ -1,9 +1,9 @@
 package ci
 
 import (
+	"context"
 	"fmt"
 	"os"
-	"os/exec"
 	"strconv"
 	"strings"
 	"time"
@@ -12,6 +12,7 @@ import (
 	"github.com/sourcegraph/sourcegraph/dev/ci/images"
 	"github.com/sourcegraph/sourcegraph/dev/ci/internal/ci/changed"
 	"github.com/sourcegraph/sourcegraph/dev/ci/runtype"
+	"github.com/sourcegraph/sourcegraph/internal/execute"
 	"github.com/sourcegraph/sourcegraph/lib/errors"
 )
 
@@ -97,7 +98,7 @@ func NewConfig(now time.Time) Config {
 			changedFiles, err = gitops.GetBranchChangedFiles(baseBranch, commit)
 		}
 	} else {
-		out, giterr := exec.Command("git", "rev-parse", "HEAD").Output()
+		out, giterr := execute.Git(context.Background(), "rev-parse", "HEAD")
 		if giterr != nil {
 			panic(giterr)
 		}

dev/sg/internal/backport/BUILD.bazel

@@ -7,8 +7,8 @@ go_library(
     tags = [TAG_INFRA_RELEASE],
     visibility = ["//dev/sg:__subpackages__"],
     deps = [
-        "//dev/sg/internal/execute",
         "//dev/sg/internal/std",
+        "//internal/execute",
         "//lib/errors",
         "//lib/output",
         "@com_github_urfave_cli_v2//:cli",

dev/sg/internal/backport/backport.go

@@ -7,8 +7,8 @@ import (
 
 	"github.com/urfave/cli/v2"
 
-	"github.com/sourcegraph/sourcegraph/dev/sg/internal/execute"
 	"github.com/sourcegraph/sourcegraph/dev/sg/internal/std"
+	"github.com/sourcegraph/sourcegraph/internal/execute"
 	"github.com/sourcegraph/sourcegraph/lib/errors"
 	"github.com/sourcegraph/sourcegraph/lib/output"
 )

dev/sg/internal/execute/BUILD.bazel

@@ -1,8 +0,0 @@
-load("@io_bazel_rules_go//go:def.bzl", "go_library")
-
-go_library(
-    name = "execute",
-    srcs = ["exec.go"],
-    importpath = "github.com/sourcegraph/sourcegraph/dev/sg/internal/execute",
-    visibility = ["//dev/sg:__subpackages__"],
-)

dev/sg/internal/execute/exec.go

@@ -1,46 +0,0 @@
-package execute
-
-import (
-	"bytes"
-	"context"
-	"os"
-	"os/exec"
-)
-
-func Git(ctx context.Context, args ...string) ([]byte, error) {
-	cmd := GitCmd(ctx, args...)
-
-	var stdout bytes.Buffer
-	var stderr bytes.Buffer
-
-	cmd.Stdout = &stdout
-	cmd.Stderr = &stderr
-	cmd.Stdin = os.Stdin
-
-	if err := cmd.Run(); err != nil {
-		return nil, err
-	}
-	return stdout.Bytes(), nil
-}
-
-func GitCmd(ctx context.Context, args ...string) *exec.Cmd {
-	return exec.CommandContext(ctx, "git", args...)
-}
-
-func GHCmd(ctx context.Context, args ...string) *exec.Cmd {
-	return exec.CommandContext(ctx, "gh", args...)
-}
-
-func GH(ctx context.Context, args ...string) ([]byte, error) {
-	cmd := GHCmd(ctx, args...)
-
-	var stdout bytes.Buffer
-	cmd.Stdout = &stdout
-	cmd.Stderr = os.Stderr
-	cmd.Stdin = os.Stdin
-
-	if err := cmd.Run(); err != nil {
-		return nil, err
-	}
-	return stdout.Bytes(), nil
-}

dev/sg/internal/release/BUILD.bazel

@@ -17,9 +17,9 @@ go_library(
     deps = [
         "//dev/sg/internal/bk",
         "//dev/sg/internal/category",
-        "//dev/sg/internal/execute",
         "//dev/sg/internal/repo",
         "//dev/sg/internal/std",
+        "//internal/execute",
         "//internal/jsonc",
         "//internal/releaseregistry",
         "//lib/errors",

dev/sg/internal/release/cut.go

@@ -15,8 +15,8 @@ import (
 
 	"github.com/sourcegraph/run"
 
-	"github.com/sourcegraph/sourcegraph/dev/sg/internal/execute"
 	"github.com/sourcegraph/sourcegraph/dev/sg/internal/repo"
+	"github.com/sourcegraph/sourcegraph/internal/execute"
 )
 
 func cutReleaseBranch(cctx *cli.Context) error {

internal/execute/BUILD.bazel

@@ -0,0 +1,17 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
+load("//dev:go_defs.bzl", "go_test")
+
+go_library(
+    name = "execute",
+    srcs = ["git.go"],
+    importpath = "github.com/sourcegraph/sourcegraph/internal/execute",
+    visibility = ["//:__subpackages__"],
+    deps = ["//lib/errors"],
+)
+
+go_test(
+    name = "execute_test",
+    srcs = ["git_test.go"],
+    embed = [":execute"],
+    deps = ["@com_github_stretchr_testify//require"],
+)

internal/execute/git.go

@@ -0,0 +1,83 @@
+package execute
+
+import (
+	"bytes"
+	"context"
+	"os"
+	"os/exec"
+	"strings"
+
+	"github.com/sourcegraph/sourcegraph/lib/errors"
+)
+
+type cmdErr struct {
+	err      error
+	exitCode int
+}
+
+func (g *cmdErr) Error() string {
+	return g.err.Error()
+}
+
+func (g *cmdErr) ExitCode() int {
+	return g.exitCode
+}
+
+func (g *cmdErr) Unwrap() error {
+	return g.err
+}
+
+// HandleGitCommandExec There's a weird behavior that occurs where an error isn't accessible in the err variable
+// from a *Cmd executing a git command after calling CombinedOutput().
+// This occurs due to how Git handles errors and how the exec package in Go interprets the command's output.
+// Git often writes error messages to stderr, but it might still exit with a status code of 0 (indicating success).
+// In this case, CombinedOutput() won't return an error, but the error message will be in the out variable.
+func handleGitCommandExec(cmd *exec.Cmd) ([]byte, error) {
+	var stdout, stderr bytes.Buffer
+	cmd.Stdout = &stdout
+	cmd.Stderr = &stderr
+
+	if err := cmd.Run(); err != nil {
+		maybeErrMessage := strings.Trim(stderr.String(), "\n")
+		if strings.HasPrefix(maybeErrMessage, "fatal:") || strings.HasPrefix(maybeErrMessage, "error:") {
+			exitCode := 1
+			var exitErr *exec.ExitError
+			if errors.As(err, &exitErr) {
+				exitCode = exitErr.ExitCode()
+			}
+			return nil, &cmdErr{
+				err:      errors.New(maybeErrMessage),
+				exitCode: exitCode,
+			}
+		}
+		return nil, err
+	}
+
+	return stdout.Bytes(), nil
+}
+
+func Git(ctx context.Context, args ...string) ([]byte, error) {
+	return handleGitCommandExec(GitCmd(ctx, args...))
+}
+
+func GitCmd(ctx context.Context, args ...string) *exec.Cmd {
+	return exec.CommandContext(ctx, "git", args...)
+}
+
+func GHCmd(ctx context.Context, args ...string) *exec.Cmd {
+	return exec.CommandContext(ctx, "gh", args...)
+}
+
+func GH(ctx context.Context, args ...string) ([]byte, error) {
+	cmd := GHCmd(ctx, args...)
+
+	var stdout bytes.Buffer
+	cmd.Stdout = &stdout
+	cmd.Stderr = os.Stderr
+	cmd.Stdin = os.Stdin
+
+	if err := cmd.Run(); err != nil {
+		return nil, err
+	}
+	return stdout.Bytes(), nil
+}

internal/execute/git_test.go

@@ -0,0 +1,76 @@
+package execute
+
+import (
+	"os"
+	"os/exec"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestHandleGitCommandExec(t *testing.T) {
+	// Store the current working directory
+	originalDir, err := os.Getwd()
+	if err != nil {
+		t.Fatalf("Failed to get current directory: %v", err)
+	}
+
+	// Create a temporary directory for the test
+	tempDir, err := os.MkdirTemp("", "git-ops-test-temp-dir")
+	if err != nil {
+		t.Fatalf("Failed to create temp directory: %v", err)
+	}
+	defer os.RemoveAll(tempDir) // Clean up after the test
+
+	// Change to the temporary directory
+	if err := os.Chdir(tempDir); err != nil {
+		t.Fatalf("Failed to change to temp directory: %v", err)
+	}
+
+	// Defer changing back to the original directory
+	defer func() {
+		if err := os.Chdir(originalDir); err != nil {
+			t.Fatalf("Failed to change back to original directory: %v", err)
+		}
+	}()
+
+	tests := []struct {
+		name           string
+		cmdSetup       func() *exec.Cmd
+		expectedOutput string
+		expectedError  string
+	}{
+		{
+			name: "Successful command",
+			cmdSetup: func() *exec.Cmd {
+				return exec.Command("echo", "file1.txt\nfile2.txt")
+			},
+			expectedOutput: "file1.txt\nfile2.txt\n",
+			expectedError:  "",
+		},
+		{
+			name: "Git fatal error",
+			cmdSetup: func() *exec.Cmd {
+				return exec.Command("git", "rev-parse", "--is-inside-work-tree")
+			},
+			expectedOutput: "",
+			expectedError:  "fatal: not a git repository",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cmd := tt.cmdSetup()
+			output, err := handleGitCommandExec(cmd)
+
+			if tt.expectedError != "" {
+				require.Error(t, err)
+				require.ErrorContains(t, err, tt.expectedError)
+			} else {
+				require.NoError(t, err)
+			}
+
+			require.Equal(t, tt.expectedOutput, string(output))
+		})
+	}
+}