Stephen Gildea <[email protected]> writes:
Hi Stephen,
>> > 3. Unmount if this process did the mount. This is your idea of an
>> > "unmount on cleanup" bit. As with case 2, if this process did not do
>> > the mount, Tramp would have to handle an unexpectedly closed connection.
>> >
>> > 4. Every Emacs uses its own mount point. Multiple mount points could
>> > still share an ssh connection but would no longer share the sshfs cache.
>> > This option seems the simplest and cleanest to implement.
>>
>> I'm just working on 3, controlled by a user option. Let's see how it
>> goes (with your testing), if not satisfying we'll have 4.
>>
>> Variant 4 has the disadvantage to leave several mount points when Emacs
>> crashes or does not pass the cleanup machinery for whatever reason.
>
> I tested the patch you sent me that implements variant 3.
Thanks for your tests!
> tramp-cleanup-this-connection fails to unmount. The problem is that
> vec is not a member of tramp-fuse-mount-points:
> vec:
> (tramp-file-name sshfs nil nil otherhost nil /home/gildea/ nil)
> tramp-fuse-mount-points:
> ((tramp-file-name sshfs nil nil otherhost nil /home/gildea/afile nil))
Fixed.
> tramp-cleanup-all-connections and exiting Emacs both do unmount, as expected.
> However, they leave behind the mount point directory,
> /tmp/tramp.sshfs.otherhost/
Fixed.
> Two Emacs processes accessing the same remote host can confuse each other:
> . open an sshfs file in Emacs 1
> . open an sshfs file in Emacs 2 on the same remote host
> . cleanup-all in Emacs 1
> . try to access file again in Emacs 2 - fails: No such file or directory
Yes, you can always shoot yourself in the foot. It might be possible to
check this case, but is it really worth the effort?
> Also while testing I came across two behaviors that could be better.
> These are not regressions, as un-patched Tramp behaves the same.
>
> If I open a non-existent file, e.g.,
> /sshfs:otherhost:/tmp/newfile
> The following gets logged to *Messages*
> File is missing: Opening input file No such file or directory
> /tmp/tramp.sshfs.otherhost/tmp/newfile
> and the buffer is shown as editing the local file
> /tmp/tramp.sshfs.otherhost/tmp/newfile
This happens for other methods as well, like /ssh:otherhost:/tmp/newfile.
It is a bug, I will work on this,
> Tramp will not share the ssh connection with an existing connection to
> the remote host, because it uses its own ControlPath. This makes it
> harder to use Tramp with a remote host that requires user action to
> authenticate, as many two-factor methods do. I would prefer that
> Tramp not set its own options and let the options in my .ssh/config
> control the connection.
See the Tramp manual, (info "(tramp) Frequently Asked Questions")
--8<---------------cut here---------------start------------->8---
• TRAMP does not use default ‘ssh’ ‘ControlPath’
TRAMP overwrites ‘ControlPath’ settings when initiating ‘ssh’
sessions. TRAMP does this to fend off a stall if a master session
opened outside the Emacs session is no longer open. That is why
TRAMP prompts for the password again even if there is an ‘ssh’
already open.
Some ‘ssh’ versions support a ‘ControlPersist’ option, which allows
you to set the ‘ControlPath’ provided the variable
‘tramp-ssh-controlmaster-options’ is customized as follows:
(customize-set-variable
'tramp-ssh-controlmaster-options
(concat
"-o ControlPath=/tmp/ssh-ControlPath-%%r@%%h:%%p "
"-o ControlMaster=auto -o ControlPersist=yes"))
Note how ‘%r’, ‘%h’ and ‘%p’ must be encoded as ‘%%r’, ‘%%h’ and
‘%%p’.
If the ‘~/.ssh/config’ file is configured appropriately for the
above behavior, then any changes to ‘ssh’ can be suppressed with
this ‘nil’ setting:
(customize-set-variable 'tramp-use-ssh-controlmaster-options nil)
This should also be set to ‘nil’ if you use the ‘ProxyCommand’ or
‘ProxyJump’ options in your ‘ssh’ configuration.
On MS Windows, ‘tramp-use-ssh-controlmaster-options’ is set to
‘nil’ by default, because the MS Windows and MSYS2 implementations
of ‘OpenSSH’ do not support this option properly.
--8<---------------cut here---------------end--------------->8---
The revised patch is appended.
Best regards, Michael.
diff --git a/lisp/tramp-cache.el b/lisp/tramp-cache.el
index 03cca2ec..30408d3a 100644
--- a/lisp/tramp-cache.el
+++ b/lisp/tramp-cache.el
@@ -319,12 +319,7 @@ KEY identifies the connection, it is either a process or a
used to cache connection properties of the local machine.
If KEY is `tramp-cache-undefined', or if the value is not set for
the connection, return DEFAULT."
- ;; Unify key by removing localname and hop from `tramp-file-name'
- ;; structure. Work with a copy in order to avoid side effects.
- (when (tramp-file-name-p key)
- (setq key (copy-tramp-file-name key))
- (setf (tramp-file-name-localname key) nil
- (tramp-file-name-hop key) nil))
+ (setq key (tramp-file-name-unify key))
(let* ((hash (tramp-get-hash-table key))
(cached (if (hash-table-p hash)
(gethash property hash tramp-cache-undefined)
@@ -350,12 +345,7 @@ used to cache connection properties of the local machine. If KEY
is `tramp-cache-undefined', nothing is set.
PROPERTY is set persistent when KEY is a `tramp-file-name' structure.
Return VALUE."
- ;; Unify key by removing localname and hop from `tramp-file-name'
- ;; structure. Work with a copy in order to avoid side effects.
- (when (tramp-file-name-p key)
- (setq key (copy-tramp-file-name key))
- (setf (tramp-file-name-localname key) nil
- (tramp-file-name-hop key) nil))
+ (setq key (tramp-file-name-unify key))
(when-let ((hash (tramp-get-hash-table key)))
(puthash property value hash))
(setq tramp-cache-data-changed
@@ -379,12 +369,7 @@ KEY identifies the connection, it is either a process or a
`tramp-file-name' structure. A special case is nil, which is
used to cache connection properties of the local machine.
PROPERTY is set persistent when KEY is a `tramp-file-name' structure."
- ;; Unify key by removing localname and hop from `tramp-file-name'
- ;; structure. Work with a copy in order to avoid side effects.
- (when (tramp-file-name-p key)
- (setq key (copy-tramp-file-name key))
- (setf (tramp-file-name-localname key) nil
- (tramp-file-name-hop key) nil))
+ (setq key (tramp-file-name-unify key))
(when-let ((hash (tramp-get-hash-table key)))
(remhash property hash))
(setq tramp-cache-data-changed
@@ -397,12 +382,7 @@ PROPERTY is set persistent when KEY is a `tramp-file-name' structure."
KEY identifies the connection, it is either a process or a
`tramp-file-name' structure. A special case is nil, which is
used to cache connection properties of the local machine."
- ;; Unify key by removing localname and hop from `tramp-file-name'
- ;; structure. Work with a copy in order to avoid side effects.
- (when (tramp-file-name-p key)
- (setq key (copy-tramp-file-name key))
- (setf (tramp-file-name-localname key) nil
- (tramp-file-name-hop key) nil))
+ (setq key (tramp-file-name-unify key))
(tramp-message
key 7 "%s %s" key
(when-let ((hash (gethash key tramp-cache-data)))
diff --git a/lisp/tramp-fuse.el b/lisp/tramp-fuse.el
index 8c5afa7c..d2bac2d0 100644
--- a/lisp/tramp-fuse.el
+++ b/lisp/tramp-fuse.el
@@ -175,15 +175,30 @@
mount)
(match-string 1 mount)))))))
+(defun tramp-fuse-get-fusermount ()
+ "Determine the local `fusermount' command."
+ ;; We use key nil for local connection properties.
+ (with-tramp-connection-property nil "fusermount"
+ (or (executable-find "fusermount3")
+ (executable-find "fusermount"))))
+
+(defvar tramp-fuse-mount-points nil
+ "List of fuse volume determined by a VEC.")
+
(defun tramp-fuse-unmount (vec)
"Unmount fuse volume determined by VEC."
- (let ((default-directory tramp-compat-temporary-file-directory)
- (command (format "fusermount3 -u %s" (tramp-fuse-mount-point vec))))
+ (let* ((default-directory tramp-compat-temporary-file-directory)
+ (mount-point (tramp-fuse-mount-point vec))
+ (command (format "%s -u %s" (tramp-fuse-get-fusermount) mount-point)))
(tramp-message vec 6 "%s\n%s" command (shell-command-to-string command))
(tramp-flush-connection-property
(tramp-get-connection-process vec) "mounted")
+ (setq tramp-fuse-mount-points
+ (delete (tramp-file-name-unify vec) tramp-fuse-mount-points))
;; Give the caches a chance to expire.
- (sleep-for 1)))
+ (sleep-for 1)
+ (when (tramp-compat-directory-empty-p mount-point)
+ (delete-directory mount-point))))
(defun tramp-fuse-local-file-name (filename)
"Return local mount name of FILENAME."
@@ -205,6 +220,36 @@
(substring localname 1) localname)
(tramp-fuse-mount-point v)))))))
+(defcustom tramp-fuse-unmount-on-cleanup nil
+ "Whether fuse volumes shall be unmounted on cleanup."
+ :group 'tramp
+ :version "28.1"
+ :type 'boolean)
+
+(defun tramp-fuse-cleanup (vec)
+ "Cleanup fuse volume determined by VEC."
+ (and tramp-fuse-unmount-on-cleanup
+ (member (tramp-file-name-unify vec) tramp-fuse-mount-points)
+ (tramp-fuse-unmount vec)))
+
+(defun tramp-fuse-cleanup-all ()
+ "Unmount all fuse volumes used by Tramp."
+ (and tramp-fuse-unmount-on-cleanup
+ (mapc #'tramp-fuse-unmount tramp-fuse-mount-points)))
+
+;; Add cleanup hooks.
+(add-hook 'tramp-cleanup-connection-hook #'tramp-fuse-cleanup)
+(add-hook 'tramp-cleanup-all-connections-hook #'tramp-fuse-cleanup-all)
+(add-hook 'kill-emacs-hook #'tramp-fuse-cleanup-all)
+(add-hook 'tramp-fuse-unload-hook
+ (lambda ()
+ (remove-hook 'tramp-cleanup-connection-hook
+ #'tramp-fuse-cleanup)
+ (remove-hook 'tramp-cleanup-all-connections-hook
+ #'tramp-fuse-cleanup-all)
+ (remove-hook 'kill-emacs-hook
+ #'tramp-fuse-cleanup-all)))
+
(add-hook 'tramp-unload-hook
(lambda ()
(unload-feature 'tramp-fuse 'force)))
diff --git a/lisp/tramp-rclone.el b/lisp/tramp-rclone.el
index 49e366c0..e9d9b4e3 100644
--- a/lisp/tramp-rclone.el
+++ b/lisp/tramp-rclone.el
@@ -386,6 +386,7 @@ connection if a previous connection has died for some reason."
(tramp-cleanup-connection vec 'keep-debug 'keep-password))
;; Mark it as connected.
+ (add-to-list 'tramp-fuse-mount-points vec)
(tramp-set-connection-property
(tramp-get-connection-process vec) "connected" t))))
diff --git a/lisp/tramp-sshfs.el b/lisp/tramp-sshfs.el
index 0019ac01..f6fe65fd 100644
--- a/lisp/tramp-sshfs.el
+++ b/lisp/tramp-sshfs.el
@@ -368,6 +368,7 @@ connection if a previous connection has died for some reason."
vec 'file-error "Error mounting %s" (tramp-fuse-mount-spec vec))))
;; Mark it as connected.
+ (add-to-list 'tramp-fuse-mount-points (tramp-file-name-unify vec))
(tramp-set-connection-property
(tramp-get-connection-process vec) "connected" t)))
diff --git a/lisp/tramp.el b/lisp/tramp.el
index c8839354..78343de3 100644
--- a/lisp/tramp.el
+++ b/lisp/tramp.el
@@ -1450,16 +1450,23 @@ If nil, return `tramp-default-port'."
(put #'tramp-file-name-port-or-default 'tramp-suppress-trace t)
+(defun tramp-file-name-unify (vec)
+ "Unify VEC by removing localname and hop from `tramp-file-name' structure.
+Make a copy copy in order to avoid side effects."
+ (when (tramp-file-name-p vec)
+ (setq vec (copy-tramp-file-name vec))
+ (setf (tramp-file-name-localname vec) nil
+ (tramp-file-name-hop vec) nil))
+ vec)
+
+(put #'tramp-file-name-unify 'tramp-suppress-trace t)
+
;; Comparison of file names is performed by `tramp-equal-remote'.
(defun tramp-file-name-equal-p (vec1 vec2)
"Check, whether VEC1 and VEC2 denote the same `tramp-file-name'."
(and (tramp-file-name-p vec1) (tramp-file-name-p vec2)
- (string-equal (tramp-file-name-method vec1)
- (tramp-file-name-method vec2))
- (string-equal (tramp-file-name-user-domain vec1)
- (tramp-file-name-user-domain vec2))
- (string-equal (tramp-file-name-host-port vec1)
- (tramp-file-name-host-port vec2))))
+ (equal (tramp-file-name-unify vec1)
+ (tramp-file-name-unify vec2))))
(defun tramp-get-method-parameter (vec param)
"Return the method parameter PARAM.
