[release-branch.go1.19] cmd/go/internal/work: make formatOutput return an error that includes the import path

This refines the error output that was previously adjusted in CL 437298.

Longer term, we should consider unraveling the call chains involving
formatOutput to avoid passing so many parameters through so many
different formatting functions.

Updates #60710.
Updates #60650.
Updates #25842.

Change-Id: I3b9d03bf5968902d8ccc4841ab4dbe114a2239e4
Reviewed-on: https://go-review.googlesource.com/c/go/+/451218
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/502196
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Bypass: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>

4 files changed, 72 insertions, 31 deletions

diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go
index ac3815511c..4840568d67 100644
--- a/src/cmd/go/internal/work/exec.go
+++ b/src/cmd/go/internal/work/exec.go
@@ -149,11 +149,15 @@ func (b *Builder) Do(ctx context.Context, root *Action) {
 		defer b.exec.Unlock()
 
 		if err != nil {
-			if b.AllowErrors {
+			if b.AllowErrors && a.Package != nil {
 				if a.Package.Error == nil {
 					a.Package.Error = &load.PackageError{Err: err}
 				}
 			} else {
+				var ipe load.ImportPathError
+				if a.Package != nil && (!errors.As(err, &ipe) || ipe.ImportPath() != a.Package.ImportPath) {
+					err = fmt.Errorf("%s: %v", a.Package.ImportPath, err)
+				}
 				base.Errorf("%s", err)
 			}
 			a.Failed = true
@@ -510,9 +514,6 @@ func (b *Builder) build(ctx context.Context, a *Action) (err error) {
 	}
 
 	defer func() {
-		if err != nil {
-			err = fmt.Errorf("go build %s: %v", p.ImportPath, err)
-		}
 		if err != nil && b.IsCmdList && b.NeedError && p.Error == nil {
 			p.Error = &load.PackageError{Err: err}
 		}
@@ -825,7 +826,7 @@ OverlayLoop:
 		}
 
 		if err != nil {
-			return errors.New(fmt.Sprint(formatOutput(b.WorkDir, p.Dir, p.Desc(), output)))
+			return formatOutput(b.WorkDir, p.Dir, p.ImportPath, p.Desc(), output)
 		} else {
 			b.showOutput(a, p.Dir, p.Desc(), output)
 		}
@@ -1506,8 +1507,8 @@ func (b *Builder) getPkgConfigFlags(p *load.Package) (cflags, ldflags []string,
 		var out []byte
 		out, err = b.runOut(nil, p.Dir, nil, b.PkgconfigCmd(), "--cflags", pcflags, "--", pkgs)
 		if err != nil {
-			prefix, suffix := formatOutput(b.WorkDir, p.Dir, b.PkgconfigCmd()+" --cflags "+strings.Join(pcflags, " ")+" -- "+strings.Join(pkgs, " "), string(out))
-			return nil, nil, errors.New(fmt.Sprint(prefix, suffix+err.Error()))
+			err = formatOutput(b.WorkDir, p.Dir, p.ImportPath, b.PkgconfigCmd()+" --cflags "+strings.Join(pcflags, " ")+" -- "+strings.Join(pkgs, " "), string(out)+err.Error())
+			return nil, nil, err
 		}
 		if len(out) > 0 {
 			cflags, err = splitPkgConfigOutput(out)
@@ -1520,8 +1521,8 @@ func (b *Builder) getPkgConfigFlags(p *load.Package) (cflags, ldflags []string,
 		}
 		out, err = b.runOut(nil, p.Dir, nil, b.PkgconfigCmd(), "--libs", pcflags, "--", pkgs)
 		if err != nil {
-			prefix, suffix := formatOutput(b.WorkDir, p.Dir, b.PkgconfigCmd()+" --libs "+strings.Join(pcflags, " ")+" -- "+strings.Join(pkgs, " "), string(out))
-			return nil, nil, errors.New(fmt.Sprint(prefix, suffix+err.Error()))
+			err = formatOutput(b.WorkDir, p.Dir, p.ImportPath, b.PkgconfigCmd()+" --libs "+strings.Join(pcflags, " ")+" -- "+strings.Join(pkgs, " "), string(out)+err.Error())
+			return nil, nil, err
 		}
 		if len(out) > 0 {
 			// NOTE: we don't attempt to parse quotes and unescapes here. pkg-config
@@ -2017,16 +2018,37 @@ func (b *Builder) Showcmd(dir string, format string, args ...any) {
 // If a is not nil and a.output is not nil, showOutput appends to that slice instead of
 // printing to b.Print.
 func (b *Builder) showOutput(a *Action, dir, desc, out string) {
-	prefix, suffix := formatOutput(b.WorkDir, dir, desc, out)
+	importPath := ""
+	if a != nil && a.Package != nil {
+		importPath = a.Package.ImportPath
+	}
+	psErr := formatOutput(b.WorkDir, dir, importPath, desc, out)
 	if a != nil && a.output != nil {
-		a.output = append(a.output, prefix...)
-		a.output = append(a.output, suffix...)
+		a.output = append(a.output, psErr.prefix...)
+		a.output = append(a.output, psErr.suffix...)
 		return
 	}
 
 	b.output.Lock()
 	defer b.output.Unlock()
-	b.Print(prefix, suffix)
+	b.Print(psErr.prefix, psErr.suffix)
+}
+
+// A prefixSuffixError is an error formatted by formatOutput.
+type prefixSuffixError struct {
+	importPath     string
+	prefix, suffix string
+}
+
+func (e *prefixSuffixError) Error() string {
+	if e.importPath != "" && !strings.HasPrefix(strings.TrimPrefix(e.prefix, "# "), e.importPath) {
+		return fmt.Sprintf("go build %s:\n%s%s", e.importPath, e.prefix, e.suffix)
+	}
+	return e.prefix + e.suffix
+}
+
+func (e *prefixSuffixError) ImportPath() string {
+	return e.importPath
 }
 
 // formatOutput prints "# desc" followed by the given output.
@@ -2052,9 +2074,9 @@ func (b *Builder) showOutput(a *Action, dir, desc, out string) {
 // formatOutput also replaces references to the work directory with $WORK.
 // formatOutput returns the output in a prefix with the description and a
 // suffix with the actual output.
-func formatOutput(workDir, dir, desc, out string) (prefix, suffix string) {
-	prefix = "# " + desc
-	suffix = "\n" + out
+func formatOutput(workDir, dir, importPath, desc, out string) *prefixSuffixError {
+	prefix := "# " + desc
+	suffix := "\n" + out
 	if reldir := base.ShortPath(dir); reldir != dir {
 		suffix = strings.ReplaceAll(suffix, " "+dir, " "+reldir)
 		suffix = strings.ReplaceAll(suffix, "\n"+dir, "\n"+reldir)
@@ -2062,7 +2084,7 @@ func formatOutput(workDir, dir, desc, out string) (prefix, suffix string) {
 	}
 	suffix = strings.ReplaceAll(suffix, " "+workDir, " $WORK")
 
-	return prefix, suffix
+	return &prefixSuffixError{importPath: importPath, prefix: prefix, suffix: suffix}
 }
 
 var cgoLine = lazyregexp.New(`\[[^\[\]]+\.(cgo1|cover)\.go:[0-9]+(:[0-9]+)?\]`)
@@ -2078,7 +2100,7 @@ func (b *Builder) run(a *Action, dir string, desc string, env []string, cmdargs
 			desc = b.fmtcmd(dir, "%s", strings.Join(str.StringList(cmdargs...), " "))
 		}
 		if err != nil {
-			err = errors.New(fmt.Sprint(formatOutput(b.WorkDir, dir, desc, b.processOutput(out))))
+			err = formatOutput(b.WorkDir, dir, a.Package.ImportPath, desc, b.processOutput(out))
 		} else {
 			b.showOutput(a, dir, desc, b.processOutput(out))
 		}
@@ -2364,7 +2386,6 @@ func (b *Builder) gfortran(a *Action, p *load.Package, workdir, out string, flag
 // ccompile runs the given C or C++ compiler and creates an object from a single source file.
 func (b *Builder) ccompile(a *Action, p *load.Package, outfile string, flags []string, file string, compiler []string) error {
 	file = mkAbs(p.Dir, file)
-	desc := p.ImportPath
 	outfile = mkAbs(p.Dir, outfile)
 
 	// Elide source directory paths if -trimpath or GOROOT_FINAL is set.
@@ -2425,9 +2446,9 @@ func (b *Builder) ccompile(a *Action, p *load.Package, outfile string, flags []s
 		}
 
 		if err != nil || os.Getenv("GO_BUILDER_NAME") != "" {
-			err = errors.New(fmt.Sprintf(formatOutput(b.WorkDir, p.Dir, desc, b.processOutput(output))))
+			err = formatOutput(b.WorkDir, p.Dir, p.ImportPath, p.Desc(), b.processOutput(output))
 		} else {
-			b.showOutput(a, p.Dir, desc, b.processOutput(output))
+			b.showOutput(a, p.Dir, p.Desc(), b.processOutput(output))
 		}
 	}
 	return err
@@ -2890,8 +2911,6 @@ func (b *Builder) cgo(a *Action, cgoExe, objdir string, pcCFLAGS, pcLDFLAGS, cgo
 		cgoflags = append(cgoflags, "-exportheader="+objdir+"_cgo_install.h")
 	}
 
-	execdir := p.Dir
-
 	// Rewrite overlaid paths in cgo files.
 	// cgo adds //line and #line pragmas in generated files with these paths.
 	var trimpath []string
@@ -2906,7 +2925,7 @@ func (b *Builder) cgo(a *Action, cgoExe, objdir string, pcCFLAGS, pcLDFLAGS, cgo
 		cgoflags = append(cgoflags, "-trimpath", strings.Join(trimpath, ";"))
 	}
 
-	if err := b.run(a, execdir, p.ImportPath, cgoenv, cfg.BuildToolexec, cgoExe, "-objdir", objdir, "-importpath", p.ImportPath, cgoflags, "--", cgoCPPFLAGS, cgoCFLAGS, cgofiles); err != nil {
+	if err := b.run(a, p.Dir, p.ImportPath, cgoenv, cfg.BuildToolexec, cgoExe, "-objdir", objdir, "-importpath", p.ImportPath, cgoflags, "--", cgoCPPFLAGS, cgoCFLAGS, cgofiles); err != nil {
 		return nil, nil, err
 	}
 	outGo = append(outGo, gofiles...)
@@ -3382,7 +3401,7 @@ func (b *Builder) swigOne(a *Action, p *load.Package, file, objdir string, pcCFL
 				return "", "", errors.New("must have SWIG version >= 3.0.6")
 			}
 			// swig error
-			return "", "", errors.New(fmt.Sprint(formatOutput(b.WorkDir, p.Dir, p.Desc(), b.processOutput(out))))
+			err = formatOutput(b.WorkDir, p.Dir, p.ImportPath, p.Desc(), b.processOutput(out))
 		}
 		return "", "", err
 	}
diff --git a/src/cmd/go/internal/work/gc.go b/src/cmd/go/internal/work/gc.go
index 3e405d448d..680ca94974 100644
--- a/src/cmd/go/internal/work/gc.go
+++ b/src/cmd/go/internal/work/gc.go
@@ -7,7 +7,6 @@ package work
 import (
 	"bufio"
 	"bytes"
-	"errors"
 	"fmt"
 	"io"
 	"log"
@@ -503,7 +502,7 @@ func (gcToolchain) pack(b *Builder, a *Action, afile string, ofiles []string) er
 		return nil
 	}
 	if err := packInternal(absAfile, absOfiles); err != nil {
-		return errors.New(fmt.Sprint(formatOutput(b.WorkDir, p.Dir, p.Desc(), err.Error()+"\n")))
+		return formatOutput(b.WorkDir, p.Dir, p.ImportPath, p.Desc(), err.Error()+"\n")
 	}
 	return nil
 }
diff --git a/src/cmd/go/testdata/script/list_export_e.txt b/src/cmd/go/testdata/script/list_export_e.txt
index f6992e221d..8e4c361fc4 100644
--- a/src/cmd/go/testdata/script/list_export_e.txt
+++ b/src/cmd/go/testdata/script/list_export_e.txt
@@ -1,6 +1,13 @@
-go list -e -export ./...
+! go list -export ./...
+stderr '^# example.com/p2\np2'${/}'main\.go:7:.*'
+! stderr '^go build '
+
+go list -f '{{with .Error}}{{.}}{{end}}' -e -export ./...
 ! stderr '.'
-go list -e -export -json ...
+stdout '^# example.com/p2\np2'${/}'main\.go:7:.*'
+
+go list -e -export -json=Error ./...
+stdout '"Err": "# example.com/p2'
 
 -- go.mod --
 module example.com
@@ -15,5 +22,5 @@ import "fmt"
 import "example.com/p1"
 
 func main() {
-        fmt.Println(p1.Name == 5)
-}
\ No newline at end of file
+	fmt.Println(p1.Name == 5)
+}
diff --git a/src/cmd/go/testdata/script/list_pkgconfig_error.txt b/src/cmd/go/testdata/script/list_pkgconfig_error.txt
new file mode 100644
index 0000000000..7d671a6438
--- /dev/null
+++ b/src/cmd/go/testdata/script/list_pkgconfig_error.txt
@@ -0,0 +1,16 @@
+[!cgo] skip 'test verifies cgo pkg-config errors'
+[!exec:pkg-config] skip 'test requires pkg-config tool'
+
+! go list -export .
+stderr '^go build example:\n# pkg-config (.*\n)+pkg-config: exit status \d+$'
+
+-- go.mod --
+module example
+go 1.20
+-- example.go --
+package example
+
+// #cgo pkg-config: libnot-a-valid-cgo-library
+import "C"
+
+package main() {}