bug#39057: 27.0.60; copy-file interactive VS from lisp disagreement

bug#39057: 27.0.60; copy-file interactive VS from lisp disagreement

[Tino Calancha]

Observed a disagreement when calling `copy-file' interactively VS calling
it from Lisp.

This is a recipe:

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
emacs -Q:
$ touch /tmp/foo
M-x copy-file RET /tmp/foo RET ~/ RET
;; Received prompt:
;; File /home/calancha already exists; copy to it anyway? (y or n) 

;; In the other hand, the following works, i.e., it creates ~/foo
M-: (copy-file "/tmp/foo" "~/")
=> nil
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

Reproduced w/ Emacs 26-27; with Emacs 25, the interactive call
creates ~/foo as expected.

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

In GNU Emacs 27.0.60 (build 30, x86_64-pc-linux-gnu, GTK+ Version 3.24.5)
 of 2020-01-09 built on calancha-pc.dy.bbexcite.jp
Repository revision: 58412402959d8f88e230f95c5fc7de072e115140
Repository branch: emacs-27
System Description: Debian GNU/Linux 10 (buster)

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
funcall-interactively: File already exists: /home/calancha
nil
Mark set

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

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

Major mode: Dired by name

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
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny format-spec rfc822 mml
easymenu mml-sec password-cache epa derived epg epg-config gnus-util
rmail rmail-loaddefs text-property-search time-date subr-x seq 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 dired dired-loaddefs term/xterm xterm byte-opt gv
bytecomp byte-compile cconv 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
tab-bar menu-bar rfn-eshadow isearch timer select scroll-bar mouse
jit-lock font-lock syntax facemenu 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 charscript charprop case-table epa-hook jka-cmpr-hook help
simple abbrev obarray 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
threads dbusbind inotify lcms2 dynamic-setting system-font-setting
font-render-setting move-toolbar gtk x-toolkit x multi-tty
make-network-process emacs)

Memory information:
((conses 16 53485 9324)
 (symbols 48 6097 1)
 (strings 32 15475 1668)
 (string-bytes 1 503791)
 (vectors 16 7253)
 (vector-slots 8 77927 8490)
 (floats 8 22 306)
 (intervals 56 2921 0)
 (buffers 1000 12))

bug#39057: 27.0.60; copy-file interactive VS from lisp disagreement

[Eli Zaretskii]

> From: Tino Calancha <tino.calancha@gmail.com>
> Date: Thu, 09 Jan 2020 22:03:19 +0100
> 
> Observed a disagreement when calling `copy-file' interactively VS calling
> it from Lisp.
> 
> This is a recipe:
> 
> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
> emacs -Q:
> $ touch /tmp/foo
> M-x copy-file RET /tmp/foo RET ~/ RET
> ;; Received prompt:
> ;; File /home/calancha already exists; copy to it anyway? (y or n) 

I cannot reproduce this, so there must be more here than meets the
eye, like whether /tmp/foo existed before the 'touch' command (and if
so, what kind of file is it and what are its attributes), the contents
of your home directory that could have something to do with this, etc.

So please step through the code and tell where it fails and why.
Alternatively, try reproducing this in a cleaner environment (a home
directory and /tmp could have a lot of files), and see if something
changes then.

Or maybe you didn't actually type the slash in "~/" when invoking the
command interactively?

Thanks.

bug#39057: 27.0.60; copy-file interactive VS from lisp disagreement

[Tino Calancha]

> I cannot reproduce this, so there must be more here than meets the
> eye
> So please step through the code and tell where it fails and why.

It seems NEWNAME, i.e., the second argument seeing by `copy-file' might be different
when called interactively; this is true even if the user introduces the 
same value.

I have printed out newname before the line
newname = expand_cp_target (file, newname);
at src/fileio.c

I)
M-: (copy-file "/tmp/foo" "~/") RET
;; it shows "~/" as expected

II)
M-x: copy-file RET /tmp/foo RET ~/ RET
;; it shows "~" (the '/' is missing)

Apparentely, this was unnoticed because before Emacs 26 we were using predicate
`file-directory-p`
M-: (file-directory-p "~") RET
=> t

After commit 'Fix race with rename-file etc. with dir NEWNAME'
(01c885f21f343045783eb9ad1ff5f9b83d6cd789)
we use `directory-name-p`, and the issue is revealed.

(directory-name-p "~")
=> nil

Since you cannot reproduce the issue, it might be platform dependent.
I am able to reproduce it in this nice site, which runs Emacs 26.3 in a linux machine:
https://repl.it/languages/elisp

M-! touch /tmp/foo RET
M-x copy-file RET /tmp/foo RET RET
;; received prompt
;; FILE /home/runner already exists; copy to it anyway? (yes or no)

;; Eval following forms from the *ielm* buffer 
(file-exists-p "~/foo") RET
=> nil

(copy-file "/tmp/foo" "~/")
=> nil

(file-exists-p "~/foo") RET
=> t

bug#39057: 27.0.60; copy-file interactive VS from lisp disagreement

[Eli Zaretskii]

> From: Tino Calancha <tino.calancha@gmail.com>
> Date: Fri, 10 Jan 2020 12:55:59 +0100 (CET)
> cc: Tino Calancha <tino.calancha@gmail.com>, 39057@debbugs.gnu.org
> 
> > I cannot reproduce this, so there must be more here than meets the
> > eye
> > So please step through the code and tell where it fails and why.
> 
> It seems NEWNAME, i.e., the second argument seeing by `copy-file' might be different
> when called interactively; this is true even if the user introduces the 
> same value.
> 
> I have printed out newname before the line
> newname = expand_cp_target (file, newname);
> at src/fileio.c
> 
> I)
> M-: (copy-file "/tmp/foo" "~/") RET
> ;; it shows "~/" as expected
> 
> II)
> M-x: copy-file RET /tmp/foo RET ~/ RET
> ;; it shows "~" (the '/' is missing)

Why would it be missing? which code removes it, if you typed it?

> Since you cannot reproduce the issue, it might be platform dependent.
> I am able to reproduce it in this nice site, which runs Emacs 26.3 in a linux machine:
> https://repl.it/languages/elisp

I couldn't reproduce on GNU/Linux either.

> M-! touch /tmp/foo RET
> M-x copy-file RET /tmp/foo RET RET
> ;; received prompt
> ;; FILE /home/runner already exists; copy to it anyway? (yes or no)

This is a different use case.

bug#39057: 27.0.60; copy-file interactive VS from lisp disagreement

[Eli Zaretskii]

> Date: Fri, 10 Jan 2020 15:48:02 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 39057@debbugs.gnu.org
> 
> > I have printed out newname before the line
> > newname = expand_cp_target (file, newname);
> > at src/fileio.c
> > 
> > I)
> > M-: (copy-file "/tmp/foo" "~/") RET
> > ;; it shows "~/" as expected
> > 
> > II)
> > M-x: copy-file RET /tmp/foo RET ~/ RET
> > ;; it shows "~" (the '/' is missing)
> 
> Why would it be missing? which code removes it, if you typed it?

Answering myself: this is a problem with completion routines: when the
user types a file name that is exactly identical to the default (or
just presses RET, which is the same), then the completion code in
read-file-name-internal returns a value that has the trailing slash
stripped.  And we now require a trailing slash to recognize NEWNAME as
a directory.

Maybe Stefan can help us out here.

bug#39057: 27.0.60; copy-file interactive VS from lisp disagreement

[Stefan Monnier]

> Maybe Stefan can help us out here.

I'll take a look at it ASAP,


        Stefan

bug#39057: 27.0.60; copy-file interactive VS from lisp disagreement

[Stefan Monnier]

> Answering myself: this is a problem with completion routines: when the
> user types a file name that is exactly identical to the default (or
> just presses RET, which is the same), then the completion code in
> read-file-name-internal returns a value that has the trailing slash
> stripped.  And we now require a trailing slash to recognize NEWNAME as
> a directory.

The problem seems to be linked to the value "" passed as `initial` to
`read-file-name`, it doesn't show up with a nil value for `initial`.

It seems to come from the following lines at the beginning of
read-file-name-default:

      (unless default-filename
        (setq default-filename (if initial (expand-file-name initial dir)
                                 buffer-file-name)))

because:

    M-: (expand-file-name "" "/tmp/")
    => "/tmp"

`copy-file` uses a "" argument via the `G` interactive code:

	case 'F':		/* Possibly nonexistent file name.  */
	  args[i] = read_file_name (Qnil, Qnil, Qnil, Qnil);
	  break;

	case 'G':		/* Possibly nonexistent file name,
				   default to directory alone.  */
	  args[i] = read_file_name (Qnil, Qnil, empty_unibyte_string, Qnil);
	  break;

IOW it uses `G` since otherwise hitting just RET would return the
`buffer-file-name` rather than the `default-directory`.

I propose the patch below.


        Stefan


diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 4a33d61afc..e12f7b1156 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -3027,8 +3027,13 @@ read-file-name-default
   (unless dir (setq dir (or default-directory "~/")))
   (unless (file-name-absolute-p dir) (setq dir (expand-file-name dir)))
   (unless default-filename
-    (setq default-filename (if initial (expand-file-name initial dir)
-                             buffer-file-name)))
+    (setq default-filename
+          (cond
+           ((null initial) buffer-file-name)
+           ;; Special-case "" because (expand-file-name "" "/tmp/") returns
+           ;; "/tmp" rather than "/tmp/" (bug#39057).
+           ((equal "" initial) dir)
+           (t (expand-file-name initial dir)))))
   ;; If dir starts with user's homedir, change that to ~.
   (setq dir (abbreviate-file-name dir))
   ;; Likewise for default-filename.

bug#39057: 27.0.60; copy-file interactive VS from lisp disagreement

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: tino.calancha@gmail.com,  39057@debbugs.gnu.org
> Date: Wed, 15 Jan 2020 15:35:37 -0500
> 
> IOW it uses `G` since otherwise hitting just RET would return the
> `buffer-file-name` rather than the `default-directory`.
> 
> I propose the patch below.

LGTM, please install on the emacs-27 branch, and thanks.

bug#39057: 27.0.60; copy-file interactive VS from lisp disagreement

[Stefan Monnier]

> LGTM, please install on the emacs-27 branch, and thanks.

OK, done,


        Stefan