[PATCH v2] org-macs.el: Do not compare wall time and file modification time

[Max Nikulin <manikulin@gmail.com>]

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

On 02/10/2022 10:49, Ihor Radchenko wrote:
> Max Nikulin writes:
> 
>> From: Max Nikulin
>> Date: Fri, 6 May 2022 23:34:52 +0700
>> Subject: [PATCH] org-macs.el: Do not compare wall time and file modification
>>   time
> 
> Max, the patch does not currently apply onto main. Can you please update
> it?

When prepared, it was intended for the bugfix branch to avoid problems 
in the case of Emacs-29 and Org as an ELPA package. Changes in the main 
branch since that time caused conflicts: Kyle backported Paul's fix from 
Emacs to Org, you added `set-file-time' to `org-babel-load-file'.

[-- Attachment #2: v2-0001-org-macs.el-Do-not-compare-wall-time-and-file-mod.patch --]
[-- Type: text/x-patch, Size: 6181 bytes --]

From cb03cdf1e7e727d596526690435c86fa5299f1ee Mon Sep 17 00:00:00 2001
In-Reply-To: <87sfk628ca.fsf@localhost>
References: <87sfk628ca.fsf@localhost>
From: Max Nikulin <manikulin@gmail.com>
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
<mbarton98@gmail.com> in
https://list.orgmode.org/BF5B9308-3FEF-4DC6-98C9-BFF36F19D36C@gmail.com
Paul Eggert <eggert@cs.ucla.edu> 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