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

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

[Stefan Monnier]

Hi Michael,

> +	;; Quote shell command.
> +	(when (and (= (length command) 3)
> +		   (stringp (nth 0 command))
> +		   (string-match-p "sh$" (nth 0 command))
> +		   (stringp (nth 1 command))
> +		   (string-equal "-c" (nth 1 command))
> +		   (stringp (nth 2 command)))
> +	  (setcar (cddr command) (tramp-shell-quote-argument (nth 2 command))))

This looks odd (what is special about the quoting needs of "sh -c" which
wouldn't apply to other commands?).  Do you have some bug# or test case
associated to it so we can better understand why it's needed?


        Stefan

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

[Michael Albinus]

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

> Hi Michael,

Hi Stefan

>> +	;; Quote shell command.
>> +	(when (and (= (length command) 3)
>> +		   (stringp (nth 0 command))
>> +		   (string-match-p "sh$" (nth 0 command))
>> +		   (stringp (nth 1 command))
>> +		   (string-equal "-c" (nth 1 command))
>> +		   (stringp (nth 2 command)))
>> +	  (setcar (cddr command) (tramp-shell-quote-argument (nth 2 command))))
>
> This looks odd (what is special about the quoting needs of "sh -c" which
> wouldn't apply to other commands?).  Do you have some bug# or test case
> associated to it so we can better understand why it's needed?

See the thread "Tramp and conversion of \r\n into \n" in
emacs-devel. Martin gave me tons of Tramp traces.

I'm also not so happy with this special handling, and I expect we'll
find a better solution. I'm still working on this.

However, it is at least a proof of concept. People using lsp-mode call
also many other asynchronous processes, and so I'll get lot of traces
from real life examples to compare.

And yes, based on these examples I will also extend tramp-tests.el.

>         Stefan

Best regards, Michael.

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

[Stefan Monnier]

>>> +	;; Quote shell command.
>>> +	(when (and (= (length command) 3)
>>> +		   (stringp (nth 0 command))
>>> +		   (string-match-p "sh$" (nth 0 command))
>>> +		   (stringp (nth 1 command))
>>> +		   (string-equal "-c" (nth 1 command))
>>> +		   (stringp (nth 2 command)))
>>> +	  (setcar (cddr command) (tramp-shell-quote-argument (nth 2 command))))
>>
>> This looks odd (what is special about the quoting needs of "sh -c" which
>> wouldn't apply to other commands?).  Do you have some bug# or test case
>> associated to it so we can better understand why it's needed?
>
> See the thread "Tramp and conversion of \r\n into \n" in
> emacs-devel. Martin gave me tons of Tramp traces.
>
> I'm also not so happy with this special handling, and I expect we'll
> find a better solution. I'm still working on this.

I commend you make that clear in a comment (including a reference to
the relevant thread).

BTW, what would go wrong if you did
(setq command (mapcar #'tramp-shell-quote-argument command))?


        Stefan

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

[Stefan Monnier]

> I commend you make that clear in a comment (including a reference to

Hmm... not sure where the "re" went,


        Stefan

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

[Michael Albinus]

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

> BTW, what would go wrong if you did
> (setq command (mapcar #'tramp-shell-quote-argument command))?

Tried that, didn't work. Imagine, you call "M-x rgrep" on a remote
machine. This ends up in a make-process call with :command

--8<---------------cut here---------------start------------->8---
("/bin/sh" "-c" "find . -type d \\( -path \\*/SCCS -o -path \\*/RCS -o -path \\*/CVS -o -path \\*/MCVS -o -path \\*/.src -o -path \\*/.svn -o -path \\*/.git -o -path \\*/.hg -o -path \\*/.bzr -o -path \\*/_MTN -o -path \\*/_darcs -o -path \\*/\\{arch\\} \\) -prune -o \\! -type d \\( -name .\\#\\* -o -name \\*.o -o -name \\*\\~ -o -name \\*.bin -o -name \\*.lbin -o -name \\*.so -o -name \\*.a -o -name \\*.ln -o -name \\*.blg -o -name \\*.bbl -o -name \\*.elc -o -name \\*.lof -o -name \\*.glo -o -name \\*.idx -o -name \\*.lot -o -name \\*.fmt -o -name \\*.tfm -o -name \\*.class -o -name \\*.fas -o -name \\*.lib -o -name \\*.mem -o -name \\*.x86f -o -name \\*.sparcf -o -name \\*.dfsl -o -name \\*.pfsl -o -name \\*.d64fsl -o -name \\*.p64fsl -o -name \\*.lx64fsl -o -name \\*.lx32fsl -o -name \\*.dx64fsl -o -name \\*.dx32fsl -o -name \\*.fx64fsl -o -name \\*.fx32fsl -o -name \\*.sx64fsl -o -name \\*.sx32fsl -o -name \\*.wx64fsl -o -name \\*.wx32fsl -o -name \\*.fasl -o -name \\*.ufsl -o -name \\*.fsl -o -name \\*.dxl -o -name \\*.lo -o -name \\*.la -o -name \\*.gmo -o -name \\*.mo -o -name \\*.toc -o -name \\*.aux -o -name \\*.cp -o -name \\*.fn -o -name \\*.ky -o -name \\*.pg -o -name \\*.tp -o -name \\*.vr -o -name \\*.cps -o -name \\*.fns -o -name \\*.kys -o -name \\*.pgs -o -name \\*.tps -o -name \\*.vrs -o -name \\*.pyc -o -name \\*.pyo \\) -prune -o  -type f \\( -name \\*.cc -o -name \\*.cxx -o -name \\*.cpp -o -name \\*.C -o -name \\*.CC -o -name \\*.c\\+\\+ \\) -exec grep --color -i -nH --null -e a \\{\\} +")
--8<---------------cut here---------------end--------------->8---

As you see, three arguments "/bin/sh" "-c" and "find ...". If I do your
proposal, Tramp would send finally

--8<---------------cut here---------------start------------->8---
ssh -q -o ControlMaster=auto -o ControlPath='tramp.%C' -o ControlPersist=no -e none detlef /bin/sh -c find\\\ .\\\ -type\\\ d\\\ \\\\\\\(\\\ -path\\\ \\\\\\\*/SCCS\\\ -o\\\ -path\\\ \\\\\\\*/RCS\\\ -o\\\ -path\\\ \\\\\\\*/CVS\\\ -o\\\ -path\\\ \\\\\\\*/MCVS\\\ -o\\\ -path\\\ \\\\\\\*/.src\\\ -o\\\ -path\\\ \\\\\\\*/.svn\\\ -o\\\ -path\\\ \\\\\\\*/.git\\\ -o\\\ -path\\\ \\\\\\\*/.hg\\\ -o\\\ -path\\\ \\\\\\\*/.bzr\\\ -o\\\ -path\\\ \\\\\\\*/_MTN\\\ -o\\\ -path\\\ \\\\\\\*/_darcs\\\ -o\\\ -path\\\ \\\\\\\*/\\\\\\\{arch\\\\\\\}\\\ \\\\\\\)\\\ -prune\\\ -o\\\ \\\\\\\!\\\ -type\\\ d\\\ \\\\\\\(\\\ -name\\\ .\\\\\\\#\\\\\\\*\\\ -o\\\ -name\\\ \\\\\\\*.o\\\ -o\\\ -name\\\ \\\\\\\*\\\\\\\~\\\ -o\\\ -name\\\ \\\\\\\*.bin\\\ -o\\\ -name\\\ \\\\\\\*.lbin\\\ -o\\\ -name\\\ \\\\\\\*.so\\\ -o\\\ -name\\\ \\\\\\\*.a\\\ -o\\\ -name\\\ \\\\\\\*.ln\\\ -o\\\ -name\\\ \\\\\\\*.blg\\\ -o\\\ -name\\\ \\\\\\\*.bbl\\\ -o\\\ -name\\\ \\\\\\\*.elc\\\ -o\\\ -name\\\ \\\\\\\*.lof\\\ -o\\\ -name\\\ \\\\\\\*.glo\\\ -o\\\ -name\\\ \\\\\\\*.idx\\\ -o\\\ -name\\\ \\\\\\\*.lot\\\ -o\\\ -name\\\ \\\\\\\*.fmt\\\ -o\\\ -name\\\ \\\\\\\*.tfm\\\ -o\\\ -name\\\ \\\\\\\*.class\\\ -o\\\ -name\\\ \\\\\\\*.fas\\\ -o\\\ -name\\\ \\\\\\\*.lib\\\ -o\\\ -name\\\ \\\\\\\*.mem\\\ -o\\\ -name\\\ \\\\\\\*.x86f\\\ -o\\\ -name\\\ \\\\\\\*.sparcf\\\ -o\\\ -name\\\ \\\\\\\*.dfsl\\\ -o\\\ -name\\\ \\\\\\\*.pfsl\\\ -o\\\ -name\\\ \\\\\\\*.d64fsl\\\ -o\\\ -name\\\ \\\\\\\*.p64fsl\\\ -o\\\ -name\\\ \\\\\\\*.lx64fsl\\\ -o\\\ -name\\\ \\\\\\\*.lx32fsl\\\ -o\\\ -name\\\ \\\\\\\*.dx64fsl\\\ -o\\\ -name\\\ \\\\\\\*.dx32fsl\\\ -o\\\ -name\\\ \\\\\\\*.fx64fsl\\\ -o\\\ -name\\\ \\\\\\\*.fx32fsl\\\ -o\\\ -name\\\ \\\\\\\*.sx64fsl\\\ -o\\\ -name\\\ \\\\\\\*.sx32fsl\\\ -o\\\ -name\\\ \\\\\\\*.wx64fsl\\\ -o\\\ -name\\\ \\\\\\\*.wx32fsl\\\ -o\\\ -name\\\ \\\\\\\*.fasl\\\ -o\\\ -name\\\ \\\\\\\*.ufsl\\\ -o\\\ -name\\\ \\\\\\\*.fsl\\\ -o\\\ -name\\\ \\\\\\\*.dxl\\\ -o\\\ -name\\\ \\\\\\\*.lo\\\ -o\\\ -name\\\ \\\\\\\*.la\\\ -o\\\ -name\\\ \\\\\\\*.gmo\\\ -o\\\ -name\\\ \\\\\\\*.mo\\\ -o\\\ -name\\\ \\\\\\\*.toc\\\ -o\\\ -name\\\ \\\\\\\*.aux\\\ -o\\\ -name\\\ \\\\\\\*.cp\\\ -o\\\ -name\\\ \\\\\\\*.fn\\\ -o\\\ -name\\\ \\\\\\\*.ky\\\ -o\\\ -name\\\ \\\\\\\*.pg\\\ -o\\\ -name\\\ \\\\\\\*.tp\\\ -o\\\ -name\\\ \\\\\\\*.vr\\\ -o\\\ -name\\\ \\\\\\\*.cps\\\ -o\\\ -name\\\ \\\\\\\*.fns\\\ -o\\\ -name\\\ \\\\\\\*.kys\\\ -o\\\ -name\\\ \\\\\\\*.pgs\\\ -o\\\ -name\\\ \\\\\\\*.tps\\\ -o\\\ -name\\\ \\\\\\\*.vrs\\\ -o\\\ -name\\\ \\\\\\\*.pyc\\\ -o\\\ -name\\\ \\\\\\\*.pyo\\\ \\\\\\\)\\\ -prune\\\ -o\\\ \\\ -type\\\ f\\\ \\\\\\\(\\\ -name\\\ \\\\\\\*.cc\\\ -o\\\ -name\\\ \\\\\\\*.cxx\\\ -o\\\ -name\\\ \\\\\\\*.cpp\\\ -o\\\ -name\\\ \\\\\\\*.C\\\ -o\\\ -name\\\ \\\\\\\*.CC\\\ -o\\\ -name\\\ \\\\\\\*.c\\\\\\\+\\\\\\\+\\\ \\\\\\\)\\\ -exec\\\ grep\\\ --color\\\ -i\\\ -nH\\\ --null\\\ -e\\\ a\\\ \\\\\\\{\\\\\\\}\\\ \\\+
--8<---------------cut here---------------end--------------->8---

This gives the error

--8<---------------cut here---------------start------------->8---
/bin/sh: 1: find . -type d \( -path \*/SCCS -o -path \*/RCS -o -path \*/CVS -o -path \*/MCVS -o -path \*/.src -o -path \*/.svn -o -path \*/.git -o -path \*/.hg -o -path \*/.bzr -o -path \*/_MTN -o -path \*/_darcs -o -path \*/\{arch\} \) -prune -o \! -type d \( -name .\#\* -o -name \*.o -o -name \*\~ -o -name \*.bin -o -name \*.lbin -o -name \*.so -o -name \*.a -o -name \*.ln -o -name \*.blg -o -name \*.bbl -o -name \*.elc -o -name \*.lof -o -name \*.glo -o -name \*.idx -o -name \*.lot -o -name \*.fmt -o -name \*.tfm -o -name \*.class -o -name \*.fas -o -name \*.lib -o -name \*.mem -o -name \*.x86f -o -name \*.sparcf -o -name \*.dfsl -o -name \*.pfsl -o -name \*.d64fsl -o -name \*.p64fsl -o -name \*.lx64fsl -o -name \*.lx32fsl -o -name \*.dx64fsl -o -name \*.dx32fsl -o -name \*.fx64fsl -o -name \*.fx32fsl -o -name \*.sx64fsl -o -name \*.sx32fsl -o -name \*.wx64fsl -o -name \*.wx32fsl -o -name \*.fasl -o -name \*.ufsl -o -name \*.fsl -o -name \*.dxl -o -name \*.lo -o -name \*.la -o -name \*.gmo -o -name \*.mo -o -name \*.toc -o -name \*.aux -o -name \*.cp -o -name \*.fn -o -name \*.ky -o -name \*.pg -o -name \*.tp -o -name \*.vr -o -name \*.cps -o -name \*.fns -o -name \*.kys -o -name \*.pgs -o -name \*.tps -o -name \*.vrs -o -name \*.pyc -o -name \*.pyo \) -prune -o  -type f \( -name \*.cc -o -name \*.cxx -o -name \*.cpp -o -name \*.C -o -name \*.CC -o -name \*.c\+\+ \) -exec grep --color -i -nH --null -e a \{\} +: not found
--8<---------------cut here---------------end--------------->8---

>         Stefan

Best regards, Michael.

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

[Stefan Monnier]

>> BTW, what would go wrong if you did
>> (setq command (mapcar #'tramp-shell-quote-argument command))?
>
> Tried that, didn't work. Imagine, you call "M-x rgrep" on a remote
> machine. This ends up in a make-process call with :command
[...]
> As you see, three arguments "/bin/sh" "-c" and "find ...". If I do your
> proposal, Tramp would send finally

I don't understand: your code leaves "sh" and "-c" and passes the third
through `tramp-shell-quote-argument`, but that should give the exact
same result as passing all three through `tramp-shell-quote-argument`
since "sh" and "-c" are both left unchanged by `tramp-shell-quote-argument`.

What am I missing?


        Stefan

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

[Andreas Schwab]

On Dez 18 2020, Michael Albinus wrote:

> Tried that, didn't work. Imagine, you call "M-x rgrep" on a remote
> machine. This ends up in a make-process call with :command
>
> ("/bin/sh" "-c" "find . -type d \\( -path \\*/SCCS -o -path \\*/RCS -o -path \\*/CVS -o -path \\*/MCVS -o -path \\*/.src -o -path \\*/.svn -o -path \\*/.git -o -path \\*/.hg -o -path \\*/.bzr -o -path \\*/_MTN -o -path \\*/_darcs -o -path \\*/\\{arch\\} \\) -prune -o \\! -type d \\( -name .\\#\\* -o -name \\*.o -o -name \\*\\~ -o -name \\*.bin -o -name \\*.lbin -o -name \\*.so -o -name \\*.a -o -name \\*.ln -o -name \\*.blg -o -name \\*.bbl -o -name \\*.elc -o -name \\*.lof -o -name \\*.glo -o -name \\*.idx -o -name \\*.lot -o -name \\*.fmt -o -name \\*.tfm -o -name \\*.class -o -name \\*.fas -o -name \\*.lib -o -name \\*.mem -o -name \\*.x86f -o -name \\*.sparcf -o -name \\*.dfsl -o -name \\*.pfsl -o -name \\*.d64fsl -o -name \\*.p64fsl -o -name \\*.lx64fsl -o -name \\*.lx32fsl -o -name \\*.dx64fsl -o -name \\*.dx32fsl -o -name \\*.fx64fsl -o -name \\*.fx32fsl -o -name \\*.sx64fsl -o -name \\*.sx32fsl -o -name \\*.wx64fsl -o -name \\*.wx32fsl -o -name \\*.fasl -o -name \\*.ufsl -o -name \\*.fsl -o -name \\*.dxl -o -name \\*.lo -o -name \\*.la -o -name \\*.gmo -o -name \\*.mo -o -name \\*.toc -o -name \\*.aux -o -name \\*.cp -o -name \\*.fn -o -name \\*.ky -o -name \\*.pg -o -name \\*.tp -o -name \\*.vr -o -name \\*.cps -o -name \\*.fns -o -name \\*.kys -o -name \\*.pgs -o -name \\*.tps -o -name \\*.vrs -o -name \\*.pyc -o -name \\*.pyo \\) -prune -o  -type f \\( -name \\*.cc -o -name \\*.cxx -o -name \\*.cpp -o -name \\*.C -o -name \\*.CC -o -name \\*.c\\+\\+ \\) -exec grep --color -i -nH --null -e a \\{\\} +")

But for this particular list,

(setq command (mapcar #'tramp-shell-quote-argument command))

and

(setcar (cddr command) (tramp-shell-quote-argument (nth 2 command)))

produce exactly the same.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

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

[Michael Albinus]

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

>>> BTW, what would go wrong if you did
>>> (setq command (mapcar #'tramp-shell-quote-argument command))?
>>
>> Tried that, didn't work. Imagine, you call "M-x rgrep" on a remote
>> machine. This ends up in a make-process call with :command
> [...]
>> As you see, three arguments "/bin/sh" "-c" and "find ...". If I do your
>> proposal, Tramp would send finally
>
> I don't understand: your code leaves "sh" and "-c" and passes the third
> through `tramp-shell-quote-argument`, but that should give the exact
> same result as passing all three through `tramp-shell-quote-argument`
> since "sh" and "-c" are both left unchanged by `tramp-shell-quote-argument`.
>
> What am I missing?

The example I gave you was simplified. If you look into
tramp-handle-make-process, you'll see a more complex machinery. And yes,
I've tried several variants of quoting before I found this working
solution.

But I don't believe it shall be solved in this function. Because it is
about quoting of a shell command, it is likely that it would be better
fixed in start-file-process-shell-command. I would prefer

(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 " ")))))

However, shell-quote-argument quotes wrt the local shell. A shell on the
remote host might quote differently. Maybe we make
start-file-process-shell-command a magic function? or shell-quote-argument?
Don't know. I'll experiment with this next days. People will be in the
Xmas break, so there's time.

>         Stefan

Best regards, Michael.

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

[Stefan Monnier]

> (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))

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.


        Stefan

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

[Michael Albinus]

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.

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

[Michael Albinus]

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.

I've dropped this.

> 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.

My recent patch (pushed) is along these lines. In
tramp-handle-make-process, but this is just an implementation detail.

It is a little bit more complicate, because it needs also some quoting
for the purpose of ssh, but it works for this.

There are still some problems when the connection is not performed via
ssh; this I will continue to investigate.

>         Stefan

Best regards, Michael.

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

[Stefan Monnier]

>> 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.
>
> I've dropped this.

Thanks.

>> 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.
>
> My recent patch (pushed) is along these lines. In
> tramp-handle-make-process, but this is just an implementation detail.

I'm glad we could find a less hackish solution (tho those levels of
quoting are involved, "hackish" is largely unavoidable).


        Stefan