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

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

[Mark Barton]

The change also breaks org-file-newer-than-p function that triggered the debugger while loading my init that uses org babel. I was able to use the example of the patch that Paul Eggert provided earlier for the desktop-save to add the time-convert to “fix” org-file-newer-than-p as shown below. Not positive that I needed to change it in both places, but it works for me now on macOS Monterey.

modified   lisp/org/org-macs.el
@@ -264,8 +264,8 @@ org-file-newer-than-p
        ;; (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)))))
+       (not (time-less-p (cl-subseq (time-convert (nth 5 (file-attributes file)) 'list) 0 2)
+			 (cl-subseq (time-convert time 'list) 0 2)))))

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

[Po Lu]

Mark Barton <mbarton98@gmail.com> writes:

> The change also breaks org-file-newer-than-p function that triggered
> the debugger while loading my init that uses org babel. I was able to
> use the example of the patch that Paul Eggert provided earlier for the
> desktop-save to add the time-convert to “fix” org-file-newer-than-p as
> shown below. Not positive that I needed to change it in both places,
> but it works for me now on macOS Monterey.
>
> modified   lisp/org/org-macs.el
> @@ -264,8 +264,8 @@ org-file-newer-than-p
>         ;; (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)))))
> +       (not (time-less-p (cl-subseq (time-convert (nth 5 (file-attributes file)) 'list) 0 2)
> +			 (cl-subseq (time-convert time 'list) 0 2)))))

Isn't the fact that changes are needed in multiple pieces of in-tree
code a wake-up call demonstrating it's too early to change the default
time format?

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

[Paul Eggert]

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

Thanks for reporting that. Fixed in Emacs master via the attached.

For the more general issue I'm planning to add a builtin boolean 
variable current-time-list soon, that is t for (HIGH LOW MICROSEC 
PICOSEC) format, nil for (TICKS . HZ) format.

[-- Attachment #2: 0001-Use-org-time-convert-to-integer-instead-of-by-hand.patch --]
[-- Type: text/x-patch, Size: 1623 bytes --]

From 3abb3681b57d7c8ca7fa808addb0a10b6b109cab Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 27 Apr 2022 00:29:26 -0700
Subject: [PATCH] Use org-time-convert-to-integer instead of by hand
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lisp/org/org-macs.el (org-file-newer-than-p):
Don’t assume list-format timestamps, by using
org-time-convert-to-integer instead of doing it by hand.
---
 lisp/org/org-macs.el | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lisp/org/org-macs.el b/lisp/org/org-macs.el
index b10725bd52..92591b5bb7 100644
--- a/lisp/org/org-macs.el
+++ b/lisp/org/org-macs.el
@@ -257,15 +257,16 @@ org-fit-window-to-buffer
 
 (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
+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 (cl-subseq (nth 5 (file-attributes file)) 0 2)
-			 (cl-subseq time 0 2)))))
+       (not (time-less-p (org-time-convert-to-integer
+			  (nth 5 (file-attributes file)))
+			 (org-time-convert-to-integer time)))))
 
 (defun org-compile-file (source process ext &optional err-msg log-buf spec)
   "Compile a SOURCE file using PROCESS.
-- 
2.32.0

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

[Stefan Monnier]

> -       (not (time-less-p (cl-subseq (nth 5 (file-attributes file)) 0 2)
> -			 (cl-subseq time 0 2)))))
> +       (not (time-less-p (org-time-convert-to-integer
> +			  (nth 5 (file-attributes file)))
> +			 (org-time-convert-to-integer time)))))

Instead of rounding the times to whole seconds, wouldn't it make more
sense to check that the difference is larger than 1s?


        Stefan

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

[Paul Eggert]

On 4/27/22 09:55, Stefan Monnier wrote:
> Instead of rounding the times to whole seconds, wouldn't it make more
> sense to check that the difference is larger than 1s?

org-file-newer-than-p is intended to work on filesystems like HFS+ that 
store just the seconds part of the last-modified time. Since these 
filesystems take the floor of the system time, taking the floor should 
be the most-accurate way to work around timestamp truncation issues, 
where comparing one timestamp that comes from an HFS+ filesystem to 
another timestamp coming from some other source (which is how 
org-file-newer-than-p is used).

The code won't work as desired on filesystems like FAT where the 
last-modified time has only 2-second resolution. Ideally Emacs Lisp code 
would have access to file timestamp resolution but that's not something 
it has now, so I merely preserved org-file-newer-than-p's assumption 
that taking the floor is good enough.

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

[Max Nikulin]

On 29/04/2022 05:27, Paul Eggert wrote:
> On 4/27/22 09:55, Stefan Monnier wrote:
>> Instead of rounding the times to whole seconds, wouldn't it make more
>> sense to check that the difference is larger than 1s?
> 
> org-file-newer-than-p is intended to work on filesystems like HFS+ that 
> store just the seconds part of the last-modified time.

I have found just 2 calls of `org-file-newer-than-p' in the Org code and 
in both cases the intention is to check whether particular file has been 
updated. I have not checked Org extensions for usage of this function. I 
would rather assume that the code was written without any considerations 
concerning filesystem timestamps precision and its difference from 
`current-time' representation. It was still working in most real-life cases.

 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, so 
`current-time' can be dropped to eliminate comparison of timestamp from 
different sources. With such modification it is better to compare file 
timestamps without truncation to whole seconds, however I have not tried 
to create an example where fractional seconds may change behavior.

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

[Paul Eggert]

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

On 4/29/22 07:22, Max Nikulin wrote:

> It was still working in most real-life cases.

Yes, the current code breaks only in fine-grained cases. Most of the 
time it'll work fine since people rarely compile the same file twice in 
the same second.

>  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?

[-- Attachment #2: 0001-Improve-org-compile-file-timestamp-handling.patch --]
[-- Type: text/x-patch, Size: 2512 bytes --]

From fbd6561952acf359236afcf7957a197376a18c66 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 29 Apr 2022 11:06:00 -0700
Subject: [PATCH] Improve org-compile-file timestamp handling
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lisp/org/org-macs.el (org-file-newer-than-p):
Don’t lose timestamp information in an attempt to work around
problems on filesystems with coarse-grained timestamps.
(org-compile-file): Use only filesystem timestamps;
don’t try to compare them to the current time, as
the filesystem clock may be different from our clock.
---
 lisp/org/org-macs.el | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

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
@@ -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'."
-  (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)))
 
 (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) "")))
     (save-window-excursion
       (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)))
     output))
 
-- 
2.34.1

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

[Max Nikulin]

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) "")))
>      (save-window-excursion
>        (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)))
>      output))

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

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

[Max Nikulin]

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

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.

I do not consider current behavior as reliable, however if you would 
prefer to keep it, the following patch may be used instead:

Paul Eggert to emacs-orgmode. Re: master 4a1f69ebca 2/2: Use (TICKS . 
HZ) for current-time etc. Wed, 27 Apr 2022 00:39:01 -0700. 
https://list.orgmode.org/f200c9ab-d1d4-d5a8-24cf-4e1082528fe7@cs.ucla.edu

The changes are not covered by unit tests at least when most babel 
languages are disabled.

On 30/04/2022 17:56, Max Nikulin wrote:
> 
>      (and mtime (not (and time (time-less-p mtime time))))

Treating equality as "newer" would break `org-compile-file', so I 
changed the condition. Previously it was not a case since file 
modification time is usually in the past in comparison to current time.

> On 30/04/2022 01:10, Paul Eggert wrote: 
>> +  (when-let ((mtime (file-attribute-modification-time (file-attributes file))))
>> +    (time-less-p time mtime))) 

> `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.

I have found `file-attribute-modification-time' in org-compat.el.

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

From d37b5bb295c69572d1615e7fb2c0bcce05cb2b58 Mon Sep 17 00:00:00 2001
From: Max Nikulin <manikulin@gmail.com>
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'.

It changes behavior of `org-babel-load-file' for the case of equal
modification time of source and tangled files that may happen on
filesystems with coarse timestamps, for example HFS+.  The file will be
tangled again.  Treating equal timestamps as up to date state would
break `org-compile-file' however.  Anyway time comparison is not really
reliable since precision of filesystem timestamps is unknown and it
differs from system clock precision.

Reported by Mark Barton <mbarton98@gmail.com>
https://list.orgmode.org/BF5B9308-3FEF-4DC6-98C9-BFF36F19D36C@gmail.com

During discussion of the issue Paul Eggert <eggert@cs.ucla.edu>
suggested over variants of the changes in the same thread.
---
 lisp/org-macs.el | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 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
-- 
2.25.1

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

[Max Nikulin]

[-- 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 <manikulin@gmail.com>
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 <mbarton98@gmail.com>
https://list.orgmode.org/BF5B9308-3FEF-4DC6-98C9-BFF36F19D36C@gmail.com

During discussion of the issue Paul Eggert <eggert@cs.ucla.edu>
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