[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: master 4a1f69ebca 2/2: Use (TICKS . HZ) for current-time etc.

From: Max Nikulin
Subject: Re: master 4a1f69ebca 2/2: Use (TICKS . HZ) for current-time etc.
Date: Sat, 30 Apr 2022 17:56:31 +0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.7.0

On 30/04/2022 01:10, Paul Eggert wrote:
On 4/29/22 07:22, Max Nikulin wrote:

 From my point of view, it is better to rewrite `org-compile-time' to treat the case when there were no file prior to the call as that the file has been updated without comparison of timestamps

Yes, that sounds simpler and better. How about the attached patch?

Thank you, Paul. I am glad to see that you agree with my idea.

From: Paul Eggert
Date: Fri, 29 Apr 2022 11:06:00 -0700
Subject: [PATCH] Improve org-compile-file timestamp handling

diff --git a/lisp/org/org-macs.el b/lisp/org/org-macs.el
index bb0562dde0..907043580a 100644
--- a/lisp/org/org-macs.el
+++ b/lisp/org/org-macs.el

It should be committed to the "main" branch of the Org repository as well. I am unsure if all Emacs developers have commit permission however.

@@ -260,14 +260,8 @@ org-file-newer-than-p
   "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'."

It is not true any more, it is expected that TIME is obtained using `file-attributes'.

I would consider replacing docstring with something like:

Non-nil if FILE modification time is greater than or equal to TIME.
TIME should be obtained from the return value of the `file-attributes'
function.  If TIME is nil (file did not exist)
then any existing FILE is considered as a newer one.
This function may give false positive for file systems with coarse
timestamp resolution such as 1 second for HFS+ or 2 seconds for FAT.
File timestamp and system clock may have different resolution,
so attempts to compare them may give unexpected results.

-  (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)))))
+  (when-let ((mtime (file-attribute-modification-time (file-attributes file))))
+    (time-less-p time mtime)))

org-macs.el does not contain (require 'subr-x) thus `when-let' will cause a warning on Emacs-26.

`file-attribute-modification-time' makes code clearer, but it causes some complications. Formally compatibility with Emacs-25 (e.g. ubuntu-18.04 LTS bionic) is not required for the "main" branch. Emacs sources have the "bugfix" Org branch of the stable release though. The latter still supports Emacs-25, so either the Emacs source tree and the Org bugfix branch will diverge at this point or it is safer to avoid `file-attribute-modification-time' till the next major Org release. Maybe Org maintainers and developers will correct me.

Likely you already guessed from the suggested docscring that I would prefer

    (and mtime (not (and time (time-less-p mtime time))))

to keep existing behavior on HFS+ and to allow nil for TIME.

 (defun org-compile-file (source process ext &optional err-msg log-buf spec)
   "Compile a SOURCE file using PROCESS.
@@ -301,7 +295,7 @@ org-compile-file
         (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) "")))
       (pcase process
@@ -320,7 +314,7 @@ org-compile-file
        (_ (error "No valid command to process %S%s" source err-msg))))
     ;; Check for process failure.  Output file is expected to be
     ;; located in the same directory as SOURCE.
-    (unless (org-file-newer-than-p output time)
+    (unless (or (not time) (org-file-newer-than-p output time))
       (error (format "File %S wasn't produced%s" output err-msg)))

I am considering handling of (not time) case inside `org-file-newer-than-p` as a more robust approach.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]