From: Max Nikulin <email@example.com> To: Paul Eggert <firstname.lastname@example.org>, Bastien <email@example.com> Cc: emacs-orgmode <firstname.lastname@example.org>, email@example.com Subject: Re: [PATCH] org-macs.el: Do not compare wall time and file modification time Date: Wed, 11 May 2022 19:28:13 +0700 [thread overview] Message-ID: <firstname.lastname@example.org> (raw) In-Reply-To: <email@example.com> [-- Attachment #1: Type: text/plain, Size: 947 bytes --] On 06/05/2022 23:56, Max Nikulin wrote: > Mark Barton to emacs-orgmode, emacs-devel. master 4a1f69ebca 2/2: Use > (TICKS . HZ) for current-time etc. Tue, 26 Apr 2022 23:37:50 -0700. > https://list.orgmode.org/BF5B9308-3FEF-4DC6-98C9-BFF36F19D36C@gmail.com >> >> The change also breaks org-file-newer-than-p function that triggered the >> debugger while loading my init that uses org babel. > > I think, it should be fixed in the bugfix Org branch. The attached > patch is a compromise to some degree, but I do not see a robust solution. Thinking more I realized that `org-file-newer-than-p' should not be reused for `org-babel-load-file'. I am attaching an updated patch for which I do not see any real drawback. The only change in behavior is that if a file had modification time in future and it is overwritten by `org-compile-time' to current time than the function reports failure. I consider such case as a rare and peculiar one. [-- Attachment #2: 0001-org-macs.el-Do-not-compare-wall-time-and-file-modifi.patch --] [-- Type: text/x-patch, Size: 5204 bytes --] From e5f98dbc729904297bef529009ade96361dd4dd2 Mon Sep 17 00:00:00 2001 From: Max Nikulin <firstname.lastname@example.org> Date: Fri, 6 May 2022 23:34:52 +0700 Subject: [PATCH] org-macs.el: Do not compare wall time and file modification time * lisp/org-macs.el (org-file-newer-than-p): Fix Emacs-29 problem with changed representation of system clock timestamp. Recommend passing file modification time and do not truncate its precision. (org-compile-file): Store file modification time instead of system clock for later comparison by `org-file-newer-than-p'. * lisp/org.el (org-babel-load-file): Do not use `org-file-newer-than-p' to consider the .el file as up to date when its modification time is the same as for the source .org file. Unchanged timestamp of a file means failure of `org-compile-file' but in `org-babel-load-file' the target may be considered up to date if its timestamp is equal to the one for prerequisite. So `org-file-newer-than-p' is not suitable for both cases. The difference matter for filesystems with coarse timestamp resolution, for example HFS+. Reported by Mark Barton <email@example.com> https://list.orgmode.org/BF5B9308-3FEF-4DC6-98C9-BFF36F19D36C@gmail.com During discussion of the issue Paul Eggert <firstname.lastname@example.org> suggested over variants of the changes in the same thread. --- lisp/org-macs.el | 32 +++++++++++++++++++++----------- lisp/org.el | 18 ++++++++++++------ 2 files changed, 33 insertions(+), 17 deletions(-) diff --git a/lisp/org-macs.el b/lisp/org-macs.el index b10725bd5..556bf658d 100644 --- a/lisp/org-macs.el +++ b/lisp/org-macs.el @@ -256,16 +256,26 @@ ignored in this case." ;;; File (defun org-file-newer-than-p (file time) - "Non-nil if FILE is newer than TIME. -FILE is a filename, as a string, TIME is a list of integers, as -returned by, e.g., `current-time'." - (and (file-exists-p file) - ;; Only compare times up to whole seconds as some file-systems - ;; (e.g. HFS+) do not retain any finer granularity. As - ;; a consequence, make sure we return non-nil when the two - ;; times are equal. - (not (time-less-p (cl-subseq (nth 5 (file-attributes file)) 0 2) - (cl-subseq time 0 2))))) + "Non-nil if FILE modification time is greater than TIME. +TIME should be obtained earlier for the same FILE name using + + (file-attribute-modification-time (file-attributes file)) + +If TIME is nil (file did not exist) then any existing FILE +is considered as a newer one. Some file systems have coarse +timestamp resolution, for example 1 second on HFS+ or 2 seconds on FAT, +so nil may be returned when file is updated twice within a short period +of time. File timestamp and system clock `current-time' may have +different resolution, so attempts to compare them may give unexpected +results. + +Attempt to check whether a derived file has been updated in +response to modification of its source file may give unreliable +result. Equal timestamps in such case may mean that the derived +file is up to date however this function returns nil assuming +that the FILE is not modified." + (let ((mtime (file-attribute-modification-time (file-attributes file)))) + (and mtime (or (not time) (time-less-p time mtime))))) (defun org-compile-file (source process ext &optional err-msg log-buf spec) "Compile a SOURCE file using PROCESS. @@ -299,7 +309,7 @@ it for output." (full-name (file-truename source)) (out-dir (or (file-name-directory source) "./")) (output (expand-file-name (concat base-name "." ext) out-dir)) - (time (current-time)) + (time (file-attribute-modification-time (file-attributes output))) (err-msg (if (stringp err-msg) (concat ". " err-msg) ""))) (save-window-excursion (pcase process diff --git a/lisp/org.el b/lisp/org.el index 54350faee..c1ce57c4d 100644 --- a/lisp/org.el +++ b/lisp/org.el @@ -232,12 +232,18 @@ and then loads the resulting file using `load-file'. With optional prefix argument COMPILE, the tangled Emacs Lisp file is byte-compiled before it is loaded." (interactive "fFile to load: \nP") - (let ((tangled-file (concat (file-name-sans-extension file) ".el"))) - ;; Tangle only if the Org file is newer than the Elisp file. - (unless (org-file-newer-than-p - tangled-file - (file-attribute-modification-time - (file-attributes (file-truename file)))) + (let* ((tangled-file (concat (file-name-sans-extension file) ".el")) + (file-mtime (file-attribute-modification-time + (file-attributes (file-truename file)))) + (tangled-mtime (file-attribute-modification-time + (file-attributes (file-truename tangled-file))))) + ;; Tangle only if the Elisp file is older than the Org file. + ;; Filesystem may have coarse timestamp resolution (HFS+, FAT) + ;; so no need to update if timestamps are equal and thus + ;; `org-file-newer-than-p' can not be used here. + (unless (and file-mtime + tangled-mtime + (not (time-less-p tangled-mtime file-mtime))) (org-babel-tangle-file file tangled-file (rx string-start -- 2.25.1
next prev parent reply other threads:[~2022-05-11 12:30 UTC|newest] Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-04-27 6:37 master 4a1f69ebca 2/2: Use (TICKS . HZ) for current-time etc Mark Barton 2022-04-27 7:20 ` Po Lu 2022-04-27 7:39 ` Paul Eggert 2022-04-27 16:55 ` Stefan Monnier 2022-04-28 22:27 ` Paul Eggert 2022-04-29 14:22 ` Max Nikulin 2022-04-29 18:10 ` Paul Eggert 2022-04-30 10:56 ` Max Nikulin 2022-05-06 16:56 ` [PATCH] org-macs.el: Do not compare wall time and file modification time Max Nikulin 2022-05-11 12:28 ` Max Nikulin [this message] 2022-05-11 16:24 ` Paul Eggert 2022-05-12 16:55 ` Max Nikulin 2022-05-12 22:52 ` Paul Eggert 2022-05-13 12:28 ` Max Nikulin 2022-05-13 18:00 ` Paul Eggert
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style List information: https://www.orgmode.org/ * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: [PATCH] org-macs.el: Do not compare wall time and file modification time' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Code repositories for project(s) associated with this inbox: https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).