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>

diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go
index d66ffb7b86e44fffcc887db4028bc5701cdb2309..3c5b9842f2870fd445205d5f6cf89041a6616e29 100644 (file)
--- a/src/cmd/go/internal/work/exec.go
+++ b/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
-       if e.importPath != "" && !strings.HasPrefix(e.desc, e.importPath) {
-               // Ensure the import path is part of the message. We checked the prefix
-               // because desc can be a package ID, which may have text in addition to
-               // the import path.
-               //
-               // TODO(austin): Checking the prefix seems flimsy. reportCmd could
-               // 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)
+       var msg string
+       if e.needsPath {
+               // Ensure the import path is part of the message.
+               // Clearly distinguish the description from the import path.
+               msg = fmt.Sprintf("# %s\n# [%s]\n", e.importPath, e.desc)
+       } else {
+               msg = "# " + e.desc + "\n"
        }
-       return msg
+       return msg + e.text
 }
 
 func (e *cmdError) ImportPath() string {

diff --git a/src/cmd/go/testdata/script/list_pkgconfig_error.txt b/src/cmd/go/testdata/script/list_pkgconfig_error.txt
index de6eafd2c222211824cd8743400560481c410064..f554d2a4ed62581eb1a4c657e8b03bcc6d991915 100644 (file)
--- a/src/cmd/go/testdata/script/list_pkgconfig_error.txt
+++ b/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