Fix OpenOS '/bin/sh cmd' failing due to lack of env table

[0x0ade]

/bin/sh.lua should be able to execute a command passed as an argument, but currently fails to do so in interactive shells as sh.execute (in /lib/sh.lua) expects an environment table as the first parameter:

That environment table is given when running commands in the interactive shell, but is missing from the non-interactive sh.execute call.

Meanwhile, /bin/source.lua passes an environment table as the first argument to /bin/sh.lua when starting the sh process manually.

As such, I've decided to add a simple check if the first parameter is a table. If it is, it's assumed to be the environment table and won't be passed on. Otherwise _ENV will be used and all arguments will be passed on.

This should maintain compatibility with any existing usages of /bin/sh.lua as a child process. Admittedly, I've only tested it locally by manipulating a copy of /bin/sh.lua after installing OpenOS onto an OC computer.

(Sidenote: I was not sure whether to remove the env table from the argument list, or whether to skip it when unpacking the arg list. I went with the second option as I thought it'd be preferred, given how returning does the same.)

[Vexatos]

This could be shortened a bit.

  local cenv = _ENV
  if type(cargs[1]) == "table" then
    -- sh can also run as a manually started process (see /bin/source.lua)
    cenv = table.remove(cargs, 1)
  end
  local result = table.pack(sh.execute(cenv, table.unpack(cargs)))

Also, what about using {...} instead of table.pack(...)?

[0x0ade]

Same as with skipping the arg with table.unpack instead of using table.remove, I figured I'd just stick to what's already being used and used table.pack instead of {...}, mixing multiple ways to achieve the same thing in the same chunk of code.

[Vexatos]

They are not completely identical. Using {...} would pack all arguments into a table, whereas table.pack actually also adds the additional key "n". Not that it matters in this case.

[0x0ade]

TIL 😅 anyway, I've just pushed another commit implementing your proposed changes.

[Vexatos]

This is up to @payonel anyway.

[payonel]

first of all, i greatly dislike that /bin/sh takes a table as an arg
no /bin program should ever have been written this way
we inherited this strangeness from ages ago and well, it's far too late to change it now.

[payonel]

nope i dont care
it just doesnt make sense to assume ENV is passed to /bin/sh
it is easy to correct this, and all my tests pass
the only way this breaks someone's code is if they call /bin/sh via loadfile
my fix corrects shell.execute and os.execute, which honestly is what everyone is using

[asiekierka]

Well, guess this is not so simple to fix.