pkg/tool/exec: show command arguments in errors as a Go slice

If we render the command arguments as a quoted Go string joining
the arguments via spaces, it's always going to be confusing if any
of those arguments contain any spaces.

We always use a slice now, even when the arguments don't contain
any spaces or even when there's just one argument,
but consistency seems more important than saving a few characters.

An alternative to Go slices with %q quoting, following Go syntax,
would be to quote a list of strings in CUE syntax.
However, we use Go syntax for lists and string quoting for errors
rather consistently, so it seems best to continue doing so here.

While here, add a godoc for mkCommand, avoid doing a lookup on the
"cmd" field twice, and simplify the code slightly.

Updates #3238.

Signed-off-by: Daniel Martí <[email protected]>
Change-Id: I1a2ae4e0c07236db566ddc9217c1722bb5e99da4
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1197452
Reviewed-by: Roger Peppe <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>

Author: mvdan

cmd/cue/cmd/testdata/script/cmd_errcode.txtar

@@ -1,6 +1,6 @@
 ! exec cue cmd errcode
 ! stdout .
-stderr '^task failed: command "ls --badflags" failed: exit status [12]$'
+stderr '^task failed: command \["ls" "--badflags"\] failed: exit status [12]$'
 
 -- task.cue --
 package home

cmd/cue/cmd/testdata/script/cmd_execerr.txtar

@@ -4,8 +4,6 @@ cmp stderr cannotFail-stderr.golden
 exec cue cmd canFail
 cmp stdout canFail-stdout.golden
 
-# TODO(mvdan): the error message should quote the argument list
-# so that any arguments with spaces do not introduce ambiguity.
 ! exec cue cmd withSpaces
 cmp stderr withSpaces-stderr.golden
 
@@ -14,7 +12,7 @@ module: "example.com"
 language: version: "v0.9.0"
 
 -- cannotFail-stderr.golden --
-task failed: command "false" failed: exit status 1
+task failed: command ["false"] failed: exit status 1
 -- canFail-stdout.golden --
 errExit:
 success=false
@@ -24,7 +22,7 @@ errNotFound:
 success=false
 hasStderr=true
 -- withSpaces-stderr.golden --
-task failed: command "false with an ignored argument" failed: exit status 1
+task failed: command ["false" "with an ignored argument"] failed: exit status 1
 -- foo_tool.cue --
 package foo
 

pkg/tool/exec/exec.go

@@ -101,45 +101,43 @@ func (c *execCmd) Run(ctx *task.Context) (res interface{}, err error) {
 	return nil, fmt.Errorf("command %q failed: %v", doc, err)
 }
 
-func mkCommand(ctx *task.Context) (c *exec.Cmd, doc string, err error) {
-	var bin string
-	var args []string
-
+// mkCommand builds an [exec.Cmd] from a CUE task value,
+// also returning the full list of arguments as a string slice
+// so that it can be used in error messages.
+func mkCommand(ctx *task.Context) (c *exec.Cmd, doc []string, err error) {
 	v := ctx.Lookup("cmd")
 	if ctx.Err != nil {
-		return nil, "", ctx.Err
+		return nil, nil, ctx.Err
 	}
 
+	var bin string
+	var args []string
 	switch v.Kind() {
 	case cue.StringKind:
-		str := ctx.String("cmd")
-		doc = str
+		str, _ := v.String()
 		list := strings.Fields(str)
-		bin = list[0]
-		args = append(args, list[1:]...)
+		bin, args = list[0], list[1:]
 
 	case cue.ListKind:
 		list, _ := v.List()
 		if !list.Next() {
-			return nil, "", errors.New("empty command list")
+			return nil, nil, errors.New("empty command list")
 		}
 		bin, err = list.Value().String()
 		if err != nil {
-			return nil, "", err
+			return nil, nil, err
 		}
-		doc += bin
 		for list.Next() {
 			str, err := list.Value().String()
 			if err != nil {
-				return nil, "", err
+				return nil, nil, err
 			}
 			args = append(args, str)
-			doc += " " + str
 		}
 	}
 
 	if bin == "" {
-		return nil, "", errors.New("empty command")
+		return nil, nil, errors.New("empty command")
 	}
 
 	cmd := exec.CommandContext(ctx.Context, bin, args...)
@@ -153,7 +151,7 @@ func mkCommand(ctx *task.Context) (c *exec.Cmd, doc string, err error) {
 		v, _ := iter.Value().Default()
 		str, err := v.String()
 		if err != nil {
-			return nil, "", errors.Wrapf(err, v.Pos(),
+			return nil, nil, errors.Wrapf(err, v.Pos(),
 				"invalid environment variable value %q", v)
 		}
 		cmd.Env = append(cmd.Env, str)
@@ -170,11 +168,11 @@ func mkCommand(ctx *task.Context) (c *exec.Cmd, doc string, err error) {
 		case cue.IntKind, cue.FloatKind, cue.NumberKind:
 			str = fmt.Sprint(v)
 		default:
-			return nil, "", errors.Newf(v.Pos(),
+			return nil, nil, errors.Newf(v.Pos(),
 				"invalid environment variable value %q", v)
 		}
 		cmd.Env = append(cmd.Env, fmt.Sprintf("%s=%s", label, str))
 	}
 
-	return cmd, doc, nil
+	return cmd, append([]string{bin}, args...), nil
 }