sg: don't print output twice if build fails (#57945)

This irritated me and I also think it's confusing to see the output
twice, once in red and once in default color.

The "easy" fix would be to change the underlying error and don't include
the output, but it turns out that `run.InRoot` is used in quite a few
things: `sg ci` helpers, migration helpers, etc. And I don't want to
suddenly stop output from appearing where it previously did.

So I special-case the case where we *do* have the output easily
available, which is where we check for the `installErr`.

It's a bit of a band-aid and ideally we'd fix the underlying issue, but
I also like errors having the output included by default because it's
helpful.

dev/sg/internal/run/helpers.go

@@ -2,12 +2,12 @@ package run
 
 import (
 	"context"
+	"fmt"
 	"os"
 	"os/exec"
 	"strings"
 
 	"github.com/sourcegraph/sourcegraph/dev/sg/root"
-	"github.com/sourcegraph/sourcegraph/lib/errors"
 )
 
 func GitCmd(args ...string) (string, error) {
@@ -25,6 +25,26 @@ func DockerCmd(args ...string) (string, error) {
 	return InRoot(exec.Command("docker", args...))
 }
 
+type errorWithoutOutputer interface {
+	ErrorWithoutOutput() string
+}
+
+type cmdInRootErr struct {
+	err    error
+	args   []string
+	output string
+}
+
+func (e cmdInRootErr) Error() string {
+	return fmt.Sprintf("'%s' failed: %s", strings.Join(e.args, " "), e.output)
+}
+
+func (e cmdInRootErr) ErrorWithoutOutput() string {
+	return fmt.Sprintf("'%s' failed with %q", strings.Join(e.args, " "), e.err)
+}
+
+func (e cmdInRootErr) Unwrap() error { return e.err }
+
 func InRoot(cmd *exec.Cmd) (string, error) {
 	repoRoot, err := root.RepositoryRoot()
 	if err != nil {
@@ -34,7 +54,7 @@ func InRoot(cmd *exec.Cmd) (string, error) {
 	cmd.Dir = repoRoot
 	out, err := cmd.CombinedOutput()
 	if err != nil {
-		return string(out), errors.Wrapf(err, "'%s' failed: %s", strings.Join(cmd.Args, " "), out)
+		return string(out), cmdInRootErr{err: err, args: cmd.Args, output: string(out)}
 	}
 
 	return string(out), nil

dev/sg/internal/run/run.go

@@ -464,7 +464,13 @@ func printCmdError(out *output.Output, cmdName string, err error) {
 	case installErr:
 		message = "Failed to build " + cmdName
 		if e.originalErr != nil {
-			message += ": " + e.originalErr.Error()
+			if errWithout, ok := e.originalErr.(errorWithoutOutputer); ok {
+				// If we can, let's strip away the output, otherwise this gets
+				// too noisy.
+				message += ": " + errWithout.ErrorWithoutOutput()
+			} else {
+				message += ": " + e.originalErr.Error()
+			}
 		}
 		cmdOut = e.output
 	case reinstallErr:

dev/sg/sg_start_test.go

@@ -82,8 +82,7 @@ func TestStartCommandSet_InstallError(t *testing.T) {
 		"",
 		"💡 Installing 1 commands...",
 		"--------------------------------------------------------------------------------",
-		"Failed to build test-cmd-1: 'bash -c echo 'booting up horsegraph' && exit 1' failed: booting up horsegraph",
-		": exit status 1:",
+		`Failed to build test-cmd-1: 'bash -c echo 'booting up horsegraph' && exit 1' failed with "exit status 1":`,
 		"booting up horsegraph",
 		"--------------------------------------------------------------------------------",
 	})