From: Michael Albinus <michael.albinus@gmx.de>
To: Aleksander Trofimowicz <trof@n90.eu>
Cc: Benjamin Orthen <benjamin@orthen.net>, 62093@debbugs.gnu.org
Subject: bug#62093: [PATCH] Let processes read nothing from stdin in tramp
Date: Sat, 11 Nov 2023 14:08:18 +0100 [thread overview]
Message-ID: <87y1f430al.fsf@gmx.de> (raw)
In-Reply-To: <877cmupx1c.fsf@n90.eu> (Aleksander Trofimowicz's message of "Mon, 06 Nov 2023 16:51:24 +0000")
[-- Attachment #1: Type: text/plain, Size: 2399 bytes --]
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.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 1146 bytes --]
*** /home/albinus/.emacs.d/elpa/magit-20231103.1516/magit-process.el~ 2023-11-04 11:43:11.848351502 +0100
--- /home/albinus/.emacs.d/elpa/magit-20231103.1516/magit-process.el 2023-11-11 13:26:23.212027014 +0100
***************
*** 583,589 ****
(let ((process-connection-type
;; Don't use a pty, because it would set icrnl
;; which would modify the input (issue #20).
! (and (not input) magit-process-connection-type))
(process-environment (magit-process-environment))
(default-process-coding-system (magit--process-coding-system)))
(apply #'start-file-process
--- 583,590 ----
(let ((process-connection-type
;; Don't use a pty, because it would set icrnl
;; which would modify the input (issue #20).
! (or (and (file-remote-p default-directory) t)
! (and (not input) magit-process-connection-type)))
(process-environment (magit-process-environment))
(default-process-coding-system (magit--process-coding-system)))
(apply #'start-file-process
next prev parent reply other threads:[~2023-11-11 13:08 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-25 11:40 bug#62093: [PATCH] Let processes read nothing from stdin in tramp Aleksander Trofimowicz
2023-11-04 17:42 ` Michael Albinus
2023-11-06 16:51 ` Aleksander Trofimowicz via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-11-11 13:08 ` Michael Albinus [this message]
2023-11-17 20:10 ` Aleksander Trofimowicz via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-11-29 13:27 ` Michael Albinus
2023-12-04 11:05 ` Aleksander Trofimowicz via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-24 10:32 ` Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87y1f430al.fsf@gmx.de \
--to=michael.albinus@gmx.de \
--cc=62093@debbugs.gnu.org \
--cc=benjamin@orthen.net \
--cc=trof@n90.eu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.