Re: [PATCH] Provide completion candidates for remote containers over a TRAMP connection
Hi Michael,
Michael Albinus wrote:
> For the futere I could imagine more different values. t and nil are fine
> ATM, but if there are more methods which support this feature, it could
> be a list. '("docker" "podman" "kubernetes") would enable this for the
> respective methods, and disable it for all other methods in multi-hop
> completions. WDYT?
I think this is a great idea. What about calling the variable
`tramp-completion-multi-hop-methods', with a default of '("docker" "podman")? I
am not confident including "kubernetes" here as I have not tested it but could
do so if you are comfortable with that.
> Furthermore, the result of tramp-container--completion-function is
> cached now. I've obeserved that the function is called 5 times in a row;
> we don't need so many shells to be applied on remote.
Ah nice. I hadn't noticed this.
> What you could perhaps contribute is documentation of
> tramp-completion-remote-containers (with its changed name) in
> tramp.texi. Maybe in node "Ad-hoc multi-hops"?
I'll get started on this and an addition to etc/NEWS about the new user option.
Re: tramp (2.7.0-pre HEAD/dc0839de9b3654837ec8f5e66d187319b9eecd6f); bracktrace when saving modified remote file
Gregor Zattler writes:
Hi Gregor,
> When I saved a modified buffer to it*'s associated
> remote file (via ssh on an Univention Corporate Server
> (debian/buster)), the file was saved but I got a
> backtrace, see below.
>
> I tried to reproduce (modify, save) with tramp-verbose
> set to 9 but no problem occured.
Well, this is a problem with Tramp for a long time. Tramp keeps a
connection to the remote host (say an ssh connection). Whenever Tramp
needs to know something, it sends a command over the connection, and
reads the reply.
This works fine when everything is serialized. But there are
asynchronous Emacs events which could trigger a new remote command while
another command is still waiting for the response. This cannot work,
because once a response arrives, Tramp wouldn't know which command is
waiting for what response.
Tramp has tried to fix this. It suppresses timers while waitng for a
response. Timers are the most likely trouble-shooter in this game.
Obviously, there are still cases where it doesn't work as expected. Like here.
> Here is the backtrace and further info (I redacted one
> directory name)
Lets' condense the backtrace to the relevant calls.
--8<---cut here---start->8---
> Debugger entered--Lisp error: (remote-file-error "Forbidden reentrant call of
> Tramp")
> signal(remote-file-error ("Forbidden reentrant call of Tramp"))
> tramp-send-command((tramp-file-name "ssh" "root" nil "fs2" nil
> "/data/projekte/Projekte//Aufgaben/" nil) "test -r
> /data/projekte/Projekte//Aufga...")
>
> tramp-sh-handle-file-readable-p("/ssh:root@fs2:/data/projekte/Projekte/...")
> dired-buffer-stale-p(t)
> auto-revert-buffer(#)
> timer-event-handler([t 25836 43972 364989 5 auto-revert-buffers nil nil
> 762000 nil])
--8<---cut here---end--->8---
--8<---cut here---start->8---
> accept-process-output(# 0 nil t)
> tramp-send-command((tramp-file-name "ssh" "root" nil "fs2" nil
> "/data/projekte/Projekte//Aufgaben/" nil) "test -e
> /data/projekte/Projekte//Aufga...")
> file-exists-p("/ssh:root@fs2:/data/projekte/Projekte/...")
> insert-directory("/ssh:root@fs2:/data/projekte/Projekte/..."
> "-altrF --group-directories-first --block-size='1" nil t)
> dired-revert(ignore-auto dont-ask)
> revert-buffer(ignore-auto dont-ask preserve-modes)
> auto-revert-handler()
> file-notify-handle-event((file-notify (# (delete)
> ".#MDMunattended--Antwortfile-Win11.txt") file-notify-callback))
> tramp-sh-inotifywait-process-filter(#
> "/data/projekte/Projekte//Aufgaben/ MOD...")
--8<---cut here---end--->8---
The lower block shows a file-notify event '(#
(delete) ".#xxx")'. This is a file lock which is deleted, because the
corresponding file was written. This calls up to "test -e
/data/projekte/Projekte//Aufga..." (checking, whether the
directory still exists, before inserting the changed contents in the
dired buffer. Everything normal.
But while it waits for the response, a timer has started (also in the
auto-revert cosmos) which wants to send "test -r
/data/projekte/Projekte//Aufga..." (checking, whether a
file is readable. Since the first command didn't finish yet, Tramp has
signalled it. Boom.
Obviously, Tramp didn't try hard enough to suppress timers. I've pushed
the appended patch to the repositories. Could you please observe next
days, whether the situation has been changed? Thanks.
> HTH, Gregor
Best regards, Michael.
diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 9d555c5621b..b34d3ff6695 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -5615,7 +5615,8 @@ tramp-accept-process-output
(v (process-get proc 'tramp-vector)))
(dolist (p (delq proc (process-list)))
(when (tramp-file-name-equal-p v (process-get p 'tramp-vector))
- (with-local-quit (accept-process-output p 0 nil t)
+ (with-tramp-suspended-timers
+ (with-local-quit (accept-process-output p 0 nil t))
(with-current-buffer (process-buffer proc)
(let ((inhibit-read-only t)
Re: [PATCH] Provide completion candidates for remote containers over a TRAMP connection
Gene Goykhman writes:
> Hi Michael,
Hi Gene,
> I have also incorporated your other suggestions and added a `defcustom
> tramp-completion-remote-containers' to allow the user to turn this
> feature on. It is off by default, but I can change this if you feel it
> appropriate.
Well, yes. I believe we could use a better name for it, because the
feature is not restricted to container methods. It could be added for
any method; it just needs to adapt the respective completion
function. A name like tramp-completion-use-ad-hoc-proxies? Perhaps you
have a better proposal; I'm notoriously bad in finding good names.
I'm kind of undecided what the default should be. nil is conservative,
no surprise of users. OTOH, this new feature will remain undetected by
the majority of users, if we don't peopagate it accordingly (for
example, enabling it by default :-)
For the futere I could imagine more different values. t and nil are fine
ATM, but if there are more methods which support this feature, it could
be a list. '("docker" "podman" "kubernetes") would enable this for the
respective methods, and disable it for all other methods in multi-hop
completions. WDYT?
> I have attached the patch in its current form. Note that this patch is
> now made against the tramp.git repository... my original patch was
> against the main Emacs repository.
Tramp repo is OK. All my development happens there. Mostly, I keep the
Tramp and Emacs repos in sync, but sometimes, when a new feature is
under development, I delay the sync.
I've pushed your patch to both the Tramp and Emacs repos. I've added
another patch afterwards, most of it are minor changes. The change which
matters is in tramp-container--completion-function. I've changed the
argument to METHOD, by this we avoid the problem in
tramp-set-completion-function checking the argument properly. Furthermore,
non-essential is bound to nil here. During completion, it is usually
bound to t, in order to suppress opening a new connection. But here, we
*want* the new connection, therefore this change. Now you can do something like
C-x C-f /ss TAB : TAB | TAB doc TAB TAB
without to connect to any host explicitly in advance.
Furthermore, the result of tramp-container--completion-function is
cached now. I've obeserved that the function is called 5 times in a row;
we don't need so many shells to be applied on remote.
What you could perhaps contribute is documentation of
tramp-completion-remote-containers (with its changed name) in
tramp.texi. Maybe in node "Ad-hoc multi-hops"?
And also a short entry for Emacs' etc/NEWS file would be great.
Best regards, Michael.
