bug#65609: 30.0.50; inhibit-eol-conversion breaks revert-buffer

bug#65609: 30.0.50; inhibit-eol-conversion breaks revert-buffer

[Andrew Scott]

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

It appears that inhibit-eol-conversion breaks insert-file-contents's
replace functionality when a change was made in the middle of the file
to be reverted as the file contents get duplicated in the buffer. The
issue can be reproduced on a fresh Emacs with a file named bug.el with
contents:

    ;; 1. Launch Emacs: nix run emacs-overlay#emacs-git -- bug.el -Q --load
bug.el
    ;; 2. Outside of Emacs, add text in below empty line:
    ;; 3. In Emacs, revert-buffer

    (setq inhibit-eol-conversion t)

The contents of the buffer after revert-buffer:

    ;; 1. Launch Emacs: nix run emacs-overlay#emacs-git -- bug.el -Q --load
bug.el
    ;; 2. Outside of Emacs, add text in below empty line:
    ;; 3. In Emacs, revert-buffer
    foo
    (setq inhibit-eol-conversion t)

    (setq inhibit-eol-conversion t)

I couldn't reproduce this on Emacs 28.2 (nixpkgs#emacs) or 29.1
(emacs-overlay#emacs-unstable).

In GNU Emacs 30.0.50 (build 1, aarch64-apple-darwin22.6.0, NS
appkit-2299.70 Version 13.5.1 (Build 22G90))
Repository revision: 09351df10a1041d58c7236484f96d6999bfe9cb0
Repository branch: master
Windowing system distributor 'Apple', version 10.3.2299
System Description:  macOS 13.5.1

Configured using:
 'configure
 --prefix=/nix/store/37hr4bsvv94zlh8abxs2bylxn2dj0fix-emacs-git-20230826.0
 --disable-build-details --with-modules --disable-ns-self-contained
 --with-ns --with-native-compilation --with-tree-sitter'

Configured features:
ACL GLIB GMP GNUTLS JPEG JSON LIBXML2 MODULES NATIVE_COMP NOTIFY KQUEUE
NS PDUMPER PNG RSVG SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER
WEBP ZLIB

Important settings:
  value of $EMACSNATIVELOADPATH: /Users/a/.cache/emacs/eln-cache
  value of $LC_COLLATE: C
  value of $LANG: en_GB.UTF-8
  locale-coding-system: utf-8-unix

Major mode: ELisp/d

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
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
epg-config gnus-util time-date mm-decode mm-bodies mm-encode mail-parse
rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045
ietf-drums mm-util mail-prsvr mail-utils compile text-property-search
comint ansi-osc ansi-color ring comp comp-cstr warnings icons subr-x rx
cl-seq cl-macs gv cl-extra help-mode bytecomp byte-compile vc-git
diff-mode easy-mmode vc-dispatcher cl-loaddefs cl-lib rmc iso-transl
tooltip cconv eldoc paren electric uniquify ediff-hook vc-hooks
lisp-float-type elisp-mode mwheel term/ns-win ns-win ucs-normalize
mule-util term/common-win touch-screen 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 nadvice seq simple cl-generic
indonesian philippine 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 abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads kqueue cocoa ns multi-tty
make-network-process native-compile emacs)

Memory information:
((conses 16 153137 16869) (symbols 48 8291 0) (strings 32 26303 2093)
 (string-bytes 1 843959) (vectors 16 18856)
 (vector-slots 8 366824 13619) (floats 8 29 52) (intervals 56 16278 0)
 (buffers 992 15))

[-- Attachment #2: Type: text/html, Size: 4522 bytes --]

bug#65609: 30.0.50; inhibit-eol-conversion breaks revert-buffer

[Eli Zaretskii]

> From: Andrew Scott <andj.scott@gmail.com>
> Date: Tue, 29 Aug 2023 16:45:39 +0100
> 
> It appears that inhibit-eol-conversion breaks insert-file-contents's
> replace functionality when a change was made in the middle of the file
> to be reverted as the file contents get duplicated in the buffer. The
> issue can be reproduced on a fresh Emacs with a file named bug.el with
> contents:
> 
>     ;; 1. Launch Emacs: nix run emacs-overlay#emacs-git -- bug.el -Q --load bug.el
>     ;; 2. Outside of Emacs, add text in below empty line:
>     ;; 3. In Emacs, revert-buffer
> 
>     (setq inhibit-eol-conversion t)
> 
> The contents of the buffer after revert-buffer:
> 
>     ;; 1. Launch Emacs: nix run emacs-overlay#emacs-git -- bug.el -Q --load bug.el
>     ;; 2. Outside of Emacs, add text in below empty line:
>     ;; 3. In Emacs, revert-buffer
>     foo
>     (setq inhibit-eol-conversion t)
> 
>     (setq inhibit-eol-conversion t)

Thanks for an easy test case.  We are reading too much from the file
on disk when reverting in this case.

Po Lu, I think we need the patch below to fix this.  Gerd, does this
solve your problem as well, including the abort during bootstrap?
Herbert, I think this could also be related to the problems you see,
so please try the patch below.

diff --git a/src/fileio.c b/src/fileio.c
index 23e1a83..8919e08 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -4805,7 +4805,7 @@ DEFUN ("insert-file-contents", Finsert_file_contents, Sinsert_file_contents,
 
 	/* 'try' is reserved in some compilers (Microsoft C).  */
 	ptrdiff_t trytry = min (gap_size, READ_BUF_SIZE);
-	if (!NILP (end))
+	if (seekable || !NILP (end))
 	  trytry = min (trytry, total - inserted);
 
 	if (!seekable && NILP (end))

bug#65609: 30.0.50; inhibit-eol-conversion breaks revert-buffer

[Gerd Möllmann]

On 30.08.23 15:26, Eli Zaretskii wrote:
> Po Lu, I think we need the patch below to fix this.  Gerd, does this
> solve your problem as well, including the abort during bootstrap?
> Herbert, I think this could also be related to the problems you see,
> so please try the patch below.
> 
> diff --git a/src/fileio.c b/src/fileio.c
> index 23e1a83..8919e08 100644
> --- a/src/fileio.c
> +++ b/src/fileio.c
> @@ -4805,7 +4805,7 @@ DEFUN ("insert-file-contents", Finsert_file_contents, Sinsert_file_contents,
>   
>   	/* 'try' is reserved in some compilers (Microsoft C).  */
>   	ptrdiff_t trytry = min (gap_size, READ_BUF_SIZE);
> -	if (!NILP (end))
> +	if (seekable || !NILP (end))
>   	  trytry = min (trytry, total - inserted);
>   
>   	if (!seekable && NILP (end))

With this patch, the abort during bootstrap no longer happens.

Thanks!  That was quick ;-).

bug#65609: 30.0.50; inhibit-eol-conversion breaks revert-buffer

[Eli Zaretskii]

> Date: Wed, 30 Aug 2023 15:42:48 +0200
> Cc: 65609@debbugs.gnu.org
> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> 
> On 30.08.23 15:26, Eli Zaretskii wrote:
> > Po Lu, I think we need the patch below to fix this.  Gerd, does this
> > solve your problem as well, including the abort during bootstrap?
> > Herbert, I think this could also be related to the problems you see,
> > so please try the patch below.
> > 
> > diff --git a/src/fileio.c b/src/fileio.c
> > index 23e1a83..8919e08 100644
> > --- a/src/fileio.c
> > +++ b/src/fileio.c
> > @@ -4805,7 +4805,7 @@ DEFUN ("insert-file-contents", Finsert_file_contents, Sinsert_file_contents,
> >   
> >   	/* 'try' is reserved in some compilers (Microsoft C).  */
> >   	ptrdiff_t trytry = min (gap_size, READ_BUF_SIZE);
> > -	if (!NILP (end))
> > +	if (seekable || !NILP (end))
> >   	  trytry = min (trytry, total - inserted);
> >   
> >   	if (!seekable && NILP (end))
> 
> With this patch, the abort during bootstrap no longer happens.

Thanks for testing.

> Thanks!  That was quick ;-).

It was an easy test case and a stunning failure.

bug#65609: 30.0.50; inhibit-eol-conversion breaks revert-buffer

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks for an easy test case.  We are reading too much from the file
> on disk when reverting in this case.
>
> Po Lu, I think we need the patch below to fix this.  Gerd, does this
> solve your problem as well, including the abort during bootstrap?
> Herbert, I think this could also be related to the problems you see,
> so please try the patch below.
>
> diff --git a/src/fileio.c b/src/fileio.c
> index 23e1a83..8919e08 100644
> --- a/src/fileio.c
> +++ b/src/fileio.c
> @@ -4805,7 +4805,7 @@ DEFUN ("insert-file-contents", Finsert_file_contents, Sinsert_file_contents,
>  
>  	/* 'try' is reserved in some compilers (Microsoft C).  */
>  	ptrdiff_t trytry = min (gap_size, READ_BUF_SIZE);
> -	if (!NILP (end))
> +	if (seekable || !NILP (end))
>  	  trytry = min (trytry, total - inserted);
>  
>  	if (!seekable && NILP (end))

I don't believe this should incur any difficulties upon the Android
port, so please proceed with installing it if it works.

bug#65609: 30.0.50; inhibit-eol-conversion breaks revert-buffer

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: Andrew Scott <andj.scott@gmail.com>,  Gerd Möllmann
>  <gerd.moellmann@gmail.com>,  "Herbert J. Skuhra" <herbert@gojira.at>,
>   65609@debbugs.gnu.org
> Date: Wed, 30 Aug 2023 21:43:23 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Thanks for an easy test case.  We are reading too much from the file
> > on disk when reverting in this case.
> >
> > Po Lu, I think we need the patch below to fix this.  Gerd, does this
> > solve your problem as well, including the abort during bootstrap?
> > Herbert, I think this could also be related to the problems you see,
> > so please try the patch below.
> >
> > diff --git a/src/fileio.c b/src/fileio.c
> > index 23e1a83..8919e08 100644
> > --- a/src/fileio.c
> > +++ b/src/fileio.c
> > @@ -4805,7 +4805,7 @@ DEFUN ("insert-file-contents", Finsert_file_contents, Sinsert_file_contents,
> >  
> >  	/* 'try' is reserved in some compilers (Microsoft C).  */
> >  	ptrdiff_t trytry = min (gap_size, READ_BUF_SIZE);
> > -	if (!NILP (end))
> > +	if (seekable || !NILP (end))
> >  	  trytry = min (trytry, total - inserted);
> >  
> >  	if (!seekable && NILP (end))
> 
> I don't believe this should incur any difficulties upon the Android
> port, so please proceed with installing it if it works.

Thanks, installed on master.  I will wait to hear from Andrew before
closing the bug.

bug#65609: 30.0.50; inhibit-eol-conversion breaks revert-buffer

[Eli Zaretskii]

> From: Andrew Scott <andj.scott@gmail.com>
> Date: Sun, 3 Sep 2023 16:16:09 +0100
> 
> Amazing. Thank you 

Thanks, I'm therefore closing this bug.

bug#65609: 30.0.50; inhibit-eol-conversion breaks revert-buffer

[Herbert J. Skuhra]

On Wed, 30 Aug 2023 15:26:35 +0200, Eli Zaretskii wrote:
> 
> > From: Andrew Scott <andj.scott@gmail.com>
> > Date: Tue, 29 Aug 2023 16:45:39 +0100
> > 
> > It appears that inhibit-eol-conversion breaks insert-file-contents's
> > replace functionality when a change was made in the middle of the file
> > to be reverted as the file contents get duplicated in the buffer. The
> > issue can be reproduced on a fresh Emacs with a file named bug.el with
> > contents:
> > 
> >     ;; 1. Launch Emacs: nix run emacs-overlay#emacs-git -- bug.el -Q --load bug.el
> >     ;; 2. Outside of Emacs, add text in below empty line:
> >     ;; 3. In Emacs, revert-buffer
> > 
> >     (setq inhibit-eol-conversion t)
> > 
> > The contents of the buffer after revert-buffer:
> > 
> >     ;; 1. Launch Emacs: nix run emacs-overlay#emacs-git -- bug.el -Q --load bug.el
> >     ;; 2. Outside of Emacs, add text in below empty line:
> >     ;; 3. In Emacs, revert-buffer
> >     foo
> >     (setq inhibit-eol-conversion t)
> > 
> >     (setq inhibit-eol-conversion t)
> 
> Thanks for an easy test case.  We are reading too much from the file
> on disk when reverting in this case.
> 
> Po Lu, I think we need the patch below to fix this.  Gerd, does this
> solve your problem as well, including the abort during bootstrap?
> Herbert, I think this could also be related to the problems you see,
> so please try the patch below.

Thanks. Unfortunately this change does not resolve my issue opening
org-files. I still have to revert 4e8d579f3da.

--
Herbert