Skip to content

Commit

Permalink
cmd/go: move fmtcmd's side effect to Showcmd
Browse files Browse the repository at this point in the history
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 <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Austin Clements <[email protected]>
  • Loading branch information
aclements authored and gopherbot committed Oct 18, 2023
1 parent 1ec427e commit ce25ad6
Showing 1 changed file with 14 additions and 10 deletions.
24 changes: 14 additions & 10 deletions src/cmd/go/internal/work/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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")
}

Expand Down

0 comments on commit ce25ad6

Please sign in to comment.