feat: system/exec() function call ( #1043)

[forbesmyester]

Feature as detailed in #1043

Changes from discussed

• For env, I did not use a dictionary / map for environmental variables but instead an array with KEY=VALUE items, as this matches the golang API
• Changed the option string_input to stdin_string as that seemed better
• Added a combined_output option which includes both STDOUT and STDERR as it seemed like a good idea and was easy with the golang API.

NOTE: I've never wrote (, or practically even looked at) any golang code before. I'm not sure on how good this is, specifically my define a variable then use an if/else afterwards (combinedOutput) to change it seems naff.

Happy to have feedback.

[johnkerl]

Awesome, thanks @forbesmyester !!! I'll take a look this weekend 😁

[johnkerl]

this could really use if ... else if ... else and handle the unexpected "none of the above" in the else

that said, miller DSL source code doesn't propagate go-style error returns as much as it should, so without reworking the BIF API, the best you could do would be to print to stderr and exit 1 (which isn't great but is what i've done all over the place in the DSL)

[johnkerl]

thanks @forbesmyester !!!!! :D

[johnkerl]

NOTE: I've never wrote (, or practically even looked at) any golang code before. I'm not sure on how good this is, specifically my define a variable then use an if/else afterwards (combinedOutput) to change it seems naff.

very much not a concern for me ... my perspective after having spent a long time in software is that readability and clarity is all that matters, and this is very clear :)

as an after-commit i'm running make dev which will update the manpage & readthedocs with the new exec function

... 5ffb8a5

[johnkerl]

... oops, I may have merged too quickly:

$ mlr repl

[mlr] ?exec
exec  (class=system #args=variadic) '$output = exec( "command", ["arg1", "arg2"], {"env": ["ENV_VAR=ENV_VALUE"], "dir": "/tmp/run_command_here", "stdin_string": "this is input fed to program", "combined_output": true )' Run a command via executable, path, args and environment, yielding its stdout minus final carriage return.
Example:
exec("echo", ["Hello", "$YOUR_NAME"], {"env": "YOUR_NAME=World"}) outputs "Hello World"

[mlr] exec("echo", ["Hello", "$YOUR_NAME"], {"env": "YOUR_NAME=World"})
"Hello $YOUR_NAME" <-- not "Hello World"

[johnkerl]

(also could use some unit tests)

[forbesmyester]

Hmmm,

So I think my code is good, but my docs are bad (because it's not what I was actually using to test).

Thinking about it, it not interpolinating the $YOUR_NAME is exactly what we want and illustrates the whole purpose of this code!

I'm a bit unsure how to go about adding tests to this given that it's purpose is to shell out to the system and that's not going to be consistent across platforms...

This (ugly thing) below how I was testing my code. As you can see it does send the environment across:

the_path/toexec

#!/bin/bash

echo "Hi $1 $2"
echo "You exec int $PWD"
echo "You live in $YOYR"


IN_SQL="$(cat /dev/stdin)"
echo "INPUT: $IN_SQL"
echo "STDERR" > /dev/fd/2

the command

$ make && echo '{"name":"bob"}' | ./mlr --ijson put '$bye = exec("./toexec", ["sir", $name], {"env":["YOYR=home"],"dir":"./the_path","stdin_string":"THE\nIN\nbob","combined_output":true})'

the output

go build github.com/johnkerl/miller/cmd/mlr
Build complete. The Miller executable is ./mlr (or .\mlr.exe on Windows).
You can use 'make check' to run tests.
name=bob,bye=Hi sir bob
You exec int /home/fozz/Projects/miller/the_path
You live in home
INPUT: THE
IN
bob
STDERR

How should we proceed? Should I just fix the docs?

[johnkerl]

yes please! :)