GNU bug report logs - #25951
26.0.50; Error when ediffing files that are visited using quoted file names

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 25951 in the body.
You can then email your comments to 25951 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Philipp Stephani <p.stephani2 <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 26.0.50;
 Error when ediffing files that are visited using quoted file names
Date: Fri, 03 Mar 2017 14:56:48 +0100

Assuming /tmp/[ab].py are existing files.

emacs -Q -f server-start /:/tmp/a.py

Then:

emacsclient --create-frame --eval '(ediff "/tmp/a.py" "/tmp/b.py")'

will result un an error:

*ERROR*: Wrong type argument: arrayp, nil

Backtrace is

Debugger entered--Lisp error: (wrong-type-argument arrayp nil)
  file-name-non-special(verify-visited-file-modtime #<buffer a.py>)
  verify-visited-file-modtime(#<buffer a.py>)
  apply(verify-visited-file-modtime #<buffer a.py>)
  tramp-run-real-handler(verify-visited-file-modtime (#<buffer a.py>))
  tramp-file-name-handler(verify-visited-file-modtime #<buffer a.py>)
  verify-visited-file-modtime(#<buffer a.py>)
  find-file-noselect("/tmp/a.py")
  ediff-find-file(file-A buf-A ediff-last-dir-A startup-hooks)
  ediff-files-internal("/tmp/a.py" "/tmp/b.py" nil nil ediff-files)
  ediff("/tmp/a.py" "/tmp/b.py")
  eval((ediff "/tmp/a.py" "/tmp/b.py"))
  server-eval-and-print("(ediff \"/tmp/a.py\" \"/tmp/b.py\")" #<process server <3>>)
  (more uninteresting frames)



In GNU Emacs 26.0.50 (build 10, x86_64-unknown-linux-gnu, GTK+ Version 3.10.8)
 of 2017-03-03 built on localhost
Repository revision: 244de7b0ed3bb23e700c9edef51e413602d8720a
Windowing system distributor 'The X.Org Foundation', version 11.0.11501000
System Description:	Ubuntu 14.04 LTS

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.

Configured using:
 'configure --enable-checking --enable-check-lisp-object-type
 --with-modules 'CFLAGS=-O0 -ggdb3''

Configured features:
XPM JPEG TIFF GIF PNG SOUND GSETTINGS NOTIFY GNUTLS FREETYPE XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 MODULES

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
  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
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message puny seq byte-opt subr-x gv
bytecomp byte-compile cl-extra help-mode cconv cl-loaddefs pcase cl-lib
dired dired-loaddefs format-spec rfc822 mml easymenu mml-sec
password-cache epa derived epg epg-config gnus-util rmail rmail-loaddefs
mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils
mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr
mail-utils time-date mule-util tooltip eldoc electric uniquify
ediff-hook vc-hooks lisp-float-type 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 elisp-mode lisp-mode
prog-mode register page menu-bar rfn-eshadow isearch timer select
scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame 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 charscript case-table epa-hook jka-cmpr-hook help
simple abbrev obarray minibuffer cl-preloaded nadvice loaddefs button
faces cus-face macroexp files text-properties overlay sha1 md5 base64
format env code-pages mule custom widget hashtable-print-readable
backquote inotify dynamic-setting system-font-setting
font-render-setting move-toolbar gtk x-toolkit x multi-tty
make-network-process emacs)

Memory information:
((conses 16 96397 9305)
 (symbols 48 20188 1)
 (miscs 40 39 181)
 (strings 32 17669 3597)
 (string-bytes 1 574142)
 (vectors 16 14048)
 (vector-slots 8 483315 4785)
 (floats 8 48 68)
 (intervals 56 217 0)
 (buffers 976 12)
 (heap 1024 19322 1007))

-- 
Google Germany GmbH
Erika-Mann-Straße 33
80636 München

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle

Diese E-Mail ist vertraulich.  Wenn Sie nicht der richtige Adressat sind,
leiten Sie diese bitte nicht weiter, informieren Sie den Absender und löschen
Sie die E-Mail und alle Anhänge.  Vielen Dank.

This e-mail is confidential.  If you are not the right addressee please do not
forward it, please inform the sender, and please erase this e-mail including
any attachments.  Thanks.

Message #8 received at 25951 <at> debbugs.gnu.org (full text, mbox):

From: Philipp Stephani <p.stephani2 <at> gmail.com>
To: 25951 <at> debbugs.gnu.org
Subject: Re: bug#25951: 26.0.50; Error when ediffing files that are visited
 using quoted file names
Date: Fri, 21 Apr 2017 22:14:25 +0000

[Message part 1 (text/plain, inline)]

Philipp Stephani <p.stephani2 <at> gmail.com> schrieb am Fr., 3. März 2017 um
14:58 Uhr:

>
> Assuming /tmp/[ab].py are existing files.
>
> emacs -Q -f server-start /:/tmp/a.py
>
> Then:
>
> emacsclient --create-frame --eval '(ediff "/tmp/a.py" "/tmp/b.py")'
>
> will result un an error:
>
> *ERROR*: Wrong type argument: arrayp, nil
>
> Backtrace is
>
> Debugger entered--Lisp error: (wrong-type-argument arrayp nil)
>   file-name-non-special(verify-visited-file-modtime #<buffer a.py>)
>   verify-visited-file-modtime(#<buffer a.py>)
>   apply(verify-visited-file-modtime #<buffer a.py>)
>   tramp-run-real-handler(verify-visited-file-modtime (#<buffer a.py>))
>   tramp-file-name-handler(verify-visited-file-modtime #<buffer a.py>)
>   verify-visited-file-modtime(#<buffer a.py>)
>   find-file-noselect("/tmp/a.py")
>   ediff-find-file(file-A buf-A ediff-last-dir-A startup-hooks)
>   ediff-files-internal("/tmp/a.py" "/tmp/b.py" nil nil ediff-files)
>   ediff("/tmp/a.py" "/tmp/b.py")
>   eval((ediff "/tmp/a.py" "/tmp/b.py"))
>   server-eval-and-print("(ediff \"/tmp/a.py\" \"/tmp/b.py\")" #<process
> server <3>>)
>   (more uninteresting frames)
>
>
Patch is attached.

[Message part 2 (text/html, inline)]

[0001-Fix-quoted-files-for-verify-visited-file-modtime.txt (text/plain, attachment)]

Message #11 received at 25951 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Philipp Stephani <p.stephani2 <at> gmail.com>
Cc: 25951 <at> debbugs.gnu.org
Subject: Re: bug#25951: 26.0.50;
 Error when ediffing files that are visited using quoted file names
Date: Fri, 21 Apr 2017 18:50:19 -0400

Philipp Stephani <p.stephani2 <at> gmail.com> writes:

> +       (let ((buffer (current-buffer)))
> +         ;; `unquote-then-quote' is only used for the
> +         ;; `verify-visited-file-modtime' action, which takes a buffer
> +         ;; as only optional argument.
> +         (with-current-buffer (or (car arguments) buffer)
> +           (let ((buffer-file-name (substring buffer-file-name 2)))
> +             ;; Make sure to hide the temporary buffer change from the
> +             ;; underlying operation.
> +             (with-current-buffer buffer
> +               (apply operation arguments))))))

I think this could be simplified by using the buffer-file-name function:

    ;; `unquote-then-quote' is only used for the
    ;; `verify-visited-file-modtime' action, which takes a buffer
    ;; as only optional argument.
    (let ((buffer-file-name
           (substring (buffer-file-name (car arguments)) 2)))
      (apply operation arguments))

Message #14 received at 25951 <at> debbugs.gnu.org (full text, mbox):

From: Philipp Stephani <p.stephani2 <at> gmail.com>
To: npostavs <at> users.sourceforge.net
Cc: 25951 <at> debbugs.gnu.org
Subject: Re: bug#25951: 26.0.50; Error when ediffing files that are visited
 using quoted file names
Date: Sat, 22 Apr 2017 19:43:09 +0000

[Message part 1 (text/plain, inline)]

<npostavs <at> users.sourceforge.net> schrieb am Sa., 22. Apr. 2017 um 00:48 Uhr:

> Philipp Stephani <p.stephani2 <at> gmail.com> writes:
>
> > +       (let ((buffer (current-buffer)))
> > +         ;; `unquote-then-quote' is only used for the
> > +         ;; `verify-visited-file-modtime' action, which takes a buffer
> > +         ;; as only optional argument.
> > +         (with-current-buffer (or (car arguments) buffer)
> > +           (let ((buffer-file-name (substring buffer-file-name 2)))
> > +             ;; Make sure to hide the temporary buffer change from the
> > +             ;; underlying operation.
> > +             (with-current-buffer buffer
> > +               (apply operation arguments))))))
>
> I think this could be simplified by using the buffer-file-name function:
>
>     ;; `unquote-then-quote' is only used for the
>     ;; `verify-visited-file-modtime' action, which takes a buffer
>     ;; as only optional argument.
>     (let ((buffer-file-name
>            (substring (buffer-file-name (car arguments)) 2)))
>       (apply operation arguments))
>

That's not the same, it will set the file name of the wrong buffer.

[Message part 2 (text/html, inline)]

Message #17 received at 25951 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Philipp Stephani <p.stephani2 <at> gmail.com>
Cc: 25951 <at> debbugs.gnu.org
Subject: Re: bug#25951: 26.0.50;
 Error when ediffing files that are visited using quoted file names
Date: Sat, 22 Apr 2017 16:32:40 -0400

Philipp Stephani <p.stephani2 <at> gmail.com> writes:

> <npostavs <at> users.sourceforge.net> schrieb am Sa., 22. Apr. 2017 um 00:48 Uhr:
>
>> Philipp Stephani <p.stephani2 <at> gmail.com> writes:
>>
>> > +       (let ((buffer (current-buffer)))
>> > +         ;; `unquote-then-quote' is only used for the
>> > +         ;; `verify-visited-file-modtime' action, which takes a buffer
>> > +         ;; as only optional argument.
>> > +         (with-current-buffer (or (car arguments) buffer)
>> > +           (let ((buffer-file-name (substring buffer-file-name 2)))
>> > +             ;; Make sure to hide the temporary buffer change from the
>> > +             ;; underlying operation.
>> > +             (with-current-buffer buffer
>> > +               (apply operation arguments))))))
>>
>> I think this could be simplified by using the buffer-file-name function:
>>
>>     (let ((buffer-file-name
>>            (substring (buffer-file-name (car arguments)) 2)))
>>       (apply operation arguments))
>>
>
> That's not the same, it will set the file name of the wrong buffer.

Oh, I think I get it now.  It could be written like this, right?

    (cl-letf* ((buf (or (car arguments) (current-buffer)))
               ((buffer-local-value buffer-file-name buf)
                (substring (buffer-file-name buf) 2)))
      (apply operation arguments))

Message #20 received at 25951 <at> debbugs.gnu.org (full text, mbox):

From: Philipp Stephani <p.stephani2 <at> gmail.com>
To: npostavs <at> users.sourceforge.net
Cc: 25951 <at> debbugs.gnu.org
Subject: Re: bug#25951: 26.0.50; Error when ediffing files that are visited
 using quoted file names
Date: Sun, 23 Apr 2017 16:54:12 +0000

[Message part 1 (text/plain, inline)]

<npostavs <at> users.sourceforge.net> schrieb am Sa., 22. Apr. 2017 um 22:31 Uhr:

> Philipp Stephani <p.stephani2 <at> gmail.com> writes:
>
> > <npostavs <at> users.sourceforge.net> schrieb am Sa., 22. Apr. 2017 um 00:48
> Uhr:
> >
> >> Philipp Stephani <p.stephani2 <at> gmail.com> writes:
> >>
> >> > +       (let ((buffer (current-buffer)))
> >> > +         ;; `unquote-then-quote' is only used for the
> >> > +         ;; `verify-visited-file-modtime' action, which takes a
> buffer
> >> > +         ;; as only optional argument.
> >> > +         (with-current-buffer (or (car arguments) buffer)
> >> > +           (let ((buffer-file-name (substring buffer-file-name 2)))
> >> > +             ;; Make sure to hide the temporary buffer change from
> the
> >> > +             ;; underlying operation.
> >> > +             (with-current-buffer buffer
> >> > +               (apply operation arguments))))))
> >>
> >> I think this could be simplified by using the buffer-file-name function:
> >>
> >>     (let ((buffer-file-name
> >>            (substring (buffer-file-name (car arguments)) 2)))
> >>       (apply operation arguments))
> >>
> >
> > That's not the same, it will set the file name of the wrong buffer.
>
> Oh, I think I get it now.  It could be written like this, right?
>
>     (cl-letf* ((buf (or (car arguments) (current-buffer)))
>                ((buffer-local-value buffer-file-name buf)
>                 (substring (buffer-file-name buf) 2)))
>       (apply operation arguments))
>

Yes, that should work, thanks. Will send a new patch in a second.

[Message part 2 (text/html, inline)]

Message #23 received at 25951 <at> debbugs.gnu.org (full text, mbox):

From: Philipp Stephani <p.stephani2 <at> gmail.com>
To: 25951 <at> debbugs.gnu.org,
	npostavs <at> users.sourceforge.net
Cc: Philipp Stephani <phst <at> google.com>
Subject: [PATCH] Fix quoted files for 'verify-visited-file-modtime'
Date: Sat, 29 Apr 2017 13:49:37 +0200

Fixes Bug#25951.

* lisp/files.el (file-name-non-special): Set the file name for the
correct buffer.

* test/lisp/files-tests.el (files-tests--file-name-non-special--buffers):
Add unit test.
---
 lisp/files.el            |  9 ++++++++-
 test/lisp/files-tests.el | 42 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/lisp/files.el b/lisp/files.el
index 6848818cad..2e9ab1aad1 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -28,6 +28,8 @@
 
 ;;; Code:
 
+(eval-when-compile (require 'cl-lib))
+
 (defvar font-lock-keywords)
 
 (defgroup backup nil
@@ -6987,7 +6989,12 @@ file-name-non-special
            (when (and visit buffer-file-name)
              (setq buffer-file-name (concat "/:" buffer-file-name))))))
       (`unquote-then-quote
-       (let ((buffer-file-name (substring buffer-file-name 2)))
+       (cl-letf* ((buffer (or (car arguments) (current-buffer)))
+                  ((buffer-local-value 'buffer-file-name buffer)
+                   (substring (buffer-file-name buffer) 2)))
+         ;; `unquote-then-quote' is only used for the
+         ;; `verify-visited-file-modtime' action, which takes a buffer
+         ;; as only optional argument.
          (apply operation arguments)))
       (_
        (apply operation arguments)))))
diff --git a/test/lisp/files-tests.el b/test/lisp/files-tests.el
index 80bbeb1bc5..9c633198f1 100644
--- a/test/lisp/files-tests.el
+++ b/test/lisp/files-tests.el
@@ -1,4 +1,4 @@
-;;; files-tests.el --- tests for files.el.
+;;; files-tests.el --- tests for files.el.  -*- lexical-binding: t; -*-
 
 ;; Copyright (C) 2012-2017 Free Software Foundation, Inc.
 
@@ -20,6 +20,7 @@
 ;;; Code:
 
 (require 'ert)
+(require 'nadvice)
 
 ;; Set to t if the local variable was set, `query' if the query was
 ;; triggered.
@@ -251,5 +252,44 @@ files-test-bug-18141-file
                       (start-file-process "foo" nil "true"))))
   (should (eq (let ((default-directory "/:/")) (shell-command "true")) 0)))
 
+(ert-deftest files-tests--file-name-non-special--buffers ()
+  "Check that Bug#25951 is fixed.
+We call `verify-visited-file-modtime' on a buffer visiting a file
+with a quoted name.  We use two different variants: first with
+the buffer current and a nil argument, second passing the buffer
+object explicitly.  In both cases no error should be raised and
+the `file-name-non-special' handler for quoted file names should
+be invoked with the right arguments."
+  (with-temp-buffer
+    (let* ((buffer-file-name "/:/tmp/bug25951.txt")
+           (buffer-visiting-file (current-buffer))
+           (actual-args ())
+           (log (lambda (&rest args) (push args actual-args))))
+      (set-visited-file-modtime '(1 2 3 4))
+      (should (equal (find-file-name-handler buffer-file-name
+                                             #'verify-visited-file-modtime)
+                     #'file-name-non-special))
+      (advice-add #'file-name-non-special :before log)
+      (unwind-protect
+          (progn
+            (should (stringp buffer-file-name))
+            ;; This should call the file name handler with the right
+            ;; buffer and not signal an error.  The file doesn't
+            ;; actually exist, so this should return nil.
+            (should-not (verify-visited-file-modtime))
+            (with-temp-buffer
+              (should (stringp (buffer-file-name buffer-visiting-file)))
+              ;; This should call the file name handler with the right
+              ;; buffer and not signal an error.  The file doesn't
+              ;; actually exist, so this should return nil.
+              (should-not (verify-visited-file-modtime buffer-visiting-file))))
+        (advice-remove #'file-name-non-special 'bug25951))
+      ;; Verify that the handler was actually called.  We called
+      ;; `verify-visited-file-modtime' twice, so both calls should be
+      ;; recorded in reverse order.
+      (should (equal actual-args
+                     `((verify-visited-file-modtime ,buffer-visiting-file)
+                       (verify-visited-file-modtime nil)))))))
+
 (provide 'files-tests)
 ;;; files-tests.el ends here
-- 
2.12.2

Message #26 received at 25951 <at> debbugs.gnu.org (full text, mbox):

From: Philipp Stephani <p.stephani2 <at> gmail.com>
To: 25951 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net, emacs-devel <at> gnu.org,
 eliz <at> gnu.org
Cc: Philipp Stephani <phst <at> google.com>
Subject: [PATCH] Fix quoted files for 'verify-visited-file-modtime'
Date: Sat, 29 Apr 2017 14:20:27 +0200

Fixes Bug#25951.

* lisp/files.el (file-name-non-special): Set the file name for the
correct buffer.

* test/lisp/files-tests.el (files-tests--file-name-non-special--buffers):
Add unit test.
(files-tests--with-advice, files-tests--with-temp-file): New helper
macros.
---
 lisp/files.el            |  9 ++++++-
 test/lisp/files-tests.el | 64 +++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 71 insertions(+), 2 deletions(-)

diff --git a/lisp/files.el b/lisp/files.el
index 6848818cad..2e9ab1aad1 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -28,6 +28,8 @@
 
 ;;; Code:
 
+(eval-when-compile (require 'cl-lib))
+
 (defvar font-lock-keywords)
 
 (defgroup backup nil
@@ -6987,7 +6989,12 @@ file-name-non-special
            (when (and visit buffer-file-name)
              (setq buffer-file-name (concat "/:" buffer-file-name))))))
       (`unquote-then-quote
-       (let ((buffer-file-name (substring buffer-file-name 2)))
+       (cl-letf* ((buffer (or (car arguments) (current-buffer)))
+                  ((buffer-local-value 'buffer-file-name buffer)
+                   (substring (buffer-file-name buffer) 2)))
+         ;; `unquote-then-quote' is only used for the
+         ;; `verify-visited-file-modtime' action, which takes a buffer
+         ;; as only optional argument.
          (apply operation arguments)))
       (_
        (apply operation arguments)))))
diff --git a/test/lisp/files-tests.el b/test/lisp/files-tests.el
index 80bbeb1bc5..4583b1af3c 100644
--- a/test/lisp/files-tests.el
+++ b/test/lisp/files-tests.el
@@ -1,4 +1,4 @@
-;;; files-tests.el --- tests for files.el.
+;;; files-tests.el --- tests for files.el.  -*- lexical-binding: t; -*-
 
 ;; Copyright (C) 2012-2017 Free Software Foundation, Inc.
 
@@ -20,6 +20,7 @@
 ;;; Code:
 
 (require 'ert)
+(require 'nadvice)
 
 ;; Set to t if the local variable was set, `query' if the query was
 ;; triggered.
@@ -251,5 +252,66 @@ files-test-bug-18141-file
                       (start-file-process "foo" nil "true"))))
   (should (eq (let ((default-directory "/:/")) (shell-command "true")) 0)))
 
+(defmacro files-tests--with-advice (symbol where function &rest body)
+  (declare (indent 3))
+  (cl-check-type symbol symbol)
+  (cl-check-type where keyword)
+  (cl-check-type function function)
+  (macroexp-let2 nil function function
+    `(progn
+       (advice-add #',symbol ,where ,function)
+       (unwind-protect
+           (progn ,@body)
+         (advice-remove #',symbol ,function)))))
+
+(defmacro files-tests--with-temp-file (name &rest body)
+  (declare (indent 1))
+  (cl-check-type name symbol)
+  `(let ((,name (make-temp-file "emacs")))
+     (unwind-protect
+         (progn ,@body)
+       (delete-file ,name))))
+
+(ert-deftest files-tests--file-name-non-special--buffers ()
+  "Check that Bug#25951 is fixed.
+We call `verify-visited-file-modtime' on a buffer visiting a file
+with a quoted name.  We use two different variants: first with
+the buffer current and a nil argument, second passing the buffer
+object explicitly.  In both cases no error should be raised and
+the `file-name-non-special' handler for quoted file names should
+be invoked with the right arguments."
+  (files-tests--with-temp-file temp-file-name
+    (with-temp-buffer
+     (let* ((buffer-visiting-file (current-buffer))
+            (actual-args ())
+            (log (lambda (&rest args) (push args actual-args))))
+       (insert-file-contents (concat "/:" temp-file-name) :visit)
+       (should (stringp buffer-file-name))
+       (should (string-prefix-p "/:" buffer-file-name))
+       (should (consp (visited-file-modtime)))
+       (should (equal (find-file-name-handler buffer-file-name
+                                              #'verify-visited-file-modtime)
+                      #'file-name-non-special))
+       (files-tests--with-advice file-name-non-special :before log
+         ;; This should call the file name handler with the right
+         ;; buffer and not signal an error.  The file hasn't been
+         ;; modified, so `verify-visited-file-modtime' should return
+         ;; t.
+         (should (equal (verify-visited-file-modtime) t))
+         (with-temp-buffer
+           (should (stringp (buffer-file-name buffer-visiting-file)))
+           ;; This should call the file name handler with the right
+           ;; buffer and not signal an error.  The file hasn't been
+           ;; modified, so `verify-visited-file-modtime' should return
+           ;; t.
+           (should (equal (verify-visited-file-modtime buffer-visiting-file)
+                          t))))
+       ;; Verify that the handler was actually called.  We called
+       ;; `verify-visited-file-modtime' twice, so both calls should be
+       ;; recorded in reverse order.
+       (should (equal actual-args
+                      `((verify-visited-file-modtime ,buffer-visiting-file)
+                        (verify-visited-file-modtime nil))))))))
+
 (provide 'files-tests)
 ;;; files-tests.el ends here
-- 
2.12.2

Message #31 received at 25951-done <at> debbugs.gnu.org (full text, mbox):

From: Philipp <p.stephani2 <at> gmail.com>
To: npostavs <at> users.sourceforge.net, emacs-devel <at> gnu.org, eliz <at> gnu.org, 
 25951-done <at> debbugs.gnu.org
Cc: Philipp Stephani <phst <at> google.com>
Subject: Re: [PATCH] Fix quoted files for 'verify-visited-file-modtime'
Date: Sat, 06 May 2017 19:27:48 +0000

[Message part 1 (text/plain, inline)]

Philipp Stephani <p.stephani2 <at> gmail.com> schrieb am Sa., 29. Apr. 2017 um
14:20 Uhr:

> Fixes Bug#25951.
>
> * lisp/files.el (file-name-non-special): Set the file name for the
> correct buffer.
>
> * test/lisp/files-tests.el (files-tests--file-name-non-special--buffers):
> Add unit test.
> (files-tests--with-advice, files-tests--with-temp-file): New helper
> macros.
> ---
>  lisp/files.el            |  9 ++++++-
>  test/lisp/files-tests.el | 64
> +++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 71 insertions(+), 2 deletions(-)
>
> diff --git a/lisp/files.el b/lisp/files.el
> index 6848818cad..2e9ab1aad1 100644
> --- a/lisp/files.el
> +++ b/lisp/files.el
> @@ -28,6 +28,8 @@
>
>  ;;; Code:
>
> +(eval-when-compile (require 'cl-lib))
> +
>  (defvar font-lock-keywords)
>
>  (defgroup backup nil
> @@ -6987,7 +6989,12 @@ file-name-non-special
>             (when (and visit buffer-file-name)
>               (setq buffer-file-name (concat "/:" buffer-file-name))))))
>        (`unquote-then-quote
> -       (let ((buffer-file-name (substring buffer-file-name 2)))
> +       (cl-letf* ((buffer (or (car arguments) (current-buffer)))
> +                  ((buffer-local-value 'buffer-file-name buffer)
> +                   (substring (buffer-file-name buffer) 2)))
> +         ;; `unquote-then-quote' is only used for the
> +         ;; `verify-visited-file-modtime' action, which takes a buffer
> +         ;; as only optional argument.
>           (apply operation arguments)))
>        (_
>         (apply operation arguments)))))
> diff --git a/test/lisp/files-tests.el b/test/lisp/files-tests.el
> index 80bbeb1bc5..4583b1af3c 100644
> --- a/test/lisp/files-tests.el
> +++ b/test/lisp/files-tests.el
> @@ -1,4 +1,4 @@
> -;;; files-tests.el --- tests for files.el.
> +;;; files-tests.el --- tests for files.el.  -*- lexical-binding: t; -*-
>
>  ;; Copyright (C) 2012-2017 Free Software Foundation, Inc.
>
> @@ -20,6 +20,7 @@
>  ;;; Code:
>
>  (require 'ert)
> +(require 'nadvice)
>
>  ;; Set to t if the local variable was set, `query' if the query was
>  ;; triggered.
> @@ -251,5 +252,66 @@ files-test-bug-18141-file
>                        (start-file-process "foo" nil "true"))))
>    (should (eq (let ((default-directory "/:/")) (shell-command "true"))
> 0)))
>
> +(defmacro files-tests--with-advice (symbol where function &rest body)
> +  (declare (indent 3))
> +  (cl-check-type symbol symbol)
> +  (cl-check-type where keyword)
> +  (cl-check-type function function)
> +  (macroexp-let2 nil function function
> +    `(progn
> +       (advice-add #',symbol ,where ,function)
> +       (unwind-protect
> +           (progn ,@body)
> +         (advice-remove #',symbol ,function)))))
> +
> +(defmacro files-tests--with-temp-file (name &rest body)
> +  (declare (indent 1))
> +  (cl-check-type name symbol)
> +  `(let ((,name (make-temp-file "emacs")))
> +     (unwind-protect
> +         (progn ,@body)
> +       (delete-file ,name))))
> +
> +(ert-deftest files-tests--file-name-non-special--buffers ()
> +  "Check that Bug#25951 is fixed.
> +We call `verify-visited-file-modtime' on a buffer visiting a file
> +with a quoted name.  We use two different variants: first with
> +the buffer current and a nil argument, second passing the buffer
> +object explicitly.  In both cases no error should be raised and
> +the `file-name-non-special' handler for quoted file names should
> +be invoked with the right arguments."
> +  (files-tests--with-temp-file temp-file-name
> +    (with-temp-buffer
> +     (let* ((buffer-visiting-file (current-buffer))
> +            (actual-args ())
> +            (log (lambda (&rest args) (push args actual-args))))
> +       (insert-file-contents (concat "/:" temp-file-name) :visit)
> +       (should (stringp buffer-file-name))
> +       (should (string-prefix-p "/:" buffer-file-name))
> +       (should (consp (visited-file-modtime)))
> +       (should (equal (find-file-name-handler buffer-file-name
> +
> #'verify-visited-file-modtime)
> +                      #'file-name-non-special))
> +       (files-tests--with-advice file-name-non-special :before log
> +         ;; This should call the file name handler with the right
> +         ;; buffer and not signal an error.  The file hasn't been
> +         ;; modified, so `verify-visited-file-modtime' should return
> +         ;; t.
> +         (should (equal (verify-visited-file-modtime) t))
> +         (with-temp-buffer
> +           (should (stringp (buffer-file-name buffer-visiting-file)))
> +           ;; This should call the file name handler with the right
> +           ;; buffer and not signal an error.  The file hasn't been
> +           ;; modified, so `verify-visited-file-modtime' should return
> +           ;; t.
> +           (should (equal (verify-visited-file-modtime
> buffer-visiting-file)
> +                          t))))
> +       ;; Verify that the handler was actually called.  We called
> +       ;; `verify-visited-file-modtime' twice, so both calls should be
> +       ;; recorded in reverse order.
> +       (should (equal actual-args
> +                      `((verify-visited-file-modtime
> ,buffer-visiting-file)
> +                        (verify-visited-file-modtime nil))))))))
> +
>  (provide 'files-tests)
>  ;;; files-tests.el ends here
> --
> 2.12.2
>
>
No further comments, so I've pushed this as 5e47c2e52b.

[Message part 2 (text/html, inline)]

Message #34 received at 25951 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: 25951 <at> debbugs.gnu.org
Cc: p.stephani2 <at> gmail.com
Subject: Re: bug#25951: [PATCH] Fix quoted files for
 'verify-visited-file-modtime'
Date: Sat, 06 May 2017 16:28:52 -0400

This fails to build: http://hydra.nixos.org/build/52549555

Message #37 received at 25951 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Glenn Morris <rgm <at> gnu.org>
Cc: 25951 <at> debbugs.gnu.org, p.stephani2 <at> gmail.com
Subject: Re: bug#25951: [PATCH] Fix quoted files for
 'verify-visited-file-modtime'
Date: Sat, 06 May 2017 16:41:26 -0400

Glenn Morris <rgm <at> gnu.org> writes:

> This fails to build: http://hydra.nixos.org/build/52549555

    Loading /tmp/nix-build-emacs-tarball-unknown.drv-0/x1vxdahi72zl4bj2yd676ichh4iwi9iw-git-export/lisp/files.el (source)...
    Warning: Unknown defun property `gv-setter' in cl-fifth
    Warning: Unknown defun property `gv-setter' in cl-sixth
    Warning: Unknown defun property `gv-setter' in cl-seventh
    Warning: Unknown defun property `gv-setter' in cl-eighth
    Warning: Unknown defun property `gv-setter' in cl-ninth
    Warning: Unknown defun property `gv-setter' in cl-tenth
    Symbol's function definition is void: gv-define-simple-setter
    make[1]: *** [bootstrap-emacs] Error 255

Looks like my suggestion of using cl-letf doesn't work because it's too
early in the bootstrap process.  Should work with the original code (but
perhaps add a comment about why we can't use cl-letf).

Message #40 received at 25951 <at> debbugs.gnu.org (full text, mbox):

From: Philipp <p.stephani2 <at> gmail.com>
To: npostavs <at> users.sourceforge.net, Glenn Morris <rgm <at> gnu.org>
Cc: 25951 <at> debbugs.gnu.org
Subject: Re: bug#25951: [PATCH] Fix quoted files for
 'verify-visited-file-modtime'
Date: Sat, 06 May 2017 21:27:40 +0000

[Message part 1 (text/plain, inline)]

<npostavs <at> users.sourceforge.net> schrieb am Sa., 6. Mai 2017 um 22:39 Uhr:

> Glenn Morris <rgm <at> gnu.org> writes:
>
> > This fails to build: http://hydra.nixos.org/build/52549555
>
>     Loading
> /tmp/nix-build-emacs-tarball-unknown.drv-0/x1vxdahi72zl4bj2yd676ichh4iwi9iw-git-export/lisp/files.el
> (source)...
>     Warning: Unknown defun property `gv-setter' in cl-fifth
>     Warning: Unknown defun property `gv-setter' in cl-sixth
>     Warning: Unknown defun property `gv-setter' in cl-seventh
>     Warning: Unknown defun property `gv-setter' in cl-eighth
>     Warning: Unknown defun property `gv-setter' in cl-ninth
>     Warning: Unknown defun property `gv-setter' in cl-tenth
>     Symbol's function definition is void: gv-define-simple-setter
>     make[1]: *** [bootstrap-emacs] Error 255
>
> Looks like my suggestion of using cl-letf doesn't work because it's too
> early in the bootstrap process.  Should work with the original code (but
> perhaps add a comment about why we can't use cl-letf).
>

Installed commit cea3b22bc7.

[Message part 2 (text/html, inline)]

Message #43 received at 25951 <at> debbugs.gnu.org (full text, mbox):

From: Philipp <phil07c1 <at> gmail.com>
To: rgm <at> gnu.org, npostavs <at> users.sourceforge.net, emacs-devel <at> gnu.org,
 25951 <at> debbugs.gnu.org
Cc: Philipp <phst <at> google.com>
Subject: [PATCH] Fix bootstrap build of files.el
Date: Sat,  6 May 2017 23:21:57 +0200

* lisp/files.el (file-name-non-special): Don't use cl-letf.
---
 lisp/files.el | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/lisp/files.el b/lisp/files.el
index 7e627d36d4..8ac1993754 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -29,7 +29,6 @@
 ;;; Code:
 
 (eval-when-compile
-  (require 'cl-lib)
   (require 'pcase)
   (require 'easy-mmode)) ; For `define-minor-mode'.
 
@@ -7032,13 +7031,18 @@ file-name-non-special
            (when (and visit buffer-file-name)
              (setq buffer-file-name (concat "/:" buffer-file-name))))))
       (`unquote-then-quote
-       (cl-letf* ((buffer (or (car arguments) (current-buffer)))
-                  ((buffer-local-value 'buffer-file-name buffer)
-                   (substring (buffer-file-name buffer) 2)))
+       ;; We can't use `cl-letf' with `(buffer-local-value)' here
+       ;; because it wouldn't work during bootstrapping.
+       (let ((buffer (current-buffer)))
          ;; `unquote-then-quote' is only used for the
          ;; `verify-visited-file-modtime' action, which takes a buffer
          ;; as only optional argument.
-         (apply operation arguments)))
+         (with-current-buffer (or (car arguments) buffer)
+           (let ((buffer-file-name (substring buffer-file-name 2)))
+             ;; Make sure to hide the temporary buffer change from the
+             ;; underlying operation.
+             (with-current-buffer buffer
+               (apply operation arguments))))))
       (_
        (apply operation arguments)))))
 
-- 
2.12.2

Message #46 received at 25951 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Philipp <phil07c1 <at> gmail.com>
Cc: Philipp <phst <at> google.com>, 25951 <at> debbugs.gnu.org, emacs-devel <at> gnu.org,
 npostavs <at> users.sourceforge.net
Subject: Re: [PATCH] Fix bootstrap build of files.el
Date: Sat, 06 May 2017 21:24:35 -0400

We have a diffs mailing list, so you don't need to send your patches to
two other mailing lists as well.

Message #49 received at 25951 <at> debbugs.gnu.org (full text, mbox):

From: Philipp <phil07c1 <at> gmail.com>
To: Glenn Morris <rgm <at> gnu.org>
Cc: Philipp <phst <at> google.com>, 25951 <at> debbugs.gnu.org,
 npostavs <at> users.sourceforge.net, emacs-devel <at> gnu.org
Subject: Re: [PATCH] Fix bootstrap build of files.el
Date: Sun, 07 May 2017 11:35:10 +0000

[Message part 1 (text/plain, inline)]

Glenn Morris <rgm <at> gnu.org> schrieb am So., 7. Mai 2017 um 03:25 Uhr:

>
> We have a diffs mailing list, so you don't need to send your patches to
> two other mailing lists as well.
>
>

Yes, I just wanted to have my patch reviewed before committing, but since
it broke bootstrapping I decided to install it without review.

[Message part 2 (text/html, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.