bug#62614: Tramp attempts to remove lock file with 'remote-file-name-inhibit-locks t

bug#62614: Tramp attempts to remove lock file with 'remote-file-name-inhibit-locks t

[Yuri D'Elia]

I have the following configuration to reduce remote latency while
saving, where the interesting tidbit is 'remote-file-name-inhibit-locks:

(setq tramp-ssh-controlmaster-options "-o ControlMaster=auto"
      remote-file-name-inhibit-cache 300
      remote-file-name-inhibit-locks t)

While editing a remote file via /scp:remote:test.txt I see the following
warning pop up:

Warning (unlock-file): Cannot remove lock file for /scp:remote:test.txt, ignored

because the lock file hasn't been created in the first place. I guess we
shouldn't attempt to unlock if there was no lock to begin with.


In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.16.0, Xaw scroll bars) of 2023-04-01 built on t14g2
Repository revision: 4bd1fc59664273c571aba8543940768c68eb336b
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101007
System Description: Debian GNU/Linux 12 (bookworm)

Configured using:
 'configure --with-x-toolkit=lucid --with-native-compilation
 --with-tree-sitter --without-gsettings 'CFLAGS=-O3 -flto -march=native
 -pipe ' 'LDFLAGS=-fuse-ld=mold -flto''

bug#62614: Tramp attempts to remove lock file with 'remote-file-name-inhibit-locks t

[Michael Albinus]

Yuri D'Elia <wavexx@thregr.org> writes:

Hi Yuri,

> I have the following configuration to reduce remote latency while
> saving, where the interesting tidbit is 'remote-file-name-inhibit-locks:
>
> (setq tramp-ssh-controlmaster-options "-o ControlMaster=auto"
>       remote-file-name-inhibit-cache 300
>       remote-file-name-inhibit-locks t)
>
> While editing a remote file via /scp:remote:test.txt I see the following
> warning pop up:
>
> Warning (unlock-file): Cannot remove lock file for /scp:remote:test.txt, ignored
>
> because the lock file hasn't been created in the first place. I guess we
> shouldn't attempt to unlock if there was no lock to begin with.

Tramp behaves like with local lock files. Imagine, you have a local file
~/test with a lock file (a symbolic link) ~/.#test -> something. Set
create-lock-files to nil. Start editing the local file ~/test, and save
it. The local lock file ~/.#test is removed.

And that's also what's documented, see (info "(elisp) File Locks")

--8<---------------cut here---------------start------------->8---
 -- User Option: remote-file-name-inhibit-locks
     You can prevent the creation of remote lock files by setting the
     variable ‘remote-file-name-inhibit-locks’ to ‘t’.
--8<---------------cut here---------------end--------------->8---

It speaks only about creation of remote lock files, and not about
removal. One possible workaround for you would be to eval the following
form, additionally to your settings:

--8<---------------cut here---------------start------------->8---
(fset #'tramp-handle-unlock-file #'ignore)
--8<---------------cut here---------------end--------------->8---

Best regards, Michael.

bug#62614: Tramp attempts to remove lock file with 'remote-file-name-inhibit-locks t

[Yuri D'Elia]

On Mon, Apr 03 2023, Michael Albinus wrote:
> Tramp behaves like with local lock files. Imagine, you have a local file
> ~/test with a lock file (a symbolic link) ~/.#test -> something. Set
> create-lock-files to nil. Start editing the local file ~/test, and save
> it. The local lock file ~/.#test is removed.
>
> And that's also what's documented, see (info "(elisp) File Locks")
>
>  -- User Option: remote-file-name-inhibit-locks
>      You can prevent the creation of remote lock files by setting the
>      variable ‘remote-file-name-inhibit-locks’ to ‘t’.
>
>
> It speaks only about creation of remote lock files, and not about
> removal.

Mmmh, maybe it should be mentioned explicitly. For me, "inhibit-locks"
meant inhibiting both creation and removal.

But even for local files, why the unlock is done? For cleanup?

> One possible workaround for you would be to eval the following form,
> additionally to your settings:
>
> (fset #'tramp-handle-unlock-file #'ignore)

Looking at the definition of #'tramp-handle-unlock-file, it does
actually look the most reasonable thing to do, but somehow having to
fset an internal function doesn't feel right, but I don't have a better
proposal since we don't have any setting that inhibit lock handling
completely.

I went with this configuration for a long time, actually. I never
noticed this behavior, because I never had remote lock files to begin
with. Only recently with the warning buffer displayed on errors I
started to see it.

bug#62614: Tramp attempts to remove lock file with 'remote-file-name-inhibit-locks t

[Michael Albinus]

Yuri D'Elia <wavexx@thregr.org> writes:

Hi Yuri,

> Mmmh, maybe it should be mentioned explicitly. For me, "inhibit-locks"
> meant inhibiting both creation and removal.
>
> But even for local files, why the unlock is done? For cleanup?

Don't know, this behavior is "since ever" (I don't remember a change here).

With Tramp I'm agnostic. Tramp follows the behavior of local files. It
could do it differently, the local behavior could change, whatever.

Opinions?

>> One possible workaround for you would be to eval the following form,
>> additionally to your settings:
>>
>> (fset #'tramp-handle-unlock-file #'ignore)
>
> Looking at the definition of #'tramp-handle-unlock-file, it does
> actually look the most reasonable thing to do, but somehow having to
> fset an internal function doesn't feel right, but I don't have a better
> proposal since we don't have any setting that inhibit lock handling
> completely.

As said, it would be a workaround for your expectations. I won't
document it as global solution.

Best regards, Michael.

bug#62614: Tramp attempts to remove lock file with 'remote-file-name-inhibit-locks t

[Yuri D'Elia]

On Mon, Apr 03 2023, Michael Albinus wrote:
> Don't know, this behavior is "since ever" (I don't remember a change here).
>
> With Tramp I'm agnostic. Tramp follows the behavior of local files. It
> could do it differently, the local behavior could change, whatever.
>
> Opinions?

I fully agree with you that it should follow the behavior of local files
too, so I wouldn't change anything here _just_ for tramp.

bug#62614: Tramp attempts to remove lock file with 'remote-file-name-inhibit-locks t

[Yuri D'Elia]

On Mon, Apr 03 2023, Yuri D'Elia wrote:
> On Mon, Apr 03 2023, Michael Albinus wrote:
>> Don't know, this behavior is "since ever" (I don't remember a change here).
>>
>> With Tramp I'm agnostic. Tramp follows the behavior of local files. It
>> could do it differently, the local behavior could change, whatever.
>>
>> Opinions?
>
> I fully agree with you that it should follow the behavior of local files
> too, so I wouldn't change anything here _just_ for tramp.

Thinking loudly, if we inhibit lock creation, what's your opinion on the
unlock warning (for the generic local case)?

IMHO if lock creation is inhibited, we could still attempt to remove the
lock to keep the old behavior, but then the warning shouldn't be
generated as you don't expect the lock to exist in the normal case.

bug#62614: Tramp attempts to remove lock file with 'remote-file-name-inhibit-locks t

[Michael Albinus]

Yuri D'Elia <wavexx@thregr.org> writes:

Hi Yuri,

>> I fully agree with you that it should follow the behavior of local files
>> too, so I wouldn't change anything here _just_ for tramp.
>
> Thinking loudly, if we inhibit lock creation, what's your opinion on the
> unlock warning (for the generic local case)?
>
> IMHO if lock creation is inhibited, we could still attempt to remove the
> lock to keep the old behavior, but then the warning shouldn't be
> generated as you don't expect the lock to exist in the normal case.

That might be an option. But it wouldn't fix your use case, where you
try to avoid the file locking machinery for remote files at all.

Best regards, Michael.

bug#62614: Tramp attempts to remove lock file with 'remote-file-name-inhibit-locks t

[Yuri D'Elia]

On Mon, Apr 03 2023, Michael Albinus wrote:
>> IMHO if lock creation is inhibited, we could still attempt to remove the
>> lock to keep the old behavior, but then the warning shouldn't be
>> generated as you don't expect the lock to exist in the normal case.
>
> That might be an option. But it wouldn't fix your use case, where you
> try to avoid the file locking machinery for remote files at all.

No, but it would fix an unexpected warning for both tramp and local
files, which I think is beneficial.

On Mon, Apr 03 2023, Eli Zaretskii wrote:
> How about trying to remove the lock file, but if creation of lock
> files is disabled, suppressing the warning?

As above.

bug#62614: Tramp attempts to remove lock file with 'remote-file-name-inhibit-locks t

[Michael Albinus]

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

Yuri D'Elia <wavexx@thregr.org> writes:

Hi Yuri & Eli,

> On Mon, Apr 03 2023, Michael Albinus wrote:
>>> IMHO if lock creation is inhibited, we could still attempt to remove the
>>> lock to keep the old behavior, but then the warning shouldn't be
>>> generated as you don't expect the lock to exist in the normal case.
>>
>> That might be an option. But it wouldn't fix your use case, where you
>> try to avoid the file locking machinery for remote files at all.
>
> No, but it would fix an unexpected warning for both tramp and local
> files, which I think is beneficial.
>
> On Mon, Apr 03 2023, Eli Zaretskii wrote:
>> How about trying to remove the lock file, but if creation of lock
>> files is disabled, suppressing the warning?
>
> As above.

See appended patch. Shall it go to the emacs-29 or master branch?

Best regards, Michael.


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

diff --git a/lisp/files.el b/lisp/files.el
index d325729bf4d..216d31c737b 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -555,7 +555,7 @@ lock-file-name-transforms
   :version "28.1")

 (defcustom remote-file-name-inhibit-locks nil
-  "Whether to use file locks for remote files."
+  "Whether to create file locks for remote files."
   :group 'files
   :version "28.1"
   :type 'boolean)
diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index df2f0850b83..4180a32b6d9 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -4585,7 +4585,8 @@ tramp-handle-unlock-file
     (condition-case err
         (delete-file lockname)
       ;; `userlock--handle-unlock-error' exists since Emacs 28.1.
-      (error (tramp-compat-funcall 'userlock--handle-unlock-error err)))))
+      (error (and (not (bound-and-true-p remote-file-name-inhibit-locks))
+                  (tramp-compat-funcall 'userlock--handle-unlock-error err))))))

 (defun tramp-handle-load (file &optional noerror nomessage nosuffix must-suffix)
   "Like `load' for Tramp files."
diff --git a/lisp/userlock.el b/lisp/userlock.el
index 61f061d3e54..562bc0a0a9f 100644
--- a/lisp/userlock.el
+++ b/lisp/userlock.el
@@ -206,11 +206,12 @@ ask-user-about-supersession-help
 ;;;###autoload
 (defun userlock--handle-unlock-error (error)
   "Report an ERROR that occurred while unlocking a file."
-  (display-warning
-   '(unlock-file)
-   ;; There is no need to explain that this is an unlock error because
-   ;; ERROR is a `file-error' condition, which explains this.
-   (message "%s, ignored" (error-message-string error))
-   :warning))
+  (when create-lockfiles
+    (display-warning
+     '(unlock-file)
+     ;; There is no need to explain that this is an unlock error because
+     ;; ERROR is a `file-error' condition, which explains this.
+     (message "%s, ignored" (error-message-string error))
+     :warning)))

 ;;; userlock.el ends here

bug#62614: Tramp attempts to remove lock file with 'remote-file-name-inhibit-locks t

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: Eli Zaretskii <eliz@gnu.org>,  62614@debbugs.gnu.org
> Date: Mon, 03 Apr 2023 18:32:25 +0200
> 
> > On Mon, Apr 03 2023, Michael Albinus wrote:
> >>> IMHO if lock creation is inhibited, we could still attempt to remove the
> >>> lock to keep the old behavior, but then the warning shouldn't be
> >>> generated as you don't expect the lock to exist in the normal case.
> >>
> >> That might be an option. But it wouldn't fix your use case, where you
> >> try to avoid the file locking machinery for remote files at all.
> >
> > No, but it would fix an unexpected warning for both tramp and local
> > files, which I think is beneficial.
> >
> > On Mon, Apr 03 2023, Eli Zaretskii wrote:
> >> How about trying to remove the lock file, but if creation of lock
> >> files is disabled, suppressing the warning?
> >
> > As above.
> 
> See appended patch.

LGTM, thanks.

> Shall it go to the emacs-29 or master branch?

Master, please.

bug#62614: Tramp attempts to remove lock file with 'remote-file-name-inhibit-locks t

[Yuri D'Elia]

On Mon, Apr 03 2023, Eli Zaretskii wrote:
>> > On Mon, Apr 03 2023, Eli Zaretskii wrote:
>> >> How about trying to remove the lock file, but if creation of lock
>> >> files is disabled, suppressing the warning?
>> >
>> > As above.
>>
>> See appended patch.
>
> LGTM, thanks.
>
>> Shall it go to the emacs-29 or master branch?
>
> Master, please.

As for this specific bug report, I'd close it as "fixed".
The spurious warning was definitely my main complaint.

As for the extra attempt to remove the remote file, I can't think of
anything better at this moment than manually fset'ing the function.

bug#62614: Tramp attempts to remove lock file with 'remote-file-name-inhibit-locks t

[Michael Albinus]

Version: 30.1

Yuri D'Elia <wavexx@thregr.org> writes:

Hi Yuri,

>>> >> How about trying to remove the lock file, but if creation of lock
>>> >> files is disabled, suppressing the warning?
>>> >
>>> > As above.
>>>
>>> See appended patch.
>>
>> LGTM, thanks.
>>
>>> Shall it go to the emacs-29 or master branch?
>>
>> Master, please.
>
> As for this specific bug report, I'd close it as "fixed".
> The spurious warning was definitely my main complaint.

Pushed to master, closing.

Best regards, Michael.

bug#62614: Tramp attempts to remove lock file with 'remote-file-name-inhibit-locks t

[Eli Zaretskii]

> Cc: 62614@debbugs.gnu.org
> From: Michael Albinus <michael.albinus@gmx.de>
> Date: Mon, 03 Apr 2023 11:54:08 +0200
> 
> Yuri D'Elia <wavexx@thregr.org> writes:
> 
> Hi Yuri,
> 
> > Mmmh, maybe it should be mentioned explicitly. For me, "inhibit-locks"
> > meant inhibiting both creation and removal.
> >
> > But even for local files, why the unlock is done? For cleanup?
> 
> Don't know, this behavior is "since ever" (I don't remember a change here).
> 
> With Tramp I'm agnostic. Tramp follows the behavior of local files. It
> could do it differently, the local behavior could change, whatever.
> 
> Opinions?

How about trying to remove the lock file, but if creation of lock
files is disabled, suppressing the warning?

bug#62614: Tramp attempts to remove lock file with 'remote-file-name-inhibit-locks t

[Eli Zaretskii]

> Cc: 62614@debbugs.gnu.org
> From: Yuri D'Elia <wavexx@thregr.org>
> Date: Mon, 03 Apr 2023 11:19:19 +0200
> 
> On Mon, Apr 03 2023, Michael Albinus wrote:
> > Tramp behaves like with local lock files. Imagine, you have a local file
> > ~/test with a lock file (a symbolic link) ~/.#test -> something. Set
> > create-lock-files to nil. Start editing the local file ~/test, and save
> > it. The local lock file ~/.#test is removed.
> >
> > And that's also what's documented, see (info "(elisp) File Locks")
> >
> >  -- User Option: remote-file-name-inhibit-locks
> >      You can prevent the creation of remote lock files by setting the
> >      variable ‘remote-file-name-inhibit-locks’ to ‘t’.
> >
> >
> > It speaks only about creation of remote lock files, and not about
> > removal.
> 
> Mmmh, maybe it should be mentioned explicitly. For me, "inhibit-locks"
> meant inhibiting both creation and removal.
> 
> But even for local files, why the unlock is done? For cleanup?

Yes, because a lock file could be created before the option to avoid
them was set.