From ce25ad60bb6b77552393de8e197e08ff06dcba6e Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Thu, 12 Oct 2023 10:54:40 -0400 Subject: [PATCH] cmd/go: move fmtcmd's side effect to Showcmd Currently, fmtcmd may have the side effect of updating Builder.scriptDir, the logical working directory of the printed script. If it does so, it also returns a two line command consisting of both a "cd" into the new scriptDir and the original command. When fmtcmd is used as part of Showcmd, that's fine, but fmtcmd is also used in a handful of places to construct command descriptions that are ultimately passed to Builder.reportCmd. In these cases, it's surprising that fmtcmd has any side effects, but the bigger problem is that reportCmd isn't expecting a two-line description and will print it wrong in the output. One option is to fix printing multi-line descriptions in reportCmd, but we can fix the surprise side effect too by instead moving the working directory update to Showcmd. With this CL, fmtcmd merely consults the working directory to shorten it in the output and does not update it. For #62067. Change-Id: I7808b279a430551f4ba51545417adf0bb132f931 Reviewed-on: https://go-review.googlesource.com/c/go/+/534857 Reviewed-by: Bryan Mills LUCI-TryBot-Result: Go LUCI Auto-Submit: Austin Clements --- src/cmd/go/internal/work/exec.go | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go index 69c27be53a..cc2cf9f623 100644 --- a/src/cmd/go/internal/work/exec.go +++ b/src/cmd/go/internal/work/exec.go @@ -2156,27 +2156,24 @@ func mayberemovefile(s string) { // fmtcmd formats a command in the manner of fmt.Sprintf but also: // -// If dir is non-empty and the script is not in dir right now, -// fmtcmd inserts "cd dir\n" before the command. -// // fmtcmd replaces the value of b.WorkDir with $WORK. -// fmtcmd replaces the value of goroot with $GOROOT. -// fmtcmd replaces the value of b.gobin with $GOBIN. // // fmtcmd replaces the name of the current directory with dot (.) // but only when it is at the beginning of a space-separated token. func (b *Builder) fmtcmd(dir string, format string, args ...any) string { cmd := fmt.Sprintf(format, args...) - if dir != "" && dir != "/" { + if dir != "" && dir != "/" && b.scriptDir == dir { + // In the output stream, scriptDir is our working directory. Replace it + // with "." in the command. + // + // We intentionally don't lock around the access to scriptDir. If this + // access is racy, that means our working directory isn't well-defined + // at this call, and we want the race detector to catch that. dot := " ." if dir[len(dir)-1] == filepath.Separator { dot += string(filepath.Separator) } cmd = strings.ReplaceAll(" "+cmd, " "+dir, dot)[1:] - if b.scriptDir != dir { - b.scriptDir = dir - cmd = "cd " + dir + "\n" + cmd - } } if b.WorkDir != "" && !strings.HasPrefix(cmd, "cat ") { cmd = strings.ReplaceAll(cmd, b.WorkDir, "$WORK") @@ -2194,6 +2191,13 @@ func (b *Builder) fmtcmd(dir string, format string, args ...any) string { func (b *Builder) Showcmd(dir string, format string, args ...any) { b.output.Lock() defer b.output.Unlock() + + if dir != "" && dir != "/" && dir != b.scriptDir { + // Show changing to dir and update the current directory. + b.Print(b.fmtcmd("", "cd %s\n", dir)) + b.scriptDir = dir + } + b.Print(b.fmtcmd(dir, format, args...) + "\n") } -- 2.50.0