Re: master 0ad1c0d: * lisp/net/tramp.el (tramp-handle-make-process): Handle shell commands.

[Michael Albinus <michael.albinus@gmx.de>]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

Hi Stefan,

>> (defun start-file-process-shell-command (name buffer &rest args)
>>   "..."
>>   (with-connection-local-variables
>>    (start-file-process
>>     name buffer
>>     shell-file-name shell-command-switch
>>     (shell-quote-argument (mapconcat 'identity args " ")))))
>
> Concatenating arguments with a " " separator is plain wrong.
> That's why we have
>
>    (declare (advertised-calling-convention (name buffer command) "23.1"))
>
> so we can hopefully soon drop support for that concatenation.
>
> Any caller which passes several `args` should be fixed.
> Any chance this is the problem you're seeing?
>
> If we disregard those wrong callers, the current definition is:
>
>   (with-connection-local-variables
>    (start-file-process
>     name buffer
>     shell-file-name shell-command-switch command))

Perhaps. Let's go this way. I have changed the function to this
definition, and I have recompiled all *.el files in Emacs master
branch. No compilation error at least, which means everybody follows the
calling conventions.

> So the `command` doesn't need any quoting here: it's
> start-file-process's responsability to make sure it starts a process
> with those 3 strings (shell-file-name as the name of the executable,
> `shell-command-switch` as the first arg and `command` as the second).
>
> In the case of Tramp's implementation of `start-file-process`, you're
> going to run this process by constructing a command to send to the
> remote shell, so you'll indeed need to turn this list of strings
> into a single string and you need to do it by quoting those strings
> using the quoting that corresponds to that of the remote shell (which
> is indeed what `tramp-shell-quote-argument` does, IIUC).
> IOW, I'd expect `start-file-process` to do something like
>
>     (mapconcat #'tramp-shell-quote-argument args " ")
>
> to construct the command to send to the remote shell.

Might work. However, some of my tests in tramp-tests.el fail now, and
other tests still work. So I need to check what's up, before I commit.

>         Stefan

Best regards, Michael.