Mina::LocalHelpers::Local.invoke is broken for non-pretty term modes

[13k]

Mina::LocalHelpers::Local.invoke receives a Shellwords.escape'd command string that breaks Kernel.exec or Kernel.system.

Breaking example

• Create a Minafile containing:

task :hello do
  to :after_hook do
    queue %[echo "hello"]
  end
end

Note: the default :term_mode setting is nil.

• Run mina hello, outputs (or errs):

sh: echo "hello": command not found

 !     Command failed.
       Failed with status 32512

Working example

• Use the same Minafile as above, but first set term_mode to :pretty:

set :term_mode, :pretty

• Run mina hello, output:

       hello
       Elapsed time: 0.01 seconds

So when either Kernel.exec or Kernel.system are called, the command string should not be shell escaped. It should be shell escaped when passed to Mina::ExecHelpers#pretty_system, which does Shellwords.shellsplit on it.

Proposed solution

Since Mina::LocalHelpers::Local.invoke is the responsible for switching between which shell execution method to use, it should be that method to decide whether to shell escape the command or not.

So change it to shell escape only when calling pretty_system and not escaping when calling Kernel methods and change all callers to send the command string as-is.

As far as I can see, the only caller is Mina::LocalHelpers#local.

Sidenote

Why the use of Kernel.exec? According to the docs, it replaces the current process by running the given external command. Is that expected behavior?

[d4be4st]

Thx for this.

The local_helper.rb is a copy of a ssh_helper.rb file. I will have to extract that into a single method.

For your last question, unfortunately that was written by former owner of mina and i have no knowledge why he used Kernel.exec
I know that we have non pretty_print mode for windows users because open4 is not supported there.

Do you believe it is not necessary at all? I am not fully aware of the cons/pros of the various methods you can call system code, so I would very grateful for some input here.