From cb03cdf1e7e727d596526690435c86fa5299f1ee Mon Sep 17 00:00:00 2001 In-Reply-To: <87sfk628ca.fsf@localhost> References: <87sfk628ca.fsf@localhost> From: Max Nikulin Date: Fri, 6 May 2022 23:34:52 +0700 Subject: [PATCH v2] org-macs.el: Do not compare wall time and file modification time To: emacs-orgmode@gnu.org * lisp/org-macs.el (org-file-newer-than-p): Recommend passing file modification time instead of wall time to avoid truncation of timestamp precision for the sake of filesystems with coarse time resolution. (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): Use `file-newer-than-file-p' instead of `org-file-newer-than-p' since the former is more suitable for target-prerequisite relation in the case of equal timestamps. Improve error reporting when source file does not exist. Update timestamp after tangling of an org file, not before it. This is assumed to be a better fix of the problem with change of time representation in Emacs-29. The problem was reported by Mark Barton in https://list.orgmode.org/BF5B9308-3FEF-4DC6-98C9-BFF36F19D36C@gmail.com Paul Eggert committed another variant to Emacs as 3abb3681b5. It was ported to Org as commit 56ba22b9df several months later. Unchanged timestamp of a file means failure of `org-compile-file' but in `org-babel-load-file' the target may be considered as up to date if its timestamp is equal to the one for the prerequisite. So `org-file-newer-than-p' is not suitable for both cases. The difference matters for filesystems with coarse timestamp resolution, for example HFS+. Earlier call of `org-babel-load-file' for a non-existing .org file caused "Bad bounding indices: 0, 2" error. Update file timestamp (introduced by the commit 1525a5a64e) after tangling of the file. Change caused by conflict resolution during rebasing of the initial version of the patch. See https://list.orgmode.org/t75efi$9pv$1@ciao.gmane.io for an argument in support of such change. --- lisp/org-macs.el | 30 ++++++++++++++++++------------ lisp/org.el | 22 +++++++++++----------- 2 files changed, 29 insertions(+), 23 deletions(-) diff --git a/lisp/org-macs.el b/lisp/org-macs.el index 2666733d0..c858f3695 100644 --- a/lisp/org-macs.el +++ b/lisp/org-macs.el @@ -312,17 +312,23 @@ If EXCLUDE-TMP is non-nil, ignore temporary buffers." ;;; 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 Lisp time value, 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 (org-time-convert-to-integer - (nth 5 (file-attributes file))) - (org-time-convert-to-integer time))))) + "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. + +Consider `file-newer-than-file-p' to check up to date state +in target-prerequisite files relation." + (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. @@ -356,7 +362,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 6f4e04306..fa73e2976 100644 --- a/lisp/org.el +++ b/lisp/org.el @@ -254,22 +254,22 @@ 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)))) + ;; Tangle only if the Elisp file is older than the Org file. + ;; Catch the case when the .el file exists while the .org file is missing. + (unless (file-exists-p file) + (error "File to tangle does not exist: %s" file)) + (when (file-newer-than-file-p file tangled-file) + (org-babel-tangle-file file + tangled-file + (rx string-start + (or "emacs-lisp" "elisp") + string-end)) ;; Make sure that tangled file modification time is ;; updated even when `org-babel-tangle-file' does not make changes. ;; This avoids re-tangling changed FILE where the changes did ;; not affect the tangled code. (when (file-exists-p tangled-file) - (set-file-times tangled-file)) - (org-babel-tangle-file file - tangled-file - (rx string-start - (or "emacs-lisp" "elisp") - string-end))) + (set-file-times tangled-file))) (if compile (progn (byte-compile-file tangled-file) -- 2.25.1