From 54df87eb6b8bd14b43e08a8d8828ad32a3488341 Mon Sep 17 00:00:00 2001 From: Rob Figueiredo Date: Thu, 19 Nov 2020 21:40:36 -0500 Subject: [PATCH 1/3] relativize paths in error messages Before: /private/var/tmp/_bazel_robfig/1db08896ecff7e5e96a745981c3b77e6/sandbox/darwin-sandbox/28708/execroot/corp/gocode/src/corp/analytics/searchterms/redshiftcopier.go:14:2: imported and not used: "corp/net/rpc" compilepkg: nogo: errors found by nogo during build-time code analysis: /private/var/tmp/_bazel_robfig/1db08896ecff7e5e96a745981c3b77e6/sandbox/darwin-sandbox/29923/execroot/corp/gocode/src/corp/net/rpc/server.go:125:7: the cancel function returned by context.WithTimeout should be called, not discarded, to avoid a context leak After: gocode/src/corp/analytics/searchterms/redshiftcopier.go:14:2: imported and not used: "corp/net/rpc" compilepkg: nogo: errors found by nogo during build-time code analysis: gocode/src/corp/net/rpc/server.go:125:7: the cancel function returned by context.WithTimeout should be called, not discarded, to avoid a context leak Fixes #2061 --- go/tools/builders/compilepkg.go | 2 +- go/tools/builders/env.go | 36 ++++++++++++++++++++++++++------- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/go/tools/builders/compilepkg.go b/go/tools/builders/compilepkg.go index 77bc28b116..4af57a774b 100644 --- a/go/tools/builders/compilepkg.go +++ b/go/tools/builders/compilepkg.go @@ -460,7 +460,7 @@ func runNogo(ctx context.Context, workDir string, nogoPath string, srcs []string cmdLine := strings.Join(args, " ") return fmt.Errorf("nogo command '%s' exited unexpectedly: %s", cmdLine, exitErr.String()) } - return errors.New(out.String()) + return errors.New(string(relativizePaths(out.Bytes()))) } else { if out.Len() != 0 { fmt.Fprintln(os.Stderr, out.String()) diff --git a/go/tools/builders/env.go b/go/tools/builders/env.go index 8a839dd618..45c55b95c9 100644 --- a/go/tools/builders/env.go +++ b/go/tools/builders/env.go @@ -60,6 +60,8 @@ type env struct { workDirPath string shouldPreserveWorkDir bool + + execDir string } // envFlags registers flags common to multiple builders and returns an env @@ -71,6 +73,7 @@ func envFlags(flags *flag.FlagSet) *env { flags.StringVar(&env.installSuffix, "installsuffix", "", "Standard library under GOROOT/pkg") flags.BoolVar(&env.verbose, "v", false, "Whether subprocess command lines should be printed") flags.BoolVar(&env.shouldPreserveWorkDir, "work", false, "if true, the temporary work directory will be preserved") + env.execDir, _ = os.Getwd() return env } @@ -133,18 +136,20 @@ func (e *env) runCommand(args []string) error { cmd := exec.Command(args[0], args[1:]...) // Redirecting stdout to stderr. This mirrors behavior in the go command: // https://go.googlesource.com/go/+/refs/tags/go1.15.2/src/cmd/go/internal/work/exec.go#1958 - cmd.Stdout = os.Stderr - cmd.Stderr = os.Stderr - return runAndLogCommand(cmd, e.verbose) + var stderr bytes.Buffer + cmd.Stdout = &stderr + cmd.Stderr = &stderr + return runAndLogCommand(cmd, &stderr, e.verbose) } // runCommandToFile executes a subprocess and writes the output to the given // writer. func (e *env) runCommandToFile(w io.Writer, args []string) error { cmd := exec.Command(args[0], args[1:]...) + var stderr bytes.Buffer cmd.Stdout = w - cmd.Stderr = os.Stderr - return runAndLogCommand(cmd, e.verbose) + cmd.Stderr = &stderr + return runAndLogCommand(cmd, &stderr, e.verbose) } func absEnv(envNameList []string, argList []string) error { @@ -158,13 +163,15 @@ func absEnv(envNameList []string, argList []string) error { return nil } -func runAndLogCommand(cmd *exec.Cmd, verbose bool) error { +func runAndLogCommand(cmd *exec.Cmd, stderr *bytes.Buffer, verbose bool) error { if verbose { fmt.Fprintln(os.Stderr, formatCommand(cmd)) } cleanup := passLongArgsInResponseFiles(cmd) defer cleanup() - if err := cmd.Run(); err != nil { + err := cmd.Run() + os.Stderr.Write(relativizePaths(stderr.Bytes())) + if err != nil { return fmt.Errorf("error running subcommand %s: %v", cmd.Path, err) } return nil @@ -332,6 +339,21 @@ func absArgs(args []string, flags []string) { } } +// relativizePaths converts absolute paths found in the given output string to +// relative, if they are within the PWD. +func relativizePaths(output []byte) []byte { + dir, err := os.Getwd() + if dir == "" || err != nil { + return output + } + dirBytes := make([]byte, len(dir), len(dir)+1) + copy(dirBytes, dir) + if !bytes.HasSuffix(dirBytes, []byte{filepath.Separator}) { + dirBytes = append(dirBytes, filepath.Separator) + } + return bytes.ReplaceAll(output, dirBytes, nil) +} + // formatCommand formats cmd as a string that can be pasted into a shell. // Spaces in environment variables and arguments are escaped as needed. func formatCommand(cmd *exec.Cmd) string { From 8c591bd8511559b9a7de09b71b2d17187af275e5 Mon Sep 17 00:00:00 2001 From: Rob Figueiredo Date: Fri, 20 Nov 2020 16:47:38 -0500 Subject: [PATCH 2/3] Address comments --- go/tools/builders/env.go | 43 ++++++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/go/tools/builders/env.go b/go/tools/builders/env.go index 45c55b95c9..0cb455665e 100644 --- a/go/tools/builders/env.go +++ b/go/tools/builders/env.go @@ -60,8 +60,6 @@ type env struct { workDirPath string shouldPreserveWorkDir bool - - execDir string } // envFlags registers flags common to multiple builders and returns an env @@ -73,7 +71,6 @@ func envFlags(flags *flag.FlagSet) *env { flags.StringVar(&env.installSuffix, "installsuffix", "", "Standard library under GOROOT/pkg") flags.BoolVar(&env.verbose, "v", false, "Whether subprocess command lines should be printed") flags.BoolVar(&env.shouldPreserveWorkDir, "work", false, "if true, the temporary work directory will be preserved") - env.execDir, _ = os.Getwd() return env } @@ -136,20 +133,14 @@ func (e *env) runCommand(args []string) error { cmd := exec.Command(args[0], args[1:]...) // Redirecting stdout to stderr. This mirrors behavior in the go command: // https://go.googlesource.com/go/+/refs/tags/go1.15.2/src/cmd/go/internal/work/exec.go#1958 - var stderr bytes.Buffer - cmd.Stdout = &stderr - cmd.Stderr = &stderr - return runAndLogCommand(cmd, &stderr, e.verbose) + return runAndLogCommandOutput(cmd, os.Stderr, os.Stderr, e.verbose) } // runCommandToFile executes a subprocess and writes the output to the given // writer. func (e *env) runCommandToFile(w io.Writer, args []string) error { cmd := exec.Command(args[0], args[1:]...) - var stderr bytes.Buffer - cmd.Stdout = w - cmd.Stderr = &stderr - return runAndLogCommand(cmd, &stderr, e.verbose) + return runAndLogCommandOutput(cmd, w, os.Stderr, e.verbose) } func absEnv(envNameList []string, argList []string) error { @@ -163,15 +154,23 @@ func absEnv(envNameList []string, argList []string) error { return nil } -func runAndLogCommand(cmd *exec.Cmd, stderr *bytes.Buffer, verbose bool) error { +func runAndLogCommandOutput(cmd *exec.Cmd, stdout, stderr io.Writer, verbose bool) error { + var bufout, buferr bytes.Buffer + cmd.Stdout = &bufout + cmd.Stderr = &buferr + err := runAndLogCommand(cmd, verbose) + io.Copy(stdout, bytes.NewReader(relativizePaths(bufout.Bytes()))) + io.Copy(stderr, bytes.NewReader(relativizePaths(buferr.Bytes()))) + return err +} + +func runAndLogCommand(cmd *exec.Cmd, verbose bool) error { if verbose { fmt.Fprintln(os.Stderr, formatCommand(cmd)) } cleanup := passLongArgsInResponseFiles(cmd) defer cleanup() - err := cmd.Run() - os.Stderr.Write(relativizePaths(stderr.Bytes())) - if err != nil { + if err := cmd.Run(); err != nil { return fmt.Errorf("error running subcommand %s: %v", cmd.Path, err) } return nil @@ -340,7 +339,7 @@ func absArgs(args []string, flags []string) { } // relativizePaths converts absolute paths found in the given output string to -// relative, if they are within the PWD. +// relative, if they are within the working directory. func relativizePaths(output []byte) []byte { dir, err := os.Getwd() if dir == "" || err != nil { @@ -348,10 +347,16 @@ func relativizePaths(output []byte) []byte { } dirBytes := make([]byte, len(dir), len(dir)+1) copy(dirBytes, dir) - if !bytes.HasSuffix(dirBytes, []byte{filepath.Separator}) { - dirBytes = append(dirBytes, filepath.Separator) + if bytes.HasSuffix(dirBytes, []byte{filepath.Separator}) { + return bytes.ReplaceAll(output, dirBytes, nil) } - return bytes.ReplaceAll(output, dirBytes, nil) + + // This is the common case. + // Replace "$CWD/" with "" and "$CWD" with "." + dirBytes = append(dirBytes, filepath.Separator) + output = bytes.ReplaceAll(output, dirBytes, nil) + dirBytes = dirBytes[:len(dirBytes)-1] + return bytes.ReplaceAll(output, dirBytes, []byte{'.'}) } // formatCommand formats cmd as a string that can be pasted into a shell. From 93b9531b600cfe567dbb9ec5a587a1a4624a105b Mon Sep 17 00:00:00 2001 From: Rob Figueiredo Date: Fri, 4 Dec 2020 13:14:05 -0500 Subject: [PATCH 3/3] Do not relativize in runAndLogCommandToFile --- go/tools/builders/env.go | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/go/tools/builders/env.go b/go/tools/builders/env.go index 0cb455665e..5faacee0ce 100644 --- a/go/tools/builders/env.go +++ b/go/tools/builders/env.go @@ -133,14 +133,21 @@ func (e *env) runCommand(args []string) error { cmd := exec.Command(args[0], args[1:]...) // Redirecting stdout to stderr. This mirrors behavior in the go command: // https://go.googlesource.com/go/+/refs/tags/go1.15.2/src/cmd/go/internal/work/exec.go#1958 - return runAndLogCommandOutput(cmd, os.Stderr, os.Stderr, e.verbose) + buf := &bytes.Buffer{} + cmd.Stdout = buf + cmd.Stderr = buf + err := runAndLogCommand(cmd, e.verbose) + fmt.Fprint(os.Stderr, relativizePaths(buf.Bytes())) + return err } // runCommandToFile executes a subprocess and writes the output to the given // writer. func (e *env) runCommandToFile(w io.Writer, args []string) error { cmd := exec.Command(args[0], args[1:]...) - return runAndLogCommandOutput(cmd, w, os.Stderr, e.verbose) + cmd.Stdout = w + cmd.Stderr = os.Stderr + return runAndLogCommand(cmd, e.verbose) } func absEnv(envNameList []string, argList []string) error { @@ -154,16 +161,6 @@ func absEnv(envNameList []string, argList []string) error { return nil } -func runAndLogCommandOutput(cmd *exec.Cmd, stdout, stderr io.Writer, verbose bool) error { - var bufout, buferr bytes.Buffer - cmd.Stdout = &bufout - cmd.Stderr = &buferr - err := runAndLogCommand(cmd, verbose) - io.Copy(stdout, bytes.NewReader(relativizePaths(bufout.Bytes()))) - io.Copy(stderr, bytes.NewReader(relativizePaths(buferr.Bytes()))) - return err -} - func runAndLogCommand(cmd *exec.Cmd, verbose bool) error { if verbose { fmt.Fprintln(os.Stderr, formatCommand(cmd)) @@ -347,7 +344,7 @@ func relativizePaths(output []byte) []byte { } dirBytes := make([]byte, len(dir), len(dir)+1) copy(dirBytes, dir) - if bytes.HasSuffix(dirBytes, []byte{filepath.Separator}) { + if bytes.HasSuffix(dirBytes, []byte{filepath.Separator}) { return bytes.ReplaceAll(output, dirBytes, nil) }