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 <bcmills@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Austin Clements <austin@google.com>

diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go
index 69c27be53a605cbb8b5e3701184cb600af94dd22..cc2cf9f623da6b3683d42b75caf989ec5fbc9f43 100644 (file)
--- 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")
 }