]> Cypherpunks repositories - gostls13.git/commitdiff
[release-branch.go1.19] cmd/go/internal/work: make formatOutput return an error that...
authorBryan C. Mills <bcmills@google.com>
Wed, 16 Nov 2022 20:36:30 +0000 (15:36 -0500)
committerMichael Pratt <mpratt@google.com>
Tue, 13 Jun 2023 20:00:09 +0000 (20:00 +0000)
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>
src/cmd/go/internal/work/exec.go
src/cmd/go/internal/work/gc.go
src/cmd/go/testdata/script/list_export_e.txt
src/cmd/go/testdata/script/list_pkgconfig_error.txt [new file with mode: 0644]

index ac3815511c2c7c77a901266beb97c02d139a1ab9..4840568d679edfc5f03984caaffd0150b92314e2 100644 (file)
@@ -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
        }
index 3e405d448d9c7f50f47d2a84a734301c6807abd0..680ca94974fe89565679d1ae01944d5227e44299 100644 (file)
@@ -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
 }
index f6992e221d0b49cdb27b5e33e34ddbaff029cd0c..8e4c361fc4ede8ef11817335b365b231dfa0580f 100644 (file)
@@ -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 (file)
index 0000000..7d671a6
--- /dev/null
@@ -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() {}