bug#58842: 29.0.50/tramp; copy-file KEEP-TIME gives the target earlier time

bug#58842: 29.0.50/tramp; copy-file KEEP-TIME gives the target earlier time

[Sam Steingold]

One property of `copy-file' that I came to expect is:

(copy-file src dst ...) is successful
==>
(file-newer-than-file-p src dst) is false

This ensures that the same file is not copied over and over.

Alas, this does not hold in the following corner case:

1. src is local on mac
2. dst is remote on linux
3. KEEP-TIME is passed to `copy-file'

In this case, after `(copy-file src dst t t)`,
--8<---------------cut here---------------start------------->8---
(file-attribute-modification-time (file-attributes dst))
=> (25435 65247 0 0)
(file-attribute-modification-time (file-attributes src))
=> (25435 65247 279367 729000)
--8<---------------cut here---------------end--------------->8---

I understand this is bad behavior is covered by the `copy-file' doc
--8<---------------cut here---------------start------------->8---
Fourth arg KEEP-TIME non-nil means give the output file the same
last-modified time as the old one.  (This works on only some systems.)
--8<---------------cut here---------------end--------------->8---

but it would seem to be relatively easy to fix by rounding _up_ the time
before calling the tramp handler for `set-file-times'.

Thank you.


In GNU Emacs 29.0.50 (build 1, x86_64-apple-darwin21.6.0, NS
 appkit-2113.60 Version 12.6 (Build 21G115)) of 2022-10-24 built on
 3c22fb11fdab.ant.amazon.com
Repository revision: 366169450392c7025665f6477e5062be72c7280f
Repository branch: master
Windowing system distributor 'Apple', version 10.3.2113
System Description:  macOS 12.6

Configured using:
 'configure --with-imagemagick --with-mailutils --with-ns
 PKG_CONFIG_PATH='

Configured features:
ACL GIF GMP GNUTLS IMAGEMAGICK JPEG JSON LCMS2 LIBXML2 MODULES NOTIFY
KQUEUE NS PDUMPER PNG SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS WEBP XIM
ZLIB

Important settings:
  value of $LANG: C
  locale-coding-system: utf-8-unix

Major mode: Org

Minor modes in effect:
  shell-dirtrack-mode: t
  pyvenv-mode: t
  flyspell-mode: t
  display-fill-column-indicator-mode: t
  global-edit-server-edit-mode: t
  winner-mode: t
  which-function-mode: t
  url-handler-mode: t
  desktop-save-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  column-number-mode: t
  line-number-mode: t
  auto-fill-function: org-auto-fill-function
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  abbrev-mode: t

Load-path shadows:
None found.

Features:
(shadow bbdb-message mailalias cookie1 emacsbug tramp-adb tramp-archive
tramp-container tramp-ftp tramp-gvfs shortdoc tramp-cmds url-http url-gw
url-auth url-cache nndoc qp flow-fill mm-archive sort smiley gnus-cite
textsec uni-scripts idna-mapping uni-confusable textsec-check gnus-async
gnus-bcklg gnus-dup mail-extr gnus-ml hl-line disp-table spam spam-stat
gnus-uu yenc nndraft nnmh gnus-agent gnus-srvr gnus-score score-mode
nnvirtual gnus-msg utf-7 gnus-cache bbdb-gnus network-stream nsm nntp
cal-move package apropos cl-print skeleton dabbrev log-edit vc-src
vc-sccs vc-svn vc-cvs vc-rcs log-view pcvs-util misearch multi-isearch
vc-annotate display-line-numbers smerge-mode diff add-log vc-hg vc-bzr
tramp-cache time-stamp tramp-sh tramp tramp-loaddefs trampver
tramp-integration tramp-compat ls-lisp remember emacs-news-mode tex-mode
shell dired-aux mhtml-mode css-mode smie color js cc-mode cc-fonts
cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs
sgml-mode facemenu bug-reference vc-dir ewoc vc company-oddmuse
company-keywords company-etags company-gtags company-dabbrev-code
company-dabbrev company-files company-clang company-template
company-cmake company-bbdb yasnippet-snippets yasnippet flymake-proc
flymake company-capf company help-fns radix-tree elpy elpy-rpc pyvenv
eshell esh-cmd esh-ext esh-opt esh-proc esh-io esh-arg esh-module
esh-groups esh-util cus-start elpy-shell elpy-profile elpy-django s
elpy-refactor ido hideshow grep compile files-x etags fileloop xref
python project compat compat-macs flyspell ispell
display-fill-column-indicator org-element avl-tree generator cl-extra
ol-eww eww xdg url-queue thingatpt mm-url ol-rmail ol-mhe ol-irc ol-info
ol-gnus nnselect gnus-art mm-uu mml2015 mm-view mml-smime smime gnutls
dig gnus-sum shr pixel-fill kinsoku url-file svg dom browse-url url
url-proxy url-privacy url-expand url-methods url-history url-cookie
generate-lisp-file url-domsuf url-util gnus-group gnus-undo gnus-start
gnus-dbus dbus xml gnus-cloud nnimap nnmail mail-source utf7 nnoo
parse-time gnus-spec gnus-int gnus-range message sendmail mailcap
yank-media puny rfc822 mml mml-sec epa derived epg rfc6068 epg-config
mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045
ietf-drums gmm-utils mailheader gnus-win ol-docview doc-view filenotify
jka-compr image-mode exif ol-bibtex ol-bbdb ol-w3m ol-doi org-link-doi
org ob ob-tangle ob-ref ob-lob ob-table ob-exp org-macro org-footnote
org-src ob-comint org-pcomplete pcomplete comint ansi-osc ansi-color
org-list org-faces org-entities noutline outline org-version
ob-emacs-lisp ob-core ob-eval org-table oc-basic bibtex iso8601 ol rx
org-keys oc org-compat org-macs org-loaddefs format-spec cal-x view
cal-china cal-bahai cal-islam holidays holiday-loaddefs bbdb-anniv
cal-iso cal-hebrew lunar cal-julian solar cal-dst appt diary-lib
diary-loaddefs cal-menu calendar cal-loaddefs vc-git diff-mode
easy-mmode vc-dispatcher midnight dired dired-loaddefs cus-edit pp
warnings icons gnus nnheader gnus-util text-property-search time-date
mail-utils range mm-util mail-prsvr wid-edit bbdb-mua bbdb-com crm
mailabbrev bbdb bbdb-site timezone modus-vivendi-theme modus-themes
pcase edit-server advice server winner ring which-func imenu edebug
debug backtrace help-mode find-func url-handlers url-parse auth-source
cl-seq eieio eieio-core cl-macs password-cache json subr-x map byte-opt
gv bytecomp byte-compile cconv url-vars help-at-pt desktop frameset
cl-loaddefs cl-lib cus-load info bbdb-autoloads edit-server-autoloads
ein-autoloads elpy-autoloads company-autoloads fb2-reader-autoloads
async-autoloads f-autoloads dash-autoloads markdown-mode-autoloads
polymode-autoloads request-autoloads s-autoloads websearch-autoloads
with-editor-autoloads compat-autoloads yaml-mode-autoloads
yasnippet-snippets-autoloads rmc iso-transl tooltip 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 lcms2
multi-tty make-network-process emacs)

Memory information:
((conses 16 802255 121214)
 (symbols 48 41505 4)
 (strings 32 285576 10877)
 (string-bytes 1 7676394)
 (vectors 16 110700)
 (vector-slots 8 2392334 272891)
 (floats 8 1495 665)
 (intervals 56 17832 6120)
 (buffers 1000 48))

-- 
Sam Steingold (https://aphar.dreamwidth.org/) on darwin Ns 10.3.2113
https://lastingimpactpsychology.com https://steingoldpsychology.com
http://think-israel.org https://www.dhimmitude.org https://memri.org
Do not worry about which side your bread is buttered on: you eat BOTH sides.

bug#58842: 29.0.50/tramp; copy-file KEEP-TIME gives the target earlier time

[Michael Albinus]

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

Sam Steingold <sds@gnu.org> writes:

Hi Sam,

> One property of `copy-file' that I came to expect is:
>
> (copy-file src dst ...) is successful
> ==>
> (file-newer-than-file-p src dst) is false
>
> This ensures that the same file is not copied over and over.
>
> Alas, this does not hold in the following corner case:
>
> 1. src is local on mac
> 2. dst is remote on linux
> 3. KEEP-TIME is passed to `copy-file'
>
> In this case, after `(copy-file src dst t t)`,
>
> (file-attribute-modification-time (file-attributes dst))
> => (25435 65247 0 0)
> (file-attribute-modification-time (file-attributes src))
> => (25435 65247 279367 729000)
>
> I understand this is bad behavior is covered by the `copy-file' doc
>
> Fourth arg KEEP-TIME non-nil means give the output file the same
> last-modified time as the old one.  (This works on only some systems.)
>
> but it would seem to be relatively easy to fix by rounding _up_ the time
> before calling the tramp handler for `set-file-times'.

Well, another fix would be to teach file-newer-than-file-p to use only
seconds w/o fractions. Does the appended patch works for you?

> Thank you.

Best regards, Michael.


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

diff --git a/lisp/tramp.el b/lisp/tramp.el
index 933bac7e..59306635 100644
--- a/lisp/tramp.el
+++ b/lisp/tramp.el
@@ -4029,6 +4029,17 @@ Let-bind it when necessary.")
   (cond
    ((not (file-exists-p file1)) nil)
    ((not (file-exists-p file2)) t)
+   ;; Tramp reads and writes timestamps on secon level.  So we round
+   ;; the timestamps to seconds w/o fractions.
+   ;; `time-convert' has been introduced with Emacs 27.1.
+   ((fboundp 'time-convert)
+    (time-less-p
+     (tramp-compat-funcall
+      'time-convert
+      (file-attribute-modification-time (file-attributes file2)) 'integer)
+     (tramp-compat-funcall
+      'time-convert
+      (file-attribute-modification-time (file-attributes file1)) 'integer)))
    (t (time-less-p
        (file-attribute-modification-time (file-attributes file2))
        (file-attribute-modification-time (file-attributes file1))))))

bug#58842: 29.0.50/tramp; copy-file KEEP-TIME gives the target earlier time

[Sam Steingold]

Hi Michael,

> * Michael Albinus <zvpunry.nyovahf@tzk.qr> [2022-10-30 16:42:21 +0100]:
>
> Sam Steingold <sds@gnu.org> writes:
>
>> One property of `copy-file' that I came to expect is:
>>
>> (copy-file src dst ...) is successful
>> ==>
>> (file-newer-than-file-p src dst) is false
>>
>> This ensures that the same file is not copied over and over.
>>
>> Alas, this does not hold in the following corner case:
>>
>> 1. src is local on mac
>> 2. dst is remote on linux
>> 3. KEEP-TIME is passed to `copy-file'
>>
>> In this case, after `(copy-file src dst t t)`,
>>
>> (file-attribute-modification-time (file-attributes dst))
>> => (25435 65247 0 0)
>> (file-attribute-modification-time (file-attributes src))
>> => (25435 65247 279367 729000)
>>
>
> Well, another fix would be to teach file-newer-than-file-p to use only
> seconds w/o fractions. Does the appended patch works for you?
>

Yes, thank you, it does appear to fix the issue.
Please note a typo in the comment in your patch.


-- 
Sam Steingold (https://aphar.dreamwidth.org/) on darwin Ns 10.3.2113
https://lastingimpactpsychology.com https://steingoldpsychology.com
https://fairforall.org http://think-israel.org https://www.dhimmitude.org
Two wrongs don't make a right, but three rights make a left.

bug#58842: 29.0.50/tramp; copy-file KEEP-TIME gives the target earlier time

[Michael Albinus]

Version: 29.1

Sam Steingold <sds@gnu.org> writes:

> Hi Michael,

Hi Sam,

>> Well, another fix would be to teach file-newer-than-file-p to use only
>> seconds w/o fractions. Does the appended patch works for you?
>
> Yes, thank you, it does appear to fix the issue.
> Please note a typo in the comment in your patch.

Thanks for the feedback. I've pushed the patch to master, incl fixing
the typo. Closing the bug.

Best regards, Michael.