pkg/tool/exec: Fix parsing cmd string args

[infogulch]

This PR fixes broken string arg parsing in pkg/tool/exec, using the github.com/kballard/go-shellquote library.

[mvdan]

Hi, thanks for sending a pull request!

I'm sure you have seen, but much like Go's os/exec, we support command arguments as a list of strings, which sidesteps this issue entirely. The majority of users, particularly those needing multiple arguments or arguments involving spaces, should use a list of strings rather than a single string that needs to be split up. This is easier for users to understand and maintain, and also doesn't require anyone to deal with shell syntax, which is unnecessary complexity. Particularly as it requires adding an extra dependency to CUE :)

I hope that makes sense. I would advocate for deprecating the string form of the command because perhaps it's a footgun that it uses https://pkg.go.dev/strings#Fields - it is rather basic. And perhaps some users would expect that it would use shell syntax, and we don't want to give that impression.

[infogulch]

Yes I ran into this issue trying to pass a string cmd like "go build -ldflags '\(vars.ldflags)'" where vars: ldflags: "-X github.com/infogulch/xtemplate/app.version=\(version)". I like the string form of cmd for simple use-cases as it is significantly easier to read. As you say, the solution was to use the list form of exec.Run.cmd, but it was a confusing problem to have in the moment. If you have the exact semantics in mind then switching to the list form is simple (if tedious), but a tool file may need small edits long after it is initially written, where the user may not have in mind the fact that you should want to switch to the list form when joining variables.

I looked and noticed that it would be low effort to integrate this small library to go from poor to very good arg parsing support, so I opened this PR to kick off a discussion with a concrete implementation in hand to refer to. I would not be too sad if the decision was no.

Consider:

• The added tests that show various ways that the current implementation using strings.Fields breaks.
• This change also uses the library for joining an args list, quoting and escaping args appropriately for documentation / error reporting purposes.

Of course adding a new dependency is something cue maintainers would want to consider carefully. Given its small size (~300loc), maybe "vendoring" / copying the code into the repo would be better if we want to avoid an explicit dependency?

[infogulch]

It just occurred to me that arg splitting / joining could be useful functionality to cue scripts in general.

What if we added SplitArgs() and JoinArgs() functions to the tool/exec package for anyone to use?

[mvdan]

Closing for now, as I explained in #3238 (comment). Thanks for contributing!