bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

[Jay Berkenbilt]

 X-Debbugs-CC: ejb@ql.org

This bug is reproducible with emacs -Q.

emacs 28.0.91 and emacs-27.2, with -Q, behave identically and correctly in the following scenario:

* outside of emacs: rm /tmp/a; echo one > /tmp/a
* In a fresh emacs -Q, C-x C-f /tmp/a
* outside of emacs: touch /tmp/a
* In /tmp/a in emacs, it is possible to edit the file freely and save, which is correct since the contents have not changed
* In /tmp/a in emacs, save any changes
* outside of emacs: echo test >> /tmp/a
* In /tmp/a in emacs: try editing and get an immediate prompt that the file has changed.

With emacs 27.2, (setq create-lockfiles nil) does not change the above behavior.

With emacs 28.0.91, (setq create-lockfiles nil) results in the following changes in behavior:

* outside of emacs: rm /tmp/a; echo one > /tmp/a
* In a fresh emacs -Q, C-x C-f /tmp/a
* outside of emacs: touch /tmp/a
* In /tmp/a in emacs, it is possible to edit the file freely, but **SAVING THE FILE RESULTS IN A PROMPT** (a has changed since visited or saved...) even though the *contents* of the file have not changed (only its modification time)
  * Say no to the prompt and M-x revert-buffer, accepting the prompt to revert the modified buffer
* outside of emacs: echo test >> /tmp/a
* In /tmp/a in emacs: try editing. **NO PROMPT IS GIVEN; EDITING CAN BE DONE FREELY**
* Try to save the file. Get the "a has changed since visited or saved" prompt at this time

Bottom line: in emacs 27.2, setting create-lockfiles to nil does not change the behavior. In emacs 28.0.91, it does. I also noticed this in 28.0.90 but hadn't had time to figure out how to reproduce it without my customizations.

I saw 'lock-file-name-transforms' in NEWS. I disable create-lockfiles because I never run emacs on a multi-user system with my default settings, and the dangling symlinks confuse too many systems. It may be that, if emacs-major-version is >= 28, I can use lock-file-name-transforms to cause lock files to save in a separate directory, but I find that I don't need lockfiles at all with the default behavior of emacs 27 and 28 regarding change detection, at least my on Ubuntu platform. 

Is this change intentional? If so, can I configure something to go back to the old behavior? It seems like a bug to me -- I don't see why create-lockfiles should have anything to do with this behavior, and it did not in 27.2. I've been running gnu emacs since version 18 in 1987 and have seen the evolution of lock files over time. I was euphoric when emacs started being able to notice when an update to a file's modification time didn't change its contents. It makes it so much easier to do things like git rebase, which I do many times a day. Thanks.

The text below came from M-x report-emacs-bug on emacs-28.0.91 -Q

In GNU Emacs 28.0.91 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.20, cairo version 1.16.0)
of 2022-01-12 built on jblin
Windowing system distributor 'The X.Org Foundation', version 11.0.12013000
System Description: Ubuntu 20.04.3 LTS

Configured using:
'configure --prefix=/usr/local/emacs-28.0.91'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS
X11 XDBE XIM XPM GTK3 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
rfc822 mml mml-sec epa derived epg rfc6068 epg-config gnus-util rmail
rmail-loaddefs auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache json map text-property-search time-date
subr-x seq byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs
cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
iso-transl tooltip eldoc paren electric uniquify ediff-hook vc-hooks
lisp-float-type elisp-mode mwheel term/x-win x-win term/common-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock
font-lock syntax font-core term/tty-colors frame minibuffer cl-generic
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice button loaddefs faces cus-face macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads dbusbind
inotify lcms2 dynamic-setting system-font-setting font-render-setting
cairo move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 51426 5400)
(symbols 48 6608 1)
(strings 32 18372 1964)
(string-bytes 1 610917)
(vectors 16 14475)
(vector-slots 8 191331 9434)
(floats 8 21 35)
(intervals 56 200 0)
(buffers 992 10))

bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

[Eli Zaretskii]

> Date: Wed, 12 Jan 2022 09:30:07 -0500
> From: "Jay Berkenbilt" <ejb@ql.org>
> 
> Bottom line: in emacs 27.2, setting create-lockfiles to nil does not change the behavior. In emacs 28.0.91, it does. I also noticed this in 28.0.90 but hadn't had time to figure out how to reproduce it without my customizations.

You seem to assume that create-lockfiles is _only_ about the creation
of the lockfiles.  But that's not true: the variable is a misnomer,
and it actually controls the entire functionality of preventing
editing collisions.  Including the test for the file being modified
behind Emacs's back.  The doc string says:

  Non-nil means use lockfiles to avoid editing collisions.

> Is this change intentional? If so, can I configure something to go
> back to the old behavior?

My suggestion is to stop setting create-lockfiles to a nil value.  Why
is the non-nil value a problem?

bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

[Jay Berkenbilt]

On Wed, Jan 12, 2022, at 12:26 PM, Eli Zaretskii wrote:
> > Date: Wed, 12 Jan 2022 09:30:07 -0500
> > From: "Jay Berkenbilt" <ejb@ql.org>
> > 
> > Bottom line: in emacs 27.2, setting create-lockfiles to nil does not change the behavior. In emacs 28.0.91, it does. I also noticed this in 28.0.90 but hadn't had time to figure out how to reproduce it without my customizations.
> 
> You seem to assume that create-lockfiles is _only_ about the creation
> of the lockfiles.  But that's not true: the variable is a misnomer,
> and it actually controls the entire functionality of preventing
> editing collisions.  Including the test for the file being modified
> behind Emacs's back.  The doc string says:
> 
>   Non-nil means use lockfiles to avoid editing collisions.

I interpreted this to mean that this was the method of preventing collisions,
as opposed to some other method.

> 
> > Is this change intentional? If so, can I configure something to go
> > back to the old behavior?
> 
> My suggestion is to stop setting create-lockfiles to a nil value.  Why
> is the non-nil value a problem?

Emacs lockfiles are dangling symbolic links. Some tools and systems don't
like those. For example, I was working on a learning project to teach myself
modern web development and was using tools that do hot reload, scanning
directories for tests. Every time I started editing, it would pick up the lock file
and give an error. You could make an argument that it is a bug in the tool,
but the cross section of people who live and breathe in emacs and who are
working on front-end web development seems to be very small, and there's
little incentive for them to add code that ignores dangling symbolic links. Most
people consider those to be errors. I get why it's done though -- I have
implemented symlink-based lock files myself since they are easier to make 
atomic, especially when network file systems are in play.

But, from a pragmatic standpoint, I don't need them, and I'd like to turn them off,
and it used to work and no longer does. This is why I'm testing 28 -- to catch
things like this.

bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

[Eli Zaretskii]

> Date: Wed, 12 Jan 2022 15:07:32 -0500
> From: "Jay Berkenbilt" <ejb@ql.org>
> Cc: 53207@debbugs.gnu.org
> 
> > My suggestion is to stop setting create-lockfiles to a nil value.  Why
> > is the non-nil value a problem?
> 
> Emacs lockfiles are dangling symbolic links. Some tools and systems don't
> like those.

Isn't that the reason we introduced lock-file-name-transforms?

bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

[Jay Berkenbilt]

On Wed, Jan 12, 2022, at 3:45 PM, Eli Zaretskii wrote:
> > Date: Wed, 12 Jan 2022 15:07:32 -0500
> > From: "Jay Berkenbilt" <ejb@ql.org>
> > Cc: 53207@debbugs.gnu.org
> > 
> > > My suggestion is to stop setting create-lockfiles to a nil value.  Why
> > > is the non-nil value a problem?
> > 
> > Emacs lockfiles are dangling symbolic links. Some tools and systems don't
> > like those.
> 
> Isn't that the reason we introduced lock-file-name-transforms?

Perhaps so, but this misses the point. I am pointing out at there is an undocumented,
perhaps undesirable change of behavior that needs to be either fixed or documented.
If the change of behavior is intentional, then it should be documented. That said, I think
the old behavior made more sense. The old behavior seems to be that setting 
create-lockfiles to nil just makes emacs stop creating lockfiles. The new behavior seems
to be that it does other things as well. It's odd for you to tell me that I shouldn't use an
option that is provided, particularly when it used to do exactly what I wanted it to do.

All that said, if this is intentional and there is some reason to decrease the functionality
of emacs by making it impossible to turn off lockfile creation without this other side effect,
then I will add conditional code in my .emacs to turn off lockfiles if 
(not (boundp 'lock-file-name-transforms)).

It just seems strange that emacs is perfectly capable of detecting when a file was changed
outside of emacs without a lockfile but doesn't do this check if it's not also creating lockfiles.

bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

[Eli Zaretskii]

> Date: Wed, 12 Jan 2022 16:35:15 -0500
> From: "Jay Berkenbilt" <ejb@ql.org>
> Cc: 53207@debbugs.gnu.org
> 
> > > > My suggestion is to stop setting create-lockfiles to a nil value.  Why
> > > > is the non-nil value a problem?
> > > 
> > > Emacs lockfiles are dangling symbolic links. Some tools and systems don't
> > > like those.
> > 
> > Isn't that the reason we introduced lock-file-name-transforms?
> 
> Perhaps so, but this misses the point.

My point was to say that if you want collision detection, you should
turn on create-lockfiles.  You said this could cause problems with
some tools, and I then pointed out that Emacs 28 has a new feature to
help you solve that side effect, so that you could use locking again.

> I am pointing out at there is an undocumented,
> perhaps undesirable change of behavior that needs to be either fixed or documented.

I understand, but that's a separate, albeit related, issue.  I was
trying to help you to get collision detection back.  Your original
report seemed to ask how to do that, so I tried to answer that part.

> If the change of behavior is intentional, then it should be documented.

We are discussing this, and will document if that's the conclusion.

> It just seems strange that emacs is perfectly capable of detecting when a file was changed
> outside of emacs without a lockfile but doesn't do this check if it's not also creating lockfiles.

Once again, this is not about the lockfiles, this is about the entire
feature of detection of editing collisions.  The variable's name is a
misnomer.

bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

[Jay Berkenbilt]

On Thu, Jan 13, 2022, at 1:43 AM, Eli Zaretskii wrote:
> > Date: Wed, 12 Jan 2022 16:35:15 -0500
> > From: "Jay Berkenbilt" <ejb@ql.org>
> > Cc: 53207@debbugs.gnu.org
> > 
> > > > > My suggestion is to stop setting create-lockfiles to a nil value.  Why
> > > > > is the non-nil value a problem?
> > > > 
> > > > Emacs lockfiles are dangling symbolic links. Some tools and systems don't
> > > > like those.
> > > 
> > > Isn't that the reason we introduced lock-file-name-transforms?
> > 
> > Perhaps so, but this misses the point.
> 
> My point was to say that if you want collision detection, you should
> turn on create-lockfiles.  You said this could cause problems with
> some tools, and I then pointed out that Emacs 28 has a new feature to
> help you solve that side effect, so that you could use locking again.

Thank you for clarifying.

I have added the following to emacs initialization code. This is satisfying my
need to avoid dangling links in places that confuse tools.

=== 8< ===
  ;; I never use emacs with my own configuration in a multi-user
  ;; environment, and lockfiles are dangling links that confuse some
  ;; tools. Conflict detection works prior to 28.1 with
  ;; create-lockfiles nil. Starting in 28.1, it no longer works with
  ;; create-lockfiles nil, but lock-file-name-transforms can be used
  ;; to create the dangling links somewhere else. See
  ;; http://debbugs.gnu.org/cgi/bugreport.cgi?bug=53207
  (if (boundp 'lock-file-name-transforms)
      (let* ((lockdir (expand-file-name "~/.emacs.d/lockfiles"))
             (replacement (concat lockdir "/\\2")))
        (make-directory lockdir t)
        (setq lock-file-name-transforms
              `(("\\`\\([^/]*/\\)*\\([^/]*\\)\\'" ,replacement t))
        )
      )
    (setq create-lockfiles nil)
  )
=== 8< ===


> > It just seems strange that emacs is perfectly capable of detecting when a file was changed
> > outside of emacs without a lockfile but doesn't do this check if it's not also creating lockfiles.
> 
> Once again, this is not about the lockfiles, this is about the entire
> feature of detection of editing collisions.  The variable's name is a
> misnomer.

In addition to any other doc clarifications, I'd suggest the fact that create-lockfiles is a misnomer
be explained in the help for create-lockfiles.

I will add some thoughts to a different part of the thread. Thanks!

bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

[Glenn Morris]

Probably a consequence of 9ce6541ac9, specifically:

  * src/filelock.c (lock_file): Don't check create_lockfiles.

which would seem to mean that ask-user-about-supersession-threat
is no longer called when create-lockfiles is nil.
Was this intentional?

bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

[Philipp Stephani]

Am Mi., 12. Jan. 2022 um 19:16 Uhr schrieb Glenn Morris <rgm@gnu.org>:
>
>
> Probably a consequence of 9ce6541ac9, specifically:
>
>   * src/filelock.c (lock_file): Don't check create_lockfiles.
>
> which would seem to mean that ask-user-about-supersession-threat
> is no longer called when create-lockfiles is nil.
> Was this intentional?


If it's intentional, then it should at least be documented in NEWS,
since it's a pretty big change.

bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

[Eli Zaretskii]

> From: Glenn Morris <rgm@gnu.org>
> Date: Wed, 12 Jan 2022 13:13:40 -0500
> Cc: michael.albinus@gmx.de, 53207@debbugs.gnu.org
> 
> Probably a consequence of 9ce6541ac9, specifically:
> 
>   * src/filelock.c (lock_file): Don't check create_lockfiles.

Actually, the more relevant part is this:

    (Flock_file): Check create_lockfiles.

which did

  -  CHECK_STRING (file);
  -  lock_file (file);
  +#ifndef MSDOS
  +  /* Don't do locking if the user has opted out.  */
  +  if (create_lockfiles)
  +    {
  +      CHECK_STRING (file);
  +      lock_file (file);
  +    }
  +#endif /* MSDOS */

> which would seem to mean that ask-user-about-supersession-threat
> is no longer called when create-lockfiles is nil.
> Was this intentional?

Michael, can you please chime in?  The long discussion we had back
then doesn't seem to mention this aspect, or maybe I'm missing
something?

We should either document this change (if we think what we have now is
the intended behavior), or we should move the call to
userlock--ask-user-about-supersession-threat into Flock_file if it's
unintended.

Personally, I prefer the former, since lock_file accesses the lock
file, which doesn't make a lot of sense if the user opted out of the
feature.  But that's me.

Lars, WDYT?

Thanks.

bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

[Jay Berkenbilt]

On Thu, Jan 13, 2022, at 5:54 AM, Eli Zaretskii wrote:
> > From: Glenn Morris <rgm@gnu.org>
> > Date: Wed, 12 Jan 2022 13:13:40 -0500
> > Cc: michael.albinus@gmx.de, 53207@debbugs.gnu.org
> > 
> > Probably a consequence of 9ce6541ac9, specifically:
> > 
> >   * src/filelock.c (lock_file): Don't check create_lockfiles.
> 
> Actually, the more relevant part is this:
> 
>     (Flock_file): Check create_lockfiles.
> 
> which did
> 
>   -  CHECK_STRING (file);
>   -  lock_file (file);
>   +#ifndef MSDOS
>   +  /* Don't do locking if the user has opted out.  */
>   +  if (create_lockfiles)
>   +    {
>   +      CHECK_STRING (file);
>   +      lock_file (file);
>   +    }
>   +#endif /* MSDOS */
> 
> > which would seem to mean that ask-user-about-supersession-threat
> > is no longer called when create-lockfiles is nil.
> > Was this intentional?
> 
> Michael, can you please chime in?  The long discussion we had back
> then doesn't seem to mention this aspect, or maybe I'm missing
> something?
> 
> We should either document this change (if we think what we have now is
> the intended behavior), or we should move the call to
> userlock--ask-user-about-supersession-threat into Flock_file if it's
> unintended.
> 
> Personally, I prefer the former, since lock_file accesses the lock
> file, which doesn't make a lot of sense if the user opted out of the
> feature.  But that's me.

For my edification, can you explain how the 27.2 behavior of noticing
when a file's contents had changed immediately is not adequate without
lockfiles? Is it that it doesn't work on some platforms, it doesn't
work reliably with a network file system, etc.?

It seems to me that there are two separate issues here. A lock file
would enable you to immediately notice if a user on a *different
system* is in the process of editing a file and has unsaved changes.
On the other hand, the other behavior I'm talking about allows you to
notice immediately when you begin editing if the file on disk has
become out of sync with the buffer contents. These seem like two
different things, both of which are valuable. I can't see how you
would ever want to disable noticing if the file's contents on disk
differ from the buffer, but if you never edit files with emacs on a
multi-user system, then I don't see why you would care about
lockfiles.

All that said, I'm perfectly happy for my own purposes to set
lock-file-name-transforms...I'm just trying to contribute a fresh,
outside perspective (from someone who has used emacs for over 30
years and has a made a career of software development, including open
source software) to the discussion.

I trust that everyone is aware of the issue and will come to a good
resolution. Thanks! I'm eager to hear the other opinions, even though
I have no "vote" in the outcome.

bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

[Philipp Stephani]

Am Do., 13. Jan. 2022 um 14:13 Uhr schrieb Jay Berkenbilt <ejb@ql.org>:
>
>
>
> On Thu, Jan 13, 2022, at 5:54 AM, Eli Zaretskii wrote:
> > > From: Glenn Morris <rgm@gnu.org>
> > > Date: Wed, 12 Jan 2022 13:13:40 -0500
> > > Cc: michael.albinus@gmx.de, 53207@debbugs.gnu.org
> > >
> > > Probably a consequence of 9ce6541ac9, specifically:
> > >
> > >   * src/filelock.c (lock_file): Don't check create_lockfiles.
> >
> > Actually, the more relevant part is this:
> >
> >     (Flock_file): Check create_lockfiles.
> >
> > which did
> >
> >   -  CHECK_STRING (file);
> >   -  lock_file (file);
> >   +#ifndef MSDOS
> >   +  /* Don't do locking if the user has opted out.  */
> >   +  if (create_lockfiles)
> >   +    {
> >   +      CHECK_STRING (file);
> >   +      lock_file (file);
> >   +    }
> >   +#endif /* MSDOS */
> >
> > > which would seem to mean that ask-user-about-supersession-threat
> > > is no longer called when create-lockfiles is nil.
> > > Was this intentional?
> >
> > Michael, can you please chime in?  The long discussion we had back
> > then doesn't seem to mention this aspect, or maybe I'm missing
> > something?
> >
> > We should either document this change (if we think what we have now is
> > the intended behavior), or we should move the call to
> > userlock--ask-user-about-supersession-threat into Flock_file if it's
> > unintended.
> >
> > Personally, I prefer the former, since lock_file accesses the lock
> > file, which doesn't make a lot of sense if the user opted out of the
> > feature.  But that's me.
>
> For my edification, can you explain how the 27.2 behavior of noticing
> when a file's contents had changed immediately is not adequate without
> lockfiles? Is it that it doesn't work on some platforms, it doesn't
> work reliably with a network file system, etc.?
>
> It seems to me that there are two separate issues here. A lock file
> would enable you to immediately notice if a user on a *different
> system* is in the process of editing a file and has unsaved changes.
> On the other hand, the other behavior I'm talking about allows you to
> notice immediately when you begin editing if the file on disk has
> become out of sync with the buffer contents. These seem like two
> different things, both of which are valuable. I can't see how you
> would ever want to disable noticing if the file's contents on disk
> differ from the buffer, but if you never edit files with emacs on a
> multi-user system, then I don't see why you would care about
> lockfiles.

I fully agree.

bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

[Eli Zaretskii]

> Date: Thu, 13 Jan 2022 08:11:34 -0500
> From: "Jay Berkenbilt" <ejb@ql.org>
> Cc: 53207@debbugs.gnu.org
> 
> For my edification, can you explain how the 27.2 behavior of noticing
> when a file's contents had changed immediately is not adequate without
> lockfiles?

First, Emacs 27 wasn't looking at the file's contents, it was looking
at the file's modification time.

More to the point, the verification of the file's modification time
that is now disabled when create-lockfiles is nil is part of detecting
editing collisions, so when the user opts out of it, it makes sense
not to detect _any_ collisions, including those detectable by
verifying that the file wasn't modified since last visited or saved.
The use of lockfiles is an implementation detail, and the fact that we
only need it for 90% of the feature, not for 100%, is not significant
enough IMO.

> It seems to me that there are two separate issues here. A lock file
> would enable you to immediately notice if a user on a *different
> system* is in the process of editing a file and has unsaved changes.

No, it also works when the same user on the same system edited the
file from another Emacs session.  That is a valid use case: some
people start more than a single Emacs session on the same system.

> On the other hand, the other behavior I'm talking about allows you to
> notice immediately when you begin editing if the file on disk has
> become out of sync with the buffer contents.

That part is done when you save the buffer.  It is unaffected by
create-lockfiles.

bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

[Jay Berkenbilt]

On Thu, Jan 13, 2022, at 9:02 AM, Eli Zaretskii wrote:
> > Date: Thu, 13 Jan 2022 08:11:34 -0500
> > From: "Jay Berkenbilt" <ejb@ql.org>
> > Cc: 53207@debbugs.gnu.org
> > 
> > For my edification, can you explain how the 27.2 behavior of noticing
> > when a file's contents had changed immediately is not adequate without
> > lockfiles?
> 
> First, Emacs 27 wasn't looking at the file's contents, it was looking
> at the file's modification time.

My original recipe for reproducing the issue demonstrated that, after
"touch file", you can continue editing freely and save, but after
changing the contents, you can't. I don't remember when this first
changed, maybe emacs 27 or 26. For ages before that, it was
modification time. I remember noticing when updating the modtime
without changing the content stopped triggering that. I was delighted.

It is definitely the case that just updating the modification time on
emacs 27.2 does not trigger this. You can try it. In emacs -Q, edit a
file and save. From the shell, touch the file. No continue editing the
file and save again. No warning. At least this is the case on my
Ubuntu Linux 20.04 system with emacs compiled from source.

> > It seems to me that there are two separate issues here. A lock file
> > would enable you to immediately notice if a user on a *different
> > system* is in the process of editing a file and has unsaved changes.
> 
> No, it also works when the same user on the same system edited the
> file from another Emacs session.  That is a valid use case: some
> people start more than a single Emacs session on the same system.

Granted. Of course it doesn't protect against another very common use
case, which is people opening the same file in emacs and
simultaneously in something like VS Code or another IDE. I know
developers that work this way day in and day out -- they use emacs for
most of their editing but hop over to an IDE to take advantage of
project-wide integrations, better test integration, a more advanced
debugger, or better out-of-the-box functionality with their
programming language or environment of choice. So lock files remain a
solution that only works in an emacs-only environment, while noticing
that the file's modification time has changed is universal, and
noticing that a file's content has changed is a great advancement over
just noticing modtime since it allows for workflows like git rebase.

> > On the other hand, the other behavior I'm talking about allows you to
> > notice immediately when you begin editing if the file on disk has
> > become out of sync with the buffer contents.
> 
> That part is done when you save the buffer.  It is unaffected by
> create-lockfiles.

It is also done when you start editing a buffer, as shown in my original
message. Really. Try it.

bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

[Michael Albinus]

"Jay Berkenbilt" <ejb@ql.org> writes:

Hi Jay,

> On Thu, Jan 13, 2022, at 9:02 AM, Eli Zaretskii wrote:
>> > Date: Thu, 13 Jan 2022 08:11:34 -0500
>> > From: "Jay Berkenbilt" <ejb@ql.org>
>> > Cc: 53207@debbugs.gnu.org
>> > 
>> > For my edification, can you explain how the 27.2 behavior of noticing
>> > when a file's contents had changed immediately is not adequate without
>> > lockfiles?
>> 
>> First, Emacs 27 wasn't looking at the file's contents, it was looking
>> at the file's modification time.
>
> My original recipe for reproducing the issue demonstrated that, after
> "touch file", you can continue editing freely and save, but after
> changing the contents, you can't. I don't remember when this first
> changed, maybe emacs 27 or 26. For ages before that, it was
> modification time. I remember noticing when updating the modtime
> without changing the content stopped triggering that. I was delighted.
>
> It is definitely the case that just updating the modification time on
> emacs 27.2 does not trigger this. You can try it. In emacs -Q, edit a
> file and save. From the shell, touch the file. No continue editing the
> file and save again. No warning. At least this is the case on my
> Ubuntu Linux 20.04 system with emacs compiled from source.

Same here. In lock_file of Emacs 27, there is the check

--8<---------------cut here---------------start------------->8---
    if (!NILP (subject_buf)
	&& NILP (Fverify_visited_file_modtime (subject_buf))
	&& !NILP (Ffile_exists_p (fn)))
      call1 (intern ("userlock--ask-user-about-supersession-threat"), fn);
--8<---------------cut here---------------end--------------->8---

It checks the file modification time. But then, if changed, it calls
userlock--ask-user-about-supersession-threat, which also checks the file
contents before warning. Therefore, a simple touch doesn't trigger the
user question.

>> > It seems to me that there are two separate issues here. A lock file
>> > would enable you to immediately notice if a user on a *different
>> > system* is in the process of editing a file and has unsaved changes.
>> 
>> No, it also works when the same user on the same system edited the
>> file from another Emacs session.  That is a valid use case: some
>> people start more than a single Emacs session on the same system.
>
> Granted. Of course it doesn't protect against another very common use
> case, which is people opening the same file in emacs and
> simultaneously in something like VS Code or another IDE. I know
> developers that work this way day in and day out -- they use emacs for
> most of their editing but hop over to an IDE to take advantage of
> project-wide integrations, better test integration, a more advanced
> debugger, or better out-of-the-box functionality with their
> programming language or environment of choice. So lock files remain a
> solution that only works in an emacs-only environment, while noticing
> that the file's modification time has changed is universal, and
> noticing that a file's content has changed is a great advancement over
> just noticing modtime since it allows for workflows like git rebase.
>
>> > On the other hand, the other behavior I'm talking about allows you to
>> > notice immediately when you begin editing if the file on disk has
>> > become out of sync with the buffer contents.
>> 
>> That part is done when you save the buffer.  It is unaffected by
>> create-lockfiles.
>
> It is also done when you start editing a buffer, as shown in my original
> message. Really. Try it.

Sure. That's because there's no visited file modification time yet for
that buffer.

In Emacs 28, the check above has been extended to

--8<---------------cut here---------------start------------->8---
  if (!NILP (subject_buf)
      && NILP (Fverify_visited_file_modtime (subject_buf))
      && !NILP (Ffile_exists_p (fn))
      && current_lock_owner (NULL, lfname) != I_OWN_IT)
    call1 (intern ("userlock--ask-user-about-supersession-threat"), fn);
--8<---------------cut here---------------end--------------->8---

So it checks also the owner of the lock file. This makes only sense, if
create-lockfiles is non-nil; otherwise there is no lock file owner ...

I agree with Eli, that the current behavior in Emacs 28 is
consistent. Since this is an incompatible change, we shall document
it. The Emacs 28 manual says

--8<---------------cut here---------------start------------->8---
   You can prevent the creation of lock files by setting the variable
‘create-lockfiles’ to ‘nil’.  *Caution:* by doing so you will lose the
benefits that this feature provides.
--8<---------------cut here---------------end--------------->8---

Maybe it shall be more explicit saying, that also detection of changing
the modification time is lost when create-lockfiles is nil.

etc/NEWS is silent about this, it should explain this subtle change as well.

Best regards, Michael.

bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: "Eli Zaretskii" <eliz@gnu.org>,  "Glenn Morris" <rgm@gnu.org>,  "Lars
>  Ingebrigtsen" <larsi@gnus.org>,  53207@debbugs.gnu.org
> Date: Fri, 14 Jan 2022 15:26:18 +0100
> 
> So it checks also the owner of the lock file. This makes only sense, if
> create-lockfiles is non-nil; otherwise there is no lock file owner ...
> 
> I agree with Eli, that the current behavior in Emacs 28 is
> consistent. Since this is an incompatible change, we shall document
> it. The Emacs 28 manual says
> 
> --8<---------------cut here---------------start------------->8---
>    You can prevent the creation of lock files by setting the variable
> ‘create-lockfiles’ to ‘nil’.  *Caution:* by doing so you will lose the
> benefits that this feature provides.
> --8<---------------cut here---------------end--------------->8---
> 
> Maybe it shall be more explicit saying, that also detection of changing
> the modification time is lost when create-lockfiles is nil.

We should say there that setting the variable to nil disables the
entire editing collision detection, not just creation of the lock
files.

> etc/NEWS is silent about this, it should explain this subtle change as well.

Right.  Assuming we agree with the current behavior.

Lars, WDYT?

bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

[Glenn Morris]

I find this report very clear (in both recipe and explaining motivation),
but I'm struggling to understand some of the responses to it.

Was it functionally necessary for the addition of lock file transforms
to break the create-lockfiles nil case? If not, why isn't restoring the
functionality being considered?

Conceptually, it is hard for me to see why lockfiles (a system for
detecting editing collisions between Emacs instances) should be tied up
with detecting changes made outside of Emacs.

bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

[Eli Zaretskii]

> From: Glenn Morris <rgm@gnu.org>
> Cc: "Jay Berkenbilt" <ejb@ql.org>,  "Eli Zaretskii" <eliz@gnu.org>,  "Lars Ingebrigtsen" <larsi@gnus.org>,  53207@debbugs.gnu.org
> Date: Fri, 14 Jan 2022 11:11:23 -0500
> 
> why isn't restoring the functionality being considered?

It was mentioned as one of the two possible solutions, so it is being
considered.

bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

[Lars Ingebrigtsen]

Glenn Morris <rgm@gnu.org> writes:

> Was it functionally necessary for the addition of lock file transforms
> to break the create-lockfiles nil case? If not, why isn't restoring the
> functionality being considered?

Oh, is this a fallout from introducing `lock-file-name-transforms'?
Then changing the semantics of `create-lockfiles' was not intended, and
the previous semantics should be restored.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Oh, is this a fallout from introducing `lock-file-name-transforms'?
> Then changing the semantics of `create-lockfiles' was not intended, and
> the previous semantics should be restored.

(I thought this was about not being asked for supersession when doing a
"touch" on the file (without modifying it), but that change was
intended.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Michael Albinus <michael.albinus@gmx.de>,  "Jay Berkenbilt"
>  <ejb@ql.org>,  "Eli Zaretskii" <eliz@gnu.org>,  53207@debbugs.gnu.org
> Date: Sat, 15 Jan 2022 09:06:17 +0100
> 
> Glenn Morris <rgm@gnu.org> writes:
> 
> > Was it functionally necessary for the addition of lock file transforms
> > to break the create-lockfiles nil case? If not, why isn't restoring the
> > functionality being considered?
> 
> Oh, is this a fallout from introducing `lock-file-name-transforms'?

No, I think this change was introduced by support for locking remote
files.

bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

[Eli Zaretskii]

> Date: Sat, 15 Jan 2022 11:23:58 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: rgm@gnu.org, ejb@ql.org, michael.albinus@gmx.de, 53207@debbugs.gnu.org
> 
> > From: Lars Ingebrigtsen <larsi@gnus.org>
> > Cc: Michael Albinus <michael.albinus@gmx.de>,  "Jay Berkenbilt"
> >  <ejb@ql.org>,  "Eli Zaretskii" <eliz@gnu.org>,  53207@debbugs.gnu.org
> > Date: Sat, 15 Jan 2022 09:06:17 +0100
> > 
> > Glenn Morris <rgm@gnu.org> writes:
> > 
> > > Was it functionally necessary for the addition of lock file transforms
> > > to break the create-lockfiles nil case? If not, why isn't restoring the
> > > functionality being considered?
> > 
> > Oh, is this a fallout from introducing `lock-file-name-transforms'?
> 
> No, I think this change was introduced by support for locking remote
> files.

Lars, what shall we do about this bug report?  Should we document the
current behavior, or should we move the call to
userlock--ask-user-about-supersession-threat out of Flock_file?  This
should be resolved before the next pretest, I think.

bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Lars, what shall we do about this bug report?  Should we document the
> current behavior, or should we move the call to
> userlock--ask-user-about-supersession-threat out of Flock_file?  This
> should be resolved before the next pretest, I think.

I think we should restore the old behaviour, so that setting
create-lockfiles to nil doesn't break file change detection.  I think
that means that we have to move
userlock--ask-user-about-supersession-threat out of Flock_file, yes.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: rgm@gnu.org,  ejb@ql.org,  michael.albinus@gmx.de,  53207@debbugs.gnu.org
> Date: Fri, 28 Jan 2022 14:42:19 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Lars, what shall we do about this bug report?  Should we document the
> > current behavior, or should we move the call to
> > userlock--ask-user-about-supersession-threat out of Flock_file?  This
> > should be resolved before the next pretest, I think.
> 
> I think we should restore the old behaviour, so that setting
> create-lockfiles to nil doesn't break file change detection.  I think
> that means that we have to move
> userlock--ask-user-about-supersession-threat out of Flock_file, yes.

Michael, could you please suggest a patch along these lines?  The
changeset which moved userlock--ask-user-about-supersession-threat
into Flock_file and put that under the create-lockfiles condition was
your change to support locking remote files.

bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

Hi Eli,

> Michael, could you please suggest a patch along these lines?  The
> changeset which moved userlock--ask-user-about-supersession-threat
> into Flock_file and put that under the create-lockfiles condition was
> your change to support locking remote files.

Will do. How urgent is this? I'd rather work on it tomorow.

Best regards, Michael.

bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: Lars Ingebrigtsen <larsi@gnus.org>,  rgm@gnu.org,  ejb@ql.org,
>   53207@debbugs.gnu.org
> Date: Fri, 28 Jan 2022 15:56:46 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> Hi Eli,
> 
> > Michael, could you please suggest a patch along these lines?  The
> > changeset which moved userlock--ask-user-about-supersession-threat
> > into Flock_file and put that under the create-lockfiles condition was
> > your change to support locking remote files.
> 
> Will do. How urgent is this? I'd rather work on it tomorow.

Tomorrow is fine, thanks.

bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

Hi everybody,

>> > Michael, could you please suggest a patch along these lines?  The
>> > changeset which moved userlock--ask-user-about-supersession-threat
>> > into Flock_file and put that under the create-lockfiles condition was
>> > your change to support locking remote files.
>>
>> Will do. How urgent is this? I'd rather work on it tomorow.
>
> Tomorrow is fine, thanks.

I've pushed ddba3c3dba to the emacs-28 branch. Tested with

--8<---------------cut here---------------start------------->8---
# make -C test files-tests filelock-tests tramp-tests SELECTOR='"lock"'
--8<---------------cut here---------------end--------------->8---

for regression. I've tested also the recipe of the OP, it works now as
expected (like in Emacs 27.2).

The recipe of the OP does not work yet for remote files. I'll continue
to fix it, but this shouldn't stall the Emacs 28 release, because the
regression tests work also for them. I'll keep the bug open, until this
remote case has been fixed, too.

Feedback, whether it works as expected, most welcome.

Best regards, Michael.

bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: larsi@gnus.org,  rgm@gnu.org,  ejb@ql.org,  53207@debbugs.gnu.org
> Date: Sat, 29 Jan 2022 11:53:55 +0100
> 
> I've pushed ddba3c3dba to the emacs-28 branch. Tested with
> 
> --8<---------------cut here---------------start------------->8---
> # make -C test files-tests filelock-tests tramp-tests SELECTOR='"lock"'
> --8<---------------cut here---------------end--------------->8---
> 
> for regression. I've tested also the recipe of the OP, it works now as
> expected (like in Emacs 27.2).

Thanks!

> The recipe of the OP does not work yet for remote files. I'll continue
> to fix it, but this shouldn't stall the Emacs 28 release, because the
> regression tests work also for them. I'll keep the bug open, until this
> remote case has been fixed, too.

Agreed.

bug#53207: 28.0.91; create-lockfiles nil breaks file change detection

[Michael Albinus]

Version 28.1

Eli Zaretskii <eliz@gnu.org> writes:

Hi,

>> I've pushed ddba3c3dba to the emacs-28 branch. Tested with
>>
>> --8<---------------cut here---------------start------------->8---
>> # make -C test files-tests filelock-tests tramp-tests SELECTOR='"lock"'
>> --8<---------------cut here---------------end--------------->8---
>>
>> for regression. I've tested also the recipe of the OP, it works now as
>> expected (like in Emacs 27.2).

I've added also a new test filelock-tests-detect-external-change on the
master branch, which checks the scenario of this bug. I haven't done
this for the emacs-28 branch, because it would need a slightly different
implementation. Doing the test on the master branch shall suffice, I believe.

>> The recipe of the OP does not work yet for remote files. I'll continue
>> to fix it, but this shouldn't stall the Emacs 28 release, because the
>> regression tests work also for them. I'll keep the bug open, until this
>> remote case has been fixed, too.
>
> Agreed.

Implementation for Tramp, plus a Tramp specific new test, has also been
pushed to the master branch. This is also kept in Tramp's
branch-2-5-stable branch, which means that it will appear with the next
Tramp ELPA package 2.5.2.2, and it will be merged to the emacs-28 branch
after the Emacs 28.1 release.

Closing the bug.

Best regards, Michael.