]> Cypherpunks.ru repositories - gostls13.git/commitdiff
cmd/go: replace formatOutput/showOutput with reportCmd
authorAustin Clements <austin@google.com>
Fri, 15 Sep 2023 16:45:17 +0000 (12:45 -0400)
committerAustin Clements <austin@google.com>
Wed, 18 Oct 2023 14:06:33 +0000 (14:06 +0000)
The general pattern is to replace

if len(cmdOut) > 0 {
output := b.processOutput(cmdOut)
if err != nil {
err = formatOutput(b.WorkDir, dir, p.ImportPath, desc, output)
} else {
b.showOutput(a, dir, desc, output)
}
}
if err != nil {
return err
}

with

if err := b.reportCmd(a, p, desc, dir, cmdOut, err); err != nil {
return err
}

However, there is a fair amount of variation between call sites. The
most common non-trivial variation is sites where errors are an
expected outcome. In this case, often we simply pass "nil" for the
error to trigger only the printing behavior of reportCmd.

For #62067, but also a nice cleanup on its own.

Change-Id: Ie5f918017c02d8558f23ad4c38261077c0fa4ea3
Reviewed-on: https://go-review.googlesource.com/c/go/+/529219
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/cover.go
src/cmd/go/internal/work/exec.go
src/cmd/go/internal/work/gc.go
src/cmd/go/internal/work/gccgo.go
src/cmd/go/testdata/script/list_pkgconfig_error.txt

index b1de4b0cb4596e8e9df077d3a521e2b43538df12..ebffe524125dcb288830103fecad6b45ca6cdcda 100644 (file)
@@ -72,9 +72,7 @@ func WriteCoveragePercent(b *Builder, runAct *Action, mf string, w io.Writer) er
        dir := filepath.Dir(mf)
        output, cerr := b.CovData(runAct, "percent", "-i", dir)
        if cerr != nil {
-               p := runAct.Package
-               return formatOutput(b.WorkDir, p.Dir, p.ImportPath,
-                       p.Desc(), string(output))
+               return b.reportCmd(runAct, nil, "", "", output, cerr)
        }
        _, werr := w.Write(output)
        return werr
@@ -89,9 +87,7 @@ func WriteCoverageProfile(b *Builder, runAct *Action, mf, outf string, w io.Writ
        dir := filepath.Dir(mf)
        output, err := b.CovData(runAct, "textfmt", "-i", dir, "-o", outf)
        if err != nil {
-               p := runAct.Package
-               return formatOutput(b.WorkDir, p.Dir, p.ImportPath,
-                       p.Desc(), string(output))
+               return b.reportCmd(runAct, nil, "", "", output, err)
        }
        _, werr := w.Write(output)
        return werr
index eecdecaa053635fb6ee698b1b0f14e98366a1790..7b5232f1b00965d7ce66f4061b25e4ebb8ae5800 100644 (file)
@@ -594,7 +594,7 @@ func (b *Builder) build(ctx context.Context, a *Action) (err error) {
        var objects, cgoObjects, pcCFLAGS, pcLDFLAGS []string
 
        if p.UsesCgo() || p.UsesSwig() {
-               if pcCFLAGS, pcLDFLAGS, err = b.getPkgConfigFlags(p); err != nil {
+               if pcCFLAGS, pcLDFLAGS, err = b.getPkgConfigFlags(a); err != nil {
                        return
                }
        }
@@ -868,15 +868,7 @@ OverlayLoop:
        // Compile Go.
        objpkg := objdir + "_pkg_.a"
        ofile, out, err := BuildToolchain.gc(b, a, objpkg, icfg.Bytes(), embedcfg, symabis, len(sfiles) > 0, gofiles)
-       if len(out) > 0 {
-               output := b.processOutput(out)
-               if err != nil {
-                       return formatOutput(b.WorkDir, p.Dir, p.ImportPath, p.Desc(), output)
-               } else {
-                       b.showOutput(a, p.Dir, p.Desc(), output)
-               }
-       }
-       if err != nil {
+       if err := b.reportCmd(a, nil, "", "", out, err); err != nil {
                return err
        }
        if ofile != objpkg {
@@ -1000,8 +992,11 @@ func (b *Builder) checkDirectives(a *Action) error {
                }
        }
        if msg != nil {
-               return formatOutput(b.WorkDir, p.Dir, p.ImportPath, p.Desc(), b.processOutput(msg.Bytes()))
-
+               // We pass a non-nil error to reportCmd to trigger the failure reporting
+               // path, but the content of the error doesn't matter because msg is
+               // non-empty.
+               err := errors.New("invalid directive")
+               return b.reportCmd(a, nil, "", "", msg.Bytes(), err)
        }
        return nil
 }
@@ -1616,8 +1611,9 @@ func splitPkgConfigOutput(out []byte) ([]string, error) {
        return flags, nil
 }
 
-// Calls pkg-config if needed and returns the cflags/ldflags needed to build the package.
-func (b *Builder) getPkgConfigFlags(p *load.Package) (cflags, ldflags []string, err error) {
+// Calls pkg-config if needed and returns the cflags/ldflags needed to build a's package.
+func (b *Builder) getPkgConfigFlags(a *Action) (cflags, ldflags []string, err error) {
+       p := a.Package
        if pcargs := p.CgoPkgConfig; len(pcargs) > 0 {
                // pkg-config permits arguments to appear anywhere in
                // the command line. Move them all to the front, before --.
@@ -1640,8 +1636,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 {
-                       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
+                       desc := b.PkgconfigCmd() + " --cflags " + strings.Join(pcflags, " ") + " -- " + strings.Join(pkgs, " ")
+                       return nil, nil, b.reportCmd(a, nil, desc, "", out, err)
                }
                if len(out) > 0 {
                        cflags, err = splitPkgConfigOutput(bytes.TrimSpace(out))
@@ -1654,8 +1650,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 {
-                       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
+                       desc := b.PkgconfigCmd() + " --libs " + strings.Join(pcflags, " ") + " -- " + strings.Join(pkgs, " ")
+                       return nil, nil, b.reportCmd(a, nil, desc, "", out, err)
                }
                if len(out) > 0 {
                        // We need to handle path with spaces so that C:/Program\ Files can pass
@@ -2511,17 +2507,10 @@ var cgoTypeSigRe = lazyregexp.New(`\b_C2?(type|func|var|macro)_\B`)
 // and returns a non-nil error.
 func (b *Builder) run(a *Action, dir string, desc string, env []string, cmdargs ...any) error {
        out, err := b.runOut(a, dir, env, cmdargs...)
-       if len(out) > 0 {
-               if desc == "" {
-                       desc = b.fmtcmd(dir, "%s", strings.Join(str.StringList(cmdargs...), " "))
-               }
-               if err != nil {
-                       err = formatOutput(b.WorkDir, dir, a.Package.ImportPath, desc, b.processOutput(out))
-               } else {
-                       b.showOutput(a, dir, desc, b.processOutput(out))
-               }
+       if desc == "" {
+               desc = b.fmtcmd(dir, "%s", strings.Join(str.StringList(cmdargs...), " "))
        }
-       return err
+       return b.reportCmd(a, nil, desc, dir, out, err)
 }
 
 // processOutput prepares the output of runOut to be output to the console.
@@ -2870,38 +2859,33 @@ func (b *Builder) ccompile(a *Action, p *load.Package, outfile string, flags []s
                overlayPath = p
        }
        output, err := b.runOut(a, filepath.Dir(overlayPath), b.cCompilerEnv(), compiler, flags, "-o", outfile, "-c", filepath.Base(overlayPath))
-       if len(output) > 0 {
-               // On FreeBSD 11, when we pass -g to clang 3.8 it
-               // invokes its internal assembler with -dwarf-version=2.
-               // When it sees .section .note.GNU-stack, it warns
-               // "DWARF2 only supports one section per compilation unit".
-               // This warning makes no sense, since the section is empty,
-               // but it confuses people.
-               // We work around the problem by detecting the warning
-               // and dropping -g and trying again.
-               if bytes.Contains(output, []byte("DWARF2 only supports one section per compilation unit")) {
-                       newFlags := make([]string, 0, len(flags))
-                       for _, f := range flags {
-                               if !strings.HasPrefix(f, "-g") {
-                                       newFlags = append(newFlags, f)
-                               }
-                       }
-                       if len(newFlags) < len(flags) {
-                               return b.ccompile(a, p, outfile, newFlags, file, compiler)
-                       }
-               }
 
-               if err == nil && os.Getenv("GO_BUILDER_NAME") != "" {
-                       output = append(output, "C compiler warning promoted to error on Go builders\n"...)
-                       err = errors.New("warning promoted to error")
+       // On FreeBSD 11, when we pass -g to clang 3.8 it
+       // invokes its internal assembler with -dwarf-version=2.
+       // When it sees .section .note.GNU-stack, it warns
+       // "DWARF2 only supports one section per compilation unit".
+       // This warning makes no sense, since the section is empty,
+       // but it confuses people.
+       // We work around the problem by detecting the warning
+       // and dropping -g and trying again.
+       if bytes.Contains(output, []byte("DWARF2 only supports one section per compilation unit")) {
+               newFlags := make([]string, 0, len(flags))
+               for _, f := range flags {
+                       if !strings.HasPrefix(f, "-g") {
+                               newFlags = append(newFlags, f)
+                       }
                }
-               if err != nil {
-                       err = formatOutput(b.WorkDir, p.Dir, p.ImportPath, p.Desc(), b.processOutput(output))
-               } else {
-                       b.showOutput(a, p.Dir, p.Desc(), b.processOutput(output))
+               if len(newFlags) < len(flags) {
+                       return b.ccompile(a, p, outfile, newFlags, file, compiler)
                }
        }
-       return err
+
+       if len(output) > 0 && err == nil && os.Getenv("GO_BUILDER_NAME") != "" {
+               output = append(output, "C compiler warning promoted to error on Go builders\n"...)
+               err = errors.New("warning promoted to error")
+       }
+
+       return b.reportCmd(a, p, "", "", output, err)
 }
 
 // gccld runs the gcc linker to create an executable from a set of object files.
@@ -2915,7 +2899,6 @@ func (b *Builder) gccld(a *Action, p *load.Package, objdir, outfile string, flag
        }
 
        cmdargs := []any{cmd, "-o", outfile, objs, flags}
-       dir := p.Dir
        out, err := b.runOut(a, base.Cwd(), b.cCompilerEnv(), cmdargs...)
 
        if len(out) > 0 {
@@ -2951,9 +2934,11 @@ func (b *Builder) gccld(a *Action, p *load.Package, objdir, outfile string, flag
                        save = append(save, line)
                }
                out = bytes.Join(save, nil)
-               if len(out) > 0 && (cfg.BuildN || cfg.BuildX) {
-                       b.showOutput(nil, dir, p.ImportPath, b.processOutput(out))
-               }
+       }
+       // Note that failure is an expected outcome here, so we report output only
+       // in debug mode and don't report the error.
+       if cfg.BuildN || cfg.BuildX {
+               b.reportCmd(a, p, "", "", out, nil)
        }
        return err
 }
@@ -3493,7 +3478,7 @@ func (b *Builder) cgo(a *Action, cgoExe, objdir string, pcCFLAGS, pcLDFLAGS, cgo
                if pkgpath := gccgoPkgpath(p); pkgpath != "" {
                        cgoflags = append(cgoflags, "-gccgopkgpath="+pkgpath)
                }
-               if !BuildToolchain.(gccgoToolchain).supportsCgoIncomplete(b) {
+               if !BuildToolchain.(gccgoToolchain).supportsCgoIncomplete(b, a) {
                        cgoflags = append(cgoflags, "-gccgo_define_cgoincomplete")
                }
        }
@@ -3990,18 +3975,11 @@ func (b *Builder) swigOne(a *Action, p *load.Package, file, objdir string, pcCFL
        }
 
        out, err := b.runOut(a, p.Dir, nil, "swig", args, file)
-       if err != nil {
-               if len(out) > 0 {
-                       if bytes.Contains(out, []byte("-intgosize")) || bytes.Contains(out, []byte("-cgo")) {
-                               return "", "", errors.New("must have SWIG version >= 3.0.6")
-                       }
-                       // swig error
-                       err = formatOutput(b.WorkDir, p.Dir, p.ImportPath, p.Desc(), b.processOutput(out))
-               }
-               return "", "", err
+       if err != nil && (bytes.Contains(out, []byte("-intgosize")) || bytes.Contains(out, []byte("-cgo"))) {
+               return "", "", errors.New("must have SWIG version >= 3.0.6")
        }
-       if len(out) > 0 {
-               b.showOutput(a, p.Dir, p.Desc(), b.processOutput(out)) // swig warning
+       if err := b.reportCmd(a, p, "", "", out, err); err != nil {
+               return "", "", err
        }
 
        // If the input was x.swig, the output is x.go in the objdir.
index 3ea9f714d24222ebb2e697d85c7ca370230fb5a3..6054e8d81c93c3de0c787121e0ddb81197078621 100644 (file)
@@ -464,7 +464,7 @@ func (gcToolchain) pack(b *Builder, a *Action, afile string, ofiles []string) er
                return nil
        }
        if err := packInternal(absAfile, absOfiles); err != nil {
-               return formatOutput(b.WorkDir, p.Dir, p.ImportPath, p.Desc(), err.Error()+"\n")
+               return b.reportCmd(a, nil, "", "", nil, err)
        }
        return nil
 }
index fa566bfd0593b690f744dc4e7c6cd54e1dca2ecf..f38d57a43f4d732af4f4ba5e58b6ca8cfb9dfd63 100644 (file)
@@ -5,6 +5,7 @@
 package work
 
 import (
+       "bytes"
        "fmt"
        "os"
        "os/exec"
@@ -243,12 +244,8 @@ func (tools gccgoToolchain) pack(b *Builder, a *Action, afile string, ofiles []s
                return b.run(a, p.Dir, p.ImportPath, nil, tools.ar(), arArgs, "rc", absAfile, absOfiles)
        }
 
-       if len(output) > 0 {
-               // Show the output if there is any even without errors.
-               b.showOutput(a, p.Dir, p.ImportPath, b.processOutput(output))
-       }
-
-       return nil
+       // Show the output if there is any even without errors.
+       return b.reportCmd(a, nil, "", "", output, nil)
 }
 
 func (tools gccgoToolchain) link(b *Builder, root *Action, out, importcfg string, allactions []*Action, buildmode, desc string) error {
@@ -617,7 +614,10 @@ type I cgo.Incomplete
 
 // supportsCgoIncomplete reports whether the gccgo/GoLLVM compiler
 // being used supports cgo.Incomplete, which was added in GCC 13.
-func (tools gccgoToolchain) supportsCgoIncomplete(b *Builder) bool {
+//
+// This takes an Action only for output reporting purposes.
+// The result value is unrelated to the Action.
+func (tools gccgoToolchain) supportsCgoIncomplete(b *Builder, a *Action) bool {
        gccgoSupportsCgoIncompleteOnce.Do(func() {
                fail := func(err error) {
                        fmt.Fprintf(os.Stderr, "cmd/go: %v\n", err)
@@ -650,14 +650,17 @@ func (tools gccgoToolchain) supportsCgoIncomplete(b *Builder) bool {
                }
                cmd := exec.Command(tools.compiler(), "-c", "-o", on, fn)
                cmd.Dir = tmpdir
-               var buf strings.Builder
+               var buf bytes.Buffer
                cmd.Stdout = &buf
                cmd.Stderr = &buf
                err = cmd.Run()
-               if out := buf.String(); len(out) > 0 && cfg.BuildX {
-                       b.showOutput(nil, tmpdir, b.fmtcmd(tmpdir, "%s -c -o %s %s", tools.compiler(), on, fn), out)
-               }
                gccgoSupportsCgoIncomplete = err == nil
+               if cfg.BuildN || cfg.BuildX {
+                       // Show output. We always pass a nil err because errors are an
+                       // expected outcome in this case.
+                       desc := b.fmtcmd(tmpdir, "%s -c -o %s %s", tools.compiler(), on, fn)
+                       b.reportCmd(a, nil, desc, tmpdir, buf.Bytes(), nil)
+               }
        })
        return gccgoSupportsCgoIncomplete
 }
index 7d671a64389b5afb22b5f6d16e7e1133e62d2b82..8e5a278dd0e5bdfa548fd2dd51f5d44c5b0210e9 100644 (file)
@@ -2,7 +2,7 @@
 [!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+$'
+stderr '^go build example:\n# pkg-config (.*\n)+Package .* not found$'
 
 -- go.mod --
 module example