]> Cypherpunks repositories - gostls13.git/commitdiff
[release-branch.go1.19] cmd/go: do not exit with non-zero code from go list -e -export
authorMichael Matloob <matloob@golang.org>
Fri, 30 Sep 2022 20:49:42 +0000 (16:49 -0400)
committerMichael Pratt <mpratt@google.com>
Tue, 13 Jun 2023 20:00:04 +0000 (20:00 +0000)
go list -e -export puts errors running build actions on the load.Package
corresponding to the failed action rather than exiting with a non zero
exit code.

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

Change-Id: I1fea85cc5a0557f514fe9d4ed3b6a858376fdcde
Reviewed-on: https://go-review.googlesource.com/c/go/+/437298
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/502195
TryBot-Bypass: Bryan Mills <bcmills@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
src/cmd/go/internal/list/list.go
src/cmd/go/internal/work/action.go
src/cmd/go/internal/work/exec.go
src/cmd/go/internal/work/gc.go
src/cmd/go/testdata/script/build_cwd_newline.txt
src/cmd/go/testdata/script/list_export_e.txt [new file with mode: 0644]

index dfb58c28ebff6d0a4fad22ccdac4bb1247a3b132..1c61e6ec708c4a5a4728a9c9630b72c9963125d6 100644 (file)
@@ -690,6 +690,9 @@ func runList(ctx context.Context, cmd *base.Command, args []string) {
        needStale := (listJson && listJsonFields.needAny("Stale", "StaleReason")) || strings.Contains(*listFmt, ".Stale")
        if needStale || *listExport || *listCompiled {
                b := work.NewBuilder("")
+               if *listE {
+                       b.AllowErrors = true
+               }
                b.IsCmdList = true
                b.NeedExport = *listExport
                b.NeedCompiledGoFiles = *listCompiled
index 4bbd23ab8e43212b10a1303cfec594a16afff59e..422cb3bd934eeb7650c767f64df78ea9cdcccfae 100644 (file)
@@ -43,6 +43,7 @@ type Builder struct {
        NeedError           bool // list needs p.Error
        NeedExport          bool // list needs p.Export
        NeedCompiledGoFiles bool // list needs p.CompiledGoFiles
+       AllowErrors         bool // errors don't immediately exit the program
 
        objdirSeq int // counter for NewObjdir
        pkgSeq    int
index 567f0459222dd0af2efbc4a7d0df21c62ff2671d..ac3815511c2c7c77a901266beb97c02d139a1ab9 100644 (file)
@@ -149,8 +149,10 @@ func (b *Builder) Do(ctx context.Context, root *Action) {
                defer b.exec.Unlock()
 
                if err != nil {
-                       if err == errPrintedOutput {
-                               base.SetExitStatus(2)
+                       if b.AllowErrors {
+                               if a.Package.Error == nil {
+                                       a.Package.Error = &load.PackageError{Err: err}
+                               }
                        } else {
                                base.Errorf("%s", err)
                        }
@@ -508,8 +510,8 @@ func (b *Builder) build(ctx context.Context, a *Action) (err error) {
        }
 
        defer func() {
-               if err != nil && err != errPrintedOutput {
-                       err = fmt.Errorf("go build %s: %v", a.Package.ImportPath, err)
+               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}
@@ -821,9 +823,11 @@ OverlayLoop:
                if p.Module != nil && !allowedVersion(p.Module.GoVersion) {
                        output += "note: module requires Go " + p.Module.GoVersion + "\n"
                }
-               b.showOutput(a, a.Package.Dir, a.Package.Desc(), output)
+
                if err != nil {
-                       return errPrintedOutput
+                       return errors.New(fmt.Sprint(formatOutput(b.WorkDir, p.Dir, p.Desc(), output)))
+               } else {
+                       b.showOutput(a, p.Dir, p.Desc(), output)
                }
        }
        if err != nil {
@@ -1502,9 +1506,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 {
-                       b.showOutput(nil, p.Dir, b.PkgconfigCmd()+" --cflags "+strings.Join(pcflags, " ")+" -- "+strings.Join(pkgs, " "), string(out))
-                       b.Print(err.Error() + "\n")
-                       return nil, nil, errPrintedOutput
+                       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()))
                }
                if len(out) > 0 {
                        cflags, err = splitPkgConfigOutput(out)
@@ -1517,9 +1520,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 {
-                       b.showOutput(nil, p.Dir, b.PkgconfigCmd()+" --libs "+strings.Join(pcflags, " ")+" -- "+strings.Join(pkgs, " "), string(out))
-                       b.Print(err.Error() + "\n")
-                       return nil, nil, errPrintedOutput
+                       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()))
                }
                if len(out) > 0 {
                        // NOTE: we don't attempt to parse quotes and unescapes here. pkg-config
@@ -1611,7 +1613,7 @@ func (b *Builder) linkShared(ctx context.Context, a *Action) (err error) {
 // BuildInstallFunc is the action for installing a single package or executable.
 func BuildInstallFunc(b *Builder, ctx context.Context, a *Action) (err error) {
        defer func() {
-               if err != nil && err != errPrintedOutput {
+               if err != nil {
                        // a.Package == nil is possible for the go install -buildmode=shared
                        // action that installs libmangledname.so, which corresponds to
                        // a list of packages, not just one.
@@ -2015,15 +2017,7 @@ 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 := "# " + 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)
-               suffix = strings.ReplaceAll(suffix, "\n\t"+dir, "\n\t"+reldir)
-       }
-       suffix = strings.ReplaceAll(suffix, " "+b.WorkDir, " $WORK")
-
+       prefix, suffix := formatOutput(b.WorkDir, dir, desc, out)
        if a != nil && a.output != nil {
                a.output = append(a.output, prefix...)
                a.output = append(a.output, suffix...)
@@ -2035,12 +2029,41 @@ func (b *Builder) showOutput(a *Action, dir, desc, out string) {
        b.Print(prefix, suffix)
 }
 
-// errPrintedOutput is a special error indicating that a command failed
-// but that it generated output as well, and that output has already
-// been printed, so there's no point showing 'exit status 1' or whatever
-// the wait status was. The main executor, builder.do, knows not to
-// print this error.
-var errPrintedOutput = errors.New("already printed output - no need to show error")
+// formatOutput prints "# desc" followed by the given output.
+// The output is expected to contain references to 'dir', usually
+// the source directory for the package that has failed to build.
+// formatOutput rewrites mentions of dir with a relative path to dir
+// when the relative path is shorter. This is usually more pleasant.
+// For example, if fmt doesn't compile and we are in src/html,
+// the output is
+//
+//     $ go build
+//     # fmt
+//     ../fmt/print.go:1090: undefined: asdf
+//     $
+//
+// instead of
+//
+//     $ go build
+//     # fmt
+//     /usr/gopher/go/src/fmt/print.go:1090: undefined: asdf
+//     $
+//
+// 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
+       if reldir := base.ShortPath(dir); reldir != dir {
+               suffix = strings.ReplaceAll(suffix, " "+dir, " "+reldir)
+               suffix = strings.ReplaceAll(suffix, "\n"+dir, "\n"+reldir)
+               suffix = strings.ReplaceAll(suffix, "\n\t"+dir, "\n\t"+reldir)
+       }
+       suffix = strings.ReplaceAll(suffix, " "+workDir, " $WORK")
+
+       return prefix, suffix
+}
 
 var cgoLine = lazyregexp.New(`\[[^\[\]]+\.(cgo1|cover)\.go:[0-9]+(:[0-9]+)?\]`)
 var cgoTypeSigRe = lazyregexp.New(`\b_C2?(type|func|var|macro)_\B`)
@@ -2054,9 +2077,10 @@ func (b *Builder) run(a *Action, dir string, desc string, env []string, cmdargs
                if desc == "" {
                        desc = b.fmtcmd(dir, "%s", strings.Join(str.StringList(cmdargs...), " "))
                }
-               b.showOutput(a, dir, desc, b.processOutput(out))
                if err != nil {
-                       err = errPrintedOutput
+                       err = errors.New(fmt.Sprint(formatOutput(b.WorkDir, dir, desc, b.processOutput(out))))
+               } else {
+                       b.showOutput(a, dir, desc, b.processOutput(out))
                }
        }
        return err
@@ -2400,11 +2424,10 @@ func (b *Builder) ccompile(a *Action, p *load.Package, outfile string, flags []s
                        }
                }
 
-               b.showOutput(a, p.Dir, desc, b.processOutput(output))
-               if err != nil {
-                       err = errPrintedOutput
-               } else if os.Getenv("GO_BUILDER_NAME") != "" {
-                       return errors.New("C compiler warning promoted to error on Go builders")
+               if err != nil || os.Getenv("GO_BUILDER_NAME") != "" {
+                       err = errors.New(fmt.Sprintf(formatOutput(b.WorkDir, p.Dir, desc, b.processOutput(output))))
+               } else {
+                       b.showOutput(a, p.Dir, desc, b.processOutput(output))
                }
        }
        return err
@@ -3358,8 +3381,8 @@ func (b *Builder) swigOne(a *Action, p *load.Package, file, objdir string, pcCFL
                        if bytes.Contains(out, []byte("-intgosize")) || bytes.Contains(out, []byte("-cgo")) {
                                return "", "", errors.New("must have SWIG version >= 3.0.6")
                        }
-                       b.showOutput(a, p.Dir, p.Desc(), b.processOutput(out)) // swig error
-                       return "", "", errPrintedOutput
+                       // swig error
+                       return "", "", errors.New(fmt.Sprint(formatOutput(b.WorkDir, p.Dir, p.Desc(), b.processOutput(out))))
                }
                return "", "", err
        }
index 842952911554efeec715b189436e46f44dcc4803..3e405d448d9c7f50f47d2a84a734301c6807abd0 100644 (file)
@@ -7,6 +7,7 @@ package work
 import (
        "bufio"
        "bytes"
+       "errors"
        "fmt"
        "io"
        "log"
@@ -502,8 +503,7 @@ func (gcToolchain) pack(b *Builder, a *Action, afile string, ofiles []string) er
                return nil
        }
        if err := packInternal(absAfile, absOfiles); err != nil {
-               b.showOutput(a, p.Dir, p.Desc(), err.Error()+"\n")
-               return errPrintedOutput
+               return errors.New(fmt.Sprint(formatOutput(b.WorkDir, p.Dir, p.Desc(), err.Error()+"\n")))
        }
        return nil
 }
index e80235698a217f4116c965c5b486b114d64546e9..87d8b344e24ef79fe45823df28fd54f84ffa6e2a 100644 (file)
@@ -35,12 +35,9 @@ stderr 'package example: invalid package directory .*uh-oh'
 [!cgo] ! go test -v main_nocgo.go main_test.go
 stderr 'package command-line-arguments: invalid package directory .*uh-oh'
 
-       # Note: this command fails on release-branch.go1.19 but succeeds
-       # on release-branch.go1.20 and later. I (bcmills) suspect it may
-       # have been fixed by CL 437298.
-! go list -compiled -e -f '{{with .CompiledGoFiles}}{{.}}{{end}}' .
-stderr 'package example: invalid package directory .*uh-oh'
+go list -compiled -e -f '{{with .CompiledGoFiles}}{{.}}{{end}}' .
 ! stdout .
+! stderr .
 ! exists obj_
 
 # The cgo tool should only accept the source file if the working directory
diff --git a/src/cmd/go/testdata/script/list_export_e.txt b/src/cmd/go/testdata/script/list_export_e.txt
new file mode 100644 (file)
index 0000000..f6992e2
--- /dev/null
@@ -0,0 +1,19 @@
+go list -e -export ./...
+! stderr '.'
+go list -e -export -json ...
+
+-- go.mod --
+module example.com
+-- p1/p1.go --
+package p1
+
+const Name = "p1"
+-- p2/main.go --
+package main
+
+import "fmt"
+import "example.com/p1"
+
+func main() {
+        fmt.Println(p1.Name == 5)
+}
\ No newline at end of file