From: Max Nikulin <manikulin@gmail.com>
To: Paul Eggert <eggert@cs.ucla.edu>
Cc: emacs-orgmode <emacs-orgmode@gnu.org>
Subject: Re: [PATCH] org-macs.el: Do not compare wall time and file modification time
Date: Thu, 12 May 2022 23:55:38 +0700 [thread overview]
Message-ID: <0fbf0584-5565-7f6d-c889-43158143f292@gmail.com> (raw)
In-Reply-To: <fe7e1484-07f6-627b-714b-d1d7d7049e2b@cs.ucla.edu>
[-- Attachment #1: Type: text/plain, Size: 1465 bytes --]
On 11/05/2022 23:24, Paul Eggert wrote:
> The comments don't seem to match the code here.
>
>> + (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)))
>
> Although this looks correct, there's no need to go to the work of
> computing file-mtime in the common case where tangled-mtime is nil.
Thank you, I missed such case. I discovered that the code below
recompiles an .el file even the .elc one is newer, moreover loading of
compiled file is broken by another modernization of emacs code (see the
dedicated thread). That is why I did not bother if the code may be
optimized a bit.
Finally I have found `file-newer-than-file-p' that looks suitable for
such case.
[-- Attachment #2: 0001-org-macs.el-Do-not-compare-wall-time-and-file-modifi.patch --]
[-- Type: text/x-patch, Size: 4747 bytes --]
From b2a546e239f32c78fb2dfaf92201a0b23eb76059 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): 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.
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+.
Before "Bad bounding indices: 0, 2" error was signalled in response to
call of `org-babel-load-file' for a non-existing .org file.
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 | 29 ++++++++++++++++++-----------
lisp/org.el | 10 +++++-----
2 files changed, 23 insertions(+), 16 deletions(-)
diff --git a/lisp/org-macs.el b/lisp/org-macs.el
index b10725bd5..23e005e6f 100644
--- a/lisp/org-macs.el
+++ b/lisp/org-macs.el
@@ -256,16 +256,23 @@ 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.
+
+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.
@@ -299,7 +306,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..7096c5098 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -233,11 +233,11 @@ 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
--
2.25.1
next prev parent reply other threads:[~2022-05-12 16:57 UTC|newest]
Thread overview: 18+ 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
2022-05-11 16:24 ` Paul Eggert
2022-05-12 16:55 ` Max Nikulin [this message]
2022-05-12 22:52 ` Paul Eggert
2022-05-13 12:28 ` Max Nikulin
2022-05-13 18:00 ` Paul Eggert
2022-10-02 3:49 ` Ihor Radchenko
2022-10-09 8:18 ` [PATCH v2] " Max Nikulin
2022-10-21 3:27 ` Ihor Radchenko
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 \
--in-reply-to=0fbf0584-5565-7f6d-c889-43158143f292@gmail.com \
--to=manikulin@gmail.com \
--cc=eggert@cs.ucla.edu \
--cc=emacs-orgmode@gnu.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public 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).