GNU bug report logs -
#30243
26.0.91; Infinite recursion in `make-auto-save-file-name' for quoted filenames
Previous Next
Reported by: phst <at> a.muc.corp.google.com
Date: Wed, 24 Jan 2018 22:23:01 UTC
Severity: normal
Tags: fixed
Found in version 26.0.91
Fixed in version 26.1
Done: Noam Postavsky <npostavs <at> users.sourceforge.net>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 30243 in the body.
You can then email your comments to 30243 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Wed, 24 Jan 2018 22:23:01 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
phst <at> a.muc.corp.google.com
:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org
.
(Wed, 24 Jan 2018 22:23:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
emacs -Q -nw -batch --eval=3D'(with-temp-buffer (let ((buffer-file-name "/:=
/tmp/x")) (make-auto-save-file-name)))'
Variable binding depth exceeds max-specpdl-size
Since find-file calls after-find-file, which calls
make-auto-save-file-name, this can be trivially reproduced by visiting
any quoted file in Emacs 26, e.g. using C-x f /:/tmp/a.
I think this should block the Emacs 26 release because it makes quoted
file names essentially unusable.
In GNU Emacs 26.0.91 (build 3, x86_64-pc-linux-gnu, GTK+ Version 3.22.24)
of 2018-01-24 built on localhost
Repository revision: 59db8dca030ba6a34d143c3cc6715f02beba1068
Windowing system distributor 'The X.Org Foundation', version 11.0.11903000
System Description: Debian GNU/Linux
Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Configured using:
'configure --without-threads --enable-gcc-warnings=3Dwarn-only
--enable-gtk-deprecation-warnings --without-pop --with-mailutils
--enable-checking --enable-check-lisp-object-type --with-modules
'CFLAGS=3D-O0 -ggdb3''
Configured features:
XPM JPEG TIFF GIF PNG SOUND DBUS 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
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 rmc puny seq byte-opt gv
bytecomp byte-compile cconv cl-loaddefs 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 elec-pair 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 charprop
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 dbusbind 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 95204 8643)
(symbols 48 20381 1)
(miscs 40 41 145)
(strings 32 28348 1660)
(string-bytes 1 755236)
(vectors 16 14065)
(vector-slots 8 497282 8868)
(floats 8 49 68)
(intervals 56 223 0)
(buffers 992 12))
--=20
Google Germany GmbH
Erika-Mann-Stra=C3=9Fe 33
80636 M=C3=BCnchen
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Gesch=C3=A4ftsf=C3=BChrer: Paul Manicle, Halimah DeLaine Prado
If you received this communication by mistake, please don=E2=80=99t forward=
it to
anyone else (it may contain confidential or privileged information), please
erase all copies of it, including all attachments, and please let the sender
know it went to the wrong person. Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Wed, 24 Jan 2018 22:44:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 30243 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
<phst <at> a.muc.corp.google.com> schrieb am Mi., 24. Jan. 2018 um 23:23 Uhr:
>
> emacs -Q -nw -batch --eval=3D'(with-temp-buffer (let ((buffer-file-name
> "/:=
> /tmp/x")) (make-auto-save-file-name)))'
> Variable binding depth exceeds max-specpdl-size
>
>
According to git bisect, the problematic commit is:
commit a1bbc490155b61a634a6d0b165000ce35b93aa35 (HEAD, refs/bisect/bad)
Author: Michael Albinus <michael.albinus <at> gmx.de>
Date: Wed Dec 6 20:49:30 2017 +0100
Fix Bug#29579
* lisp/files.el (file-name-non-special):
Inhibit `file-name-handler-alist' only for some operations.
Add missing operations. (Bug#29579)
[...]
which makes sense because it touches the part of the code that's causing
issues.
Since this commit was a bug fix for a related issue with quoted file names,
reverting it is probably not the best way forward. We should push a fix and
make a new pretest.
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Wed, 24 Jan 2018 23:05:02 GMT)
Full text and
rfc822 format available.
Message #11 received at 30243 <at> debbugs.gnu.org (full text, mbox):
Philipp Stephani <p.stephani2 <at> gmail.com> writes:
> Since this commit was a bug fix for a related issue with quoted file
> names, reverting it is probably not the best way forward. We should
> push a fix and make a new pretest.
The following seems to fix it. We should review other file handler
operations of course, but can we really expect to learn anything from
another pretest?
--- i/lisp/files.el
+++ w/lisp/files.el
@@ -7004,6 +7004,11 @@ file-name-non-special
(expand-file-name
(unhandled-file-name-directory default-directory)))
default-directory))
+ (buffer-file-name
+ (if (and (memq operation '(make-auto-save-file-name))
+ (string-match "\\`/:" buffer-file-name))
+ (substring buffer-file-name (match-end 0))
+ buffer-file-name))
;; Get a list of the indices of the args which are file names.
(file-arg-indices
(cdr (or (assq operation
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Wed, 24 Jan 2018 23:26:02 GMT)
Full text and
rfc822 format available.
Message #14 received at 30243 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Noam Postavsky <npostavs <at> users.sourceforge.net> schrieb am Do., 25. Jan.
2018 um 00:04 Uhr:
> Philipp Stephani <p.stephani2 <at> gmail.com> writes:
>
> > Since this commit was a bug fix for a related issue with quoted file
> > names, reverting it is probably not the best way forward. We should
> > push a fix and make a new pretest.
>
> The following seems to fix it. We should review other file handler
> operations of course, but can we really expect to learn anything from
> another pretest?
>
> --- i/lisp/files.el
> +++ w/lisp/files.el
> @@ -7004,6 +7004,11 @@ file-name-non-special
> (expand-file-name
> (unhandled-file-name-directory default-directory)))
> default-directory))
> + (buffer-file-name
> + (if (and (memq operation '(make-auto-save-file-name))
> + (string-match "\\`/:" buffer-file-name))
> + (substring buffer-file-name (match-end 0))
> + buffer-file-name))
> ;; Get a list of the indices of the args which are file names.
> (file-arg-indices
> (cdr (or (assq operation
>
Thanks, when you commit this, could you please also add a regression test
so we don't reintroduce the bug later?
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Thu, 25 Jan 2018 05:58:02 GMT)
Full text and
rfc822 format available.
Message #17 received at 30243 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Philipp Stephani <p.stephani2 <at> gmail.com> writes:
> Noam Postavsky <npostavs <at> users.sourceforge.net> schrieb am Do., 25.
> Jan. 2018 um 00:04 Uhr:
> + (buffer-file-name
> + (if (and (memq operation '(make-auto-save-file-name))
> + (string-match "\\`/:" buffer-file-name))
> + (substring buffer-file-name (match-end 0))
> + buffer-file-name))
> Thanks, when you commit this, could you please also add a regression
> test so we don't reintroduce the bug later?
Yeah, we need to test more than just that operation though. I've added
a test for all the operations listed in `(elisp) Magic File Names'. I
discovered several other similar problems. With
`file-name-all-completions' I even got Emacs to crash (I presume due to
stack corruption during the recursion). Also note that the above patch
breaks find-file for /: quoted files, buffer-file-name needs to be bound
more selectively (as in the new patch, attached below).
`file-notify-rm-watch' and `file-notify-valid-p' are still broken, I
can't immediately see how to fix them and it's getting past my bedtime.
[v2-0001-Test-and-fix-quoted-file-name-handlers-Bug-30243.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Thu, 25 Jan 2018 09:50:01 GMT)
Full text and
rfc822 format available.
Message #20 received at 30243 <at> debbugs.gnu.org (full text, mbox):
Noam Postavsky <npostavs <at> users.sourceforge.net> writes:
Hi Noam,
> Yeah, we need to test more than just that operation though. I've added
> a test for all the operations listed in `(elisp) Magic File Names'. I
> discovered several other similar problems. With
> `file-name-all-completions' I even got Emacs to crash (I presume due to
> stack corruption during the recursion). Also note that the above patch
> breaks find-file for /: quoted files, buffer-file-name needs to be bound
> more selectively (as in the new patch, attached below).
>
> `file-notify-rm-watch' and `file-notify-valid-p' are still broken, I
> can't immediately see how to fix them and it's getting past my bedtime.
Maybe I could be of some help? Pls commit your changes you have worked
on, and tell me which missing part I shall try to follow.
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Thu, 25 Jan 2018 14:08:02 GMT)
Full text and
rfc822 format available.
Message #23 received at 30243 <at> debbugs.gnu.org (full text, mbox):
Michael Albinus <michael.albinus <at> gmx.de> writes:
>> `file-notify-rm-watch' and `file-notify-valid-p' are still broken, I
>> can't immediately see how to fix them and it's getting past my bedtime.
>
> Maybe I could be of some help? Pls commit your changes you have worked
> on, and tell me which missing part I shall try to follow.
I pushed my patch to the scratch/nonspecial-handlers branch. If you run
make -C test files-tests SELECTOR='\"non-special\"'
you will see the relevant tests,
files-file-name-non-special-notify-handlers being the one which
currently fails. Maybe it's just a matter of binding
file-name-handler-alist to nil again for those operations, not sure.
PS we should probably update test/Makefile.in to require less escaping
for SELECTOR.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Thu, 25 Jan 2018 16:37:01 GMT)
Full text and
rfc822 format available.
Message #26 received at 30243 <at> debbugs.gnu.org (full text, mbox):
Noam Postavsky <npostavs <at> users.sourceforge.net> writes:
Hi Noam,
> I pushed my patch to the scratch/nonspecial-handlers branch. If you run
>
> make -C test files-tests SELECTOR='\"non-special\"'
>
> you will see the relevant tests,
Thanks.
> files-file-name-non-special-notify-handlers being the one which
> currently fails. Maybe it's just a matter of binding
> file-name-handler-alist to nil again for those operations, not sure.
I tried to debug this interactively. First, I had to give the debug
property to files-tests--with-temp-file and files-tests--with-temp-dir,
in order to be able to debug interactively.
I've tried to instrument file-notify-valid-p for debugging, but I've got
the error
Invalid read syntax: "Expected lambda expression"
Strange. No idea what's up, could it be due to the when-let* macro?
Anyway, I've instrumented files-file-name-non-special-notify-handlers.
When running the test, it returns file-notify-add-watch as expected, but
when entering file-notify-valid-p, it returns the error
Lisp nesting exceeds 'max-lisp-eval-depth'
Emacs is frozen, and there is the error message in the shell Emacs is
started from:
emacs: malloc.c:2427: sysmalloc: Assertion `(old_top == initial_top (av) && old_size == 0) || ((unsigned long) (old_size) >= MINSIZE && prev_inuse (old_top) && ((unsigned long) old_end & (pagesize - 1)) == 0)' failed.
Fatal error 6: Aborted
No idea what's up. I'd say you've uncovered a bug :-)
I'll try to continue tomorrow, running Emacs under gdb, but for now I
have to stop (there's a life outside Emacs ...)
> PS we should probably update test/Makefile.in to require less escaping
> for SELECTOR.
Go on. But pls remember, that SELECTOR is not a string only, it can be
also a Lisp form.
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Thu, 25 Jan 2018 16:47:02 GMT)
Full text and
rfc822 format available.
Message #29 received at 30243 <at> debbugs.gnu.org (full text, mbox):
On Thu, Jan 25, 2018 at 11:36 AM, Michael Albinus
<michael.albinus <at> gmx.de> wrote:
> Emacs is frozen, and there is the error message in the shell Emacs is
> started from:
>
> emacs: malloc.c:2427: sysmalloc: Assertion `(old_top == initial_top (av) && old_size == 0) || ((unsigned long) (old_size) >= MINSIZE && prev_inuse (old_top) && ((unsigned long) old_end & (pagesize - 1)) == 0)' failed.
> Fatal error 6: Aborted
Oh, that's what I got from `file-name-all-completions' before fixing
it. My guess is that it's due to stack overflow. Although it is a bit
strange to get heap corruption from stack usage, maybe a bad
interaction with the stack overflow recovery mechanism?
I'll take a look at the edebug problems later today.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Fri, 26 Jan 2018 01:47:01 GMT)
Full text and
rfc822 format available.
Message #32 received at 30243 <at> debbugs.gnu.org (full text, mbox):
Noam Postavsky <npostavs <at> users.sourceforge.net> writes:
> On Thu, Jan 25, 2018 at 11:36 AM, Michael Albinus
> <michael.albinus <at> gmx.de> wrote:
>
>> Emacs is frozen, and there is the error message in the shell Emacs is
>> started from:
>>
>> emacs: malloc.c:2427: sysmalloc: Assertion `(old_top == initial_top
>> (av) && old_size == 0) || ((unsigned long) (old_size) >= MINSIZE &&
>> prev_inuse (old_top) && ((unsigned long) old_end & (pagesize - 1))
>> == 0)' failed.
>> Fatal error 6: Aborted
>
> Oh, that's what I got from `file-name-all-completions' before fixing
> it. My guess is that it's due to stack overflow. Although it is a bit
> strange to get heap corruption from stack usage, maybe a bad
> interaction with the stack overflow recovery mechanism?
It's not related to stack overflow recovery, it still happens with
attempt-stack-overflow-recovery set to nil. The problem appears to be
that we hit the limit in grow_specpdl(), and then call signal_error
which does Ffuncall and then record_in_backtrace writes to specdl, this
latter write is invalid since we failed to grow specdl before. Thus
memory corruption, undefined behaviour, etc.
#0 0x000000000063999d in record_in_backtrace (function=XIL(0xd9ea380), args=0xffef5b188, nargs=2)
at ../../src/eval.c:2096
#1 0x000000000063b8c9 in Ffuncall (nargs=3, args=0xffef5b180) at ../../src/eval.c:2746
#2 0x000000000063b320 in call2 (fn=XIL(0xd9ea380), arg1=XIL(0x5250), arg2=XIL(0x1161fc03))
at ../../src/eval.c:2625
#3 0x00000000006381db in signal_or_quit (error_symbol=XIL(0x5250), data=XIL(0x1161fc03),
keyboard_quit=false) at ../../src/eval.c:1565
#4 0x000000000063806d in Fsignal (error_symbol=XIL(0x5250), data=XIL(0x1161fc03))
at ../../src/eval.c:1514
#5 0x000000000057939a in xsignal (error_symbol=XIL(0x5250), data=XIL(0x1161fc03))
at ../../src/lisp.h:3861
#6 0x0000000000638704 in signal_error (s=0x75e388 "Variable binding depth exceeds max-specpdl-size",
arg=XIL(0)) at ../../src/eval.c:1688
#7 0x00000000006398cd in grow_specpdl () at ../../src/eval.c:2080
(More stack frames follow...)
Lisp Backtrace:
"stringp" (0xfef5b398)
"file-name-non-special" (0xfef5bb58)
"file-newer-than-file-p" (0xfef5bf38)
"apply" (0xfef5c130)
"file-name-non-special" (0xfef5c6f8)
"file-newer-than-file-p" (0xfef5c998)
"apply" (0xfef5cb90)
"file-name-non-special" (0xfef5d158)
"file-newer-than-file-p" (0xfef5d3f8)
"apply" (0xfef5d5f0)
"file-name-non-special" (0xfef5dbb8)
"file-newer-than-file-p" (0xfef5de58)
"apply" (0xfef5e050)
[...]
> I'll take a look at the edebug problems later today.
Turns out this is just Bug#10577. You can work around it by loading
subr-x before instrumenting forms which use when-let*.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Fri, 26 Jan 2018 11:02:02 GMT)
Full text and
rfc822 format available.
Message #35 received at 30243 <at> debbugs.gnu.org (full text, mbox):
Noam Postavsky <npostavs <at> users.sourceforge.net> writes:
Hi Noam,
> Lisp Backtrace:
> "stringp" (0xfef5b398)
> "file-name-non-special" (0xfef5bb58)
> "file-newer-than-file-p" (0xfef5bf38)
> "apply" (0xfef5c130)
> "file-name-non-special" (0xfef5c6f8)
> "file-newer-than-file-p" (0xfef5c998)
> "apply" (0xfef5cb90)
> "file-name-non-special" (0xfef5d158)
> "file-newer-than-file-p" (0xfef5d3f8)
> "apply" (0xfef5d5f0)
> "file-name-non-special" (0xfef5dbb8)
> "file-newer-than-file-p" (0xfef5de58)
> "apply" (0xfef5e050)
> [...]
Thanks! I've fixed this in filenotify.el (in fact, the underlying watch
descriptor should not use quoted file names). Patch pushed to the branch.
In my interactive test, there was also another error:
--8<---------------cut here---------------start------------->8---
F files-file-name-non-special-handlers
(ert-test-failed
((should
(equal
(find-backup-file-name nospecial)
(mapcar
(lambda ... ...)
(find-backup-file-name tmpfile))))
:form
(equal
("/:/tmp/emacsqk0Dcl~")
("/:/tmp/emacsqk0Dcl.~1~"))
:value nil :explanation
(list-elt 0
(arrays-of-different-length 19 22 "/:/tmp/emacsqk0Dcl~" "/:/tmp/emacsqk0Dcl.~1~" first-mismatch-at 18))))
--8<---------------cut here---------------end--------------->8---
I guess, it is because I have set `version-control' to t in my .emacs
(not investigated further).
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Fri, 26 Jan 2018 22:12:02 GMT)
Full text and
rfc822 format available.
Message #38 received at 30243 <at> debbugs.gnu.org (full text, mbox):
On Fri, Jan 26, 2018 at 6:01 AM, Michael Albinus <michael.albinus <at> gmx.de> wrote:
> Thanks! I've fixed this in filenotify.el (in fact, the underlying watch
> descriptor should not use quoted file names). Patch pushed to the branch.
Ah, makes sense.
> (equal
> ("/:/tmp/emacsqk0Dcl~")
> ("/:/tmp/emacsqk0Dcl.~1~"))
> I guess, it is because I have set `version-control' to t in my .emacs
> (not investigated further).
Hmm, couldn't reproduce here, but it looks like a bug (although not as
severe as the inf loop stuff).
I've tried running the tests on w32, and discovered I missed testing
file-newer-than-file-p with both args quoted. There's also some
apparently w32-specific trouble with dired-compress-file and
insert-directory (although looking at the code, I'm not entirely sure
why dired-compress-file passed on GNU/Linux). I pushed 2 more patches
to fix file-newer-than-file-p and avoid the test w32 errors.
I'm thinking also the massive test needs to be split up, as it's a bit
unwieldy. Having one test per operation almost seems like overkill,
but I think it will make it easier to check we've covered everything,
so I'll probably go with that.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Sun, 28 Jan 2018 10:29:02 GMT)
Full text and
rfc822 format available.
Message #41 received at 30243 <at> debbugs.gnu.org (full text, mbox):
Noam Postavsky <npostavs <at> users.sourceforge.net> writes:
Hi Noam,
>> (equal
>> ("/:/tmp/emacsqk0Dcl~")
>> ("/:/tmp/emacsqk0Dcl.~1~"))
>
>> I guess, it is because I have set `version-control' to t in my .emacs
>> (not investigated further).
>
> Hmm, couldn't reproduce here, but it looks like a bug (although not as
> severe as the inf loop stuff).
Well, if I start emacs -Q, the error does not appear. Hmm, maybe I will
track it later.
> I'm thinking also the massive test needs to be split up, as it's a bit
> unwieldy. Having one test per operation almost seems like overkill,
> but I think it will make it easier to check we've covered everything,
> so I'll probably go with that.
Yes, would be better. There shall also be cleanup; I've added a missing
delete-file which was missing.
The branch scratch/nonspecial-handlers is based on Emacs 26.0.91, do you
plan to merge it there? I feel a little bit uncomfortable with this,
because my fix in file-notify-add-watch didn't get heavy testing.
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Sun, 28 Jan 2018 14:59:01 GMT)
Full text and
rfc822 format available.
Message #44 received at 30243 <at> debbugs.gnu.org (full text, mbox):
Michael Albinus <michael.albinus <at> gmx.de> writes:
>> I'm thinking also the massive test needs to be split up, as it's a bit
>> unwieldy. Having one test per operation almost seems like overkill,
>> but I think it will make it easier to check we've covered everything,
>> so I'll probably go with that.
>
> Yes, would be better. There shall also be cleanup; I've added a missing
> delete-file which was missing.
Right, should all be done now.
> The branch scratch/nonspecial-handlers is based on Emacs 26.0.91, do you
> plan to merge it there?
Yeah, I agree with Phillip that breaking "/:" files for the release
would be quite a severe regression.
> I feel a little bit uncomfortable with this, because my fix in
> file-notify-add-watch didn't get heavy testing.
However, if you think the file-notify fix might possibly affect other
things, I think leaving that part out for 26.1 would be acceptable.
Now that the code is ready, perhaps Eli has something to say on where it
should go?
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Sun, 28 Jan 2018 19:18:02 GMT)
Full text and
rfc822 format available.
Message #47 received at 30243 <at> debbugs.gnu.org (full text, mbox):
Noam Postavsky <npostavs <at> users.sourceforge.net> writes:
>> Yes, would be better. There shall also be cleanup; I've added a missing
>> delete-file which was missing.
>
> Right, should all be done now.
I've pushed another fix, which uses file-name-quote{,d-p} instead of
messing with "/:". This is cleaner, and shall also be an example for
people who care about quoted file names.
Futhermore, I've added a small test, which demonstrates how to use these
functions.
>> I feel a little bit uncomfortable with this, because my fix in
>> file-notify-add-watch didn't get heavy testing.
>
> However, if you think the file-notify fix might possibly affect other
> things, I think leaving that part out for 26.1 would be acceptable.
LGTM.
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Tue, 30 Jan 2018 13:47:02 GMT)
Full text and
rfc822 format available.
Message #50 received at 30243 <at> debbugs.gnu.org (full text, mbox):
> From: Noam Postavsky <npostavs <at> users.sourceforge.net>
> Date: Sun, 28 Jan 2018 09:58:21 -0500
> Cc: 30243 <at> debbugs.gnu.org, Philipp Stephani <p.stephani2 <at> gmail.com>
>
> > The branch scratch/nonspecial-handlers is based on Emacs 26.0.91, do you
> > plan to merge it there?
>
> Yeah, I agree with Phillip that breaking "/:" files for the release
> would be quite a severe regression.
>
> > I feel a little bit uncomfortable with this, because my fix in
> > file-notify-add-watch didn't get heavy testing.
>
> However, if you think the file-notify fix might possibly affect other
> things, I think leaving that part out for 26.1 would be acceptable.
>
> Now that the code is ready, perhaps Eli has something to say on where it
> should go?
I think to make up my mind I'd need a few words about each part of the
changes (with the exception of the test suite changes): why is each of
them needed, and what does it do to fix which part of the original
problem.
I also wonder how come we've succeeded to break quoted file names so
fundamentally -- what change did that, and why did we make it on the
release branch?
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Tue, 30 Jan 2018 16:03:02 GMT)
Full text and
rfc822 format available.
Message #53 received at 30243 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
Hi Eli,
> I think to make up my mind I'd need a few words about each part of the
> changes (with the exception of the test suite changes): why is each of
> them needed, and what does it do to fix which part of the original
> problem.
The change in `file-notify-valid-p' is not related to the original
problem; it was uncovered by the extensive tests Noam has written.
I have already proposed, not to push *this* change into the emacs-26
branch. It shall go to the master.
> Thanks.
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Tue, 30 Jan 2018 19:23:01 GMT)
Full text and
rfc822 format available.
Message #56 received at 30243 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> schrieb am Di., 30. Jan. 2018 um 14:46 Uhr:
> > From: Noam Postavsky <npostavs <at> users.sourceforge.net>
> > Date: Sun, 28 Jan 2018 09:58:21 -0500
> > Cc: 30243 <at> debbugs.gnu.org, Philipp Stephani <p.stephani2 <at> gmail.com>
> >
> > > The branch scratch/nonspecial-handlers is based on Emacs 26.0.91, do
> you
> > > plan to merge it there?
> >
> > Yeah, I agree with Phillip that breaking "/:" files for the release
> > would be quite a severe regression.
> >
> > > I feel a little bit uncomfortable with this, because my fix in
> > > file-notify-add-watch didn't get heavy testing.
> >
> > However, if you think the file-notify fix might possibly affect other
> > things, I think leaving that part out for 26.1 would be acceptable.
> >
> > Now that the code is ready, perhaps Eli has something to say on where it
> > should go?
>
> I think to make up my mind I'd need a few words about each part of the
> changes (with the exception of the test suite changes): why is each of
> them needed, and what does it do to fix which part of the original
> problem.
>
> I also wonder how come we've succeeded to break quoted file names so
> fundamentally -- what change did that, and why did we make it on the
> release branch?
>
>
If my git bisect is correct, it was
commit a1bbc490155b61a634a6d0b165000ce35b93aa35 to fix Bug#29579. So by
fixing one bug we introduced another one :(
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Wed, 31 Jan 2018 00:03:01 GMT)
Full text and
rfc822 format available.
Message #59 received at 30243 <at> debbugs.gnu.org (full text, mbox):
Philipp Stephani <p.stephani2 <at> gmail.com> writes:
> If my git bisect is correct, it was
> commit a1bbc490155b61a634a6d0b165000ce35b93aa35 to fix Bug#29579. So
> by fixing one bug we introduced another one :(
Actually, looking at Bug#29579 again, it doesn't seem *that* bad, and as
far as I can tell, it has existed for a long time (still occurs back in
24.3). So reverting that fix seems like a reasonable option too. I can
confirm that doing so fixes this bug.
> Eli Zaretskii <eliz <at> gnu.org> schrieb am Di., 30. Jan. 2018 um
> 14:46 Uhr:
>
> I think to make up my mind I'd need a few words about each part of
> the changes (with the exception of the test suite changes): why is
> each of them needed, and what does it do to fix which part of the
> original problem.
The file-name-non-special function handles all file-handler operations
for "/:" quoted files. It has an alist inside, to decide which
arguments are filenames and so require the "/:" to be removed before
calling the real operation. The alist was not complete, so for many
operations the "/:" would only be stripped from the first argument
(that's a fallback for operations not listed in the alist).
Up until the fix for Bug#29579 this didn't matter so much, because
file-name-non-special would bind file-name-handler-alist to nil, thus
preventing any further filenames from being passed to the handler
anyway. So multi-filename-arg handlers were broken, but unobtrusively
so. With the fix for Bug#29579, such handlers got stuck in infinite
recursion, as they kept getting called, not stripping the "/:" prefix,
thus getting called again, etc...
The proposed patch just fixes up this alist to finally list all the
arguments for all the handlers correctly:
@@ -7000,7 +7000,7 @@ file-name-non-special
;; Bug#25949.
(if (memq operation
'(insert-directory process-file start-file-process
- shell-command))
+ shell-command temporary-file-directory))
(directory-file-name
(expand-file-name
(unhandled-file-name-directory default-directory)))
@@ -7024,15 +7024,23 @@ file-name-non-special
;; temporarily to unquoted filename.
(verify-visited-file-modtime unquote-then-quote)
;; List the arguments which are filenames.
- (file-name-completion 1)
- (file-name-all-completions 1)
+ (file-name-completion 0 1)
+ (file-name-all-completions 0 1)
+ (file-equal-p 0 1)
+ (file-newer-than-file-p 0 1)
(write-region 2 5)
(rename-file 0 1)
(copy-file 0 1)
(copy-directory 0 1)
(file-in-directory-p 0 1)
(make-symbolic-link 0 1)
- (add-name-to-file 0 1))))
+ (add-name-to-file 0 1)
+ (make-auto-save-file-name buffer-file-name)
+ (set-visited-file-modtime buffer-file-name)
+ ;; These file-notify-* operations take a
+ ;; descriptor.
+ (file-notify-rm-watch . nil)
+ (file-notify-valid-p . nil))))
;; For all other operations, treat the first argument only
;; as the file name.
'(nil 0))))
@@ -7055,6 +7063,12 @@ file-name-non-special
(pcase method
(`identity (car arguments))
(`add (file-name-quote (apply operation arguments)))
+ (`buffer-file-name
+ (let ((buffer-file-name
+ (if (string-match "\\`/:" buffer-file-name)
+ (substring buffer-file-name (match-end 0))
+ buffer-file-name)))
+ (apply operation arguments)))
(`insert-file-contents
(let ((visit (nth 1 arguments)))
(unwind-protect
> I also wonder how come we've succeeded to break quoted file names
> so fundamentally -- what change did that, and why did we make it
> on the release branch?
IMO, the root cause is pretty clearly lack of adequate tests for this.
There are more than 60 file-handler operations; it's crazy to expect to
be able to make a correct change without an automated test that at least
exercises each one.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Wed, 31 Jan 2018 15:39:02 GMT)
Full text and
rfc822 format available.
Message #62 received at 30243 <at> debbugs.gnu.org (full text, mbox):
> From: Philipp Stephani <p.stephani2 <at> gmail.com>
> Date: Tue, 30 Jan 2018 19:22:34 +0000
> Cc: Noam Postavsky <npostavs <at> users.sourceforge.net>, michael.albinus <at> gmx.de,
> 30243 <at> debbugs.gnu.org
>
> If my git bisect is correct, it was commit a1bbc490155b61a634a6d0b165000ce35b93aa35 to fix Bug#29579.
> So by fixing one bug we introduced another one :(
Thanks. yes, this happens to us all the time.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Wed, 31 Jan 2018 16:03:02 GMT)
Full text and
rfc822 format available.
Message #65 received at 30243 <at> debbugs.gnu.org (full text, mbox):
> From: Noam Postavsky <npostavs <at> users.sourceforge.net>
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 30243 <at> debbugs.gnu.org, michael.albinus <at> gmx.de
> Date: Tue, 30 Jan 2018 19:01:52 -0500
>
> Philipp Stephani <p.stephani2 <at> gmail.com> writes:
>
> > If my git bisect is correct, it was
> > commit a1bbc490155b61a634a6d0b165000ce35b93aa35 to fix Bug#29579. So
> > by fixing one bug we introduced another one :(
>
> Actually, looking at Bug#29579 again, it doesn't seem *that* bad, and as
> far as I can tell, it has existed for a long time (still occurs back in
> 24.3). So reverting that fix seems like a reasonable option too. I can
> confirm that doing so fixes this bug.
I tend to agree. Michael, WDYT?
> > I also wonder how come we've succeeded to break quoted file names
> > so fundamentally -- what change did that, and why did we make it
> > on the release branch?
>
> IMO, the root cause is pretty clearly lack of adequate tests for this.
> There are more than 60 file-handler operations; it's crazy to expect to
> be able to make a correct change without an automated test that at least
> exercises each one.
Right, thanks.
So if Michael agrees, I think we should revert the fix for bug#29579
on the release branch, and merge the branch you two worked on onto
master.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Wed, 31 Jan 2018 18:08:01 GMT)
Full text and
rfc822 format available.
Message #68 received at 30243 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
Hi Eli,
>> Actually, looking at Bug#29579 again, it doesn't seem *that* bad, and as
>> far as I can tell, it has existed for a long time (still occurs back in
>> 24.3). So reverting that fix seems like a reasonable option too. I can
>> confirm that doing so fixes this bug.
>
> I tend to agree. Michael, WDYT?
Noam is right. The bug exist "since ever", and it wasn't detected in the
wild but when I was working on my tramp-tests. So it might be OK to
revert it in the emacs-26 branch.
> So if Michael agrees, I think we should revert the fix for bug#29579
> on the release branch, and merge the branch you two worked on onto
> master.
Agreed. I expect some broken tests in tramp-tests then (IIRC, copy-file
and rename-file were affected). I will skip those tests for Emacs 26,
immediately after the revert has happened. Pls ping me when it is
reverted.
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Wed, 31 Jan 2018 18:17:02 GMT)
Full text and
rfc822 format available.
Message #71 received at 30243 <at> debbugs.gnu.org (full text, mbox):
On Wed, Jan 31, 2018 at 1:07 PM, Michael Albinus <michael.albinus <at> gmx.de> wrote:
> Agreed. I expect some broken tests in tramp-tests then (IIRC, copy-file
> and rename-file were affected). I will skip those tests for Emacs 26,
> immediately after the revert has happened. Pls ping me when it is
> reverted.
Actually, if it's not too much trouble, I think it would be better if
you did the reverting; it looks to me like not all of the commit is
directly related to this, and I think you're probably the best able to
untangle that.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Wed, 31 Jan 2018 18:23:01 GMT)
Full text and
rfc822 format available.
Message #74 received at 30243 <at> debbugs.gnu.org (full text, mbox):
Noam Postavsky <npostavs <at> users.sourceforge.net> writes:
> Actually, if it's not too much trouble, I think it would be better if
> you did the reverting; it looks to me like not all of the commit is
> directly related to this, and I think you're probably the best able to
> untangle that.
Will do, but rather tomorrow. I'm just attending the monthly Berlin
Emacs Meetup.
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Thu, 01 Feb 2018 14:03:02 GMT)
Full text and
rfc822 format available.
Message #77 received at 30243 <at> debbugs.gnu.org (full text, mbox):
Noam Postavsky <npostavs <at> users.sourceforge.net> writes:
> Actually, if it's not too much trouble, I think it would be better if
> you did the reverting; it looks to me like not all of the commit is
> directly related to this, and I think you're probably the best able to
> untangle that.
I've pushed this to the emacs-26 branch. Philipp, could you pls check
whether everything is OK now for you?
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Thu, 01 Feb 2018 16:41:01 GMT)
Full text and
rfc822 format available.
Message #80 received at 30243 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Michael Albinus <michael.albinus <at> gmx.de> schrieb am Do., 1. Feb. 2018 um
15:01 Uhr:
> Noam Postavsky <npostavs <at> users.sourceforge.net> writes:
>
> > Actually, if it's not too much trouble, I think it would be better if
> > you did the reverting; it looks to me like not all of the commit is
> > directly related to this, and I think you're probably the best able to
> > untangle that.
>
> I've pushed this to the emacs-26 branch. Philipp, could you pls check
> whether everything is OK now for you?
>
>
Seems to work. I've tested both the batch command and visiting a quoted
file, they appear to work as expected. Thanks!
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Thu, 01 Feb 2018 18:53:02 GMT)
Full text and
rfc822 format available.
Message #83 received at 30243 <at> debbugs.gnu.org (full text, mbox):
Philipp Stephani <p.stephani2 <at> gmail.com> writes:
Hi Philipp,
> Seems to work. I've tested both the batch command and visiting a
> quoted file, they appear to work as expected. Thanks!
Thanks for checking. So we seem to be OK now with the emacs-26 branch.
I propose to close this bug after Noam has merged the
scratch/nonspecial-handlers branch into master, and the tests pass
successfully.
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Fri, 02 Feb 2018 01:18:02 GMT)
Full text and
rfc822 format available.
Message #86 received at 30243 <at> debbugs.gnu.org (full text, mbox):
Michael Albinus <michael.albinus <at> gmx.de> writes:
> Philipp Stephani <p.stephani2 <at> gmail.com> writes:
>
> Hi Philipp,
>
>> Seems to work. I've tested both the batch command and visiting a
>> quoted file, they appear to work as expected. Thanks!
>
> Thanks for checking. So we seem to be OK now with the emacs-26 branch.
>
> I propose to close this bug after Noam has merged the
> scratch/nonspecial-handlers branch into master, and the tests pass
> successfully.
I'v pushed to master.
[1: 65da409e41]: 2018-02-01 20:14:57 -0500
Test and fix "/:" quoted file name handlers (Bug#30243)
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=65da409e411a0cdfa1932d21ce8a7f87ceae9e25
[2: 00c65bcf4e]: 2018-02-01 20:15:11 -0500
Use file-name-quote{,d-p} in files-tests.el
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=00c65bcf4ee8ca4ce04ad46907de29c832b8310b
[3: e08de2bae2]: 2018-02-01 20:15:12 -0500
Handle quoted file names in filenotify.el
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=e08de2bae2a8e91c0245259dfcbfdca1d191a119
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Fri, 02 Feb 2018 17:58:01 GMT)
Full text and
rfc822 format available.
Message #89 received at 30243 <at> debbugs.gnu.org (full text, mbox):
Noam Postavsky <npostavs <at> users.sourceforge.net> writes:
Hi Noam,
> I'v pushed to master.
>
> [1: 65da409e41]: 2018-02-01 20:14:57 -0500
> Test and fix "/:" quoted file name handlers (Bug#30243)
> https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=65da409e411a0cdfa1932d21ce8a7f87ceae9e25
>
> [2: 00c65bcf4e]: 2018-02-01 20:15:11 -0500
> Use file-name-quote{,d-p} in files-tests.el
> https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=00c65bcf4ee8ca4ce04ad46907de29c832b8310b
>
> [3: e08de2bae2]: 2018-02-01 20:15:12 -0500
> Handle quoted file names in filenotify.el
> https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=e08de2bae2a8e91c0245259dfcbfdca1d191a119
Thanks. I did rerun filenotify-tests.el interactively, after applying
(setq temporary-file-directory (file-name-quote temporary-file-directory))
And indeed, the tests for a quoted remote temporary-file-directory did
fail. Fixed now, and pushed to master.
I've also extended files-tests-file-name-non-special-quote-unquote with
some more quoting/unquoting scenarii.
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#30243
; Package
emacs
.
(Sat, 03 Feb 2018 20:35:01 GMT)
Full text and
rfc822 format available.
Message #92 received at 30243 <at> debbugs.gnu.org (full text, mbox):
tags 30243 fixed
close 30243 26.1
quit
Michael Albinus <michael.albinus <at> gmx.de> writes:
> Thanks. I did rerun filenotify-tests.el interactively, after applying
> (setq temporary-file-directory (file-name-quote temporary-file-directory))
>
> And indeed, the tests for a quoted remote temporary-file-directory did
> fail. Fixed now, and pushed to master.
>
> I've also extended files-tests-file-name-non-special-quote-unquote with
> some more quoting/unquoting scenarii.
Okay, I think we can consider this bug done.
Added tag(s) fixed.
Request was from
Noam Postavsky <npostavs <at> users.sourceforge.net>
to
control <at> debbugs.gnu.org
.
(Sat, 03 Feb 2018 20:35:02 GMT)
Full text and
rfc822 format available.
bug marked as fixed in version 26.1, send any further explanations to
30243 <at> debbugs.gnu.org and phst <at> a.muc.corp.google.com
Request was from
Noam Postavsky <npostavs <at> users.sourceforge.net>
to
control <at> debbugs.gnu.org
.
(Sat, 03 Feb 2018 20:35:04 GMT)
Full text and
rfc822 format available.
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org
.
(Sun, 04 Mar 2018 12:24:04 GMT)
Full text and
rfc822 format available.
This bug report was last modified 7 years and 192 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.