bug#68805: 29.2; Tramp: out-of-band copy-file fails when tramp-copy-keep-date is nil

bug#68805: 29.2; Tramp: out-of-band copy-file fails when tramp-copy-keep-date is nil

[Sean Devlin]

Hi folks,

I created a custom Tramp method with support for out-of-band file
transfer using tramp-copy-program, and I left tramp-copy-keep-date
unset. When I try to copy a file to the remote using copy-file with the
keep-date argument set to t, the transfer succeeds, but Emacs raises an
error. If I set tramp-copy-keep-date to t, there is no error.

You can reproduce the error by modifying the scp method:

1. Emacs -Q
2. Evaluate these forms:

(setf (alist-get 'tramp-copy-keep-date 
		 (alist-get "scp" tramp-methods nil nil #'string-equal))
      '(nil))

(copy-file "/path/to/bigfile" "/scp:your-remote:bigfile" nil t)

3. Emacs will raise a file-missing error
4. Type q to dismiss the backtrace
5. Evaluate:

(dired "/scp:your-remote:")

6. Verify the file was transferred despite the error

I think the issue is due to caching. Before copying, Tramp checks to see
if the file exists, and it does not. After the transfer, Tramp tries to
check again because the keep-date argument was set, tramp-copy-keep-date
is nil or unset, and Tramp wants to set the file time manually. Instead
of actually doing this file existence check, Tramp instead gets the
property from cache that was set just prior to the file transfer. It
then raises a file-missing error.

Thanks,

Sean

In GNU Emacs 29.2 (build 1, aarch64-apple-darwin21.6.0, NS
appkit-2113.60 Version 12.6.6 (Build 21G646)) of 2024-01-18 built on
armbob.lan
Windowing system distributor 'Apple'
System Description:  macOS 14

Configured using:
'configure --with-ns '--enable-locallisppath=/Library/Application
Support/Emacs/${version}/site-lisp:/Library/Application
Support/Emacs/site-lisp' --with-modules 'CFLAGS=-DFD_SETSIZE=10000
-DDARWIN_UNLIMITED_SELECT' --with-x-toolkit=no'

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

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

Major mode: Lisp Interaction

Minor modes in effect:
  shell-dirtrack-mode: t
  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 epa derived epg rfc6068 epg-config
gnus-util text-property-search mm-decode mm-bodies mm-encode mail-parse
rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045
ietf-drums mm-util mail-prsvr mail-utils help-fns radix-tree debug
backtrace help-mode find-func tramp-cmds cl-print ielm pp tramp-cache
time-stamp tramp-sh tramp tramp-loaddefs trampver tramp-integration
files-x tramp-compat rx shell pcomplete comint ansi-osc ring parse-time
iso8601 time-date format-spec auth-source cl-seq eieio eieio-core
cl-macs cl-loaddefs cl-lib password-cache json subr-x map byte-opt gv
bytecomp byte-compile ansi-color 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
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 emacs)

Memory information:
((conses 16 88380 7496)
(symbols 48 8801 0)
(strings 32 27273 1929)
(string-bytes 1 912327)
(vectors 16 18170)
(vector-slots 8 252906 8762)
(floats 8 42 224)
(intervals 56 1620 55)
(buffers 984 16))

bug#68805: 29.2; Tramp: out-of-band copy-file fails when tramp-copy-keep-date is nil

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

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

Sean Devlin <spd@toadstyle.org> writes:

> Hi folks,

Hi Sean,

> I think the issue is due to caching. Before copying, Tramp checks to see
> if the file exists, and it does not. After the transfer, Tramp tries to
> check again because the keep-date argument was set, tramp-copy-keep-date
> is nil or unset, and Tramp wants to set the file time manually. Instead
> of actually doing this file existence check, Tramp instead gets the
> property from cache that was set just prior to the file transfer. It
> then raises a file-missing error.

Thanks for the report and the analysis. It is correct. Could you please
check, whether this patch fixes it:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 597 bytes --]

diff --git a/lisp/tramp-sh.el b/lisp/tramp-sh.el
index 1301cd63..50a33cc3 100644
--- a/lisp/tramp-sh.el
+++ b/lisp/tramp-sh.el
@@ -2521,6 +2521,10 @@ The method used must be an out-of-band method."
 	    ;; cached password).
 	    (tramp-cleanup-connection v 'keep-debug 'keep-password))))

+      ;; The cached file properties might be wrong if NEWNAME didn't
+      ;; exist.  Flush them.
+      (tramp-flush-file-properties v (tramp-file-local-name newname))
+
       ;; Handle KEEP-DATE argument.
       (when (and keep-date (not copy-keep-date))
 	(tramp-compat-set-file-times

[-- Attachment #3: Type: text/plain, Size: 44 bytes --]


> Thanks,
>
> Sean

Best regards, Michael.

bug#68805: 29.2; Tramp: out-of-band copy-file fails when tramp-copy-keep-date is nil

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

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

Michael Albinus via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

Hi Sean,

> Thanks for the report and the analysis. It is correct. Could you please
> check, whether this patch fixes it:

Oops, that's too lax. Pls try the following instead:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 638 bytes --]

diff --git a/lisp/tramp-sh.el b/lisp/tramp-sh.el
index 1301cd63..44c0bdc7 100644
--- a/lisp/tramp-sh.el
+++ b/lisp/tramp-sh.el
@@ -2521,6 +2521,12 @@ The method used must be an out-of-band method."
 	    ;; cached password).
 	    (tramp-cleanup-connection v 'keep-debug 'keep-password))))

+      ;; The cached file properties might be wrong if NEWNAME didn't
+      ;; exist.  Flush them.
+      (when v2
+	(with-parsed-tramp-file-name newname v2
+	  (tramp-flush-file-properties v2 v2-localname)))
+
       ;; Handle KEEP-DATE argument.
       (when (and keep-date (not copy-keep-date))
 	(tramp-compat-set-file-times

[-- Attachment #3: Type: text/plain, Size: 24 bytes --]


Best regards, Michael.

bug#68805: 29.2; Tramp: out-of-band copy-file fails when tramp-copy-keep-date is nil

[Sean Devlin]

Hi Michael,

I can confirm the below patch fixes the issue. Thanks for your help!

Regarding the comment, couldn’t some cached file properties (i.e. other than
file-exists-p) be wrong even if NEWNAME did already exist? I haven’t really
thought this through, just curious.

Thanks!

> On Feb 1, 2024, at 8:49 AM, Michael Albinus <michael.albinus@gmx.de> wrote:
> 
> Michael Albinus via "Bug reports for GNU Emacs, the Swiss army knife of
> text editors" <bug-gnu-emacs@gnu.org> writes:
> 
> Hi Sean,
> 
>> Thanks for the report and the analysis. It is correct. Could you please
>> check, whether this patch fixes it:
> 
> Oops, that's too lax. Pls try the following instead:
> 
> diff --git a/lisp/tramp-sh.el b/lisp/tramp-sh.el
> index 1301cd63..44c0bdc7 100644
> --- a/lisp/tramp-sh.el
> +++ b/lisp/tramp-sh.el
> @@ -2521,6 +2521,12 @@ The method used must be an out-of-band method."
> 	    ;; cached password).
> 	    (tramp-cleanup-connection v 'keep-debug 'keep-password))))
> 
> +      ;; The cached file properties might be wrong if NEWNAME didn't
> +      ;; exist.  Flush them.
> +      (when v2
> +	(with-parsed-tramp-file-name newname v2
> +	  (tramp-flush-file-properties v2 v2-localname)))
> +
>       ;; Handle KEEP-DATE argument.
>       (when (and keep-date (not copy-keep-date))
> 	(tramp-compat-set-file-times
> 
> Best regards, Michael.

bug#68805: 29.2; Tramp: out-of-band copy-file fails when tramp-copy-keep-date is nil

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

Version: 29.3

Sean Devlin <spd@toadstyle.org> writes:

> Hi Michael,

Hi Sean,

> I can confirm the below patch fixes the issue. Thanks for your help!

Thanks for the feedback. I've pushed the fix to the repositories. Will
appear with the next Tramp release on GNU ELPA (2.6.2.2) later this
month. If there is an Emacs 29.3, it will also contain the fix.

In Emacs 30.0.50 (Tramp 2.7) this patch isn't needed, because due to
reorganization of the code, the cache is flushed in time.

> Regarding the comment, couldn’t some cached file properties (i.e. other than
> file-exists-p) be wrong even if NEWNAME did already exist? I haven’t really
> thought this through, just curious.

Hmm, the comment isn't precise enough, yes. It reflects this bug. But
the code is.

If you like to contribute to Tramp (Emacs), send a patch :-)

> Thanks!

Best regards, Michael.

bug#68805: 29.2; Tramp: out-of-band copy-file fails when tramp-copy-keep-date is nil

[Sean Devlin]

Hi Michael,

> On Feb 1, 2024, at 11:32 AM, Michael Albinus <michael.albinus@gmx.de> wrote:
> 
> Version: 29.3
> 
> Sean Devlin <spd@toadstyle.org> writes:
> 
>> Hi Michael,
> 
> Hi Sean,
> 
>> I can confirm the below patch fixes the issue. Thanks for your help!
> 
> Thanks for the feedback. I've pushed the fix to the repositories. Will
> appear with the next Tramp release on GNU ELPA (2.6.2.2) later this
> month. If there is an Emacs 29.3, it will also contain the fix.
> 
> In Emacs 30.0.50 (Tramp 2.7) this patch isn't needed, because due to
> reorganization of the code, the cache is flushed in time.

Sounds good. Thanks!

> 
>> Regarding the comment, couldn’t some cached file properties (i.e. other than
>> file-exists-p) be wrong even if NEWNAME did already exist? I haven’t really
>> thought this through, just curious.
> 
> Hmm, the comment isn't precise enough, yes. It reflects this bug. But
> the code is.
> 
> If you like to contribute to Tramp (Emacs), send a patch :-)

I would like to, but my current job makes it difficult for me to contribute in this
way. Maybe someday that will change.

For now, I will try to file good bug reports. :)

> 
>> Thanks!
> 
> Best regards, Michael.