From 186643ea8a8e36bf3264b36c4106793cea25c6b3 Mon Sep 17 00:00:00 2001 From: Michael Albinus Date: Mon, 6 Mar 2023 13:35:49 +0100 Subject: [PATCH 1/2] Add tramp-use-ssh-controlmaster-options value `suppress'. * doc/misc/tramp.texi (Ssh setup): Explain tramp-use-ssh-controlmaster-options value `suppress'. (Remote processes): Add reference to "Using ssh connection sharing". * etc/NEWS: Extend 'tramp-use-ssh-controlmaster-options' values. ;; Fix typos. * lisp/net/tramp-sh.el (tramp-use-ssh-controlmaster-options): Allow new value `suppress'. (tramp-ssh-option-exists-p): New defun. (tramp-ssh-controlmaster-options): Implement `suppress' actions. --- doc/misc/tramp.texi | 29 ++++++++++++---- etc/NEWS | 14 +++++--- lisp/net/tramp-sh.el | 80 ++++++++++++++++++++++++-------------------- 3 files changed, 75 insertions(+), 48 deletions(-) diff --git a/doc/misc/tramp.texi b/doc/misc/tramp.texi index 7f6182ae17c..acf32726895 100644 --- a/doc/misc/tramp.texi +++ b/doc/misc/tramp.texi @@ -2728,6 +2728,7 @@ entry, @option{Seconds between keepalives} option. Set this to 5. There is no counter which could be set. +@anchor{Using ssh connection sharing} @subsection Using ssh connection sharing @vindex ControlPath@r{, ssh option} @@ -2758,19 +2759,32 @@ Note how @samp{%r}, @samp{%h} and @samp{%p} must be encoded as @samp{%%r}, @samp{%%h} and @samp{%%p}. @vindex tramp-use-ssh-controlmaster-options -If the @file{~/.ssh/config} file is configured appropriately for the -above behavior, then any changes to @command{ssh} can be suppressed -with this @code{nil} setting: +Using a predefined string in @code{tramp-ssh-controlmaster-options}, +or puzzling an own string, happens only when user option +@code{tramp-use-ssh-controlmaster-options} is set to @code{t}. If the +@file{~/.ssh/config} file is configured appropriately for the above +behavior, then any changes to @command{ssh} can be suppressed with +this @code{nil} setting: @lisp (customize-set-variable 'tramp-use-ssh-controlmaster-options nil) @end lisp +Sometimes, it is not possible to use OpenSSH's @option{ControlMaster} +option for remote processes. This could result in concurrent access +to the OpenSSH socket when reading data by different processes, which +could block Emacs. In this case, setting +@code{tramp-use-ssh-controlmaster-options} to @code{suppress} disables +shared access. It is not needed to set this user option permanently +to @code{suppress}, binding the user option prior calling +@code{make-process} is sufficient. @value{tramp} does this for +esxample for compilation processes on its own. + @vindex ProxyCommand@r{, ssh option} @vindex ProxyJump@r{, ssh option} -This should also be set to @code{nil} if you use the -@option{ProxyCommand} or @option{ProxyJump} options in your -@command{ssh} configuration. +@code{tramp-use-ssh-controlmaster-options} should also be set to +@code{nil} or @code{suppress} if you use the @option{ProxyCommand} or +@option{ProxyJump} options in your @command{ssh} configuration. In order to use the @option{ControlMaster} option, @value{tramp} must check whether the @command{ssh} client supports this option. This is @@ -4288,7 +4302,8 @@ In order to gain even more performance, it is recommended to bind @code{start-file-process}. Furthermore, you might set @code{tramp-use-ssh-controlmaster-options} to @code{nil} in order to bypass @value{tramp}'s handling of the @option{ControlMaster} options, -and use your own settings in @file{~/.ssh/config}. +and use your own settings in @file{~/.ssh/config}, @ref{Using ssh +connection sharing}. @node Cleanup remote connections diff --git a/etc/NEWS b/etc/NEWS index 116b60d8b11..7e0454b3b9e 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -100,7 +100,7 @@ If you want to get back the old behavior, set the user option to the value *** New user option 'grep-use-headings'. When non-nil, the output of Grep is split into sections, one for each file, instead of having file names prefixed to each line. It is -equivalent to the --heading option of some tools such as 'git grep' +equivalent to the "--heading" option of some tools such as 'git grep' and 'rg'. The headings are displayed using the new 'grep-heading' face. @@ -116,7 +116,7 @@ switches for shortlogs, such as the one produced by 'C-x v L'. +++ *** 'diff-ignore-whitespace-hunk' can now be applied to all hunks. -When called with a non-nil prefix argument +When called with a non-nil prefix argument, 'diff-ignore-whitespace-hunk' now iterates over all the hunks in the current diff, regenerating them without whitespace changes. @@ -161,7 +161,7 @@ this to your configuration: --- *** You can now properly unload Eshell. -Calling "(unload-feature 'eshell)" no longer signals an error, and now +Calling '(unload-feature 'eshell)' no longer signals an error, and now correctly unloads Eshell and all of its modules. +++ @@ -184,6 +184,12 @@ point is not in a comment or a string. It is by default bound to *** New connection method "toolbox". This allows accessing system containers provided by Toolbox. ++++ +*** New value 'suppress' for user option 'tramp-use-ssh-controlmaster-options'. +This user option allows now the values t, nil, and 'suppress'. The +latter suppresses also "ControlMaster" settings in the user's +"~/.ssh/config" file. + ** EWW +++ @@ -211,7 +217,7 @@ asynchronously (which is the default behavior). * New Modes and Packages in Emacs 30.1 -** New major modes based on the tree-sitter library. +** New major modes based on the tree-sitter library +++ *** New major mode 'html-ts-mode'. diff --git a/lisp/net/tramp-sh.el b/lisp/net/tramp-sh.el index 49da0e425ff..3ae5208154a 100644 --- a/lisp/net/tramp-sh.el +++ b/lisp/net/tramp-sh.el @@ -108,11 +108,18 @@ detected as prompt when being sent on echoing hosts, therefore.") (defcustom tramp-use-ssh-controlmaster-options (not (eq system-type 'windows-nt)) "Whether to use `tramp-ssh-controlmaster-options'. +Set it to t, if you want Tramp to apply these options. Set it to nil, if you use Control* or Proxy* options in your ssh -configuration." +configuration. +Set it to `suppress' if you want to disable settings in your +\"~/.ssh/config¸\"." :group 'tramp - :version "28.1" - :type 'boolean) + :version "29.2" + :type '(choice (const :tag "Set ControlMaster" t) + (const :tag "Don't set ControlMaster" nil) + (const :tag "Suppress ControlMaster" suppress)) + ;; Check with (safe-local-variable-p 'tramp-use-ssh-controlmaster-options 'suppress) + :safe (lambda (val) (and (memq val '(t nil suppress)) t))) (defvar tramp-ssh-controlmaster-options nil "Which ssh Control* arguments to use. @@ -123,8 +130,8 @@ If it is a string, it should have the form spec must be doubled, because the string is used as format string. Otherwise, it will be auto-detected by Tramp, if -`tramp-use-ssh-controlmaster-options' is non-nil. The value -depends on the installed local ssh version. +`tramp-use-ssh-controlmaster-options' is t. The value depends on +the installed local ssh version. The string is used in `tramp-methods'.") @@ -4811,6 +4818,15 @@ Goes through the list `tramp-inline-compress-commands'." (tramp-message vec 2 "Couldn't find an inline transfer compress command"))))) +(defun tramp-ssh-option-exists-p (vec option) + "Check, whether local ssh OPTION is applicable." + ;; We don't want to cache it persistently. + (with-tramp-connection-property nil option + ;; We use a non-existing IP address for check, in order to avoid + ;; useless connections, and DNS timeouts. + (zerop + (tramp-call-process vec "ssh" nil nil nil "-G" "-o" option "0.0.0.1")))) + (defun tramp-ssh-controlmaster-options (vec) "Return the Control* arguments of the local ssh." (cond @@ -4820,40 +4836,30 @@ Goes through the list `tramp-inline-compress-commands'." "") ;; There is already a value to be used. - ((stringp tramp-ssh-controlmaster-options) tramp-ssh-controlmaster-options) + ((and (eq tramp-use-ssh-controlmaster-options t) + (stringp tramp-ssh-controlmaster-options)) + tramp-ssh-controlmaster-options) ;; Determine the options. - (t (setq tramp-ssh-controlmaster-options "") - (let ((case-fold-search t)) - (ignore-errors - (with-tramp-progress-reporter - vec 4 "Computing ControlMaster options" - ;; We use a non-existing IP address, in order to avoid - ;; useless connections, and DNS timeouts. - (when (zerop - (tramp-call-process - vec "ssh" nil nil nil - "-G" "-o" "ControlMaster=auto" "0.0.0.1")) - (setq tramp-ssh-controlmaster-options - "-o ControlMaster=auto") - (if (zerop - (tramp-call-process - vec "ssh" nil nil nil - "-G" "-o" "ControlPath=tramp.%C" "0.0.0.1")) - (setq tramp-ssh-controlmaster-options - (concat tramp-ssh-controlmaster-options - " -o ControlPath=tramp.%%C")) - (setq tramp-ssh-controlmaster-options - (concat tramp-ssh-controlmaster-options - " -o ControlPath=tramp.%%r@%%h:%%p"))) - (when (zerop - (tramp-call-process - vec "ssh" nil nil nil - "-G" "-o" "ControlPersist=no" "0.0.0.1")) - (setq tramp-ssh-controlmaster-options - (concat tramp-ssh-controlmaster-options - " -o ControlPersist=no"))))))) - tramp-ssh-controlmaster-options))) + (t (ignore-errors + ;; ControlMaster and ControlPath options are introduced in OpenSSH 3.9. + (when (tramp-ssh-option-exists-p vec "ControlMaster=auto") + (concat + "-o ControlMaster=" + (if (eq tramp-use-ssh-controlmaster-options 'suppress) + "no" "auto") + + " -o ControlPath=" + (if (eq tramp-use-ssh-controlmaster-options 'suppress) + "none" + ;; Hashed tokens are introduced in OpenSSH 6.7. + (if (tramp-ssh-option-exists-p vec "ControlPath=tramp.%C") + "tramp.%%C" "tramp.%%r@%%h:%%p")) + + ;; ControlPersist option is introduced in OpenSSH 5.6. + (when (and (not (eq tramp-use-ssh-controlmaster-options 'suppress)) + (tramp-ssh-option-exists-p vec "ControlPersist=no")) + " -o ControlPersist=no"))))))) (defun tramp-scp-strict-file-name-checking (vec) "Return the strict file name checking argument of the local scp." From 1e5393a57a3bbe3f9167fee59232c2e424afadf2 Mon Sep 17 00:00:00 2001 From: Vibhav Pant Date: Wed, 1 Mar 2023 15:04:34 +0530 Subject: [PATCH 2/2] Don't modify interactive closures destructively (Bug#60974). * lisp/emacs-lisp/cconv.el (cconv-convert): When form is an interactive lambda form, don't destructively modify it, as it might be a constant literal. Instead, create a new list with the relevant place(s) changed. * test/lisp/emacs-lisp/cconv-tests.el (cconv-tests-interactive-form-modify-bug60974): New test. --- lisp/emacs-lisp/cconv.el | 37 +++++++++++++++++++++-------- test/lisp/emacs-lisp/cconv-tests.el | 12 ++++++++++ 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/lisp/emacs-lisp/cconv.el b/lisp/emacs-lisp/cconv.el index ad9d8ab0a51..601e2c13d61 100644 --- a/lisp/emacs-lisp/cconv.el +++ b/lisp/emacs-lisp/cconv.el @@ -477,7 +477,7 @@ places where they originally did not directly appear." branch)) cond-forms))) - (`(function (lambda ,args . ,body) . ,_) + (`(function (lambda ,args . ,body) . ,rest) (let* ((docstring (if (eq :documentation (car-safe (car body))) (cconv-convert (cadr (pop body)) env extend))) (bf (if (stringp (car body)) (cdr body) body)) @@ -485,15 +485,32 @@ places where they originally did not directly appear." (gethash form cconv--interactive-form-funs))) (wrapped (pcase if (`#'(lambda (&rest _cconv--dummy) .,_) t) (_ nil))) (cif (when if (cconv-convert if env extend))) - (_ (pcase cif - ('nil nil) - (`#',f - (setf (cadr (car bf)) (if wrapped (nth 2 f) cif)) - (setq cif nil)) - ;; The interactive form needs special treatment, so the form - ;; inside the `interactive' won't be used any further. - (_ (setf (cadr (car bf)) nil)))) - (cf (cconv--convert-function args body env form docstring))) + (cf nil)) + ;; TODO: Because we need to non-destructively modify body, this code + ;; is particularly ugly. This should ideally be moved to + ;; cconv--convert-function. + (pcase cif + ('nil (setq bf nil)) + (`#',f + (pcase-let ((`((,f1 . (,_ . ,f2)) . ,f3) bf)) + (setq bf `((,f1 . (,(if wrapped (nth 2 f) cif) . ,f2)) . ,f3))) + (setq cif nil)) + ;; The interactive form needs special treatment, so the form + ;; inside the `interactive' won't be used any further. + (_ (pcase-let ((`((,f1 . (,_ . ,f2)) . ,f3) bf)) + (setq bf `((,f1 . (nil . ,f2)) . ,f3))))) + (when bf + ;; If we modified bf, re-build body and form as + ;; copies with the modified bits. + (setq body (if (stringp (car body)) + (cons (car body) bf) + bf) + form `(function (lambda ,args . ,body) . ,rest)) + ;; Also, remove the current old entry on the alist, replacing + ;; it with the new one. + (let ((entry (pop cconv-freevars-alist))) + (push (cons body (cdr entry)) cconv-freevars-alist))) + (setq cf (cconv--convert-function args body env form docstring)) (if (not cif) ;; Normal case, the interactive form needs no special treatment. cf diff --git a/test/lisp/emacs-lisp/cconv-tests.el b/test/lisp/emacs-lisp/cconv-tests.el index 349ffeb7e47..6facd3452ea 100644 --- a/test/lisp/emacs-lisp/cconv-tests.el +++ b/test/lisp/emacs-lisp/cconv-tests.el @@ -376,6 +376,18 @@ (eval '(lambda (x) :closure-dont-trim-context (+ x 1)) `((y . ,magic-string))))))) +(ert-deftest cconv-tests-interactive-form-modify-bug60974 () + (let* ((f '(function (lambda (&optional arg) + (interactive + (list (if current-prefix-arg + (prefix-numeric-value current-prefix-arg) + 'toggle))) + (ignore arg)))) + (if (cadr (nth 2 (cadr f)))) + (if2)) + (cconv-closure-convert f) + (setq if2 (cadr (nth 2 (cadr f)))) + (should (eq if if2)))) (provide 'cconv-tests) ;;; cconv-tests.el ends here