this repo has no description
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

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í <mvdan@mvdan.cc>
Change-Id: I1a2ae4e0c07236db566ddc9217c1722bb5e99da4
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1197452
Reviewed-by: Roger Peppe <rogpeppe@gmail.com>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>

+19 -23
+1 -1
cmd/cue/cmd/testdata/script/cmd_errcode.txtar
··· 1 1 ! exec cue cmd errcode 2 2 ! stdout . 3 - stderr '^task failed: command "ls --badflags" failed: exit status [12]$' 3 + stderr '^task failed: command \["ls" "--badflags"\] failed: exit status [12]$' 4 4 5 5 -- task.cue -- 6 6 package home
+2 -4
cmd/cue/cmd/testdata/script/cmd_execerr.txtar
··· 4 4 exec cue cmd canFail 5 5 cmp stdout canFail-stdout.golden 6 6 7 - # TODO(mvdan): the error message should quote the argument list 8 - # so that any arguments with spaces do not introduce ambiguity. 9 7 ! exec cue cmd withSpaces 10 8 cmp stderr withSpaces-stderr.golden 11 9 ··· 14 12 language: version: "v0.9.0" 15 13 16 14 -- cannotFail-stderr.golden -- 17 - task failed: command "false" failed: exit status 1 15 + task failed: command ["false"] failed: exit status 1 18 16 -- canFail-stdout.golden -- 19 17 errExit: 20 18 success=false ··· 24 22 success=false 25 23 hasStderr=true 26 24 -- withSpaces-stderr.golden -- 27 - task failed: command "false with an ignored argument" failed: exit status 1 25 + task failed: command ["false" "with an ignored argument"] failed: exit status 1 28 26 -- foo_tool.cue -- 29 27 package foo 30 28
+16 -18
pkg/tool/exec/exec.go
··· 101 101 return nil, fmt.Errorf("command %q failed: %v", doc, err) 102 102 } 103 103 104 - func mkCommand(ctx *task.Context) (c *exec.Cmd, doc string, err error) { 105 - var bin string 106 - var args []string 107 - 104 + // mkCommand builds an [exec.Cmd] from a CUE task value, 105 + // also returning the full list of arguments as a string slice 106 + // so that it can be used in error messages. 107 + func mkCommand(ctx *task.Context) (c *exec.Cmd, doc []string, err error) { 108 108 v := ctx.Lookup("cmd") 109 109 if ctx.Err != nil { 110 - return nil, "", ctx.Err 110 + return nil, nil, ctx.Err 111 111 } 112 112 113 + var bin string 114 + var args []string 113 115 switch v.Kind() { 114 116 case cue.StringKind: 115 - str := ctx.String("cmd") 116 - doc = str 117 + str, _ := v.String() 117 118 list := strings.Fields(str) 118 - bin = list[0] 119 - args = append(args, list[1:]...) 119 + bin, args = list[0], list[1:] 120 120 121 121 case cue.ListKind: 122 122 list, _ := v.List() 123 123 if !list.Next() { 124 - return nil, "", errors.New("empty command list") 124 + return nil, nil, errors.New("empty command list") 125 125 } 126 126 bin, err = list.Value().String() 127 127 if err != nil { 128 - return nil, "", err 128 + return nil, nil, err 129 129 } 130 - doc += bin 131 130 for list.Next() { 132 131 str, err := list.Value().String() 133 132 if err != nil { 134 - return nil, "", err 133 + return nil, nil, err 135 134 } 136 135 args = append(args, str) 137 - doc += " " + str 138 136 } 139 137 } 140 138 141 139 if bin == "" { 142 - return nil, "", errors.New("empty command") 140 + return nil, nil, errors.New("empty command") 143 141 } 144 142 145 143 cmd := exec.CommandContext(ctx.Context, bin, args...) ··· 153 151 v, _ := iter.Value().Default() 154 152 str, err := v.String() 155 153 if err != nil { 156 - return nil, "", errors.Wrapf(err, v.Pos(), 154 + return nil, nil, errors.Wrapf(err, v.Pos(), 157 155 "invalid environment variable value %q", v) 158 156 } 159 157 cmd.Env = append(cmd.Env, str) ··· 170 168 case cue.IntKind, cue.FloatKind, cue.NumberKind: 171 169 str = fmt.Sprint(v) 172 170 default: 173 - return nil, "", errors.Newf(v.Pos(), 171 + return nil, nil, errors.Newf(v.Pos(), 174 172 "invalid environment variable value %q", v) 175 173 } 176 174 cmd.Env = append(cmd.Env, fmt.Sprintf("%s=%s", label, str)) 177 175 } 178 176 179 - return cmd, doc, nil 177 + return cmd, append([]string{bin}, args...), nil 180 178 }