Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Relative paths in error messages #2736

Merged
merged 3 commits into from Dec 7, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion go/tools/builders/compilepkg.go
Expand Up @@ -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())
Expand Down
39 changes: 33 additions & 6 deletions go/tools/builders/env.go
Expand Up @@ -133,18 +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
cmd.Stdout = os.Stderr
cmd.Stderr = os.Stderr
return runAndLogCommand(cmd, e.verbose)
return runAndLogCommandOutput(cmd, os.Stderr, os.Stderr, e.verbose)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of introducing runAndLogCommandOutput, I think it would be simpler to add the relativization here, especially since runCommandToFile should not use use it. Something like:

buf := &bytes.Buffer{}
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.Stdout = buf
cmd.Stderr = buf
err := runAndLogCommand(cmd, e.verbose)
fmt.Fprint(os.Stderr, relativizePaths(buf.Bytes())
return err

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Updated

}

// 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:]...)
cmd.Stdout = w
cmd.Stderr = os.Stderr
return runAndLogCommand(cmd, e.verbose)
return runAndLogCommandOutput(cmd, w, os.Stderr, e.verbose)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's be careful that runCommandToFile does not relativize the output. It should just call runAndLogCommand.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies, missed that requirement. Updated

}

func absEnv(envNameList []string, argList []string) error {
Expand All @@ -158,6 +154,16 @@ 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))
Expand Down Expand Up @@ -332,6 +338,27 @@ func absArgs(args []string, flags []string) {
}
}

// relativizePaths converts absolute paths found in the given output string to
// relative, if they are within the working directory.
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}) {
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.
// Spaces in environment variables and arguments are escaped as needed.
func formatCommand(cmd *exec.Cmd) string {
Expand Down