Aleksander Trofimowicz <trof@n90.eu> writes:

> Hi Michael,

Hi Alexander,

>> The failed test runs have in common that there is a complex Tramp target
>> for the test using multi-hops, like above. I suspect a problem with
>> timing due to the several hops, but I don't know exactly what's up.
>>
> Triggered by your multi-hop tests (host B via host A) I run git-apply
> actions over Tramp with the patch I submitted. I settled with 3
> different test environment configurations; all of them should provide
> the same functionality:
>
> 1. An explicit multi-hop target "/ssh:host_A|/ssh:host_B:..."
>
> 2. The SSH client option ProxyCommand set to "ssh host_A nc host_B 22" and
> simple "/ssh:host_B"
>
> 3. The SSH client option ProxyJump set to "host_A" and simple "/ssh:host_B"
>
> tramp-use-connection-share was set to nil in each case.
>
> It worked as expected only in the first two cases. As far as the last one
> is concerned, the workflow wasn't stalled, but it turned out to be no-op
> after all.
>
> It seems the last test environment enables the most responsive data
> stream: no additional userland process is forked on a middle box and
> Nagle's algorithm is disabled for all TCP connections involved (which is
> not true for the case no 2.). In the end such results might corroborate
> your theory.

Well, I've tried deeper debugging with this. But for whatever values
I've taken for stty's min and time in tramp-sh-handle-make-process,
there were always cases it didn't work. Rare cases, and not always
reproducible due to race conditions, but they exist.

Going a step back to magit, I see the following comment in magit-start-process:

--8<---------------cut here---------------start------------->8---
               ;; Don't use a pty, because it would set icrnl
               ;; which would modify the input (issue #20).
--8<---------------cut here---------------end--------------->8---

I cannot speak for the local case. But in Tramp, we need to set "stty
-icanon ..." for the pipe connection type in order avoid blocking
situations with larger hunks of data, see the comment there. And the use
case of magit-start-process would be better with the connection type
pty, at least when calling Tramp.

The appended (rather naïve) patch fixes the reported use case in my
local enviroment.

> at

Best regards, Michael.