Package: emacs;
Reported by: João Távora <joaotavora <at> gmail.com>
Date: Tue, 14 Mar 2023 23:07:01 UTC
Severity: normal
Found in version 30.0.50
Done: Michael Albinus <michael.albinus <at> gmx.de>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 62194 in the body.
You can then email your comments to 62194 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
View this report as an mbox folder, status mbox, maintainer mbox
michael.albinus <at> gmx.de, bug-gnu-emacs <at> gnu.org:bug#62194; Package emacs.
(Tue, 14 Mar 2023 23:07:01 GMT) Full text and rfc822 format available.João Távora <joaotavora <at> gmail.com>:michael.albinus <at> gmx.de, bug-gnu-emacs <at> gnu.org.
(Tue, 14 Mar 2023 23:07:02 GMT) Full text and rfc822 format available.Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: João Távora <joaotavora <at> gmail.com> To: bug-gnu-emacs <at> gnu.org Subject: 30.0.50; Two Eglot-over-Tramp tests are failing on master, passing on emacs-29 Date: Tue, 14 Mar 2023 23:08:17 +0000
Hi Michael,
I've noticd that these two Eglot tests are passing on emacs-29 but
failing on emacs master. I haven't checked if this is due to your
recent work on Tramp or some other factor.
Sometimes one of the tests passes, but having the two of them pass is
rare. In emacs-29, they always pass. Both emacs-master and emacs-29
have the version of Eglot _with_ the ControlMaster-neutering workaround.
To reproduce, you need the "clangd" language server installed (which is
one of the easiest to get language servers, should be available in your
package manager).
Then:
make -C test lisp/progmodes/eglot-tests SELECTOR=\"tramp\"
The error output is after my sig. Let me know if you need more info.
João
make: Entering directory '/home/capitaomorte/Source/Emacs/emacs-master/test'
make[1]: Entering directory '/home/capitaomorte/Source/Emacs/emacs-master/test'
GEN lisp/progmodes/eglot-tests.log
Running 2 tests (2023-03-14 23:04:34+0000, selector ‘"tramp"’)
[eglot-tests] [eglot-test-tramp-test]: test start
[eglot] Connected! Server `clangd' now managing `(c-mode c-ts-mode c++-mode c++-ts-mode)' buffers in project `project'.
File error: Couldn’t find exit status of ‘\readlink --canonicalize-missing /tmp/eglot--fixturezQyQYv/project/merdix.c’
[eglot-tests] [eglot-test-tramp-test]: FAILED
[eglot] Asking EGLOT (project/(c-mode c-ts-mode c++-mode c++-ts-mode)) politely to terminate
[jsonrpc] Server exited with status 9
[eglot-tests] contents of ` *EGLOT (project/(c-mode c-ts-mode c++-mode c++-ts-mode)) output*':
[eglot-tests] contents of ` *EGLOT (project/(c-mode c-ts-mode c++-mode c++-ts-mode)) stderr*':
I[23:04:35.143] clangd version 15.0.7
I[23:04:35.143] Features: linux
I[23:04:35.143] PID: 251491
I[23:04:35.143] Working directory: /tmp/eglot--fixturezQyQYv/project
I[23:04:35.143] argv[0]: /usr/bin/clangd
I[23:04:35.143] Starting LSP over stdin/stdout
I[23:04:35.143] <-- initialize(1)
I[23:04:35.144] --> reply:initialize(1) 0 ms
I[23:04:35.145] <-- initialized
I[23:04:35.153] <-- textDocument/didOpen
I[23:04:35.153] <-- workspace/didChangeConfiguration
I[23:04:35.153] Failed to find compilation database for /tmp/eglot--fixturezQyQYv/project/coiso.c
I[23:04:35.153] ASTWorker building file /tmp/eglot--fixturezQyQYv/project/coiso.c version 0 with command clangd fallback
[/tmp/eglot--fixturezQyQYv/project]
/usr/bin/clang -resource-dir=/usr/lib/clang/15.0.7 -- /tmp/eglot--fixturezQyQYv/project/coiso.c
I[23:04:35.163] --> textDocument/publishDiagnostics
I[23:04:35.184] <-- textDocument/didOpen
I[23:04:35.184] Failed to find compilation database for /tmp/eglot--fixturezQyQYv/project/merdix.c
I[23:04:35.184] ASTWorker building file /tmp/eglot--fixturezQyQYv/project/merdix.c version 0 with command clangd fallback
[/tmp/eglot--fixturezQyQYv/project]
/usr/bin/clang -resource-dir=/usr/lib/clang/15.0.7 -- /tmp/eglot--fixturezQyQYv/project/merdix.c
I[23:04:35.194] --> textDocument/publishDiagnostics
I[23:04:35.274] <-- shutdown(2)
I[23:04:35.274] --> reply:shutdown(2) 0 ms
I[23:04:35.274] <-- exit
I[23:04:35.274] LSP finished, exiting with status 0
[eglot-tests] contents of `*EGLOT (project/(c-mode c-ts-mode c++-mode c++-ts-mode)) events*':
[stderr] nil
[stderr]
[stderr] nil
[stderr]
[stderr] nil
[stderr]
[stderr] nil
[stderr]
[stderr] nil
[stderr]
[stderr] foo
[stderr] foo
[stderr] foo
[stderr]
[stderr] foo
[stderr] bar
[stderr] nil
[stderr] bar
[stderr]
[stderr] nil
[stderr]
[stderr] nil
[stderr]
[stderr] nil
[stderr]
[stderr] nil
[stderr]
[stderr] "/dev/pts/8"
[stderr] tramp_exit_status 0
[stderr] tramp_exit_status 0
[stderr] tramp_exit_status 0
[stderr] "/dev/pts/8"
[stderr]
[stderr] nil
[stderr]
[stderr] nil
[stderr]
[stderr] 251483
[stderr] tramp_exit_status 0
[stderr] tramp_exit_status 0
[stderr] tramp_exit_status 0
[stderr] 251483
[stderr]
[internal] Tue Mar 14 23:04:35 2023:
(:message "Running language server: /bin/sh -c stty raw > /dev/null; /usr/bin/clangd")
[client-request] (id:1) Tue Mar 14 23:04:35 2023:
(:jsonrpc "2.0" :id 1 :method "initialize" :params
(:processId nil :rootPath "/tmp/eglot--fixturezQyQYv/project/" :rootUri "file:///tmp/eglot--fixturezQyQYv/project" :initializationOptions #s(hash-table size 1 test eql rehash-size 1.5 rehash-threshold 0.8125 data
())
:capabilities
(:workspace
(:applyEdit t :executeCommand
(:dynamicRegistration :json-false)
:workspaceEdit
(:documentChanges t)
:didChangeWatchedFiles
(:dynamicRegistration :json-false)
:symbol
(:dynamicRegistration :json-false)
:configuration t :workspaceFolders t)
:textDocument
(:synchronization
(:dynamicRegistration :json-false :willSave t :willSaveWaitUntil t :didSave t)
:completion
(:dynamicRegistration :json-false :completionItem
(:snippetSupport :json-false :deprecatedSupport t :resolveSupport
(:properties
["documentation" "details" "additionalTextEdits"])
:tagSupport
(:valueSet
[1]))
:contextSupport t)
:hover
(:dynamicRegistration :json-false :contentFormat
["plaintext"])
:signatureHelp
(:dynamicRegistration :json-false :signatureInformation
(:parameterInformation
(:labelOffsetSupport t)
:activeParameterSupport t))
:references
(:dynamicRegistration :json-false)
:definition
(:dynamicRegistration :json-false :linkSupport t)
:declaration
(:dynamicRegistration :json-false :linkSupport t)
:implementation
(:dynamicRegistration :json-false :linkSupport t)
:typeDefinition
(:dynamicRegistration :json-false :linkSupport t)
:documentSymbol
(:dynamicRegistration :json-false :hierarchicalDocumentSymbolSupport t :symbolKind
(:valueSet
[1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26]))
:documentHighlight
(:dynamicRegistration :json-false)
:codeAction
(:dynamicRegistration :json-false :codeActionLiteralSupport
(:codeActionKind
(:valueSet
["quickfix" "refactor" "refactor.extract" "refactor.inline" "refactor.rewrite" "source" "source.organizeImports"]))
:isPreferredSupport t)
:formatting
(:dynamicRegistration :json-false)
:rangeFormatting
(:dynamicRegistration :json-false)
:rename
(:dynamicRegistration :json-false)
:inlayHint
(:dynamicRegistration :json-false)
:publishDiagnostics
(:relatedInformation :json-false :codeDescriptionSupport :json-false :tagSupport
(:valueSet
[1 2])))
:general
(:positionEncodings
["utf-32" "utf-8" "utf-16"])
:experimental #s(hash-table size 1 test eql rehash-size 1.5 rehash-threshold 0.8125 data
()))
:workspaceFolders
[(:uri "file:///tmp/eglot--fixturezQyQYv/project" :name "/mock:krug:/tmp/eglot--fixturezQyQYv/project/")]))
[stderr] I[23:04:35.143] clangd version 15.0.7
[stderr] I[23:04:35.143] Features: linux
[stderr] I[23:04:35.143] PID: 251491
[stderr] I[23:04:35.143] Working directory: /tmp/eglot--fixturezQyQYv/project
[stderr] I[23:04:35.143] argv[0]: /usr/bin/clangd
[stderr] I[23:04:35.143] Starting LSP over stdin/stdout
[stderr] I[23:04:35.143] <-- initialize(1)
[server-reply] (id:1) Tue Mar 14 23:04:35 2023:
(:id 1 :jsonrpc "2.0" :result
(:capabilities
(:astProvider t :callHierarchyProvider t :clangdInlayHintsProvider t :codeActionProvider
(:codeActionKinds
["quickfix" "refactor" "info"])
:compilationDatabase
(:automaticReload t)
:completionProvider
(:resolveProvider :json-false :triggerCharacters
["." "<" ">" ":" "\"" "/" "*"])
:declarationProvider t :definitionProvider t :documentFormattingProvider t :documentHighlightProvider t :documentLinkProvider
(:resolveProvider :json-false)
:documentOnTypeFormattingProvider
(:firstTriggerCharacter "\n" :moreTriggerCharacter
[])
:documentRangeFormattingProvider t :documentSymbolProvider t :executeCommandProvider
(:commands
["clangd.applyFix" "clangd.applyTweak"])
:hoverProvider t :implementationProvider t :inlayHintProvider t :memoryUsageProvider t :referencesProvider t :renameProvider t :selectionRangeProvider t :semanticTokensProvider
(:full
(:delta t)
:legend
(:tokenModifiers
["declaration" "deprecated" "deduced" "readonly" "static" "abstract" "virtual" "dependentName" "defaultLibrary" "usedAsMutableReference" "functionScope" "classScope" "fileScope" "globalScope"]
:tokenTypes
["variable" "variable" "parameter" "function" "method" "function" "property" "variable" "class" "interface" "enum" "enumMember" "type" "type" "unknown" "namespace" "typeParameter" "concept" "type" "macro" "comment"])
:range :json-false)
:signatureHelpProvider
(:triggerCharacters
["(" ")" "{" "}" "<" ">" ","])
:standardTypeHierarchyProvider t :textDocumentSync
(:change 2 :openClose t :save t)
:typeDefinitionProvider t :typeHierarchyProvider t :workspaceSymbolProvider t)
:serverInfo
(:name "clangd" :version "clangd version 15.0.7 linux x86_64-pc-linux-gnu")))
[client-notification] Tue Mar 14 23:04:35 2023:
(:jsonrpc "2.0" :method "initialized" :params #s(hash-table size 1 test eql rehash-size 1.5 rehash-threshold 0.8125 data
()))
[stderr] I[23:04:35.144] --> reply:initialize(1) 0 ms
[stderr] I[23:04:35.145] <-- initialized
[client-notification] Tue Mar 14 23:04:35 2023:
(:jsonrpc "2.0" :method "textDocument/didOpen" :params
(:textDocument
(:uri "file:///tmp/eglot--fixturezQyQYv/project/coiso.c" :version 0 :languageId "c" :text "bla")))
[client-notification] Tue Mar 14 23:04:35 2023:
(:jsonrpc "2.0" :method "workspace/didChangeConfiguration" :params
(:settings #s(hash-table size 1 test eql rehash-size 1.5 rehash-threshold 0.8125 data
())))
[stderr] I[23:04:35.153] <-- textDocument/didOpen
[stderr] I[23:04:35.153] <-- workspace/didChangeConfiguration
[stderr] I[23:04:35.153] Failed to find compilation database for /tmp/eglot--fixturezQyQYv/project/coiso.c
[stderr] I[23:04:35.153] ASTWorker building file /tmp/eglot--fixturezQyQYv/project/coiso.c version 0 with command clangd fallback
[stderr] [/tmp/eglot--fixturezQyQYv/project]
[stderr] /usr/bin/clang -resource-dir=/usr/lib/clang/15.0.7 -- /tmp/eglot--fixturezQyQYv/project/coiso.c
[server-notification] Tue Mar 14 23:04:35 2023:
(:jsonrpc "2.0" :method "textDocument/publishDiagnostics" :params
(:diagnostics
[(:code "unknown_typename" :message "Unknown type name 'bla'" :range
(:end
(:character 3 :line 0)
:start
(:character 0 :line 0))
:severity 1 :source "clang")
(:code "expected_either" :message "Expected identifier or '('" :range
(:end
(:character 3 :line 0)
:start
(:character 3 :line 0))
:severity 1 :source "clang")]
:uri "file:///tmp/eglot--fixturezQyQYv/project/coiso.c" :version 0))
[stderr] I[23:04:35.163] --> textDocument/publishDiagnostics
[client-notification] Tue Mar 14 23:04:35 2023:
(:jsonrpc "2.0" :method "textDocument/didOpen" :params
(:textDocument
(:uri "file:///tmp/eglot--fixturezQyQYv/project/merdix.c" :version 0 :languageId "c" :text "bla")))
[stderr] I[23:04:35.184] <-- textDocument/didOpen
[stderr] I[23:04:35.184] Failed to find compilation database for /tmp/eglot--fixturezQyQYv/project/merdix.c
[stderr] I[23:04:35.184] ASTWorker building file /tmp/eglot--fixturezQyQYv/project/merdix.c version 0 with command clangd fallback
[stderr] [/tmp/eglot--fixturezQyQYv/project]
[stderr] /usr/bin/clang -resource-dir=/usr/lib/clang/15.0.7 -- /tmp/eglot--fixturezQyQYv/project/merdix.c
[server-notification] Tue Mar 14 23:04:35 2023:
(:jsonrpc "2.0" :method "textDocument/publishDiagnostics" :params
(:diagnostics
[(:code "unknown_typename" :message "Unknown type name 'bla'" :range
(:end
(:character 3 :line 0)
:start
(:character 0 :line 0))
:severity 1 :source "clang")
(:code "expected_either" :message "Expected identifier or '('" :range
(:end
(:character 3 :line 0)
:start
(:character 3 :line 0))
:severity 1 :source "clang")]
:uri "file:///tmp/eglot--fixturezQyQYv/project/merdix.c" :version 0))
[stderr] I[23:04:35.194] --> textDocument/publishDiagnostics
[client-request] (id:2) Tue Mar 14 23:04:35 2023:
(:jsonrpc "2.0" :id 2 :method "shutdown" :params nil)
[server-reply] (id:2) Tue Mar 14 23:04:35 2023:
(:id 2 :jsonrpc "2.0" :result nil)
[client-notification] Tue Mar 14 23:04:35 2023:
(:jsonrpc "2.0" :method "exit" :params nil)
[internal] Tue Mar 14 23:04:35 2023:
(:message "Connection state changed" :change "killed\n")
----------b---y---e---b---y---e----------
[stderr] I[23:04:35.274] <-- shutdown(2)
[stderr] I[23:04:35.274] --> reply:shutdown(2) 0 ms
[stderr] I[23:04:35.274] <-- exit
[stderr] I[23:04:35.274] LSP finished, exiting with status 0
[eglot-tests] Killing (coiso.c merdix.c), wiping /mock:krug:/tmp/eglot--fixturezQyQYv, restoring nil
Test eglot-test-tramp-test backtrace:
signal(wrong-type-argument ("listp /tmp/tramp.G0HtZ3"))
tramp-error((tramp-file-name "mock" nil nil "krug" nil "/tmp/eglot--
tramp-signal-hook-function(wrong-type-argument ("listp /tmp/tramp.G0
signal(wrong-type-argument ("listp /tmp/tramp.G0HtZ3"))
tramp-handle-insert-file-contents("/mock:krug:/tmp/eglot--fixturezQy
apply(tramp-handle-insert-file-contents ("/mock:krug:/tmp/eglot--fix
tramp-sh-file-name-handler(insert-file-contents "/mock:krug:/tmp/egl
apply(tramp-sh-file-name-handler insert-file-contents ("/mock:krug:/
tramp-file-name-handler(insert-file-contents "/mock:krug:/tmp/eglot-
insert-file-contents("/mock:krug:/tmp/eglot--fixturezQyQYv/project/R
vc-insert-file("/mock:krug:/tmp/eglot--fixturezQyQYv/project/RCS/m..
vc-rcs-fetch-master-state("/mock:krug:/tmp/eglot--fixturezQyQYv/proj
vc-rcs-state("/mock:krug:/tmp/eglot--fixturezQyQYv/project/merdi..."
apply(vc-rcs-state "/mock:krug:/tmp/eglot--fixturezQyQYv/project/mer
vc-call-backend(RCS state "/mock:krug:/tmp/eglot--fixturezQyQYv/proj
vc-state-refresh("/mock:krug:/tmp/eglot--fixturezQyQYv/project/merdi
vc-state("/mock:krug:/tmp/eglot--fixturezQyQYv/project/merdi..." RCS
vc-rcs-find-file-hook()
apply(vc-rcs-find-file-hook nil)
vc-call-backend(RCS find-file-hook)
vc-refresh-state()
run-hooks(find-file-hook)
after-find-file(nil t)
find-file-noselect-1(#<killed buffer> "/mock:krug:/tmp/eglot--fixtur
find-file-noselect("project/merdix.c")
eglot--find-file-noselect("project/merdix.c")
(set-buffer (eglot--find-file-noselect "project/merdix.c"))
(save-current-buffer (set-buffer (eglot--find-file-noselect "project
(closure ((server . #<eglot-lsp-server eglot-lsp-server-15755b2a4d56
funcall((closure ((server . #<eglot-lsp-server eglot-lsp-server-1575
(prog1 (funcall fn) (setq test-body-successful-p t))
(let* ((process-environment (append (list "XDG_CONFIG_HOME=/dev/null
(unwind-protect (let* ((process-environment (append (list "XDG_CONFI
(let* ((fixture-directory (make-nearby-temp-file "eglot--fixture" t)
eglot--call-with-fixture((("project" ("coiso.c" . "bla") ("merdix.c"
(let (server) (eglot--call-with-fixture '(("project" ("coiso.c" . "b
eglot-tests--auto-detect-running-server-1()
funcall(eglot-tests--auto-detect-running-server-1)
(let* ((tramp-remote-path (cons 'tramp-own-remote-path tramp-remote-
eglot--call-with-tramp-test(eglot-tests--auto-detect-running-server-
(closure (tramp-histfile-override company-candidates typescript-mode
ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
ert-run-test(#s(ert-test :name eglot-test-tramp-test :documentation
ert-run-or-rerun-test(#s(ert--stats :selector "tramp" :tests ... :te
ert-run-tests("tramp" #f(compiled-function (event-type &rest event-a
ert-run-tests-batch("tramp")
ert-run-tests-batch-and-exit("tramp")
eval((ert-run-tests-batch-and-exit '"tramp") t)
command-line-1(("-L" ":." "-l" "ert" "-l" "lisp/progmodes/eglot-test
command-line()
normal-top-level()
Test eglot-test-tramp-test condition:
(wrong-type-argument "listp /tmp/tramp.G0HtZ3")
FAILED 1/2 eglot-test-tramp-test (0.744733 sec) at lisp/progmodes/eglot-tests.el:1285
[eglot-tests] [eglot-test-tramp-test-2]: test start
[eglot] Connected! Server `clangd' now managing `(c-mode)' buffers in project `project'.
[eglot-tests] waiting for `(should (equal 71 (cadddr (cadadr (aref (cadddr params) 0)))))'
[eglot-tests] detected: textDocument/didChange
[eglot-tests] [eglot-test-tramp-test-2]: OK
[eglot] Asking EGLOT (project/(c-mode)) politely to terminate
[jsonrpc] Server exited with status 9
[eglot-tests] Killing (foo.c), wiping /mock:krug:/tmp/eglot--fixtureQ3JgHX, restoring nil
File is missing: /mock:krug:/tmp/eglot--fixtureQ3JgHX/project/foo.c~
passed 2/2 eglot-test-tramp-test-2 (0.618120 sec)
Ran 2 tests, 1 results as expected, 1 unexpected (2023-03-14 23:04:36+0000, 1.444125 sec)
1 unexpected results:
FAILED eglot-test-tramp-test
make[1]: *** [Makefile:174: lisp/progmodes/eglot-tests.log] Error 1
make[1]: Leaving directory '/home/capitaomorte/Source/Emacs/emacs-master/test'
make: *** [Makefile:240: lisp/progmodes/eglot-tests] Error 2
make: Leaving directory '/home/capitaomorte/Source/Emacs/emacs-master/test'
bug-gnu-emacs <at> gnu.org:bug#62194; Package emacs.
(Wed, 15 Mar 2023 09:41:02 GMT) Full text and rfc822 format available.Message #8 received at 62194 <at> debbugs.gnu.org (full text, mbox):
From: Michael Albinus <michael.albinus <at> gmx.de> To: João Távora <joaotavora <at> gmail.com> Cc: 62194 <at> debbugs.gnu.org Subject: Re: bug#62194: 30.0.50; Two Eglot-over-Tramp tests are failing on master, passing on emacs-29 Date: Wed, 15 Mar 2023 10:40:32 +0100
[Message part 1 (text/plain, inline)]
João Távora <joaotavora <at> gmail.com> writes: > Hi Michael, Hi João, > I've noticd that these two Eglot tests are passing on emacs-29 but > failing on emacs master. I haven't checked if this is due to your > recent work on Tramp or some other factor. > > Sometimes one of the tests passes, but having the two of them pass is > rare. In emacs-29, they always pass. Both emacs-master and emacs-29 > have the version of Eglot _with_ the ControlMaster-neutering workaround. > > To reproduce, you need the "clangd" language server installed (which is > one of the easiest to get language servers, should be available in your > package manager). > > Then: > > make -C test lisp/progmodes/eglot-tests SELECTOR=\"tramp\" FTR, the following woulöd be sufficient: --8<---------------cut here---------------start------------->8--- make -C test eglot-tests ... --8<---------------cut here---------------end--------------->8--- Well, if I call it w/o a SELECTOR, I get 14 failing tests. clangd is installed: --8<---------------cut here---------------start------------->8--- # which clangd /var/lib/snapd/snap/bin/clangd --8<---------------cut here---------------end--------------->8--- This might not be the canonical path, but at least the local tests shall find it. I'm running Fedora 37. Could you, pls, check? The log file is appended. > João Best regards, Michael.
[eglot-tests.log (text/plain, attachment)]
bug-gnu-emacs <at> gnu.org:bug#62194; Package emacs.
(Wed, 15 Mar 2023 11:46:01 GMT) Full text and rfc822 format available.Message #11 received at 62194 <at> debbugs.gnu.org (full text, mbox):
From: Michael Albinus <michael.albinus <at> gmx.de> To: João Távora <joaotavora <at> gmail.com> Cc: 62194 <at> debbugs.gnu.org Subject: Re: bug#62194: 30.0.50; Two Eglot-over-Tramp tests are failing on master, passing on emacs-29 Date: Wed, 15 Mar 2023 12:45:23 +0100
Michael Albinus <michael.albinus <at> gmx.de> writes:
Hi João,
> This might not be the canonical path, but at least the local tests shall
> find it. I'm running Fedora 37. Could you, pls, check? The log file is appended.
In order to force analysis, I have used as remote server another one,
which runs Ubuntu 22.10. clangd is installed there at a canonical
location:
--8<---------------cut here---------------start------------->8---
# which clangd
/usr/bin/clangd
--8<---------------cut here---------------end--------------->8---
And I run the tests from my Fedora laptop like
--8<---------------cut here---------------start------------->8---
env REMOTE_TEMPORARY_FILE_DIRECTORY='/ssh:albinus <at> detlef:/tmp' make -C test eglot-tests.log SELECTOR='"tramp"'
--8<---------------cut here---------------end--------------->8---
Sometimes the tests pass, sometimes not. In the latter case, it is the
same picture as you have reported. But I wanted to reproduce it locally.
The most interesting part is the backtrace:
--8<---------------cut here---------------start------------->8---
Test eglot-test-tramp-test backtrace:
tramp-sh-handle-file-attributes("/ssh:albinus <at> detlef:/tmp/eglot--fix
apply(tramp-sh-handle-file-attributes "/ssh:albinus <at> detlef:/tmp/eglo
tramp-vc-file-name-handler(file-attributes "/ssh:albinus <at> detlef:/tmp
file-attributes("/ssh:albinus <at> detlef:/tmp/eglot--fixture2kRNPt/proj.
tramp-handle-file-symlink-p("/ssh:albinus <at> detlef:/tmp/eglot--fixture
apply(tramp-handle-file-symlink-p "/ssh:albinus <at> detlef:/tmp/eglot--f
tramp-vc-file-name-handler(file-symlink-p "/ssh:albinus <at> detlef:/tmp/
file-symlink-p("/ssh:albinus <at> detlef:/tmp/eglot--fixture2kRNPt/proj..
tramp-sh-handle-file-truename("/ssh:albinus <at> detlef:/tmp/eglot--fixtu
apply(tramp-sh-handle-file-truename "/ssh:albinus <at> detlef:/tmp/eglot-
tramp-vc-file-name-handler(file-truename "/ssh:albinus <at> detlef:/tmp/e
file-truename("/ssh:albinus <at> detlef:/tmp/eglot--fixture2kRNPt/proj...
find-buffer-visiting("/ssh:albinus <at> detlef:/tmp/eglot--fixture2kRNPt/
#f(compiled-function (arg1 arg2 &rest rest) "Handle notification pub
apply(#f(compiled-function (arg1 arg2 &rest rest) "Handle notificati
eglot-handle-notification(#<eglot-lsp-server eglot-lsp-server-9d34aa
apply(eglot-handle-notification #<eglot-lsp-server eglot-lsp-server-
#f(compiled-function (server method params) #<bytecode -0xa39641d878
jsonrpc-connection-receive(#<eglot-lsp-server eglot-lsp-server-9d34a
jsonrpc--process-filter(#<process EGLOT (project/(c-mode c-ts-mode c
accept-process-output(#<process EGLOT (project/(c-mode c-ts-mode c++
tramp-accept-process-output(#<process *tramp/ssh albinus <at> detlef*>)
tramp-wait-for-regexp(#<process *tramp/ssh albinus <at> detlef*> nil "\\(
tramp-wait-for-output(#<process *tramp/ssh albinus <at> detlef*>)
tramp-send-command((tramp-file-name "ssh" "albinus" nil "detlef" nil
tramp-send-command-and-check((tramp-file-name "ssh" "albinus" nil "d
tramp-run-test("-d" "/ssh:albinus <at> detlef:/tmp/eglot--fixture2kRNPt")
tramp-sh-handle-file-directory-p("/ssh:albinus <at> detlef:/tmp/eglot--fi
apply(tramp-sh-handle-file-directory-p "/ssh:albinus <at> detlef:/tmp/egl
tramp-vc-file-name-handler(file-directory-p "/ssh:albinus <at> detlef:/tm
file-directory-p("/ssh:albinus <at> detlef:/tmp/eglot--fixture2kRNPt/")
locate-dominating-file("/ssh:albinus <at> detlef:/tmp/eglot--fixture2kRNP
vc-find-root("/ssh:albinus <at> detlef:/tmp/eglot--fixture2kRNPt/proj..."
vc-svn-registered("/ssh:albinus <at> detlef:/tmp/eglot--fixture2kRNPt/pro
apply(vc-svn-registered "/ssh:albinus <at> detlef:/tmp/eglot--fixture2kRN
vc-call-backend(SVN registered "/ssh:albinus <at> detlef:/tmp/eglot--fixt
#f(compiled-function (b) #<bytecode 0x133be07a00af7442>)(SVN)
mapc(#f(compiled-function (b) #<bytecode 0x133be07a00af7442>) (RCS C
vc-registered("/ssh:albinus <at> detlef:/tmp/eglot--fixture2kRNPt/proj...
apply(vc-registered "/ssh:albinus <at> detlef:/tmp/eglot--fixture2kRNPt/p
tramp-run-real-handler(vc-registered ("/ssh:albinus <at> detlef:/tmp/eglo
tramp-sh-handle-vc-registered("/ssh:albinus <at> detlef:/tmp/eglot--fixtu
apply(tramp-sh-handle-vc-registered "/ssh:albinus <at> detlef:/tmp/eglot-
tramp-sh-file-name-handler(vc-registered "/ssh:albinus <at> detlef:/tmp/e
apply(tramp-sh-file-name-handler vc-registered "/ssh:albinus <at> detlef:
tramp-file-name-handler(vc-registered "/ssh:albinus <at> detlef:/tmp/eglo
vc-registered("/ssh:albinus <at> detlef:/tmp/eglot--fixture2kRNPt/proj...
vc-backend("/ssh:albinus <at> detlef:/tmp/eglot--fixture2kRNPt/proj...")
vc-refresh-state()
run-hooks(find-file-hook)
after-find-file(nil t)
find-file-noselect-1(#<killed buffer> "/ssh:albinus <at> detlef:/tmp/eglo
find-file-noselect("project/merdix.c")
eglot--find-file-noselect("project/merdix.c")
(set-buffer (eglot--find-file-noselect "project/merdix.c"))
(save-current-buffer (set-buffer (eglot--find-file-noselect "project
(closure ((server . #<eglot-lsp-server eglot-lsp-server-9d34aa>)) ni
funcall((closure ((server . #<eglot-lsp-server eglot-lsp-server-9d34
(prog1 (funcall fn) (setq test-body-successful-p t))
(let* ((process-environment (append (list "XDG_CONFIG_HOME=/dev/null
(unwind-protect (let* ((process-environment (append (list "XDG_CONFI
(let* ((fixture-directory (make-nearby-temp-file "eglot--fixture" t)
eglot--call-with-fixture((("project" ("coiso.c" . "bla") ("merdix.c"
(let (server) (eglot--call-with-fixture '(("project" ("coiso.c" . "b
eglot-tests--auto-detect-running-server-1()
funcall(eglot-tests--auto-detect-running-server-1)
(let* ((tramp-remote-path (cons 'tramp-own-remote-path tramp-remote-
eglot--call-with-tramp-test(eglot-tests--auto-detect-running-server-
(closure (tramp-histfile-override company-candidates typescript-mode
ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
ert-run-test(#s(ert-test :name eglot-test-tramp-test :documentation
ert-run-or-rerun-test(#s(ert--stats :selector "tramp" :tests ... :te
ert-run-tests("tramp" #f(compiled-function (event-type &rest event-a
ert-run-tests-batch("tramp")
ert-run-tests-batch-and-exit("tramp")
eval((ert-run-tests-batch-and-exit '"tramp") t)
command-line-1(("-L" ":." "-l" "ert" "-l" "lisp/progmodes/eglot-test
command-line()
normal-top-level()
--8<---------------cut here---------------end--------------->8---
Let's analyze bottom up. The very first time Tramp is involved is here:
--8<---------------cut here---------------start------------->8---
tramp-file-name-handler(vc-registered "/ssh:albinus <at> detlef:/tmp/eglo
vc-registered("/ssh:albinus <at> detlef:/tmp/eglot--fixture2kRNPt/proj...
vc-backend("/ssh:albinus <at> detlef:/tmp/eglot--fixture2kRNPt/proj...")
vc-refresh-state()
run-hooks(find-file-hook)
after-find-file(nil t)
find-file-noselect-1(#<killed buffer> "/ssh:albinus <at> detlef:/tmp/eglo
find-file-noselect("project/merdix.c")
eglot--find-file-noselect("project/merdix.c")
--8<---------------cut here---------------end--------------->8---
That is, file "project/merdix.c" is viewed, which has the absolute path
"/ssh:albinus <at> detlef:/tmp/eglot--fixture2kRNPt/project/merdix.c". It has
been loaded into the buffer successfully, and 'run-hooks(find-file-hook)'
is applied. Since the file is under vc control, 'vc-registerd' is invoked.
Some lines up in the backtrace, we see
--8<---------------cut here---------------start------------->8---
jsonrpc--process-filter(#<process EGLOT (project/(c-mode c-ts-mode c
accept-process-output(#<process EGLOT (project/(c-mode c-ts-mode c++
tramp-accept-process-output(#<process *tramp/ssh albinus <at> detlef*>)
tramp-wait-for-regexp(#<process *tramp/ssh albinus <at> detlef*> nil "\\(
tramp-wait-for-output(#<process *tramp/ssh albinus <at> detlef*>)
tramp-send-command((tramp-file-name "ssh" "albinus" nil "detlef" nil
tramp-send-command-and-check((tramp-file-name "ssh" "albinus" nil "d
tramp-run-test("-d" "/ssh:albinus <at> detlef:/tmp/eglot--fixture2kRNPt")
tramp-sh-handle-file-directory-p("/ssh:albinus <at> detlef:/tmp/eglot--fi
--8<---------------cut here---------------end--------------->8---
Tramp needs to know, whether a remote file is a directory, and it
sends the command "test -d /tmp/eglot--fixture2kRNPt". It waits for the
result in 'tramp-accept-process-output(#<process *tramp/ssh albinus <at> detlef*>)'
However, 'tramp-accept-process-output' reads also the jsonrpc output by
'accept-process-output(#<process EGLOT (project/(c-mode c-ts-mode c++...',
which triggers 'jsonrpc--process-filter'. This results finally, on top of
the backtrace, in another remote Tramp command:
--8<---------------cut here---------------start------------->8---
tramp-sh-handle-file-attributes("/ssh:albinus <at> detlef:/tmp/eglot--fix
apply(tramp-sh-handle-file-attributes "/ssh:albinus <at> detlef:/tmp/eglo
tramp-vc-file-name-handler(file-attributes "/ssh:albinus <at> detlef:/tmp
file-attributes("/ssh:albinus <at> detlef:/tmp/eglot--fixture2kRNPt/proj.
--8<---------------cut here---------------end--------------->8---
It isn't visible in the backtrace, but it is for sure the famous
"Forbidden reentrant call of Tramp" error.
So I reommend the following:
- At least in 'eglot--call-with-tramp-test', better in your Eglot
package, suppress vc related checks, by binding
--8<---------------cut here---------------start------------->8---
(let ((vc-ignore-dir-regexp
(format "\\(%s\\)\\|\\(%s\\)"
vc-ignore-dir-regexp
tramp-file-name-regexp)))
...)
--8<---------------cut here---------------end--------------->8---
- If this works sufficiently, this bug shall be either closed, or merged
with bug#60534, which tracks this error.
Best regards, Michael.
bug-gnu-emacs <at> gnu.org:bug#62194; Package emacs.
(Wed, 15 Mar 2023 20:15:02 GMT) Full text and rfc822 format available.Message #14 received at 62194 <at> debbugs.gnu.org (full text, mbox):
From: João Távora <joaotavora <at> gmail.com> To: Michael Albinus <michael.albinus <at> gmx.de> Cc: 62194 <at> debbugs.gnu.org Subject: Re: bug#62194: 30.0.50; Two Eglot-over-Tramp tests are failing on master, passing on emacs-29 Date: Wed, 15 Mar 2023 20:16:24 +0000
Michael Albinus <michael.albinus <at> gmx.de> writes: > Well, if I call it w/o a SELECTOR, I get 14 failing tests. clangd is > installed: > > # which clangd > /var/lib/snapd/snap/bin/clangd > > This might not be the canonical path, but at least the local tests shall > find it. I'm running Fedora 37. Could you, pls, check? The log file is appended. I cant' reproduce, but I haven't got Fedora. The logs only tell that -- apparently -- the LSP server never replied to Eglot. João
bug-gnu-emacs <at> gnu.org:bug#62194; Package emacs.
(Wed, 15 Mar 2023 20:23:02 GMT) Full text and rfc822 format available.Message #17 received at 62194 <at> debbugs.gnu.org (full text, mbox):
From: João Távora <joaotavora <at> gmail.com> To: Michael Albinus <michael.albinus <at> gmx.de> Cc: 62194 <at> debbugs.gnu.org Subject: Re: bug#62194: 30.0.50; Two Eglot-over-Tramp tests are failing on master, passing on emacs-29 Date: Wed, 15 Mar 2023 20:24:41 +0000
merge 62194 61350
thanks
Michael Albinus <michael.albinus <at> gmx.de> writes:
> So I reommend the following:
>
> - At least in 'eglot--call-with-tramp-test', better in your Eglot
> package, suppress vc related checks, by binding
>
> (let ((vc-ignore-dir-regexp
> (format "\\(%s\\)\\|\\(%s\\)"
> vc-ignore-dir-regexp
> tramp-file-name-regexp)))
> ...)
>
> - If this works sufficiently, this bug shall be either closed, or merged
> with bug#60534, which tracks this error.
I don't think I should be doing something this in eglot-tests.el,
because the tests pass on emacs-29 and this failure was introduced
very recently.
I've just confirmed inequivocally that these failures are direct fallout
from the fix to bug#61350, so I'm merging the two bugs: 62194 and 61350.
As I wrote in bug#61350, if I revert that commit and apply the simpler
patch that we discussed, everything's good, and I can remove the
ControlMaster supression workaround in Eglot, too.
João
diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 47173b95bea..885b29f9471 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -5800,6 +5800,11 @@ tramp-accept-process-output
This is needed in order to hide `last-coding-system-used', which is set
for process communication also.
If the user quits via `C-g', it is propagated up to `tramp-file-name-handler'."
+ (when-let (((process-get proc 'shared-socket))
+ (v (process-get proc 'vector)))
+ (dolist (p (delq proc (process-list)))
+ (when (tramp-file-name-equal-p v (process-get p 'vector))
+ (while (accept-process-output p 0 nil t)))))
(with-current-buffer (process-buffer proc)
(let ((inhibit-read-only t)
last-coding-system-used
bug-gnu-emacs <at> gnu.org:bug#62194; Package emacs.
(Wed, 15 Mar 2023 20:32:02 GMT) Full text and rfc822 format available.Message #20 received at 62194 <at> debbugs.gnu.org (full text, mbox):
From: miha <at> kamnitnik.top To: Michael Albinus <michael.albinus <at> gmx.de>, João Távora <joaotavora <at> gmail.com> Cc: 62194 <at> debbugs.gnu.org Subject: Re: bug#62194: 30.0.50; Two Eglot-over-Tramp tests are failing on master, passing on emacs-29 Date: Wed, 15 Mar 2023 21:36:05 +0100
[Message part 1 (text/plain, inline)]
Michael Albinus <michael.albinus <at> gmx.de> writes:
> That is, file "project/merdix.c" is viewed, which has the absolute path
> "/ssh:albinus <at> detlef:/tmp/eglot--fixture2kRNPt/project/merdix.c". It has
> been loaded into the buffer successfully, and 'run-hooks(find-file-hook)'
> is applied. Since the file is under vc control, 'vc-registerd' is invoked.
> Some lines up in the backtrace, we see
>
> --8<---------------cut here---------------start------------->8---
> jsonrpc--process-filter(#<process EGLOT (project/(c-mode c-ts-mode c
> accept-process-output(#<process EGLOT (project/(c-mode c-ts-mode c++
> tramp-accept-process-output(#<process *tramp/ssh albinus <at> detlef*>)
> tramp-wait-for-regexp(#<process *tramp/ssh albinus <at> detlef*> nil "\\(
> tramp-wait-for-output(#<process *tramp/ssh albinus <at> detlef*>)
> tramp-send-command((tramp-file-name "ssh" "albinus" nil "detlef" nil
> tramp-send-command-and-check((tramp-file-name "ssh" "albinus" nil "d
> tramp-run-test("-d" "/ssh:albinus <at> detlef:/tmp/eglot--fixture2kRNPt")
> tramp-sh-handle-file-directory-p("/ssh:albinus <at> detlef:/tmp/eglot--fi
> --8<---------------cut here---------------end--------------->8---
>
> Tramp needs to know, whether a remote file is a directory, and it
> sends the command "test -d /tmp/eglot--fixture2kRNPt". It waits for the
> result in 'tramp-accept-process-output(#<process *tramp/ssh albinus <at> detlef*>)'
> However, 'tramp-accept-process-output' reads also the jsonrpc output by
> 'accept-process-output(#<process EGLOT (project/(c-mode c-ts-mode c++...',
I guess that this must be due to
(defun tramp-accept-process-output (proc &optional _timeout)
"Like `accept-process-output' for Tramp processes.
This is needed in order to hide `last-coding-system-used', which is set
for process communication also.
If the user quits via `C-g', it is propagated up to `tramp-file-name-handler'."
(declare (advertised-calling-convention (proc) "29.2"))
;; There could be other processes which use the same socket for
;; communication. This could block the output for the current
;; process. Read such output first. (Bug#61350)
(when-let (((process-get proc 'shared-socket))
(v (process-get proc 'vector)))
(dolist (p (delq proc (process-list)))
(when (tramp-file-name-equal-p v (process-get p 'vector))
THIS --> (accept-process-output p 0 nil t))))
I'm somewhat against these additional accept-process-output calls. While
we do limit them to processes from the same ssh connection, I'd argue
that usually, such processes aren't really related to the main tramp ssh
process. They could be a random *shell* process that doesn't really
belong to tramp.el any more IMO. Or in this case, eglot's jsonrpc
process.
On a bright note, I've been playing around with the original Eglot
freeze bug#61350. It seems that, given two ControlMaster ssh processes
where one refuses to produce output due to the other filling up the
shared ssh connection, we can unblock the blocked process by sending it
SIGWINCH.
Here is a POC tramp-accept-process-output implementation that does this,
and it seems to fix the freeze bug:
(defun tramp-accept-process-output (proc &optional _timeout)
"Like `accept-process-output' for Tramp processes.
This is needed in order to hide `last-coding-system-used', which is set
for process communication also.
If the user quits via `C-g', it is propagated up to `tramp-file-name-handler'."
(declare (advertised-calling-convention (proc) "29.2"))
(with-current-buffer (process-buffer proc)
(let ((inhibit-read-only t)
last-coding-system-used
result)
;; This must be protected by the "locked" property.
(with-tramp-locked-connection proc
;; JUST-THIS-ONE is set due to Bug#12145. `with-local-quit'
;; returns t in order to report success.
(if (with-local-quit
(setq result (accept-process-output proc 0 nil t))
(when (and (not result) (process-get proc 'shared-socket))
;; bug#62194
(tramp-message
proc 10 "%s: Trying to unblock process with SIGWINCH" proc)
(internal-default-signal-process proc 'sigwinch))
t)
(tramp-message
proc 10 "%s %s %s\n%s"
proc (process-status proc) result (buffer-string))
;; Propagate quit.
(keyboard-quit)))
result)))
Best regards.
[signature.asc (application/pgp-signature, inline)]
bug-gnu-emacs <at> gnu.org:bug#62194; Package emacs.
(Wed, 15 Mar 2023 20:45:02 GMT) Full text and rfc822 format available.Message #23 received at 62194 <at> debbugs.gnu.org (full text, mbox):
From: João Távora <joaotavora <at> gmail.com> To: miha <at> kamnitnik.top Cc: Michael Albinus <michael.albinus <at> gmx.de>, 62194 <at> debbugs.gnu.org Subject: Re: bug#62194: 30.0.50; Two Eglot-over-Tramp tests are failing on master, passing on emacs-29 Date: Wed, 15 Mar 2023 20:45:47 +0000
On Wed, Mar 15, 2023 at 8:31 PM <miha <at> kamnitnik.top> wrote: > > (dolist (p (delq proc (process-list))) > (when (tramp-file-name-equal-p v (process-get p 'vector)) > THIS --> (accept-process-output p 0 nil t)))) > > I'm somewhat against these additional accept-process-output calls. While > we do limit them to processes from the same ssh connection, I'd argue > that usually, such processes aren't really related to the main tramp ssh > process. They could be a random *shell* process that doesn't really > belong to tramp.el any more IMO. Or in this case, eglot's jsonrpc > process. I'm "against" this too, but hard facts here trump arguments and opinion. This is exactly what fixes the ControlMaster hang, that's well understood, and is really intended. It brings about some other failures but I've just confirmed that if you put (while (accept-process-output p 0 nil t)) then all's well. Of course, it's only _one_ of the ways to fix the problem. If you have another more elegant one, but I don't know if signals is a great one. I'd guess SIGWINCH to be somewhat limited in platform support, right? João
bug-gnu-emacs <at> gnu.org:bug#62194; Package emacs.
(Thu, 16 Mar 2023 12:03:02 GMT) Full text and rfc822 format available.Message #26 received at 62194 <at> debbugs.gnu.org (full text, mbox):
From: Michael Albinus <michael.albinus <at> gmx.de> To: João Távora <joaotavora <at> gmail.com> Cc: 62194 <at> debbugs.gnu.org, miha <at> kamnitnik.top Subject: Re: bug#62194: 30.0.50; Two Eglot-over-Tramp tests are failing on master, passing on emacs-29 Date: Thu, 16 Mar 2023 13:02:01 +0100
João Távora <joaotavora <at> gmail.com> writes: Hi, > I'm "against" this too, but hard facts here trump arguments and opinion. > This is exactly what fixes the ControlMaster hang, that's well understood, > and is really intended. It brings about some other failures but I've just > confirmed that if you put > > (while (accept-process-output p 0 nil t)) > > then all's well. > > Of course, it's only _one_ of the ways to fix the problem. If you have another > more elegant one, but I don't know if signals is a great one. I'd guess > SIGWINCH to be somewhat limited in platform support, right? I'm also against using SIGWINCH at this point. It isn't portable, and it isn't even known whether it is needed always. The proposed patch wraps it by --8<---------------cut here---------------start------------->8--- (when (and (not result) (process-get proc 'shared-socket)) --8<---------------cut here---------------end--------------->8--- But the reason why result is nil could be simply the case that the remote side hasn't something to tell yet, and we need to wait. And this would complicate the scenario further. We have already a very fragile tramp-accept-process-output, which has changed again and again ove the last 20 years. I *really* would like to see a simple and robust implementation. My attempt with the additional process property `shared-socket' has failed, and I've reverted it. Pushed to master. The Eglot Tramp tests pass now, so I recommend to close this bug. > João Best regards, Michael.
bug-gnu-emacs <at> gnu.org:bug#62194; Package emacs.
(Thu, 16 Mar 2023 12:19:01 GMT) Full text and rfc822 format available.Message #29 received at 62194 <at> debbugs.gnu.org (full text, mbox):
From: João Távora <joaotavora <at> gmail.com> To: Michael Albinus <michael.albinus <at> gmx.de> Cc: 62194 <at> debbugs.gnu.org, miha <at> kamnitnik.top Subject: Re: bug#62194: 30.0.50; Two Eglot-over-Tramp tests are failing on master, passing on emacs-29 Date: Thu, 16 Mar 2023 12:20:22 +0000
Michael Albinus <michael.albinus <at> gmx.de> writes: > João Távora <joaotavora <at> gmail.com> writes: > > Hi, > >> I'm "against" this too, but hard facts here trump arguments and opinion. >> This is exactly what fixes the ControlMaster hang, that's well understood, >> and is really intended. It brings about some other failures but I've just >> confirmed that if you put >> >> (while (accept-process-output p 0 nil t)) >> >> then all's well. >> >> Of course, it's only _one_ of the ways to fix the problem. If you have another >> more elegant one, but I don't know if signals is a great one. I'd guess >> SIGWINCH to be somewhat limited in platform support, right? > > I'm also against using SIGWINCH at this point. It isn't portable, and it > isn't even known whether it is needed always. The proposed patch wraps > it by > > (when (and (not result) (process-get proc 'shared-socket)) > > But the reason why result is nil could be simply the case that the > remote side hasn't something to tell yet, and we need to wait. > > And this would complicate the scenario further. We have already a very > fragile tramp-accept-process-output, which has changed again and again > ove the last 20 years. I *really* would like to see a simple and robust > implementation. > > My attempt with the additional process property `shared-socket' has > failed, and I've reverted it. > > Pushed to master. The Eglot Tramp tests pass now, so I recommend to > close this bug. Michael, now you've brought back the Eglot/Tramp hang of bug#61350! Really Michael, please consider reverting 0330cff65ae (your latest) and 54ef338ba36 (from two days ago), and going back to the simpler version that you originally proposed, with just the 'while' added there. It fixes all this mess. _Then_ we can go get to thinking about the "simple and robust" implementation. João
bug-gnu-emacs <at> gnu.org:bug#62194; Package emacs.
(Thu, 16 Mar 2023 14:58:02 GMT) Full text and rfc822 format available.Message #32 received at 62194 <at> debbugs.gnu.org (full text, mbox):
From: Michael Albinus <michael.albinus <at> gmx.de> To: João Távora <joaotavora <at> gmail.com> Cc: 62194 <at> debbugs.gnu.org, miha <at> kamnitnik.top Subject: Re: bug#62194: 30.0.50; Two Eglot-over-Tramp tests are failing on master, passing on emacs-29 Date: Thu, 16 Mar 2023 15:57:17 +0100
João Távora <joaotavora <at> gmail.com> writes: Hi João, > Michael, now you've brought back the Eglot/Tramp hang of bug#61350! > > Really Michael, please consider reverting 0330cff65ae (your latest) and > 54ef338ba36 (from two days ago), and going back to the simpler version > that you originally proposed, with just the 'while' added there. Honestly, I'm lost with all the patches back forth and back. Could you pls show me the change you propose, based on the current master state? I don't believe we need to revert the timeout change I have applied, because in fact any tramp-accept-process-output shall use a timeout of 0; otherwise it could block. > It fixes all this mess. _Then_ we can go get to thinking about the > "simple and robust" implementation. Yes. Although I have no clue yet what shall be done. > João Best regards, Michael.
bug-gnu-emacs <at> gnu.org:bug#62194; Package emacs.
(Thu, 16 Mar 2023 15:03:02 GMT) Full text and rfc822 format available.Message #35 received at 62194 <at> debbugs.gnu.org (full text, mbox):
From: Michael Albinus <michael.albinus <at> gmx.de> To: João Távora <joaotavora <at> gmail.com> Cc: 62194 <at> debbugs.gnu.org Subject: Re: bug#62194: 30.0.50; Two Eglot-over-Tramp tests are failing on master, passing on emacs-29 Date: Thu, 16 Mar 2023 16:02:36 +0100
João Távora <joaotavora <at> gmail.com> writes: Hi João, >> Well, if I call it w/o a SELECTOR, I get 14 failing tests. clangd is >> installed: >> >> # which clangd >> /var/lib/snapd/snap/bin/clangd >> >> This might not be the canonical path, but at least the local tests shall >> find it. I'm running Fedora 37. Could you, pls, check? The log file is appended. > > I cant' reproduce, but I haven't got Fedora. The logs only tell that > -- apparently -- the LSP server never replied to Eglot. Yes. I don't remember when and from where I installed the snap package. But this clangd looks strange; it even doesn't respond to "clangd --version". Further search has told me, that Fedora keeps clangd in the package clang-tools-extra. After installing this package, all Eglot tests pass as expected. > João Best regards, Michael.
bug-gnu-emacs <at> gnu.org:bug#62194; Package emacs.
(Thu, 16 Mar 2023 15:11:02 GMT) Full text and rfc822 format available.Message #38 received at 62194 <at> debbugs.gnu.org (full text, mbox):
From: João Távora <joaotavora <at> gmail.com> To: Michael Albinus <michael.albinus <at> gmx.de> Cc: 62194 <at> debbugs.gnu.org, miha <at> kamnitnik.top Subject: Re: bug#62194: 30.0.50; Two Eglot-over-Tramp tests are failing on master, passing on emacs-29 Date: Thu, 16 Mar 2023 15:12:29 +0000
[Message part 1 (text/plain, inline)]
Michael Albinus <michael.albinus <at> gmx.de> writes: > João Távora <joaotavora <at> gmail.com> writes: > > Hi João, > >> Michael, now you've brought back the Eglot/Tramp hang of bug#61350! >> >> Really Michael, please consider reverting 0330cff65ae (your latest) and >> 54ef338ba36 (from two days ago), and going back to the simpler version >> that you originally proposed, with just the 'while' added there. > > Honestly, I'm lost with all the patches back forth and back. Could you > pls show me the change you propose, based on the current master state? OK. Based on the current master state, I'm sending you 4 (four patches). I known it looks a lot, but is quite simple. '0001-Revert-Michael-Albinus-Fix-regression-in-Tramp-bug-6.patch' Reverts your latest commit 0330cff65ae, where some settings of 'shared-socket' are commented out. '0002-Revert-Michael-Albinus-Improve-Tramp-processes-to-ac.patch' Reverts 54ef338ba36 from two days ago, for all purposes restoring the state of Tramp to what is in emacs-29. '0003-A-simpler-fix-for-bug-61350-a-small-tweak-Michael-s-.patch' This is the simpler patch that adds the (while (accept-process-output)). It is directly based on your first idea. It keeps the timeout. It is the most conservative patch. '0004-Just-for-testing-Remove-Tramp-specific-workaround-in.patch' This removes the Tramp-specific Eglot workaround. It's good for testing the original hang reporting in bug#61350 After you have applied these four patches, the following should happen: * Tramp tests should have the same pass rate in master as in emacs-29 * bug#61350 should be fixed -- even without the Eglot workaround. Let me know if you need help testing this (I have a nifty dockerfile and some command-line incantations that make testing very easy). João
[0001-Revert-Michael-Albinus-Fix-regression-in-Tramp-bug-6.patch (text/x-patch, inline)]
From a168ef267aadf2d83627806c18853a7aaf1e88ca Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <joaotavora <at> gmail.com>
Date: Thu, 16 Mar 2023 12:24:29 +0000
Subject: [PATCH 1/4] Revert Michael Albinus' "Fix regression in Tramp
(bug#62194)"
This reverts commit 0330cff65ae837e93ae4d059acf643734d16386d.
---
lisp/net/tramp-sh.el | 6 +++---
lisp/net/tramp.el | 3 +--
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/lisp/net/tramp-sh.el b/lisp/net/tramp-sh.el
index 2f990af334d..882b79b3ee7 100644
--- a/lisp/net/tramp-sh.el
+++ b/lisp/net/tramp-sh.el
@@ -2427,7 +2427,7 @@ tramp-do-copy-or-rename-file-out-of-band
;; This is neded for ssh or PuTTY based processes, and
;; only if the respective options are set. Perhaps,
;; the setting could be more fine-grained.
- ;; (process-put p 'shared-socket t)
+ (process-put p 'shared-socket t)
(process-put p 'adjust-window-size-function #'ignore)
(set-process-query-on-exit-flag p nil)
@@ -3760,7 +3760,7 @@ tramp-sh-handle-file-notify-add-watch
;; This is neded for ssh or PuTTY based processes, and only if
;; the respective options are set. Perhaps, the setting could
;; be more fine-grained.
- ;; (process-put p 'shared-socket t)
+ (process-put p 'shared-socket t)
;; Needed for process filter.
(process-put p 'events events)
(process-put p 'watch-name localname)
@@ -5124,7 +5124,7 @@ tramp-maybe-open-connection
;; This is neded for ssh or PuTTY based processes, and
;; only if the respective options are set. Perhaps,
;; the setting could be more fine-grained.
- ;; (process-put p 'shared-socket t)
+ (process-put p 'shared-socket t)
(process-put p 'adjust-window-size-function #'ignore)
(set-process-query-on-exit-flag p nil)
(setq tramp-current-connection (cons vec (current-time)))
diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 0c8f8acc07d..b6e985db6b1 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -5091,7 +5091,7 @@ tramp-handle-make-process
;; This is neded for ssh or PuTTY based processes, and
;; only if the respective options are set. Perhaps, the
;; setting could be more fine-grained.
- ;; (process-put p 'shared-socket t)
+ (process-put p 'shared-socket t)
(process-put p 'remote-command orig-command)
(tramp-set-connection-property p "remote-command" orig-command)
@@ -5809,7 +5809,6 @@ tramp-accept-process-output
;; There could be other processes which use the same socket for
;; communication. This could block the output for the current
;; process. Read such output first. (Bug#61350)
- ;; The process property isn't set anymore due to Bug#62194.
(when-let (((process-get proc 'shared-socket))
(v (process-get proc 'vector)))
(dolist (p (delq proc (process-list)))
--
2.39.2
[0002-Revert-Michael-Albinus-Improve-Tramp-processes-to-ac.patch (text/x-patch, inline)]
From 14e37318dd5f170771fb1a74d69e89d5e756d1cd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <joaotavora <at> gmail.com>
Date: Thu, 16 Mar 2023 12:26:04 +0000
Subject: [PATCH 2/4] Revert Michael Albinus' "Improve Tramp processes to
accept..."
It was found to cause the failure of test
file-notify-test04-autorevert-remote, among other instabilities.
This reverts commit 54ef338ba3670415cf47fabc33a92d4904707c7e.
---
lisp/net/tramp-adb.el | 2 +-
lisp/net/tramp-gvfs.el | 2 +-
lisp/net/tramp-sh.el | 14 +-------------
lisp/net/tramp-smb.el | 6 +++---
lisp/net/tramp-sudoedit.el | 2 +-
lisp/net/tramp.el | 33 +++++++++------------------------
6 files changed, 16 insertions(+), 43 deletions(-)
diff --git a/lisp/net/tramp-adb.el b/lisp/net/tramp-adb.el
index d338201ab72..64f45e7958d 100644
--- a/lisp/net/tramp-adb.el
+++ b/lisp/net/tramp-adb.el
@@ -990,7 +990,7 @@ tramp-adb-handle-make-process
(progn
(goto-char (point-min))
(not (search-forward "\n" nil t)))
- (tramp-accept-process-output p))
+ (tramp-accept-process-output p 0))
(delete-region (point-min) (point)))
;; Provide error buffer. This shows only
;; initial error messages; messages
diff --git a/lisp/net/tramp-gvfs.el b/lisp/net/tramp-gvfs.el
index c1ad37de1d2..266724c587f 100644
--- a/lisp/net/tramp-gvfs.el
+++ b/lisp/net/tramp-gvfs.el
@@ -1469,7 +1469,7 @@ tramp-gvfs-handle-file-notify-add-watch
(set-process-sentinel p #'tramp-file-notify-process-sentinel)
;; There might be an error if the monitor is not supported.
;; Give the filter a chance to read the output.
- (while (tramp-accept-process-output p))
+ (while (tramp-accept-process-output p 0))
(unless (process-live-p p)
(tramp-error
p 'file-notify-error "Monitoring not supported for `%s'" file-name))
diff --git a/lisp/net/tramp-sh.el b/lisp/net/tramp-sh.el
index 882b79b3ee7..48ebfff6cfe 100644
--- a/lisp/net/tramp-sh.el
+++ b/lisp/net/tramp-sh.el
@@ -2424,10 +2424,6 @@ tramp-do-copy-or-rename-file-out-of-band
copy-program copy-args)))
(tramp-message v 6 "%s" (string-join (process-command p) " "))
(process-put p 'vector v)
- ;; This is neded for ssh or PuTTY based processes, and
- ;; only if the respective options are set. Perhaps,
- ;; the setting could be more fine-grained.
- (process-put p 'shared-socket t)
(process-put p 'adjust-window-size-function #'ignore)
(set-process-query-on-exit-flag p nil)
@@ -3757,10 +3753,6 @@ tramp-sh-handle-file-notify-add-watch
(string-join sequence " "))
(tramp-message v 6 "Run `%s', %S" (string-join sequence " ") p)
(process-put p 'vector v)
- ;; This is neded for ssh or PuTTY based processes, and only if
- ;; the respective options are set. Perhaps, the setting could
- ;; be more fine-grained.
- (process-put p 'shared-socket t)
;; Needed for process filter.
(process-put p 'events events)
(process-put p 'watch-name localname)
@@ -3769,7 +3761,7 @@ tramp-sh-handle-file-notify-add-watch
(set-process-sentinel p #'tramp-file-notify-process-sentinel)
;; There might be an error if the monitor is not supported.
;; Give the filter a chance to read the output.
- (while (tramp-accept-process-output p))
+ (while (tramp-accept-process-output p 0))
(unless (process-live-p p)
(tramp-error
p 'file-notify-error "Monitoring not supported for `%s'" file-name))
@@ -5121,10 +5113,6 @@ tramp-maybe-open-connection
;; Set sentinel and query flag. Initialize variables.
(set-process-sentinel p #'tramp-process-sentinel)
(process-put p 'vector vec)
- ;; This is neded for ssh or PuTTY based processes, and
- ;; only if the respective options are set. Perhaps,
- ;; the setting could be more fine-grained.
- (process-put p 'shared-socket t)
(process-put p 'adjust-window-size-function #'ignore)
(set-process-query-on-exit-flag p nil)
(setq tramp-current-connection (cons vec (current-time)))
diff --git a/lisp/net/tramp-smb.el b/lisp/net/tramp-smb.el
index bb4ab9e3057..1aa4520eeb6 100644
--- a/lisp/net/tramp-smb.el
+++ b/lisp/net/tramp-smb.el
@@ -757,7 +757,7 @@ tramp-smb-action-get-acl
"Read ACL data from connection buffer."
(unless (process-live-p proc)
;; Accept pending output.
- (while (tramp-accept-process-output proc))
+ (while (tramp-accept-process-output proc 0))
(with-current-buffer (tramp-get-connection-buffer vec)
;; There might be a hidden password prompt.
(widen)
@@ -1363,7 +1363,7 @@ tramp-smb-action-set-acl
"Set ACL data."
(unless (process-live-p proc)
;; Accept pending output.
- (while (tramp-accept-process-output proc))
+ (while (tramp-accept-process-output proc 0))
(tramp-message
vec 10 "\n%s" (tramp-get-buffer-string (tramp-get-connection-buffer vec)))
(throw 'tramp-action 'ok)))
@@ -2023,7 +2023,7 @@ tramp-smb-wait-for-output
;; Read pending output.
(while (not (re-search-forward tramp-smb-prompt nil t))
- (while (tramp-accept-process-output p))
+ (while (tramp-accept-process-output p 0))
(goto-char (point-min)))
(tramp-message vec 6 "\n%s" (buffer-string))
diff --git a/lisp/net/tramp-sudoedit.el b/lisp/net/tramp-sudoedit.el
index 3cacde2468c..abb9afc570b 100644
--- a/lisp/net/tramp-sudoedit.el
+++ b/lisp/net/tramp-sudoedit.el
@@ -692,7 +692,7 @@ tramp-sudoedit-action-sudo
"Check, whether a sudo process has finished. Remove unneeded output."
;; There might be pending output for the exit status.
(unless (process-live-p proc)
- (while (tramp-accept-process-output proc))
+ (while (tramp-accept-process-output proc 0))
;; Delete narrowed region, it would be in the way reading a Lisp form.
(goto-char (point-min))
(widen)
diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index b6e985db6b1..47173b95bea 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -5087,11 +5087,6 @@ tramp-handle-make-process
;; t. See Bug#51177.
(when filter
(set-process-filter p filter))
- (process-put p 'vector v)
- ;; This is neded for ssh or PuTTY based processes, and
- ;; only if the respective options are set. Perhaps, the
- ;; setting could be more fine-grained.
- (process-put p 'shared-socket t)
(process-put p 'remote-command orig-command)
(tramp-set-connection-property p "remote-command" orig-command)
@@ -5494,7 +5489,7 @@ tramp-handle-file-notify-rm-watch
;; There might be pending output. Avoid problems with reentrant
;; call of Tramp.
(ignore-errors
- (while (tramp-accept-process-output proc)))
+ (while (tramp-accept-process-output proc 0)))
(tramp-message proc 6 "Kill %S" proc)
(delete-process proc))
@@ -5646,13 +5641,13 @@ tramp-action-process-alive
"Check, whether a process has finished."
(unless (process-live-p proc)
;; There might be pending output.
- (while (tramp-accept-process-output proc))
+ (while (tramp-accept-process-output proc 0))
(throw 'tramp-action 'process-died)))
(defun tramp-action-out-of-band (proc vec)
"Check, whether an out-of-band copy has finished."
;; There might be pending output for the exit status.
- (while (tramp-accept-process-output proc))
+ (while (tramp-accept-process-output proc 0))
(cond ((and (not (process-live-p proc))
(zerop (process-exit-status proc)))
(tramp-message vec 3 "Process has finished.")
@@ -5683,7 +5678,7 @@ tramp-process-one-action
(while (not found)
;; Reread output once all actions have been performed.
;; Obviously, the output was not complete.
- (while (tramp-accept-process-output proc))
+ (while (tramp-accept-process-output proc 0))
(setq todo actions)
(while todo
(setq item (pop todo)
@@ -5800,21 +5795,11 @@ with-tramp-locked-connection
,@body)
(tramp-flush-connection-property ,proc "locked"))))
-(defun tramp-accept-process-output (proc &optional _timeout)
+(defun tramp-accept-process-output (proc &optional timeout)
"Like `accept-process-output' for Tramp processes.
This is needed in order to hide `last-coding-system-used', which is set
for process communication also.
If the user quits via `C-g', it is propagated up to `tramp-file-name-handler'."
- (declare (advertised-calling-convention (proc) "29.2"))
- ;; There could be other processes which use the same socket for
- ;; communication. This could block the output for the current
- ;; process. Read such output first. (Bug#61350)
- (when-let (((process-get proc 'shared-socket))
- (v (process-get proc 'vector)))
- (dolist (p (delq proc (process-list)))
- (when (tramp-file-name-equal-p v (process-get p 'vector))
- (accept-process-output p 0 nil t))))
-
(with-current-buffer (process-buffer proc)
(let ((inhibit-read-only t)
last-coding-system-used
@@ -5824,10 +5809,10 @@ tramp-accept-process-output
;; JUST-THIS-ONE is set due to Bug#12145. `with-local-quit'
;; returns t in order to report success.
(if (with-local-quit
- (setq result (accept-process-output proc 0 nil t)) t)
+ (setq result (accept-process-output proc timeout nil t)) t)
(tramp-message
- proc 10 "%s %s %s\n%s"
- proc (process-status proc) result (buffer-string))
+ proc 10 "%s %s %s %s\n%s"
+ proc timeout (process-status proc) result (buffer-string))
;; Propagate quit.
(keyboard-quit)))
result)))
@@ -6840,7 +6825,7 @@ tramp-interrupt-process
(tramp-get-remote-null-device (process-get proc 'vector))))
;; Wait, until the process has disappeared. If it doesn't,
;; fall back to the default implementation.
- (while (tramp-accept-process-output proc))
+ (while (tramp-accept-process-output proc 0))
(not (process-live-p proc))))))
(add-hook 'interrupt-process-functions #'tramp-interrupt-process)
--
2.39.2
[0003-A-simpler-fix-for-bug-61350-a-small-tweak-Michael-s-.patch (text/x-patch, inline)]
From 08f32d4026bcd657c78d147e61af467d88e12748 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <joaotavora <at> gmail.com>
Date: Wed, 15 Mar 2023 20:02:43 +0000
Subject: [PATCH 3/4] A simpler fix for bug#61350, a small tweak Michael's
original idea.
* lisp/net/tramp.el (tramp-accept-process-output): Accept process
from related processes.
---
lisp/net/tramp.el | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 47173b95bea..885b29f9471 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -5800,6 +5800,11 @@ tramp-accept-process-output
This is needed in order to hide `last-coding-system-used', which is set
for process communication also.
If the user quits via `C-g', it is propagated up to `tramp-file-name-handler'."
+ (when-let (((process-get proc 'shared-socket))
+ (v (process-get proc 'vector)))
+ (dolist (p (delq proc (process-list)))
+ (when (tramp-file-name-equal-p v (process-get p 'vector))
+ (while (accept-process-output p 0 nil t)))))
(with-current-buffer (process-buffer proc)
(let ((inhibit-read-only t)
last-coding-system-used
--
2.39.2
[0004-Just-for-testing-Remove-Tramp-specific-workaround-in.patch (text/x-patch, inline)]
From 7bb5dc7cfa7ff3d553369270f7cc6cd9868c69e1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <joaotavora <at> gmail.com>
Date: Wed, 15 Mar 2023 19:52:34 +0000
Subject: [PATCH 4/4] Just for testing: Remove Tramp-specific workaround in
Eglot
* lisp/progmodes/eglot.el (eglot--connect): No longer bind tramp vars.
(tramp-ssh-controlmaster-options)
(tramp-use-ssh-controlmaster-options): Remove.
---
lisp/progmodes/eglot.el | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index 6c1b9eafe43..f96f7dd254f 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -130,8 +130,6 @@
(defvar markdown-fontify-code-blocks-natively)
(defvar company-backends)
(defvar company-tooltip-align-annotations)
-(defvar tramp-ssh-controlmaster-options)
-(defvar tramp-use-ssh-controlmaster-options)
;;; User tweakable stuff
@@ -1251,15 +1249,7 @@ eglot--connect
(contact (cl-subseq contact 0 probe)))
`(:process
,(lambda ()
- (let ((default-directory default-directory)
- ;; bug#61350: Tramp turns on a feature
- ;; by default that can't (yet) handle
- ;; very much data so we turn it off
- ;; unconditionally -- just for our
- ;; process.
- (tramp-use-ssh-controlmaster-options t)
- (tramp-ssh-controlmaster-options
- "-o ControlMaster=no -o ControlPath=none"))
+ (let ((default-directory default-directory))
(make-process
:name readable-name
:command (setq server-info (eglot--cmd contact))
--
2.39.2
bug-gnu-emacs <at> gnu.org:bug#62194; Package emacs.
(Thu, 16 Mar 2023 17:36:01 GMT) Full text and rfc822 format available.Message #41 received at 62194 <at> debbugs.gnu.org (full text, mbox):
From: Michael Albinus <michael.albinus <at> gmx.de> To: João Távora <joaotavora <at> gmail.com> Cc: 62194 <at> debbugs.gnu.org, miha <at> kamnitnik.top Subject: Re: bug#62194: 30.0.50; Two Eglot-over-Tramp tests are failing on master, passing on emacs-29 Date: Thu, 16 Mar 2023 18:35:34 +0100
João Távora <joaotavora <at> gmail.com> writes: Hi João, >> Honestly, I'm lost with all the patches back forth and back. Could you >> pls show me the change you propose, based on the current master state? > > OK. Based on the current master state, I'm sending you 4 (four > patches). I known it looks a lot, but is quite simple. I've applied the 4 patches. As expected, eglot-tests, tramp-tests, and filenotify-tests run successfully. > * bug#61350 should be fixed -- even without the Eglot workaround. Let > me know if you need help testing this (I have a nifty dockerfile and > some command-line incantations that make testing very easy). The example given by Thomas blocks, again. So we are back to the beginning, aren't we? > João Best regards, Michael.
bug-gnu-emacs <at> gnu.org:bug#62194; Package emacs.
(Thu, 16 Mar 2023 17:58:02 GMT) Full text and rfc822 format available.Message #44 received at 62194 <at> debbugs.gnu.org (full text, mbox):
From: João Távora <joaotavora <at> gmail.com> To: Michael Albinus <michael.albinus <at> gmx.de> Cc: 62194 <at> debbugs.gnu.org, miha <at> kamnitnik.top Subject: Re: bug#62194: 30.0.50; Two Eglot-over-Tramp tests are failing on master, passing on emacs-29 Date: Thu, 16 Mar 2023 17:59:30 +0000
[Message part 1 (text/plain, inline)]
Michael Albinus <michael.albinus <at> gmx.de> writes:
> João Távora <joaotavora <at> gmail.com> writes:
>
> Hi João,
>
>>> Honestly, I'm lost with all the patches back forth and back. Could you
>>> pls show me the change you propose, based on the current master state?
>>
>> OK. Based on the current master state, I'm sending you 4 (four
>> patches). I known it looks a lot, but is quite simple.
>
> I've applied the 4 patches. As expected, eglot-tests, tramp-tests, and
> filenotify-tests run successfully.
>
>> * bug#61350 should be fixed -- even without the Eglot workaround. Let
>> me know if you need help testing this (I have a nifty dockerfile and
>> some command-line incantations that make testing very easy).
>
> The example given by Thomas blocks, again. So we are back to the
> beginning, aren't we?
Doh! I gave you a wrong patch-0003. It needs a tweak.
You can either apply this one-line diff or re-aplply attached patchset
of 4 over origin/master again.
Then please try again, and sorry for the extra confusion.
diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 885b29f9471..a7406a9d80e 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -5804,7 +5804,7 @@ tramp-accept-process-output
(v (process-get proc 'vector)))
(dolist (p (delq proc (process-list)))
(when (tramp-file-name-equal-p v (process-get p 'vector))
- (while (accept-process-output p 0 nil t)))))
+ (while (accept-process-output p 0)))))
(with-current-buffer (process-buffer proc)
(let ((inhibit-read-only t)
last-coding-system-used
I've tested Thomas's example 5 times in a row, moving around energically, in
the Emacs -Q, exercising multiple Eglot features, etc. 0% failures.
[0001-Revert-Michael-Albinus-Fix-regression-in-Tramp-bug-6.patch (text/x-patch, inline)]
From a168ef267aadf2d83627806c18853a7aaf1e88ca Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <joaotavora <at> gmail.com>
Date: Thu, 16 Mar 2023 12:24:29 +0000
Subject: [PATCH 1/4] Revert Michael Albinus' "Fix regression in Tramp
(bug#62194)"
This reverts commit 0330cff65ae837e93ae4d059acf643734d16386d.
---
lisp/net/tramp-sh.el | 6 +++---
lisp/net/tramp.el | 3 +--
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/lisp/net/tramp-sh.el b/lisp/net/tramp-sh.el
index 2f990af334d..882b79b3ee7 100644
--- a/lisp/net/tramp-sh.el
+++ b/lisp/net/tramp-sh.el
@@ -2427,7 +2427,7 @@ tramp-do-copy-or-rename-file-out-of-band
;; This is neded for ssh or PuTTY based processes, and
;; only if the respective options are set. Perhaps,
;; the setting could be more fine-grained.
- ;; (process-put p 'shared-socket t)
+ (process-put p 'shared-socket t)
(process-put p 'adjust-window-size-function #'ignore)
(set-process-query-on-exit-flag p nil)
@@ -3760,7 +3760,7 @@ tramp-sh-handle-file-notify-add-watch
;; This is neded for ssh or PuTTY based processes, and only if
;; the respective options are set. Perhaps, the setting could
;; be more fine-grained.
- ;; (process-put p 'shared-socket t)
+ (process-put p 'shared-socket t)
;; Needed for process filter.
(process-put p 'events events)
(process-put p 'watch-name localname)
@@ -5124,7 +5124,7 @@ tramp-maybe-open-connection
;; This is neded for ssh or PuTTY based processes, and
;; only if the respective options are set. Perhaps,
;; the setting could be more fine-grained.
- ;; (process-put p 'shared-socket t)
+ (process-put p 'shared-socket t)
(process-put p 'adjust-window-size-function #'ignore)
(set-process-query-on-exit-flag p nil)
(setq tramp-current-connection (cons vec (current-time)))
diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 0c8f8acc07d..b6e985db6b1 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -5091,7 +5091,7 @@ tramp-handle-make-process
;; This is neded for ssh or PuTTY based processes, and
;; only if the respective options are set. Perhaps, the
;; setting could be more fine-grained.
- ;; (process-put p 'shared-socket t)
+ (process-put p 'shared-socket t)
(process-put p 'remote-command orig-command)
(tramp-set-connection-property p "remote-command" orig-command)
@@ -5809,7 +5809,6 @@ tramp-accept-process-output
;; There could be other processes which use the same socket for
;; communication. This could block the output for the current
;; process. Read such output first. (Bug#61350)
- ;; The process property isn't set anymore due to Bug#62194.
(when-let (((process-get proc 'shared-socket))
(v (process-get proc 'vector)))
(dolist (p (delq proc (process-list)))
--
2.39.2
[0002-Revert-Michael-Albinus-Improve-Tramp-processes-to-ac.patch (text/x-patch, inline)]
From 14e37318dd5f170771fb1a74d69e89d5e756d1cd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <joaotavora <at> gmail.com>
Date: Thu, 16 Mar 2023 12:26:04 +0000
Subject: [PATCH 2/4] Revert Michael Albinus' "Improve Tramp processes to
accept..."
It was found to cause the failure of test
file-notify-test04-autorevert-remote, among other instabilities.
This reverts commit 54ef338ba3670415cf47fabc33a92d4904707c7e.
---
lisp/net/tramp-adb.el | 2 +-
lisp/net/tramp-gvfs.el | 2 +-
lisp/net/tramp-sh.el | 14 +-------------
lisp/net/tramp-smb.el | 6 +++---
lisp/net/tramp-sudoedit.el | 2 +-
lisp/net/tramp.el | 33 +++++++++------------------------
6 files changed, 16 insertions(+), 43 deletions(-)
diff --git a/lisp/net/tramp-adb.el b/lisp/net/tramp-adb.el
index d338201ab72..64f45e7958d 100644
--- a/lisp/net/tramp-adb.el
+++ b/lisp/net/tramp-adb.el
@@ -990,7 +990,7 @@ tramp-adb-handle-make-process
(progn
(goto-char (point-min))
(not (search-forward "\n" nil t)))
- (tramp-accept-process-output p))
+ (tramp-accept-process-output p 0))
(delete-region (point-min) (point)))
;; Provide error buffer. This shows only
;; initial error messages; messages
diff --git a/lisp/net/tramp-gvfs.el b/lisp/net/tramp-gvfs.el
index c1ad37de1d2..266724c587f 100644
--- a/lisp/net/tramp-gvfs.el
+++ b/lisp/net/tramp-gvfs.el
@@ -1469,7 +1469,7 @@ tramp-gvfs-handle-file-notify-add-watch
(set-process-sentinel p #'tramp-file-notify-process-sentinel)
;; There might be an error if the monitor is not supported.
;; Give the filter a chance to read the output.
- (while (tramp-accept-process-output p))
+ (while (tramp-accept-process-output p 0))
(unless (process-live-p p)
(tramp-error
p 'file-notify-error "Monitoring not supported for `%s'" file-name))
diff --git a/lisp/net/tramp-sh.el b/lisp/net/tramp-sh.el
index 882b79b3ee7..48ebfff6cfe 100644
--- a/lisp/net/tramp-sh.el
+++ b/lisp/net/tramp-sh.el
@@ -2424,10 +2424,6 @@ tramp-do-copy-or-rename-file-out-of-band
copy-program copy-args)))
(tramp-message v 6 "%s" (string-join (process-command p) " "))
(process-put p 'vector v)
- ;; This is neded for ssh or PuTTY based processes, and
- ;; only if the respective options are set. Perhaps,
- ;; the setting could be more fine-grained.
- (process-put p 'shared-socket t)
(process-put p 'adjust-window-size-function #'ignore)
(set-process-query-on-exit-flag p nil)
@@ -3757,10 +3753,6 @@ tramp-sh-handle-file-notify-add-watch
(string-join sequence " "))
(tramp-message v 6 "Run `%s', %S" (string-join sequence " ") p)
(process-put p 'vector v)
- ;; This is neded for ssh or PuTTY based processes, and only if
- ;; the respective options are set. Perhaps, the setting could
- ;; be more fine-grained.
- (process-put p 'shared-socket t)
;; Needed for process filter.
(process-put p 'events events)
(process-put p 'watch-name localname)
@@ -3769,7 +3761,7 @@ tramp-sh-handle-file-notify-add-watch
(set-process-sentinel p #'tramp-file-notify-process-sentinel)
;; There might be an error if the monitor is not supported.
;; Give the filter a chance to read the output.
- (while (tramp-accept-process-output p))
+ (while (tramp-accept-process-output p 0))
(unless (process-live-p p)
(tramp-error
p 'file-notify-error "Monitoring not supported for `%s'" file-name))
@@ -5121,10 +5113,6 @@ tramp-maybe-open-connection
;; Set sentinel and query flag. Initialize variables.
(set-process-sentinel p #'tramp-process-sentinel)
(process-put p 'vector vec)
- ;; This is neded for ssh or PuTTY based processes, and
- ;; only if the respective options are set. Perhaps,
- ;; the setting could be more fine-grained.
- (process-put p 'shared-socket t)
(process-put p 'adjust-window-size-function #'ignore)
(set-process-query-on-exit-flag p nil)
(setq tramp-current-connection (cons vec (current-time)))
diff --git a/lisp/net/tramp-smb.el b/lisp/net/tramp-smb.el
index bb4ab9e3057..1aa4520eeb6 100644
--- a/lisp/net/tramp-smb.el
+++ b/lisp/net/tramp-smb.el
@@ -757,7 +757,7 @@ tramp-smb-action-get-acl
"Read ACL data from connection buffer."
(unless (process-live-p proc)
;; Accept pending output.
- (while (tramp-accept-process-output proc))
+ (while (tramp-accept-process-output proc 0))
(with-current-buffer (tramp-get-connection-buffer vec)
;; There might be a hidden password prompt.
(widen)
@@ -1363,7 +1363,7 @@ tramp-smb-action-set-acl
"Set ACL data."
(unless (process-live-p proc)
;; Accept pending output.
- (while (tramp-accept-process-output proc))
+ (while (tramp-accept-process-output proc 0))
(tramp-message
vec 10 "\n%s" (tramp-get-buffer-string (tramp-get-connection-buffer vec)))
(throw 'tramp-action 'ok)))
@@ -2023,7 +2023,7 @@ tramp-smb-wait-for-output
;; Read pending output.
(while (not (re-search-forward tramp-smb-prompt nil t))
- (while (tramp-accept-process-output p))
+ (while (tramp-accept-process-output p 0))
(goto-char (point-min)))
(tramp-message vec 6 "\n%s" (buffer-string))
diff --git a/lisp/net/tramp-sudoedit.el b/lisp/net/tramp-sudoedit.el
index 3cacde2468c..abb9afc570b 100644
--- a/lisp/net/tramp-sudoedit.el
+++ b/lisp/net/tramp-sudoedit.el
@@ -692,7 +692,7 @@ tramp-sudoedit-action-sudo
"Check, whether a sudo process has finished. Remove unneeded output."
;; There might be pending output for the exit status.
(unless (process-live-p proc)
- (while (tramp-accept-process-output proc))
+ (while (tramp-accept-process-output proc 0))
;; Delete narrowed region, it would be in the way reading a Lisp form.
(goto-char (point-min))
(widen)
diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index b6e985db6b1..47173b95bea 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -5087,11 +5087,6 @@ tramp-handle-make-process
;; t. See Bug#51177.
(when filter
(set-process-filter p filter))
- (process-put p 'vector v)
- ;; This is neded for ssh or PuTTY based processes, and
- ;; only if the respective options are set. Perhaps, the
- ;; setting could be more fine-grained.
- (process-put p 'shared-socket t)
(process-put p 'remote-command orig-command)
(tramp-set-connection-property p "remote-command" orig-command)
@@ -5494,7 +5489,7 @@ tramp-handle-file-notify-rm-watch
;; There might be pending output. Avoid problems with reentrant
;; call of Tramp.
(ignore-errors
- (while (tramp-accept-process-output proc)))
+ (while (tramp-accept-process-output proc 0)))
(tramp-message proc 6 "Kill %S" proc)
(delete-process proc))
@@ -5646,13 +5641,13 @@ tramp-action-process-alive
"Check, whether a process has finished."
(unless (process-live-p proc)
;; There might be pending output.
- (while (tramp-accept-process-output proc))
+ (while (tramp-accept-process-output proc 0))
(throw 'tramp-action 'process-died)))
(defun tramp-action-out-of-band (proc vec)
"Check, whether an out-of-band copy has finished."
;; There might be pending output for the exit status.
- (while (tramp-accept-process-output proc))
+ (while (tramp-accept-process-output proc 0))
(cond ((and (not (process-live-p proc))
(zerop (process-exit-status proc)))
(tramp-message vec 3 "Process has finished.")
@@ -5683,7 +5678,7 @@ tramp-process-one-action
(while (not found)
;; Reread output once all actions have been performed.
;; Obviously, the output was not complete.
- (while (tramp-accept-process-output proc))
+ (while (tramp-accept-process-output proc 0))
(setq todo actions)
(while todo
(setq item (pop todo)
@@ -5800,21 +5795,11 @@ with-tramp-locked-connection
,@body)
(tramp-flush-connection-property ,proc "locked"))))
-(defun tramp-accept-process-output (proc &optional _timeout)
+(defun tramp-accept-process-output (proc &optional timeout)
"Like `accept-process-output' for Tramp processes.
This is needed in order to hide `last-coding-system-used', which is set
for process communication also.
If the user quits via `C-g', it is propagated up to `tramp-file-name-handler'."
- (declare (advertised-calling-convention (proc) "29.2"))
- ;; There could be other processes which use the same socket for
- ;; communication. This could block the output for the current
- ;; process. Read such output first. (Bug#61350)
- (when-let (((process-get proc 'shared-socket))
- (v (process-get proc 'vector)))
- (dolist (p (delq proc (process-list)))
- (when (tramp-file-name-equal-p v (process-get p 'vector))
- (accept-process-output p 0 nil t))))
-
(with-current-buffer (process-buffer proc)
(let ((inhibit-read-only t)
last-coding-system-used
@@ -5824,10 +5809,10 @@ tramp-accept-process-output
;; JUST-THIS-ONE is set due to Bug#12145. `with-local-quit'
;; returns t in order to report success.
(if (with-local-quit
- (setq result (accept-process-output proc 0 nil t)) t)
+ (setq result (accept-process-output proc timeout nil t)) t)
(tramp-message
- proc 10 "%s %s %s\n%s"
- proc (process-status proc) result (buffer-string))
+ proc 10 "%s %s %s %s\n%s"
+ proc timeout (process-status proc) result (buffer-string))
;; Propagate quit.
(keyboard-quit)))
result)))
@@ -6840,7 +6825,7 @@ tramp-interrupt-process
(tramp-get-remote-null-device (process-get proc 'vector))))
;; Wait, until the process has disappeared. If it doesn't,
;; fall back to the default implementation.
- (while (tramp-accept-process-output proc))
+ (while (tramp-accept-process-output proc 0))
(not (process-live-p proc))))))
(add-hook 'interrupt-process-functions #'tramp-interrupt-process)
--
2.39.2
[0003-A-simpler-fix-for-bug-61350-a-small-tweak-Michael-s-.patch (text/x-patch, inline)]
From ac334523b4a7ba23a5198ad60a97456055ffbfbd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <joaotavora <at> gmail.com>
Date: Wed, 15 Mar 2023 20:02:43 +0000
Subject: [PATCH 3/4] A simpler fix for bug#61350, a small tweak Michael's
original idea.
* lisp/net/tramp.el (tramp-accept-process-output): Accept process
from related processes.
---
lisp/net/tramp.el | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 47173b95bea..a7406a9d80e 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -5800,6 +5800,11 @@ tramp-accept-process-output
This is needed in order to hide `last-coding-system-used', which is set
for process communication also.
If the user quits via `C-g', it is propagated up to `tramp-file-name-handler'."
+ (when-let (((process-get proc 'shared-socket))
+ (v (process-get proc 'vector)))
+ (dolist (p (delq proc (process-list)))
+ (when (tramp-file-name-equal-p v (process-get p 'vector))
+ (while (accept-process-output p 0)))))
(with-current-buffer (process-buffer proc)
(let ((inhibit-read-only t)
last-coding-system-used
--
2.39.2
[0004-Just-for-testing-Remove-Tramp-specific-workaround-in.patch (text/x-patch, inline)]
From 566a4de4bd5c39f35a5fce245bc722ee23590ded Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <joaotavora <at> gmail.com>
Date: Wed, 15 Mar 2023 19:52:34 +0000
Subject: [PATCH 4/4] Just for testing: Remove Tramp-specific workaround in
Eglot
* lisp/progmodes/eglot.el (eglot--connect): No longer bind tramp vars.
(tramp-ssh-controlmaster-options)
(tramp-use-ssh-controlmaster-options): Remove.
---
lisp/progmodes/eglot.el | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index 6c1b9eafe43..f96f7dd254f 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -130,8 +130,6 @@
(defvar markdown-fontify-code-blocks-natively)
(defvar company-backends)
(defvar company-tooltip-align-annotations)
-(defvar tramp-ssh-controlmaster-options)
-(defvar tramp-use-ssh-controlmaster-options)
;;; User tweakable stuff
@@ -1251,15 +1249,7 @@ eglot--connect
(contact (cl-subseq contact 0 probe)))
`(:process
,(lambda ()
- (let ((default-directory default-directory)
- ;; bug#61350: Tramp turns on a feature
- ;; by default that can't (yet) handle
- ;; very much data so we turn it off
- ;; unconditionally -- just for our
- ;; process.
- (tramp-use-ssh-controlmaster-options t)
- (tramp-ssh-controlmaster-options
- "-o ControlMaster=no -o ControlPath=none"))
+ (let ((default-directory default-directory))
(make-process
:name readable-name
:command (setq server-info (eglot--cmd contact))
--
2.39.2
bug-gnu-emacs <at> gnu.org:bug#62194; Package emacs.
(Thu, 16 Mar 2023 21:15:01 GMT) Full text and rfc822 format available.Message #47 received at 62194 <at> debbugs.gnu.org (full text, mbox):
From: <miha <at> kamnitnik.top> To: João Távora <joaotavora <at> gmail.com>, Michael Albinus <michael.albinus <at> gmx.de> Cc: 62194 <at> debbugs.gnu.org Subject: Re: bug#62194: 30.0.50; Two Eglot-over-Tramp tests are failing on master, passing on emacs-29 Date: Thu, 16 Mar 2023 22:18:59 +0100
[Message part 1 (text/plain, inline)]
João Távora <joaotavora <at> gmail.com> writes: > From ac334523b4a7ba23a5198ad60a97456055ffbfbd Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <joaotavora <at> gmail.com> > Date: Wed, 15 Mar 2023 20:02:43 +0000 > Subject: [PATCH 3/4] A simpler fix for bug#61350, a small tweak Michael's > original idea. > > * lisp/net/tramp.el (tramp-accept-process-output): Accept process > from related processes. > --- > lisp/net/tramp.el | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el > index 47173b95bea..a7406a9d80e 100644 > --- a/lisp/net/tramp.el > +++ b/lisp/net/tramp.el > @@ -5800,6 +5800,11 @@ tramp-accept-process-output > This is needed in order to hide `last-coding-system-used', which is set > for process communication also. > If the user quits via `C-g', it is propagated up to `tramp-file-name-handler'." > + (when-let (((process-get proc 'shared-socket)) > + (v (process-get proc 'vector))) > + (dolist (p (delq proc (process-list))) > + (when (tramp-file-name-equal-p v (process-get p 'vector)) > + (while (accept-process-output p 0))))) I think that accept-process-output with JUST-THIS-ONE=nil is dangerous here. We are now allowing 'file-exists-p', 'expand-file-name' and all other functions listed in 'tramp-sh-file-name-handler-alist' to call any timer or process filter, without even documenting this. And even if this behaviour was documented, I don't think it is what Elisp programmers want. It's hard to be 100% sure that calling a simple functions such as 'expand-file-name' will work as expected in presence of arbitrary timers or process filters. Remember that Emacs can have pretty hardcore timers or process filters: In M-x shell, the process filter may call 'read-passwd', entering a recursive edit, in which the user can kill any buffer or even a process. Or with midnight-mode enabled, a timer kills buffers older than 3 days. I consider these "the problem of timing errors that usually plague parallel programming", to quote '(elisp) Output from Processes'. I'm not saying that my proposal with SIGWINCH is flawless and I agree that its unacceptable. I'm just saying that we shouldn't be quite satisfied with this solution yet, though it should be fine for some time. What I was thinking was perhaps to introduce a function called 'accumulate-process-output' which would be similar to 'accept-process-output', except that it would only save process output on the heap without calling any process filters or timers. Subsequent calls to 'accept-process-output', or more accurately, wait_reading_process_output, would then first call the process filters on the previously saved output from the heap. This solution would probably be quite complex to implement, but having thought about this problem for two days, I haven't come to any other idea. If there aren't any objections or better ideas, I could start working on it after I finally receive copyright paperwork. > (with-current-buffer (process-buffer proc) > (let ((inhibit-read-only t) > last-coding-system-used > -- > 2.39.2 >
[signature.asc (application/pgp-signature, inline)]
bug-gnu-emacs <at> gnu.org:bug#62194; Package emacs.
(Thu, 16 Mar 2023 21:56:02 GMT) Full text and rfc822 format available.Message #50 received at 62194 <at> debbugs.gnu.org (full text, mbox):
From: João Távora <joaotavora <at> gmail.com> To: miha <at> kamnitnik.top Cc: Michael Albinus <michael.albinus <at> gmx.de>, 62194 <at> debbugs.gnu.org Subject: Re: bug#62194: 30.0.50; Two Eglot-over-Tramp tests are failing on master, passing on emacs-29 Date: Thu, 16 Mar 2023 21:57:22 +0000
On Thu, Mar 16, 2023 at 9:14 PM <miha <at> kamnitnik.top> wrote: > > João Távora <joaotavora <at> gmail.com> writes: > > + (dolist (p (delq proc (process-list))) > > + (when (tramp-file-name-equal-p v (process-get p 'vector)) > > + (while (accept-process-output p 0))))) > > I think that accept-process-output with JUST-THIS-ONE=nil is dangerous > here. We are now allowing 'file-exists-p', 'expand-file-name' and all > other functions listed in 'tramp-sh-file-name-handler-alist' to call any > timer or process filter, That's exactly what we want. We _don't_ want to stop other output to be inhibited from going into the filter. We want to let other processes slurp their input as often as possible. The reason it hung with JUST-THIS-ONE=t is precisely the inverse reason that it hangs without this hunk (say, in emacs-29). One process (say tprocess) is holding its input and jprocess can't read it because it comes from a shared underlying SSH buffer and that buffer is likely full (until tprocess empties it a little bit). At the moment, it is the best we have, both theoretically and experimentally. > without even documenting this. This is the most normal usage of accept-process-output. Look at all the Emacs code base. Basically, only Tramp uses JUST-THIS-ONE. It's JUST-THIS-ONE who is the aberration. None of the process-heavy SLIME, SLY, CIDER IDEs use it. You can read the manual about JUST-THIS-ONE. It's in A-O-P. > filter may call 'read-passwd', entering a recursive edit, in which the > user can kill any buffer or even a process. Or with midnight-mode > enabled, a timer kills buffers older than 3 days. I consider these "the > problem of timing errors that usually plague parallel programming", to > quote '(elisp) Output from Processes'. Sorry, I don't immediately see the problem. I can't prove that there isn't one :-) that's near impossible. Normally what is done is for the problem to be demonstrated with some experiment. > I'm not saying that my proposal with SIGWINCH is flawless and I agree > that its unacceptable. I'm just saying that we shouldn't be quite > satisfied with this solution yet, though it should be fine for some > time. I'm not satisfied with it myself. This is very slow (not that it matters much). But is also all around ugly. The more correct solution is IMHO, described in bug#61350. But it will take time to see if it fits into Tramp (I think it does) but most importantly how (that's the hard bit, Tramp is a lot of code) > What I was thinking was perhaps to introduce a function called > 'accumulate-process-output' which would be similar to > 'accept-process-output', except that it would only save process output You can propose this in emacs-devel, but if you permit me to be slightly immodest, I've been coding with accept-process-output + filters for almost 15 years and I've never had the need for this. I normally want timers and other processes to run, I don't want to stop the world. João
bug-gnu-emacs <at> gnu.org:bug#62194; Package emacs.
(Thu, 16 Mar 2023 23:38:02 GMT) Full text and rfc822 format available.Message #53 received at 62194 <at> debbugs.gnu.org (full text, mbox):
From: João Távora <joaotavora <at> gmail.com> To: miha <at> kamnitnik.top Cc: Michael Albinus <michael.albinus <at> gmx.de>, 62194 <at> debbugs.gnu.org Subject: Re: bug#62194: 30.0.50; Two Eglot-over-Tramp tests are failing on master, passing on emacs-29 Date: Thu, 16 Mar 2023 23:38:49 +0000
On Thu, Mar 16, 2023 at 9:57 PM João Távora <joaotavora <at> gmail.com> wrote: > > On Thu, Mar 16, 2023 at 9:14 PM <miha <at> kamnitnik.top> wrote: > > > > João Távora <joaotavora <at> gmail.com> writes: > > > > + (dolist (p (delq proc (process-list))) > > > + (when (tramp-file-name-equal-p v (process-get p 'vector)) > > > + (while (accept-process-output p 0))))) > > > > I think that accept-process-output with JUST-THIS-ONE=nil is dangerous > > here. We are now allowing 'file-exists-p', 'expand-file-name' and all > > other functions listed in 'tramp-sh-file-name-handler-alist' to call any > > timer or process filter, > > That's exactly what we want. We _don't_ want to stop other > output to be inhibited from going into the filter. Doh, this is a silly double negative. I meant "we don't want to inhibit other output from going into the filters".
bug-gnu-emacs <at> gnu.org:bug#62194; Package emacs.
(Fri, 17 Mar 2023 10:45:02 GMT) Full text and rfc822 format available.Message #56 received at 62194 <at> debbugs.gnu.org (full text, mbox):
From: Michael Albinus <michael.albinus <at> gmx.de> To: João Távora <joaotavora <at> gmail.com> Cc: 62194 <at> debbugs.gnu.org, miha <at> kamnitnik.top Subject: Re: bug#62194: 30.0.50; Two Eglot-over-Tramp tests are failing on master, passing on emacs-29 Date: Fri, 17 Mar 2023 11:44:33 +0100
João Távora <joaotavora <at> gmail.com> writes:
Hi João,
>> The example given by Thomas blocks, again. So we are back to the
>> beginning, aren't we?
>
> Doh! I gave you a wrong patch-0003. It needs a tweak.
>
> You can either apply this one-line diff or re-aplply attached patchset
> of 4 over origin/master again.
>
> Then please try again, and sorry for the extra confusion.
>
> diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
> index 885b29f9471..a7406a9d80e 100644
> --- a/lisp/net/tramp.el
> +++ b/lisp/net/tramp.el
> @@ -5804,7 +5804,7 @@ tramp-accept-process-output
> (v (process-get proc 'vector)))
> (dolist (p (delq proc (process-list)))
> (when (tramp-file-name-equal-p v (process-get p 'vector))
> - (while (accept-process-output p 0 nil t)))))
> + (while (accept-process-output p 0)))))
> (with-current-buffer (process-buffer proc)
> (let ((inhibit-read-only t)
> last-coding-system-used
>
>
> I've tested Thomas's example 5 times in a row, moving around energically, in
> the Emacs -Q, exercising multiple Eglot features, etc. 0% failures.
I've applied the patch, but Thomas' recipe still blocks for me. Sorry to
tell you.
For comparison, here's the version of tramp-accept-process-output used
for this test:
--8<---------------cut here---------------start------------->8---
(defun tramp-accept-process-output (proc &optional timeout)
"Like `accept-process-output' for Tramp processes.
This is needed in order to hide `last-coding-system-used', which is set
for process communication also.
If the user quits via `C-g', it is propagated up to `tramp-file-name-handler'."
(when-let (((process-get proc 'shared-socket))
(v (process-get proc 'vector)))
(dolist (p (delq proc (process-list)))
(when (tramp-file-name-equal-p v (process-get p 'vector))
(while (accept-process-output p 0)))))
(with-current-buffer (process-buffer proc)
(let ((inhibit-read-only t)
last-coding-system-used
result)
;; This must be protected by the "locked" property.
(with-tramp-locked-connection proc
;; JUST-THIS-ONE is set due to Bug#12145. `with-local-quit'
;; returns t in order to report success.
(if (with-local-quit
(setq result (accept-process-output proc timeout nil t)) t)
(tramp-message
proc 10 "%s %s %s %s\n%s"
proc timeout (process-status proc) result (buffer-string))
;; Propagate quit.
(keyboard-quit)))
result)))
--8<---------------cut here---------------end--------------->8---
Best regards, Michael.
bug-gnu-emacs <at> gnu.org:bug#62194; Package emacs.
(Fri, 17 Mar 2023 11:18:01 GMT) Full text and rfc822 format available.Message #59 received at 62194 <at> debbugs.gnu.org (full text, mbox):
From: João Távora <joaotavora <at> gmail.com> To: Michael Albinus <michael.albinus <at> gmx.de> Cc: 62194 <at> debbugs.gnu.org, miha <at> kamnitnik.top Subject: Re: bug#62194: 30.0.50; Two Eglot-over-Tramp tests are failing on master, passing on emacs-29 Date: Fri, 17 Mar 2023 11:19:16 +0000
Michael Albinus <michael.albinus <at> gmx.de> writes: >> I've tested Thomas's example 5 times in a row, moving around energically, in >> the Emacs -Q, exercising multiple Eglot features, etc. 0% failures. > > I've applied the patch, but Thomas' recipe still blocks for me. Sorry to > tell you. Drats. Reproduced. I honestly don't know what's going on. Yesterday I got 0% failures, today I get 100% failures with the very same code. Maybe there is some SSH/ControlMaster thing at the OS level that's tainting the experiment. FWIW, removing the JUST-THIS-ONE make Thomas' example always pass, but it has other implications like the re-entrancy thing, which I don't understand. I don't have any better ideas at the moment, other than just biting the bullet and reading Tramp's code very closely. I'll try my hand at adapting a process-filter into it as I described in bug#61350, but I don't know if I'll manage of course, since I'm not closely acquainted with the API. Very sorry for the noise, João
bug-gnu-emacs <at> gnu.org:bug#62194; Package emacs.
(Fri, 17 Mar 2023 16:41:01 GMT) Full text and rfc822 format available.Message #62 received at 62194 <at> debbugs.gnu.org (full text, mbox):
From: <miha <at> kamnitnik.top> To: João Távora <joaotavora <at> gmail.com> Cc: Michael Albinus <michael.albinus <at> gmx.de>, 62194 <at> debbugs.gnu.org Subject: Re: bug#62194: 30.0.50; Two Eglot-over-Tramp tests are failing on master, passing on emacs-29 Date: Fri, 17 Mar 2023 17:45:43 +0100
[Message part 1 (text/plain, inline)]
João Távora <joaotavora <at> gmail.com> writes: >> I think that accept-process-output with JUST-THIS-ONE=nil is dangerous >> here. We are now allowing 'file-exists-p', 'expand-file-name' and all >> other functions listed in 'tramp-sh-file-name-handler-alist' to call any >> timer or process filter, > > That's exactly what we want. We _don't_ want to stop other > output to be inhibited from going into the filter. We want > to let other processes slurp their input as often as possible. Generally, I agree. JUST-THIS-ONE=nil is better and it's fine to use it in most functions as long as the programmer is careful that foreign timers and proc filters won't interfere. Though I do think that if a function that calls a-p-o with JUST-THIS-ONE=nil isn't internal, it should mention that it can run timers in its doc string. But here we are talking about 'expand-file-name' and other functions from tramp-sh-file-name-handler. For these, I disagree that we want them to accept output from other processes and call their filters. 'expand-file-name' and others are used often and, for most of the existence of Emacs, they did not call proc filters. I think it's dangerous to have them do it now. > The reason it hung with JUST-THIS-ONE=t is precisely the inverse > reason that it hangs without this hunk (say, in emacs-29). One > process (say tprocess) is holding its input and jprocess can't read > it because it comes from a shared underlying SSH buffer and that > buffer is likely full (until tprocess empties it a little bit). > > At the moment, it is the best we have, both theoretically and > experimentally. > >> without even documenting this. > > This is the most normal usage of accept-process-output. > Look at all the Emacs code base. Basically, only Tramp uses > JUST-THIS-ONE. It's JUST-THIS-ONE who is the aberration. > None of the process-heavy SLIME, SLY, CIDER IDEs use it. > > You can read the manual about JUST-THIS-ONE. It's in A-O-P. Sorry I wasn't clear enough. I meant documenting in expand-file-name's doc string that calling expand-file-name can now run arbitrary process filters and timers. The same goes for other functions from 'tramp-sh-file-name-handler-alist'. >> filter may call 'read-passwd', entering a recursive edit, in which the >> user can kill any buffer or even a process. Or with midnight-mode >> enabled, a timer kills buffers older than 3 days. I consider these "the >> problem of timing errors that usually plague parallel programming", to >> quote '(elisp) Output from Processes'. > > Sorry, I don't immediately see the problem. I can't prove that there > isn't one :-) that's near impossible. Normally what is done > is for the problem to be demonstrated with some experiment. > >> I'm not saying that my proposal with SIGWINCH is flawless and I agree >> that its unacceptable. I'm just saying that we shouldn't be quite >> satisfied with this solution yet, though it should be fine for some >> time. > > I'm not satisfied with it myself. This is very slow (not that it matters > much). But is also all around ugly. The more correct solution is IMHO, > described in bug#61350. But it will take time to see if it fits into > Tramp (I think it does) but most importantly how (that's the hard bit, > Tramp is a lot of code) I'm not against your way of refactoring tramp process filters but I don't think it will solve our problems in any fundamental way for the following simple reason: 'expand-file-name' is synchronous, therefore tramp has to make it use accept-process-filter (tell me if there's another way). It can use it in roughly two ways: with JUST-THIS-ONE=t or JUST-THIS-ONE=nil. In the first case, we hit the eglot-over-ssh freeze problem and in the second case, we'll have 'expand-file-name' call timers and process filters, which I explained above why I don't like. >> What I was thinking was perhaps to introduce a function called >> 'accumulate-process-output' which would be similar to >> 'accept-process-output', except that it would only save process output > > You can propose this in emacs-devel, but if you permit me to > be slightly immodest, I've been coding with > accept-process-output + filters for almost 15 years and I've never > had the need for this. I normally want timers and other processes > to run, I don't want to stop the world. This is great for other packages, but tramp.el carries the special burden of having to re-implement 'expand-file-name' and other functions of synchronous nature.
[signature.asc (application/pgp-signature, inline)]
bug-gnu-emacs <at> gnu.org:bug#62194; Package emacs.
(Fri, 17 Mar 2023 16:48:01 GMT) Full text and rfc822 format available.Message #65 received at 62194 <at> debbugs.gnu.org (full text, mbox):
From: Eli Zaretskii <eliz <at> gnu.org> To: <miha <at> kamnitnik.top> Cc: michael.albinus <at> gmx.de, joaotavora <at> gmail.com, 62194 <at> debbugs.gnu.org Subject: Re: bug#62194: 30.0.50; Two Eglot-over-Tramp tests are failing on master, passing on emacs-29 Date: Fri, 17 Mar 2023 18:47:28 +0200
> Cc: Michael Albinus <michael.albinus <at> gmx.de>, 62194 <at> debbugs.gnu.org > Date: Fri, 17 Mar 2023 17:45:43 +0100 > From: miha--- via "Bug reports for GNU Emacs, > the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org> > > Though I do think that if a function that calls a-p-o with > JUST-THIS-ONE=nil isn't internal, it should mention that it can run > timers in its doc string. I don't see why: timers can run in Emacs almost anywhere, so mentioning that in doc strings of every function makes little sense. > But here we are talking about 'expand-file-name' and other functions > from tramp-sh-file-name-handler. For these, I disagree that we want them > to accept output from other processes and call their filters. > 'expand-file-name' and others are used often and, for most of the > existence of Emacs, they did not call proc filters. I think it's > dangerous to have them do it now. Process filters can be also called at any time, because they basically depend on when the output from a subprocess arrives. Why would you want that to stop while Emacs processes expand-file-name? > 'expand-file-name' is synchronous, therefore tramp has to make it use > accept-process-filter (tell me if there's another way). It can use it in > roughly two ways: with JUST-THIS-ONE=t or JUST-THIS-ONE=nil. In the > first case, we hit the eglot-over-ssh freeze problem and in the second > case, we'll have 'expand-file-name' call timers and process filters, > which I explained above why I don't like. I think you should teach yourself to like that.
bug-gnu-emacs <at> gnu.org:bug#62194; Package emacs.
(Fri, 17 Mar 2023 17:18:02 GMT) Full text and rfc822 format available.Message #68 received at 62194 <at> debbugs.gnu.org (full text, mbox):
From: <miha <at> kamnitnik.top> To: Eli Zaretskii <eliz <at> gnu.org> Cc: michael.albinus <at> gmx.de, joaotavora <at> gmail.com, 62194 <at> debbugs.gnu.org Subject: Re: bug#62194: 30.0.50; Two Eglot-over-Tramp tests are failing on master, passing on emacs-29 Date: Fri, 17 Mar 2023 18:22:14 +0100
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes: >> Cc: Michael Albinus <michael.albinus <at> gmx.de>, 62194 <at> debbugs.gnu.org >> Date: Fri, 17 Mar 2023 17:45:43 +0100 >> From: miha--- via "Bug reports for GNU Emacs, >> the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org> >> >> Though I do think that if a function that calls a-p-o with >> JUST-THIS-ONE=nil isn't internal, it should mention that it can run >> timers in its doc string. > > I don't see why: timers can run in Emacs almost anywhere, so > mentioning that in doc strings of every function makes little sense. I'm not completely sure how timers work, but >> But here we are talking about 'expand-file-name' and other functions >> from tramp-sh-file-name-handler. For these, I disagree that we want them >> to accept output from other processes and call their filters. >> 'expand-file-name' and others are used often and, for most of the >> existence of Emacs, they did not call proc filters. I think it's >> dangerous to have them do it now. > > Process filters can be also called at any time, because they basically > depend on when the output from a subprocess arrives. Why would you > want that to stop while Emacs processes expand-file-name? From '(elisp) Output from Processes': Output from a subprocess can arrive only while Emacs is waiting: when reading terminal input, in ‘sit-for’ and ‘sleep-for’, in ‘accept-process-output’, and in functions which send data to processes. From my understanding of process.c, wait_reading_process_output normally needs to be in the call stack in order for process filters to run. >> 'expand-file-name' is synchronous, therefore tramp has to make it use >> accept-process-filter (tell me if there's another way). It can use it in >> roughly two ways: with JUST-THIS-ONE=t or JUST-THIS-ONE=nil. In the >> first case, we hit the eglot-over-ssh freeze problem and in the second >> case, we'll have 'expand-file-name' call timers and process filters, >> which I explained above why I don't like. > > I think you should teach yourself to like that.
[signature.asc (application/pgp-signature, inline)]
bug-gnu-emacs <at> gnu.org:bug#62194; Package emacs.
(Fri, 17 Mar 2023 17:23:01 GMT) Full text and rfc822 format available.Message #71 received at 62194 <at> debbugs.gnu.org (full text, mbox):
From: Eli Zaretskii <eliz <at> gnu.org> To: <miha <at> kamnitnik.top> Cc: michael.albinus <at> gmx.de, joaotavora <at> gmail.com, 62194 <at> debbugs.gnu.org Subject: Re: bug#62194: 30.0.50; Two Eglot-over-Tramp tests are failing on master, passing on emacs-29 Date: Fri, 17 Mar 2023 19:22:26 +0200
> From: <miha <at> kamnitnik.top> > Cc: michael.albinus <at> gmx.de, joaotavora <at> gmail.com, 62194 <at> debbugs.gnu.org > Date: Fri, 17 Mar 2023 18:22:14 +0100 > > From '(elisp) Output from Processes': > > Output from a subprocess can arrive only while Emacs is waiting: when > reading terminal input, in ‘sit-for’ and ‘sleep-for’, in > ‘accept-process-output’, and in functions which send data to processes. > > From my understanding of process.c, wait_reading_process_output normally > needs to be in the call stack in order for process filters to run. And it is, when Tramp calls expand-file-name and waits for the response from the remote. Moreover, there are situations in Emacs when we eventually call wait_reading_process_output as result of Lisp code that doesn't necessarily imagine something like that could happen, nor can control that. IOW, it is not easy to know from the Lisp level whether Emacs is going to "wait for something" as part of running some Lisp program.
bug-gnu-emacs <at> gnu.org:bug#62194; Package emacs.
(Sat, 18 Mar 2023 09:39:02 GMT) Full text and rfc822 format available.Message #74 received at 62194 <at> debbugs.gnu.org (full text, mbox):
From: Michael Albinus <michael.albinus <at> gmx.de> To: João Távora <joaotavora <at> gmail.com> Cc: 62194 <at> debbugs.gnu.org, miha <at> kamnitnik.top Subject: Re: bug#62194: 30.0.50; Two Eglot-over-Tramp tests are failing on master, passing on emacs-29 Date: Sat, 18 Mar 2023 10:38:39 +0100
João Távora <joaotavora <at> gmail.com> writes:
Hi João,
>> I've applied the patch, but Thomas' recipe still blocks for me. Sorry to
>> tell you.
>
> Drats. Reproduced.
>
> I honestly don't know what's going on. Yesterday I got 0% failures,
> today I get 100% failures with the very same code. Maybe there is some
> SSH/ControlMaster thing at the OS level that's tainting the experiment.
So I propose we let the state as-it-is in master. The relevant tests
pass successfully, and we have your workaround in eglot.el, which makes
the situation a little bit better. Just one remark: perhaps you could
change this to
--8<---------------cut here---------------start------------->8---
(let ((default-directory default-directory)
;; bug#61350: Tramp turns on a feature
;; by default that can't (yet) handle
;; very much data so we turn it off
;; unconditionally -- just for our
;; process.
(tramp-use-ssh-controlmaster-options 'suppress)
(tramp-ssh-controlmaster-options
"-o ControlMaster=no -o ControlPath=none"))
--8<---------------cut here---------------end--------------->8---
For the Tramp < 2.6.0.3 it still works, because
tramp-use-ssh-controlmaster-options is non-nil, and
tramp-ssh-controlmaster-options is used. Starting with Tramp 2.6.0.3,
the value `suppress' forces Tramp to compute its own
tramp-ssh-controlmaster-options, which might be the same, or not. But it
is Tramp's responsibility to DTRT.
Note that I have plans to enable shared connections also for PuTTY, by a
similar option tramp-use-shared-connection (or similar, not decided
yet). But this will be relevant for MS Windows users only; I don't know
how many of them use eglot. And it will definitively be in Tramp 2.7
only.
As proposed. we shall close *this* bug. The reported problem is fixed,
and for everything else we have bug#61350.
> FWIW, removing the JUST-THIS-ONE make Thomas' example always pass, but
> it has other implications like the re-entrancy thing, which I don't
> understand.
>
> I don't have any better ideas at the moment, other than just biting the
> bullet and reading Tramp's code very closely. I'll try my hand at
> adapting a process-filter into it as I described in bug#61350, but I
> don't know if I'll manage of course, since I'm not closely acquainted
> with the API.
I will continue to bring threads into play with Tramp, again. Slow
progress only. But perhaps, it helps to improve the situation.
> Very sorry for the noise,
> João
Best regards, Michael.
bug-gnu-emacs <at> gnu.org:bug#62194; Package emacs.
(Sat, 18 Mar 2023 11:28:01 GMT) Full text and rfc822 format available.Message #77 received at 62194 <at> debbugs.gnu.org (full text, mbox):
From: João Távora <joaotavora <at> gmail.com> To: Michael Albinus <michael.albinus <at> gmx.de> Cc: 62194 <at> debbugs.gnu.org, miha <at> kamnitnik.top Subject: Re: bug#62194: 30.0.50; Two Eglot-over-Tramp tests are failing on master, passing on emacs-29 Date: Sat, 18 Mar 2023 11:29:21 +0000
On Sat, Mar 18, 2023 at 9:38 AM Michael Albinus <michael.albinus <at> gmx.de> wrote: > > João Távora <joaotavora <at> gmail.com> writes: > > Hi João, > > >> I've applied the patch, but Thomas' recipe still blocks for me. Sorry to > >> tell you. > > > > Drats. Reproduced. > > > > I honestly don't know what's going on. Yesterday I got 0% failures, > > today I get 100% failures with the very same code. Maybe there is some > > SSH/ControlMaster thing at the OS level that's tainting the experiment. > > So I propose we let the state as-it-is in master. The relevant tests > pass successfully, and we have your workaround in eglot.el, which makes > the situation a little bit better. I propose we still revert the two of your latest patches. The situation stays exactly the same in practice for now (bug#31350 broken, this bug fixed, workaround in place, no-one knows what is really happening), but with the added advantage that the code is still the same as in emacs-29. > Just one remark: perhaps you could > change this to > > --8<---------------cut here---------------start------------->8--- > (let ((default-directory default-directory) > ;; bug#61350: Tramp turns on a feature > ;; by default that can't (yet) handle > ;; very much data so we turn it off > ;; unconditionally -- just for our > ;; process. > (tramp-use-ssh-controlmaster-options 'suppress) > (tramp-ssh-controlmaster-options > "-o ControlMaster=no -o ControlPath=none")) > --8<---------------cut here---------------end--------------->8--- > For the Tramp < 2.6.0.3 it still works, because > tramp-use-ssh-controlmaster-options is non-nil, and > tramp-ssh-controlmaster-options is used. Starting with Tramp 2.6.0.3, > the value `suppress' forces Tramp to compute its own > tramp-ssh-controlmaster-options, which might be the same, or not. But it > is Tramp's responsibility to DTRT. Makes sense. If it's just this change, you can push this yourself. Thanks in advance. > Note that I have plans to enable shared connections also for PuTTY, by a > similar option tramp-use-shared-connection (or similar, not decided > yet). But this will be relevant for MS Windows users only; I don't know > how many of them use eglot. And it will definitively be in Tramp 2.7 > only. > > As proposed. we shall close *this* bug. The reported problem is fixed, > and for everything else we have bug#61350. OK. Just a heads up, I asked for bug#61350 and this one to be "merged" earlier. Don't know what debbugs did about that, but didn't see any practical effect > > FWIW, removing the JUST-THIS-ONE make Thomas' example always pass, but > > it has other implications like the re-entrancy thing, which I don't > > understand. > > > > I don't have any better ideas at the moment, other than just biting the > > bullet and reading Tramp's code very closely. I'll try my hand at > > adapting a process-filter into it as I described in bug#61350, but I > > don't know if I'll manage of course, since I'm not closely acquainted > > with the API. > > I will continue to bring threads into play with Tramp, again. Slow > progress only. But perhaps, it helps to improve the situation. I think you should consider bring stuff _out_ of Tramp instead of _in_. Consider removing tramp-a-o-p entirely, and segregating/segmenting messages in a process filter. This segregation is entirely textual (no fs primitives) and does run the risk of reentrancy. Then -- for sync APIs -- 'throw' the complete message into whoever is blockingly waiting for the answer with (catch ... (while (accept-process-output p))). I've given a working example in bug#61350. If you need timeouts I can show you how to add them. João
bug-gnu-emacs <at> gnu.org:bug#62194; Package emacs.
(Sat, 18 Mar 2023 12:24:02 GMT) Full text and rfc822 format available.Message #80 received at 62194 <at> debbugs.gnu.org (full text, mbox):
From: Michael Albinus <michael.albinus <at> gmx.de> To: João Távora <joaotavora <at> gmail.com> Cc: 62194 <at> debbugs.gnu.org, miha <at> kamnitnik.top Subject: Re: bug#62194: 30.0.50; Two Eglot-over-Tramp tests are failing on master, passing on emacs-29 Date: Sat, 18 Mar 2023 13:23:38 +0100
João Távora <joaotavora <at> gmail.com> writes: Hi João, >> So I propose we let the state as-it-is in master. The relevant tests >> pass successfully, and we have your workaround in eglot.el, which makes >> the situation a little bit better. > > I propose we still revert the two of your latest patches. The situation > stays exactly the same in practice for now (bug#31350 broken, this bug > fixed, workaround in place, no-one knows what is really happening), but > with the added advantage that the code is still the same as in emacs-29. Nope, I'd like to keep the changes. It is the basis for further work on the problem. And these changes will also be in Tramp 2.6.0.3. >> Just one remark: perhaps you could >> change this to >> >> --8<---------------cut here---------------start------------->8--- >> (let ((default-directory default-directory) >> ;; bug#61350: Tramp turns on a feature >> ;; by default that can't (yet) handle >> ;; very much data so we turn it off >> ;; unconditionally -- just for our >> ;; process. >> (tramp-use-ssh-controlmaster-options 'suppress) >> (tramp-ssh-controlmaster-options >> "-o ControlMaster=no -o ControlPath=none")) >> --8<---------------cut here---------------end--------------->8--- > > >> For the Tramp < 2.6.0.3 it still works, because >> tramp-use-ssh-controlmaster-options is non-nil, and >> tramp-ssh-controlmaster-options is used. Starting with Tramp 2.6.0.3, >> the value `suppress' forces Tramp to compute its own >> tramp-ssh-controlmaster-options, which might be the same, or not. But it >> is Tramp's responsibility to DTRT. > > Makes sense. If it's just this change, you can push this yourself. > Thanks in advance. Pushed to the emacs-29 branch. >> Note that I have plans to enable shared connections also for PuTTY, by a >> similar option tramp-use-shared-connection (or similar, not decided >> yet). But this will be relevant for MS Windows users only; I don't know >> how many of them use eglot. And it will definitively be in Tramp 2.7 >> only. >> >> As proposed. we shall close *this* bug. The reported problem is fixed, >> and for everything else we have bug#61350. > > OK. > > Just a heads up, I asked for bug#61350 and this one to be "merged" > earlier. Don't know what debbugs did about that, but didn't see any > practical effect This are two different bugs. Bug#61350 is about using a shared connection, and this is still open, although mitigated by Eglot's workaround. Bug#62194 is about a patch which tried to fix bug#61350. This patch has been reverted, and so there's nothing left to do for bug#62194. It can be closed. >> > FWIW, removing the JUST-THIS-ONE make Thomas' example always pass, but >> > it has other implications like the re-entrancy thing, which I don't >> > understand. >> > >> > I don't have any better ideas at the moment, other than just biting the >> > bullet and reading Tramp's code very closely. I'll try my hand at >> > adapting a process-filter into it as I described in bug#61350, but I >> > don't know if I'll manage of course, since I'm not closely acquainted >> > with the API. >> >> I will continue to bring threads into play with Tramp, again. Slow >> progress only. But perhaps, it helps to improve the situation. > > I think you should consider bring stuff _out_ of Tramp instead of _in_. > > Consider removing tramp-a-o-p entirely, and segregating/segmenting > messages in a process filter. This segregation is entirely textual > (no fs primitives) and does run the risk of reentrancy. Then -- for > sync APIs -- 'throw' the complete message into whoever is blockingly > waiting for the answer with (catch ... (while (accept-process-output p))). > > I've given a working example in bug#61350. If you need timeouts > I can show you how to add them. The problem is even more complex than discussed so far. All of asynchronity can happen to Tramp's reading of process output. Process filters and process sentinels from other processes, timers, callbacks from file notifications or D-Bus events, whatever. I'm not convinced (yet?), that moving Tramp's process output reading to a process filter will solve it. There is also the additional problem in Emacs, that process filters do not cascade. Any package can activate an own process filter, even for Tramp processes, and Tramp would be lost then immediately. > João Best regards, Michael.
bug-gnu-emacs <at> gnu.org:bug#62194; Package emacs.
(Sat, 18 Mar 2023 12:33:02 GMT) Full text and rfc822 format available.Message #83 received at 62194 <at> debbugs.gnu.org (full text, mbox):
From: João Távora <joaotavora <at> gmail.com> To: Michael Albinus <michael.albinus <at> gmx.de> Cc: 62194 <at> debbugs.gnu.org, miha <at> kamnitnik.top Subject: Re: bug#62194: 30.0.50; Two Eglot-over-Tramp tests are failing on master, passing on emacs-29 Date: Sat, 18 Mar 2023 12:33:48 +0000
On Sat, Mar 18, 2023 at 12:23 PM Michael Albinus <michael.albinus <at> gmx.de> wrote: > There is also the additional problem in Emacs, that process filters do > not cascade. Any package can activate an own process filter, even for > Tramp processes, and Tramp would be lost then immediately. If this were true, then a large number of Emacs extensions that rely on the scheme I described would be failing horribly. Read the example code I posted in bug#61350, you'll notice that a process filter does nothing more than put text into a buffer and potentially act on that text _only_ if it has been told to. So it's very healthy to design your system to let timers and filters -- yours and others -- run freely. João
bug-gnu-emacs <at> gnu.org:bug#62194; Package emacs.
(Sun, 19 Mar 2023 12:20:02 GMT) Full text and rfc822 format available.Message #86 received at 62194 <at> debbugs.gnu.org (full text, mbox):
From: Michael Albinus <michael.albinus <at> gmx.de> To: João Távora <joaotavora <at> gmail.com> Cc: 62194 <at> debbugs.gnu.org, miha <at> kamnitnik.top Subject: Re: bug#62194: 30.0.50; Two Eglot-over-Tramp tests are failing on master, passing on emacs-29 Date: Sun, 19 Mar 2023 13:19:49 +0100
Michael Albinus <michael.albinus <at> gmx.de> writes: Hi João, > Note that I have plans to enable shared connections also for PuTTY, by a > similar option tramp-use-shared-connection (or similar, not decided > yet). But this will be relevant for MS Windows users only; I don't know > how many of them use eglot. And it will definitively be in Tramp 2.7 > only. For the records, I have finally renamed tramp-use-ssh-controlmaster-options to tramp-use-connection-share in Tramp 2.7 / Emacs master. This handles also PuTTY methods. You don't need to adapt your Tramp workaround in eglot.el, the former option still exists as defvaralias of the latter. Best regards, Michael.
Michael Albinus <michael.albinus <at> gmx.de>:João Távora <joaotavora <at> gmail.com>:Message #91 received at 62194-done <at> debbugs.gnu.org (full text, mbox):
From: Michael Albinus <michael.albinus <at> gmx.de> To: João Távora <joaotavora <at> gmail.com> Cc: 62194-done <at> debbugs.gnu.org Subject: Re: bug#62194: 30.0.50; Two Eglot-over-Tramp tests are failing on master, passing on emacs-29 Date: Tue, 28 Mar 2023 12:51:23 +0200
Michael Albinus <michael.albinus <at> gmx.de> writes: Hi João, > Further search has told me, that Fedora keeps clangd in the package > clang-tools-extra. After installing this package, all Eglot tests pass > as expected. Closing, since there's nothing left to do. >> João Best regards, Michael.
Debbugs Internal Request <help-debbugs <at> gnu.org>
to internal_control <at> debbugs.gnu.org.
(Tue, 25 Apr 2023 11:24:13 GMT) Full text and rfc822 format available.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.