bug#62093: [PATCH] Let processes read nothing from stdin in tramp

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

[-- Attachment #1: Type: text/plain, Size: 1234 bytes --]

Aleksander Trofimowicz <trof@n90.eu> writes:

> Hi Michael,

Hi Aleksander,

> These are reasons magit stopped working, and my patch was a half-baked
> attempt to address the problem by other means. Please drop it. At the
> same time I would encourage you to revert the change that introduced the
> noncanonical mode, since the current state is very confusing and breaks
> existing code. A dedicated defcustom should cater for those who deal
> with large input data.

I've tried several approaches, none of them did satisfy me
completely. So I have introduced a new user option
tramp-pipe-stty-settings, which you can use for different stty
settings. See appended patch. Let-binding this to "-icanon min 0 time 1"
in magit-start-process has at least solved the problem as shown here in
this bug report. I'm not very happy with this, but it is the best I
could do now. Could you please check?

> Ideally no pty should be allocated if one asks for 'pipe' (by employing
> ssh connection multiplexing?).

We have already ssh multiplexing. If you like to avoid pty's at all, you
might try to use direct async processes:

(info "(tramp)Improving performance of asynchronous remote processes")

> Kind regards,
> at

Best regards, Michael.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 1991 bytes --]

diff --git a/lisp/tramp-sh.el b/lisp/tramp-sh.el
index aa1d025b..f0e23950 100644
--- a/lisp/tramp-sh.el
+++ b/lisp/tramp-sh.el
@@ -2876,7 +2876,16 @@ the result will be a local, non-Tramp, file name."
 		(tramp-run-real-handler
 		 #'expand-file-name (list localname))))))))))

-;;; Remote commands:
+;;; Remote processes:
+
+(defcustom tramp-pipe-stty-settings "-icanon min 1 time 0"
+  "How to prevent blocking read in pipeline processes.
+This is used in `make-process' with `connection-type' `pipe'."
+  :group 'tramp
+  :version "29.2"
+  :type '(choice (const :tag "Use size limit" "-icanon min 1 time 0")
+		 (const :tag "Use timeout" "-icanon min 0 time 1")
+		 string))

 ;; We use BUFFER also as connection buffer during setup.  Because of
 ;; this, its original contents must be saved, and restored once
@@ -3087,12 +3096,21 @@ implementation will be used."
 			      ;; otherwise strings larger than 4096
 			      ;; bytes, sent by the process, could
 			      ;; block, see termios(3) and Bug#61341.
+			      ;; In order to prevent blocking read
+			      ;; from pipe processes, "stty -icanon"
+			      ;; is used.  By default, it expects at
+			      ;; least one character to read.  When a
+			      ;; process does not read from stdin,
+			      ;; like magit, it should set a timeout
+			      ;; instead. See`tramp-pipe-stty-settings'.
+			      ;; (Bug#62093)
 			      ;; FIXME: Shall we rather use "stty raw"?
-			      (if (tramp-check-remote-uname v "Darwin")
-				  (tramp-send-command
-				   v "stty -icanon min 1 time 0")
-				(tramp-send-command
-				 v "stty -icrnl -icanon min 1 time 0")))
+			      (tramp-send-command
+			       v (format
+				  "stty %s %s"
+				  (if (tramp-check-remote-uname v "Darwin")
+				      "" "-icrnl")
+				  tramp-pipe-stty-settings)))
 			    ;; `tramp-maybe-open-connection' and
 			    ;; `tramp-send-command-and-read' could
 			    ;; have trashed the connection buffer.