cmd/go: clean up adding import path to command error

Currently, cmdError makes a somewhat fuzzy attempt to ensure the
package import path is part of the printed error, using a string
prefix check. Also, if it decides it does need to add the import path,
it prints it as a "go build" line, which could be misleading because
it can happen outside of "go build".

Clean up the whole code path by explicitly checking the provided error
description against Package.Desc(), and instead of emitting "go build"
in the error message, print it as "# importPath" just like we do in
the common case.

Change-Id: Idb61ac8ffd6920a3d2d282697f4d7d5555ebae0c
Reviewed-on: https://go-review.googlesource.com/c/go/+/534655
Auto-Submit: Austin Clements <austin@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Bryan Mills <bcmills@google.com>

src/cmd/go/internal/work/exec.go

@@ -2321,7 +2321,11 @@ func (b *Builder) reportCmd(a *Action, desc, dir string, cmdOut []byte, cmdErr e
 		out = cgoTypeSigRe.ReplaceAllString(out, "C.")
 	}
 
-	err := &cmdError{desc, out, importPath}
+	// Usually desc is already p.Desc(), but if not, signal cmdError.Error to
+	// add a line explicitly metioning the import path.
+	needsPath := importPath != "" && p != nil && desc != p.Desc()
+
+	err := &cmdError{desc, out, importPath, needsPath}
 	if cmdErr != nil {
 		// The command failed. Report the output up as an error.
 		return err
@@ -2360,21 +2364,19 @@ type cmdError struct {
 	desc       string
 	text       string
 	importPath string
+	needsPath  bool // Set if desc does not already include the import path
 }
 
 func (e *cmdError) Error() string {
-	msg := "# " + e.desc + "\n" + e.text
+	var msg string
-	if e.importPath != "" && !strings.HasPrefix(e.desc, e.importPath) {
+	if e.needsPath {
-		// Ensure the import path is part of the message. We checked the prefix
+		// Ensure the import path is part of the message.
-		// because desc can be a package ID, which may have text in addition to
+		// Clearly distinguish the description from the import path.
-		// the import path.
+		msg = fmt.Sprintf("# %s\n# [%s]\n", e.importPath, e.desc)
-		//
+	} else {
-		// TODO(austin): Checking the prefix seems flimsy. reportCmd could
+		msg = "# " + e.desc + "\n"
-		// instead check if desc != p.Desc() and leave a flag in cmdError to
-		// signal this code path.
-		msg = fmt.Sprintf("go build %s:\n%s", e.importPath, msg)
 	}
-	return msg
+	return msg + e.text
 }
 
 func (e *cmdError) ImportPath() string {

src/cmd/go/testdata/script/list_pkgconfig_error.txt

@@ -2,7 +2,7 @@
 [!exec:pkg-config] skip 'test requires pkg-config tool'
 
 ! go list -export .
-stderr '^go build example:\n# pkg-config (.*\n)+Package .* not found'
+stderr '^# example\n# \[pkg-config .*\]\n(.*\n)*Package .* not found'
 
 -- go.mod --
 module example