* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones @ 2022-04-07 12:37 Max Nikulin 2022-04-09 7:52 ` Paul Eggert ` (2 more replies) 0 siblings, 3 replies; 45+ messages in thread From: Max Nikulin @ 2022-04-07 12:37 UTC (permalink / raw) To: 54764; +Cc: eggert, emacs-orgmode Consider the following change of `encode-time' calling convention: last 3 elements of the TIME argument as a list should be optional. I mean IGNORED, DST, and ZONE. (encode-time '(0 30 20 07 04 2022 nil -1 nil)) (encode-time '(0 30 20 07 04 2022)) ; currently causes an error Since Emacs-27 time fields as separated arguments are considered obsolete for calls of `encode-time'. Org mode keeps compatibility with Emacs-26 where passing all time components as a single list is not supported. Moreover, some time ago an attempt to use new style argument in the Emacs git repository (the change was never committed to the Org repository) caused a bug with handling of daylight saving time. See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=54731 for more details, the commit reverted the changes is 8ef37913d3. I have tried to create a compatibility wrapper for Org mode that chooses calling convention of `encode-time' in dependence of Emacs version. I have realized that there are enough call sites where components of time are gathered from scratch and not obtained from e.g. prior call of `decode-time'. It is inconvenient to add 3 extra mandatory components at the each place. I am reluctant to add a helper that accepts 6-components list and adds 3 fields to the end of the list. I am afraid that it may affect e.g. agenda performance. From my point of view it is better to change implementation of `encode-time' so that it may accept 6-component list SECOND...YEAR. It should not add noticeable performance penalty but makes the function more convenient in use. Old-style separate arguments for time components permits optional fields ended with ZONE. I do not mind that it should be deprecated since it is the source of surprise similar to the mentioned bug. Daylight saving time field matters only as a list component and ignored as a separate argument (by the way, it should be stressed in the docstring). It is too easy to confuse list and separate arguments in the code since both ways works but with a subtle difference: nil does not mean ignore the value. (encode-time '(0 30 20 07 04 2022 nil nil nil)) ; wrong! (encode-time 0 30 20 07 04 2022 nil nil nil) ; no problem In the Org code it is unsure which way to call `encode-time' is more convenient. In a half of the cases a list is obtained from another function, but another half is timestamp built from computed components. Unless the inconsistency with DST I would say that both ways to call the function should be supported. So my proposal is to not force Org mode to use new calling convention for `encode-time' till DST and ZONE list components will became optional ones in a released Emacs version. For a while minor changes in a couple of places in Org code should make it immune to accidental usage of new calling convention (modulo compatibility). ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones 2022-04-07 12:37 bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones Max Nikulin @ 2022-04-09 7:52 ` Paul Eggert [not found] ` <149de00f-115b-5367-414f-c7700ef8966b@cs.ucla.edu> [not found] ` <handler.54764.D54764.165091617725815.notifdone@debbugs.gnu.org> 2 siblings, 0 replies; 45+ messages in thread From: Paul Eggert @ 2022-04-09 7:52 UTC (permalink / raw) To: Max Nikulin, 54764; +Cc: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 4044 bytes --] On 4/7/22 05:37, Max Nikulin wrote: > (encode-time '(0 30 20 07 04 2022 nil nil nil)) ; wrong! Yes, and I see a couple of places (org-parse-time-string, org-read-date-analyze) where Org mode returns the wrong decoded timestamps, ending in (nil nil nil) even though the DST flag is unknown so they should end in (nil -1 nil). Please see attached proposed patch which fixes this (also see below). > Since Emacs-27 time fields as separated arguments are considered > obsolete for calls of `encode-time'. Obsolescent, not obsolete. The old form still works and it's not going away any time soon. If the efficiency concerns you mention are significant, we should keep the old form indefinitely. > t is inconvenient to add 3 extra mandatory components at > the each place. Then let's keep using the obsolescent calling convention for places where that's convenient. Perhaps we should change the documentation to say "older" instead of "obsolescent". > From my point of view it is better to change implementation of > `encode-time' so that it may accept 6-component list SECOND...YEAR. It > should not add noticeable performance penalty but makes the function > more convenient in use. Unfortunately it makes the function more convenient to use incorrectly. This was part of the motivation for the API change. The obsolescent calling convention has no way to deal with ambiguous timestamps like 2022-11-06 01:30 when TZ="America/Los_Angeles". Org mode surely has bugs in this area, although I don't have time to scout them out. > Daylight saving > time field matters only as a list component and ignored as a separate > argument (by the way, it should be stressed in the docstring). Do you have a wording suggestion? (The doc string already covers the topic concisely; however, conciseness is not always a virtue. :-) The reason -1 is the default in the obsolete API is backward compatibility. If we could have designed the API from scratch it would have been different. > In the Org code it is unsure which way to call `encode-time' is more > convenient. In a half of the cases a list is obtained from another > function, but another half is timestamp built from computed components. > Unless the inconsistency with DST I would say that both ways to call the > function should be supported. Yes, that's the idea. > So my proposal is to not force Org mode to use new calling convention > for `encode-time' till DST and ZONE list components will became optional > ones in a released Emacs version. This would delay things for ten years or so, no? We'd have to wait until Org mode supported only Emacs 29 and later. Instead, I suggest that we stick with what we have when that's cleaner. That is, Org mode can use the obsolescent encode-time API when it's cleaner to do that. It would be helpful for Org mode to use the new encode-time form in some cases, when the new form is cleaner. It's easy to support the new form efficiently even in older Emacs, using a compatibility shim. This is also in the attached proposed patch. This patch has a few other minor cleanups in the area. I haven't installed the patch, or tested it other than via 'make check'. PS. Org mode usually uses encode-time for calendrical calculations. This is dicey, as some days don't exist (for example, December 30, 2011 does not exist if TZ="Pacific/Apia", because Samoa moved across the International Date Line that day). And it's also dicey when Org mode uses 00:00:00 (midnight at the start of the day) as a timestamp representing the entire day, as it's all too common for midnight to not exist (or to be duplicated) due to a DST transition. Generally speaking, when Org mode is doing calendrical calculations it should use calendrical functions rather than encode-time+decode-time, which are best used for time calculations not calendar calculations. (I realize that fixing this in Org would be nontrivial; perhaps I should file this "PS" as an Org bug report for whoever has time to fix it....) [-- Attachment #2: 0001-Improve-Org-usage-of-timestamps.patch --] [-- Type: text/x-patch, Size: 10568 bytes --] From 094345e10ad45e06f7b32e2f8017592210f43463 Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Sat, 9 Apr 2022 00:17:09 -0700 Subject: [PATCH] Improve Org usage of timestamps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The main thing is to follow the (encode-time X) convention where X’s DST component of nil means standard time, -1 means unknown. * lisp/org/ol.el (org-store-link): Prefer plain (encode-time ...) to (apply 'encode-time ...), for speed. * lisp/org/org-clock.el (org-clock-sum) (org-clock-update-time-maybe): Prefer org-time-string-to-seconds to doing it by hand. * lisp/org/org-colview.el (org-colview-construct-allowed-dates): * lisp/org/org.el (org-time-string-to-time, org-timestamp-change): Prefer (org-encode-time-1 X) to (apply #'encode-time X) when X is the output of org-parse-time-string. They're equivalent and the former is a bit cleaner. * lisp/org/org-compat.el (org-encode-time-1): New function. * lisp/org/org-macro.el (org-macro--vc-modified-time): Prefer (org-encode-time-1 X) to (apply #'encode-time X) when X is the output of parse-time-string. They're equivalent and the former is a bit cleaner. * lisp/org/org-macs.el (org-2ft): Prefer org-time-string-to-seconds to doing it by hand. (org-parse-time-string): Return unknown DST, not standard time. (org-matcher-time): Omit an unnecessary call to ‘append’. * lisp/org/org-table.el (org-table-eval-formula): Prefer org-time-string-to-time to doing it by hand. * lisp/org/org.el (org-add-planning-info, org-read-date) (org-read-date-display, org-display-custom-time): Prefer (org-encode-time-1 X) to (apply #'encode-time X) when X is the output of org-read-date-analyze. They're equivalent and the former is a bit cleaner. (org-read-date-analyze): Return a timestamp with a DST flag of -1 (unknown) rather than nil (standard time). --- lisp/org/ol.el | 4 +--- lisp/org/org-clock.el | 17 ++++++----------- lisp/org/org-colview.el | 2 +- lisp/org/org-compat.el | 21 +++++++++++++++++++++ lisp/org/org-macro.el | 2 +- lisp/org/org-macs.el | 8 ++++---- lisp/org/org-table.el | 3 +-- lisp/org/org.el | 14 +++++++------- 8 files changed, 42 insertions(+), 29 deletions(-) diff --git a/lisp/org/ol.el b/lisp/org/ol.el index a03d85f618..fe6e97e928 100644 --- a/lisp/org/ol.el +++ b/lisp/org/ol.el @@ -1575,9 +1575,7 @@ org-store-link (setq link (format-time-string (car org-time-stamp-formats) - (apply 'encode-time - (list 0 0 0 (nth 1 cd) (nth 0 cd) (nth 2 cd) - nil nil nil)))) + (encode-time 0 0 0 (nth 1 cd) (nth 0 cd) (nth 2 cd)))) (org-link-store-props :type "calendar" :date cd))) ((eq major-mode 'w3-mode) diff --git a/lisp/org/org-clock.el b/lisp/org/org-clock.el index 7395669109..24d2b61b48 100644 --- a/lisp/org/org-clock.el +++ b/lisp/org/org-clock.el @@ -1903,13 +1903,10 @@ org-clock-sum (cond ((match-end 2) ;; Two time stamps. - (let* ((ts (float-time - (apply #'encode-time - (save-match-data - (org-parse-time-string (match-string 2)))))) - (te (float-time - (apply #'encode-time - (org-parse-time-string (match-string 3))))) + (let* ((ss (match-string 2)) + (se (match-string 3)) + (ts (org-time-string-to-seconds ss)) + (te (org-time-string-to-seconds se)) (dt (- (if tend (min te tend) te) (if tstart (max ts tstart) ts)))) (when (> dt 0) (cl-incf t1 (floor dt 60))))) @@ -3041,10 +3038,8 @@ org-clock-update-time-maybe (end-of-line 1) (setq ts (match-string 1) te (match-string 3)) - (setq s (- (float-time - (apply #'encode-time (org-parse-time-string te))) - (float-time - (apply #'encode-time (org-parse-time-string ts)))) + (setq s (- (org-time-string-to-seconds te) + (org-time-string-to-seconds ts)) neg (< s 0) s (abs s) h (floor (/ s 3600)) diff --git a/lisp/org/org-colview.el b/lisp/org/org-colview.el index 829fcbbe3f..190c2d5a4f 100644 --- a/lisp/org/org-colview.el +++ b/lisp/org/org-colview.el @@ -782,7 +782,7 @@ org-colview-construct-allowed-dates (setq time-after (copy-sequence time)) (setf (nth 3 time-before) (1- (nth 3 time))) (setf (nth 3 time-after) (1+ (nth 3 time))) - (mapcar (lambda (x) (format-time-string fmt (apply #'encode-time x))) + (mapcar (lambda (x) (format-time-string fmt (org-encode-time-1 x))) (list time-before time time-after))))) (defun org-columns-open-link (&optional arg) diff --git a/lisp/org/org-compat.el b/lisp/org/org-compat.el index 819ce74d93..247373d6b9 100644 --- a/lisp/org/org-compat.el +++ b/lisp/org/org-compat.el @@ -115,6 +115,27 @@ org-table1-hline-regexp (defun org-time-convert-to-list (time) (seconds-to-time (float-time time)))) +;; Like Emacs 27+ `encode-time' with one argument. +(if (ignore-errors (encode-time (decode-time))) + (defsubst org-encode-time-1 (time) + (encode-time time)) + (defun org-encode-time-1 (time) + (let ((dst-zone (nthcdr 7 time))) + (unless (consp (cdr dst-zone)) + (signal wrong-type-argument (list time))) + (let ((etime (apply #'encode-time time)) + (dst (car dst-zone)) + (zone (cadr dst-zone))) + (when (and (symbolp dst) (not (integerp zone)) (not (consp zone))) + (let* ((detime (decode-time etime)) + (dedst (nth 7 detime))) + (when (and (not (eq dedst dst)) (symbolp dedst)) + ;; Assume one-hour DST and adjust the timestamp. + (setq etime (time-add etime (seconds-to-time + (- (if dedst 3600 0) + (if dst 3600 0)))))))) + etime)))) + ;; `newline-and-indent' did not take a numeric argument before 27.1. (if (version< emacs-version "27") (defsubst org-newline-and-indent (&optional _arg) diff --git a/lisp/org/org-macro.el b/lisp/org/org-macro.el index 0921f3aa27..b8e4346002 100644 --- a/lisp/org/org-macro.el +++ b/lisp/org/org-macro.el @@ -378,7 +378,7 @@ org-macro--vc-modified-time (buffer-substring (point) (line-end-position))))) (when (cl-some #'identity time) - (setq date (apply #'encode-time time)))))))) + (setq date (org-encode-time-1 time)))))))) (let ((proc (get-buffer-process buf))) (while (and proc (accept-process-output proc .5 nil t))))) (kill-buffer buf)) diff --git a/lisp/org/org-macs.el b/lisp/org/org-macs.el index b10725bd52..0916da89ac 100644 --- a/lisp/org/org-macs.el +++ b/lisp/org/org-macs.el @@ -1185,7 +1185,7 @@ org-2ft ((numberp s) s) ((stringp s) (condition-case nil - (float-time (apply #'encode-time (org-parse-time-string s))) + (org-time-string-to-seconds s) (error 0))) (t 0))) @@ -1242,7 +1242,7 @@ org-parse-time-string (string-to-number (match-string 4 s)) (string-to-number (match-string 3 s)) (string-to-number (match-string 2 s)) - nil nil nil)) + nil -1 nil)) (defun org-matcher-time (s) "Interpret a time comparison value S as a floating point time. @@ -1252,8 +1252,8 @@ org-matcher-time \"<tomorrow>\", and \"<yesterday>\". Return 0. if S is not recognized as a valid value." - (let ((today (float-time (apply #'encode-time - (append '(0 0 0) (nthcdr 3 (decode-time))))))) + (let ((today (float-time (apply #'encode-time 0 0 0 + (nthcdr 3 (decode-time)))))) (save-match-data (cond ((string= s "<now>") (float-time)) diff --git a/lisp/org/org-table.el b/lisp/org/org-table.el index c4daed1665..a192fb991b 100644 --- a/lisp/org/org-table.el +++ b/lisp/org/org-table.el @@ -2606,8 +2606,7 @@ org-table-eval-formula (format-time-string (org-time-stamp-format (string-match-p "[0-9]\\{1,2\\}:[0-9]\\{2\\}" ts)) - (apply #'encode-time - (save-match-data (org-parse-time-string ts)))))) + (save-match-data (org-time-string-to-time ts))))) form t t)) (setq ev (if (and duration (string-match "^[0-9]+:[0-9]+\\(?::[0-9]+\\)?$" form)) diff --git a/lisp/org/org.el b/lisp/org/org.el index d656a51591..1bceb0f53a 100644 --- a/lisp/org/org.el +++ b/lisp/org/org.el @@ -10792,7 +10792,7 @@ org-add-planning-info (if (stringp time) ;; This is a string (relative or absolute), set ;; proper date. - (apply #'encode-time + (org-encode-time-1 (org-read-date-analyze time default-time (decode-time default-time))) ;; If necessary, get the time from the user @@ -14059,7 +14059,7 @@ org-read-date "range representable on this machine")) (ding)) - (setq final (apply #'encode-time final)) + (setq final (org-encode-time-1 final)) (setq org-read-date-final-answer ans) @@ -14096,7 +14096,7 @@ org-read-date-display (and (boundp 'org-time-was-given) org-time-was-given)) (cdr fmts) (car fmts))) - (txt (format-time-string fmt (apply #'encode-time f))) + (txt (format-time-string fmt (org-encode-time-1 f))) (txt (if org-read-date-inactive (concat "[" (substring txt 1 -1) "]") txt)) (txt (concat "=> " txt))) (when (and org-end-time-was-given @@ -14334,7 +14334,7 @@ org-read-date-analyze (setq year (nth 5 org-defdecode)) (setq org-read-date-analyze-forced-year t)))) (setq org-read-date-analyze-futurep futurep) - (list second minute hour day month year))) + (list second minute hour day month year nil -1 nil))) (defvar parse-time-weekdays) (defun org-read-date-get-relative (s today default) @@ -14470,7 +14470,7 @@ org-display-custom-time time (org-fix-decoded-time t1) str (org-add-props (format-time-string - (substring tf 1 -1) (apply 'encode-time time)) + (substring tf 1 -1) (org-encode-time-1 time)) nil 'mouse-face 'highlight)) (put-text-property beg end 'display str))) @@ -14725,7 +14725,7 @@ org-make-tdiff-string (defun org-time-string-to-time (s) "Convert timestamp string S into internal time." - (apply #'encode-time (org-parse-time-string s))) + (org-encode-time-1 (org-parse-time-string s))) (defun org-time-string-to-seconds (s) "Convert a timestamp string S into a number of seconds." @@ -15155,7 +15155,7 @@ org-timestamp-change (setcar time0 (or (car time0) 0)) (setcar (nthcdr 1 time0) (or (nth 1 time0) 0)) (setcar (nthcdr 2 time0) (or (nth 2 time0) 0)) - (setq time (apply 'encode-time time0)))) + (setq time (org-encode-time-1 time0)))) ;; Insert the new time-stamp, and ensure point stays in the same ;; category as before (i.e. not after the last position in that ;; category). -- 2.35.1 ^ permalink raw reply related [flat|nested] 45+ messages in thread
[parent not found: <149de00f-115b-5367-414f-c7700ef8966b@cs.ucla.edu>]
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones [not found] ` <149de00f-115b-5367-414f-c7700ef8966b@cs.ucla.edu> @ 2022-04-09 11:36 ` Max Nikulin 2022-04-13 14:40 ` Max Nikulin ` (3 subsequent siblings) 4 siblings, 0 replies; 45+ messages in thread From: Max Nikulin @ 2022-04-09 11:36 UTC (permalink / raw) To: 54764 Paul, I am sorry that I forced you to make more changes in the Org code. While I was filing this bug, my intention was to add something like "if (!NILP(...)) { .... }" around several lines in src/timefns.c. Now we should decide whether we leave this bug for possible changes in the `encode-time' implementation and moving the discussion related to further changes in Org to the emacs-orgmode mail list or we change the title of this bug and maybe create another one for `encode-time'. On 09/04/2022 14:52, Paul Eggert wrote: > On 4/7/22 05:37, Max Nikulin wrote: > >> (encode-time '(0 30 20 07 04 2022 nil nil nil)) ; wrong! > > Yes, and I see a couple of places (org-parse-time-string, > org-read-date-analyze) where Org mode returns the wrong decoded > timestamps, ending in (nil nil nil) I see you even change `org-store-link' that has the same problem. > Obsolescent, not obsolete. The old form still works and it's not going > away any time soon. If the efficiency concerns you mention are > significant, we should keep the old form indefinitely. I am against preserving the old form because it ignores the DST field. It is confusing and error prone to be a part of a well designed interface. Actually I have a draft of `org-encode-time' macro that transforms 6 elements list to 9 elements one at load or compile time, so it should not hurt runtime performance. I have not tried to replace all calls of the `encode-time' function yet however. But I still prefer to drop 6/9 elements branch even if it may happen a decade later. >> From my point of view it is better to change implementation of >> `encode-time' so that it may accept 6-component list SECOND...YEAR. It >> should not add noticeable performance penalty but makes the function >> more convenient in use. > > Unfortunately it makes the function more convenient to use incorrectly. > This was part of the motivation for the API change. The obsolescent > calling convention has no way to deal with ambiguous timestamps like > 2022-11-06 01:30 when TZ="America/Los_Angeles". Org mode surely has bugs > in this area, although I don't have time to scout them out. I do not see your point here. Old calling convention did not allow to specify DST flag at all and it was a problem. With the list argument even if last 3 fields are optional, it will not prevent adding DST value when it is known from some source. I think, requiring 3 extra fields when DST value is unknown is too hing price just to make a developer aware that it may be ambiguous (moreover it will not work without a clear statement in the docstring). Org timestamps does not allow to specify timezone abbreviation such as PDT/PST to distinguish DST. If it were added then users would have false impression of full time zones support in Org. It would require huge amount of work. So guessed DST is the best that often can be offered. >> Daylight saving time field matters only as a list component and >> ignored as a separate argument (by the way, it should be stressed in >> the docstring). > > Do you have a wording suggestion? (The doc string already covers the > topic concisely; however, conciseness is not always a virtue. :-) My point is that subtle breaking changes must be prominent and hard to ignore. I do not have yet ready phases and you highlighted another issue missed in the docstring. >> So my proposal is to not force Org mode to use new calling convention >> for `encode-time' till DST and ZONE list components will became >> optional ones in a released Emacs version. > > This would delay things for ten years or so, no? We'd have to wait until > Org mode supported only Emacs 29 and later. I do not think it is a problem. > Instead, I suggest that we stick with what we have when that's cleaner. > That is, Org mode can use the obsolescent encode-time API when it's > cleaner to do that. I considered such approach to defense against "aggressive" modernizing of Emacs code. Then I decided it is better to allow to deprecate one of the styles of calling `encode-time'. I tried to express it in the original report and above. > I haven't installed the patch, or tested it other than via 'make check'. Org has its own repository and changes should be committed there at first. Org has unit tests, see https://git.savannah.gnu.org/cgit/emacs/org-mode.git/tree/testing/README you changes should be accompanied by some adjustments in test expectations. I believe that at least in some cases Org test suite should be run for the bundled version of Org after Emacs builds. There are may be copyright issues with including the tests into the Emacs source tree. Maybe some changes in tests were accepted without paperwork. > PS. Org mode usually uses encode-time for calendrical calculations. This > is dicey, as some days don't exist (for example, December 30, 2011 does > not exist if TZ="Pacific/Apia", because Samoa moved across the > International Date Line that day). And it's also dicey when Org mode > uses 00:00:00 (midnight at the start of the day) as a timestamp > representing the entire day, as it's all too common for midnight to not > exist (or to be duplicated) due to a DST transition. Generally speaking, > when Org mode is doing calendrical calculations it should use > calendrical functions rather than encode-time+decode-time, which are > best used for time calculations not calendar calculations. (I realize > that fixing this in Org would be nontrivial; perhaps I should file this > "PS" as an Org bug report for whoever has time to fix it....) I agree with you that dates should not be represented as timestamps and date modifications (day, week, month, year increments and decrements) should be performed by dedicated functions. I even had 2011-12-30 example in my notes. However I expect that underlying normalization of date-time fields may mitigate such issues to some extent. > diff --git a/lisp/org/org-compat.el b/lisp/org/org-compat.el > index 819ce74d93..247373d6b9 100644 > --- a/lisp/org/org-compat.el > +++ b/lisp/org/org-compat.el > @@ -115,6 +115,27 @@ org-table1-hline-regexp > (defun org-time-convert-to-list (time) > (seconds-to-time (float-time time)))) > > +;; Like Emacs 27+ `encode-time' with one argument. > +(if (ignore-errors (encode-time (decode-time))) > + (defsubst org-encode-time-1 (time) > + (encode-time time)) > + (defun org-encode-time-1 (time) > + (let ((dst-zone (nthcdr 7 time))) > + (unless (consp (cdr dst-zone)) > + (signal wrong-type-argument (list time))) > + (let ((etime (apply #'encode-time time)) > + (dst (car dst-zone)) > + (zone (cadr dst-zone))) > + (when (and (symbolp dst) (not (integerp zone)) (not (consp zone))) > + (let* ((detime (decode-time etime)) > + (dedst (nth 7 detime))) > + (when (and (not (eq dedst dst)) (symbolp dedst)) > + ;; Assume one-hour DST and adjust the timestamp. > + (setq etime (time-add etime (seconds-to-time > + (- (if dedst 3600 0) > + (if dst 3600 0)))))))) > + etime)))) > + > ;; `newline-and-indent' did not take a numeric argument before 27.1. > (if (version< emacs-version "27") > (defsubst org-newline-and-indent (&optional _arg) > diff --git a/lisp/org/org-macro.el b/lisp/org/org-macro.el > index 0921f3aa27..b8e4346002 100644 I do not have complicated agenda setup to profile performance after such change. I will post my simple macro when we decide to continue discussion on debbugs or in a dedicated thread of the emacs-orgmode list. In my opinion, the code obtaining DST value requires unit tests. I like you idea to reuse `org-time-string-to-seconds' in more places. My original plan was to use `org-time-string-to-time' there. ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones [not found] ` <149de00f-115b-5367-414f-c7700ef8966b@cs.ucla.edu> 2022-04-09 11:36 ` Max Nikulin @ 2022-04-13 14:40 ` Max Nikulin 2022-04-13 18:35 ` Paul Eggert 2022-04-13 15:12 ` Max Nikulin ` (2 subsequent siblings) 4 siblings, 1 reply; 45+ messages in thread From: Max Nikulin @ 2022-04-13 14:40 UTC (permalink / raw) To: Paul Eggert, 54764; +Cc: emacs-orgmode On 09/04/2022 14:52, Paul Eggert wrote: > On 4/7/22 05:37, Max Nikulin wrote: > >> From my point of view it is better to change implementation of >> `encode-time' so that it may accept 6-component list SECOND...YEAR. It >> should not add noticeable performance penalty but makes the function >> more convenient in use. > > Unfortunately it makes the function more convenient to use incorrectly. > This was part of the motivation for the API change. The obsolescent > calling convention has no way to deal with ambiguous timestamps like > 2022-11-06 01:30 when TZ="America/Los_Angeles". Org mode surely has bugs > in this area, although I don't have time to scout them out. Handling DST is a step forward, it is an important case. Unfortunately there are enough peculiarities in the time zoo. I do not think new interface can be used correctly in the following case of time transition with no change of DST: #+name: encode-time-and-format #+begin_src elisp :var time=nil :var tz=() :var dst=-1 (let* ((seconds-year (reverse (mapcar #'string-to-number (split-string time "[- :]")))) (time-list (append seconds-year (list nil dst tz)))) (format-time-string "%F %T %Z %z" (encode-time time-list) tz)) #+end_src zdump -v Africa/Juba ... Africa/Juba Sun Jan 31 20:59:59 2021 UT = Sun Jan 31 23:59:59 2021 EAT isdst=0 gmtoff=10800 Africa/Juba Sun Jan 31 21:00:00 2021 UT = Sun Jan 31 23:00:00 2021 CAT isdst=0 gmtoff=7200 #+call: encode-time-and-format(time="2021-01-31 23:30:00", tz="Africa/Juba", dst=-1) #+RESULTS: : 2021-01-31 23:30:00 CAT +0200 #+call: encode-time-and-format(time="2021-01-31 23:30:00", tz="Africa/Juba", dst=()) #+RESULTS: : 2021-01-31 23:30:00 CAT +0200 #+call: encode-time-and-format(time="2021-01-31 23:30:00", tz="Africa/Juba", dst='t) : Debugger entered--Lisp error: (error "Specified time is not representable") I do not see a way to get 23:30 EAT +0300. For you example with regular DST transition there is no any problem: #+call: encode-time-and-format(time="2022-11-06 01:30:00", tz="America/Los_Angeles", dst=-1) #+RESULTS: : 2022-11-06 01:30:00 PDT -0700 #+call: encode-time-and-format(time="2022-11-06 01:30:00", tz="America/Los_Angeles", dst=()) #+RESULTS: : 2022-11-06 01:30:00 PST -0800 #+call: encode-time-and-format(time="2022-11-06 01:30:00", tz="America/Los_Angeles", dst='t) #+RESULTS: : 2022-11-06 01:30:00 PDT -0700 > PS. Org mode usually uses encode-time for calendrical calculations. This > is dicey, as some days don't exist (for example, December 30, 2011 does > not exist if TZ="Pacific/Apia", because Samoa moved across the > International Date Line that day). And it's also dicey when Org mode > uses 00:00:00 (midnight at the start of the day) as a timestamp > representing the entire day, as it's all too common for midnight to not > exist (or to be duplicated) due to a DST transition. Generally speaking, > when Org mode is doing calendrical calculations it should use > calendrical functions rather than encode-time+decode-time, which are > best used for time calculations not calendar calculations. (I realize > that fixing this in Org would be nontrivial; perhaps I should file this > "PS" as an Org bug report for whoever has time to fix it....) Then `encode-time' should only accept time zone as time offset and should not allow default or named value that may be ambiguous. However my opinion is that is should be possible to provide hints to `encode-time' to get deterministic behavior in the case of time transitions. > diff --git a/lisp/org/org-compat.el b/lisp/org/org-compat.el > index 819ce74d93..247373d6b9 100644 > --- a/lisp/org/org-compat.el > +++ b/lisp/org/org-compat.el > @@ -115,6 +115,27 @@ org-table1-hline-regexp > (defun org-time-convert-to-list (time) > (seconds-to-time (float-time time)))) > > +;; Like Emacs 27+ `encode-time' with one argument. > +(if (ignore-errors (encode-time (decode-time))) > + (defsubst org-encode-time-1 (time) > + (encode-time time)) > + (defun org-encode-time-1 (time) > + (let ((dst-zone (nthcdr 7 time))) > + (unless (consp (cdr dst-zone)) > + (signal wrong-type-argument (list time))) > + (let ((etime (apply #'encode-time time)) > + (dst (car dst-zone)) > + (zone (cadr dst-zone))) > + (when (and (symbolp dst) (not (integerp zone)) (not (consp zone))) > + (let* ((detime (decode-time etime)) > + (dedst (nth 7 detime))) > + (when (and (not (eq dedst dst)) (symbolp dedst)) > + ;; Assume one-hour DST and adjust the timestamp. > + (setq etime (time-add etime (seconds-to-time > + (- (if dedst 3600 0) > + (if dst 3600 0)))))))) I am against this workaround. It fixes (to some extent) usual DST transitions, but it adds another bug Australia/Lord_Howe Sat Apr 2 14:59:59 2022 UT = Sun Apr 3 01:59:59 2022 +11 isdst=1 gmtoff=39600 Australia/Lord_Howe Sat Apr 2 15:00:00 2022 UT = Sun Apr 3 01:30:00 2022 +1030 isdst=0 gmtoff=37800 Australia/Lord_Howe Sat Oct 1 15:29:59 2022 UT = Sun Oct 2 01:59:59 2022 +1030 isdst=0 gmtoff=37800 Australia/Lord_Howe Sat Oct 1 15:30:00 2022 UT = Sun Oct 2 02:30:00 2022 +11 isdst=1 gmtoff=39600 > + etime)))) > + > ;; `newline-and-indent' did not take a numeric argument before 27.1. > (if (version< emacs-version "27") > (defsubst org-newline-and-indent (&optional _arg) ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones 2022-04-13 14:40 ` Max Nikulin @ 2022-04-13 18:35 ` Paul Eggert 2022-04-14 13:19 ` Max Nikulin [not found] ` <4a23f3a4-fe8f-d396-49d8-10034803be63@gmail.com> 0 siblings, 2 replies; 45+ messages in thread From: Paul Eggert @ 2022-04-13 18:35 UTC (permalink / raw) To: Max Nikulin; +Cc: emacs-orgmode, 54764 On 4/13/22 07:40, Max Nikulin wrote: > > I do not see a way to get 23:30 EAT +0300. Are you asking for a function F where you say, "I want to give F a possibly-ambiguous decoded local time D, and for F to return all timestamps that map to D"? If so, encode-time doesn't do that, because the underlying C API (namely, mktime) doesn't do that. All mktime and encode-time do is give you *one* timestamp that maps to D; it won't give you any other timestamps. If you're worried about possibly-ambiguous decoded local times, you could probe (say) one day before and one day after encode-time's result to see if the UTC offset changes, and let that guide you to find other possible timestamps that map to the decoded time. Although this is just a heuristic it should be good enough. I doubt whether you need to do that, though. Code that is not careful about local time offsets doesn't care how ambiguous decoded times are resolved. And code that does care should record UTC offsets anyway, and you can use those offsets to disambiguate the decoded times. Something like this, say: (defun encode-and-format-time (time tz) (let ((etime (encode-time (parse-time-string time)))) (format-time-string "%F %T %Z %z" etime tz))) With this definition, (encode-and-format-time "2021-01-31 23:30:00 +0300" "Africa/Juba") yields "2021-01-31 23:30:00 EAT +0300", which is the timestamp you want. > `encode-time' should only accept time zone as time offset and should not allow default or named value that may be ambiguous. If we're talking about Org's encode-time substitute, you can of course do what you like. But Emacs encode-time has supported ambiguous timestamps for some time and I expect it's used by apps that don't care how ambiguous decoded times are resolved, which means we shouldn't remove that support without having a very good reason. > should be possible to provide hints to `encode-time' to get deterministic behavior in the case of time transitions Yes, that feature is already there. The hint is the UTC offset, as illustrated above. ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones 2022-04-13 18:35 ` Paul Eggert @ 2022-04-14 13:19 ` Max Nikulin [not found] ` <4a23f3a4-fe8f-d396-49d8-10034803be63@gmail.com> 1 sibling, 0 replies; 45+ messages in thread From: Max Nikulin @ 2022-04-14 13:19 UTC (permalink / raw) To: Paul Eggert; +Cc: emacs-orgmode, 54764 On 14/04/2022 01:35, Paul Eggert wrote: > On 4/13/22 07:40, Max Nikulin wrote: >> >> I do not see a way to get 23:30 EAT +0300. > > Are you asking for a function F where you say, "I want to give F a > possibly-ambiguous decoded local time D, and for F to return all > timestamps that map to D"? If so, encode-time doesn't do that, because > the underlying C API (namely, mktime) doesn't do that. All mktime and > encode-time do is give you *one* timestamp that maps to D; it won't give > you any other timestamps. I am just trying to convince you that new API still can not handle ambiguities in response to: >>> Unfortunately it makes the function more convenient to >>> use incorrectly. This was part of the motivation for the API change. >>> The obsolescent calling convention has no way to deal with ambiguous >>> timestamps like 2022-11-06 01:30 when TZ="America/Los_Angeles". Completely ignoring DST in old API is a bug. Possibility to omit DST and ZONE is a feature for convenience and it is not related to correctly/incorrectly. It has no common with allowing 6-values SECONONDS...YEAR list. I am aware of mktime and my opinion is that libc in such cases (dealing with formats for humans) is hardly usable for applications more complex than "hello world". I do not want all ambiguous time values. I want to provide hints how they should be resolved and get in response an additional value that says in which way it is actually resolved. > If you're worried about possibly-ambiguous decoded local times, you > could probe (say) one day before and one day after encode-time's result > to see if the UTC offset changes, and let that guide you to find other > possible timestamps that map to the decoded time. Although this is just > a heuristic it should be good enough. I would prefer to avoid dances with +/-1 day timestamps. I would not be surprised when one day they will give wrong result. During computations it is available to which interval of time current time offset is valid. Such value may be included in result of conversion. So date-time + "America/Los_Angeles" input should not be reduced to timezone offset in the output. Zone internal object or identifier is important for calculation of other date-time values based on the origin value. > I doubt whether you need to do that, though. Code that is not careful > about local time offsets doesn't care how ambiguous decoded times are > resolved. And code that does care should record UTC offsets anyway, and > you can use those offsets to disambiguate the decoded times. Something > like this, say: > > (defun encode-and-format-time (time tz) > (let ((etime (encode-time (parse-time-string time)))) > (format-time-string "%F %T %Z %z" etime tz))) > > With this definition, (encode-and-format-time "2021-01-31 23:30:00 > +0300" "Africa/Juba") yields "2021-01-31 23:30:00 EAT +0300", which is > the timestamp you want. I want hints like "in the case of ambiguity resolve to transition time immediately before/immediately after transition" or "provide suitable time prior to/after to transition". I hope, they may work without explicitly providing time zone offset to the input that anyway requires additional calculations. ±n hours may mean ±n*3600 seconds or time with same minutes and seconds values but hours value is changed by n even if a 30 min DST transition happens in between. If agenda interval start falls on skipped time span in local time then the value immediately after transition should be used (and maybe user should be warned by additional entry). For agenda end interval vice versa local time immediately before transition is more suitable. `parse-time-string' has another set of problems. It is impossible to specify particular format like for strptime(3). Actually parsing a string to numerical values and resolving ambiguities in numerical values are different tasks and it may be useful to have separate functions for them. I am unsure however concerning implementing constraints to parse free-form dates. >> `encode-time' should only accept time zone as time offset and should >> not allow default or named value that may be ambiguous. > > If we're talking about Org's encode-time substitute, you can of course > do what you like. But Emacs encode-time has supported ambiguous > timestamps for some time and I expect it's used by apps that don't care > how ambiguous decoded times are resolved, which means we shouldn't > remove that support without having a very good reason. There is no reason to impose such restrictions for helpers in Org since Org relies on resolving ambiguities with minimal efforts. `encode-time' supports just one kind of ambiguities. Even though it is the most common one, it is rather (partially false) impression than real support. The truth is that most of developers are not aware of real complexity of dealing with time. Documentation rarely describes limitations and corner cases that should be handled. Even when correct time handling gets more priority it appears that available libraries are full of bugs and require ugly workarounds. >> should be possible to provide hints to `encode-time' to get >> deterministic behavior in the case of time transitions > > Yes, that feature is already there. The hint is the UTC offset, as > illustrated above. No, UTC offset is another feature and implementing the hints I have tried to describe may require implementing from scratch full stack of time handling functions. In some cases offset should be replaced by time zone identifier to avoid incorrect result in further calculations. So I still do not see any point in mandatory DST and ZONE fields in new interface of `encode-time'. I do not see an alternative to convert SECONDS...YEAR values to timestamp assuming local time zone as well. Anyway DST is not available in advance and sometimes DST value is not coupled with step in local time, so its ability to resolve ambiguities is limited. ^ permalink raw reply [flat|nested] 45+ messages in thread
[parent not found: <4a23f3a4-fe8f-d396-49d8-10034803be63@gmail.com>]
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones [not found] ` <4a23f3a4-fe8f-d396-49d8-10034803be63@gmail.com> @ 2022-04-14 22:46 ` Paul Eggert [not found] ` <52fb10fb-892a-f273-3be8-28793f27e204@cs.ucla.edu> 1 sibling, 0 replies; 45+ messages in thread From: Paul Eggert @ 2022-04-14 22:46 UTC (permalink / raw) To: Max Nikulin; +Cc: emacs-orgmode, 54764 On 4/14/22 06:19, Max Nikulin wrote: > date-time + > "America/Los_Angeles" input should not be reduced to timezone offset in > the output. It depends on the application. For some applications (e.g., generating "Date:" lines in email), it is entirely correct to output a timestamp like "14 Apr 2022 15:16:04 -0700", thus losing the fact that the timestamp was generated with TZ="America/Los_Angeles". > Zone internal object or identifier is important for calculation of other date-time values based on the origin value. Again, that depends on the application. It's typically wrong to store an old timestamp in a form like "1950-07-01 00:00 Europe/Lisbon", because there is no standard for what "Europe/Lisbon" means. If you update your copy of TZDB, or interpret such a timestamp on another computer, that can change the interpretation of such a timestamp. In this particular case, a change in TZDB release 2021b altered the interpretation of this old timestamp because we discovered that DST was observed in 1950 in Portugal. If you want to keep the TZDB identifier for advice about how to interpret dates relative to a timestamp, that's fine. But you should keep the UT offset in addition to the TZDB identifier, if you want your app to be fully accurate and useful. For example, you should store "1950-07-01 00:00:00 +0000 Europe/Lisbon" for a timestamp generated by TZDB release 2021a, so that when you interpret the timestamp in release 2021b you'll have an idea of what you're dealing with. > I want hints like "in the case of ambiguity resolve to transition time immediately before/immediately after transition" or "provide suitable time prior to/after to transition". Although that might be nice it's not what mktime gives us, and I doubt whether it's a good idea to try to implement it from scratch in Emacs. > I hope, they may work without explicitly providing time zone offset to the input that anyway requires additional calculations. It doesn't require additional calculations on the Emacs Lisp user's part. All you need to do is save the UT offset, and use it later. There's so little overhead to this that it's not worth worrying about. > ±n hours may mean ±n*3600 seconds or time with same minutes and seconds > values but hours value is changed by n even if a 30 min DST transition > happens in between. Sorry, I don't understand what this sentence is intended to mean. > `parse-time-string' has another set of problems. Sure, but that was just an example. You can write your own date parser. The point is that when you save a localtime timestamp, you should save its UT offset too, in whatever notation is appropriate. > UTC offset is another feature and implementing the hints I have > tried to describe may require implementing from scratch full stack of > time handling functions. I doubt whether that's a good idea. I've written that sort of code, and it's a lot more work than one might think and it's notoriously difficult to do it correctly. You have better things to do. > So I still do not see any point in mandatory DST and ZONE fields in new > interface of `encode-time'. I think we're in agreement here. As I mentioned earlier, I plan to modify Emacs encode-time so that you can pass it a 6-arg list as well as an 9-arg list. Once this change is in, the DST and ZONE fields will not be mandatory. ^ permalink raw reply [flat|nested] 45+ messages in thread
[parent not found: <52fb10fb-892a-f273-3be8-28793f27e204@cs.ucla.edu>]
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones [not found] ` <52fb10fb-892a-f273-3be8-28793f27e204@cs.ucla.edu> @ 2022-04-15 2:14 ` Tim Cross 2022-04-15 17:23 ` Max Nikulin [not found] ` <5cd820d4-ae67-43d4-9e63-c284d51ff1e4@gmail.com> 2 siblings, 0 replies; 45+ messages in thread From: Tim Cross @ 2022-04-15 2:14 UTC (permalink / raw) To: Paul Eggert; +Cc: Max Nikulin, emacs-orgmode, 54764 Paul Eggert <eggert@cs.ucla.edu> writes: > On 4/14/22 06:19, Max Nikulin wrote: > >> date-time + "America/Los_Angeles" input should not be reduced to timezone offset >> in the output. > > It depends on the application. For some applications (e.g., generating "Date:" lines > in email), it is entirely correct to output a timestamp like "14 Apr 2022 15:16:04 > -0700", thus losing the fact that the timestamp was generated with > TZ="America/Los_Angeles". > >> Zone internal object or identifier is important for calculation of other date-time values based on the origin value. > > Again, that depends on the application. It's typically wrong to store an old > timestamp in a form like "1950-07-01 00:00 Europe/Lisbon", because there is no > standard for what "Europe/Lisbon" means. If you update your copy of TZDB, or > interpret such a timestamp on another computer, that can change the interpretation of > such a timestamp. In this particular case, a change in TZDB release 2021b altered the > interpretation of this old timestamp because we discovered that DST was observed in > 1950 in Portugal. > > If you want to keep the TZDB identifier for advice about how to interpret dates > relative to a timestamp, that's fine. But you should keep the UT offset in addition > to the TZDB identifier, if you want your app to be fully accurate and useful. For > example, you should store "1950-07-01 00:00:00 +0000 Europe/Lisbon" for a timestamp > generated by TZDB release 2021a, so that when you interpret the timestamp in release > 2021b you'll have an idea of what you're dealing with. > I think this is a very important point. Timezone data is not static. If you only record the timezone name, offsets will be calculated using the current definition, which may not be correct for past timestamps. A good example of this is the DST values and the date when a TZ transitions between DST and non-DST periods. That date can change, either temporarily or permanently. That change can be days or even weeks. Any date related calculations which only have knowledge about TZ names and not the specific offset of a timestamp can therefore be out by a significant amount. ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones [not found] ` <52fb10fb-892a-f273-3be8-28793f27e204@cs.ucla.edu> 2022-04-15 2:14 ` Tim Cross @ 2022-04-15 17:23 ` Max Nikulin [not found] ` <5cd820d4-ae67-43d4-9e63-c284d51ff1e4@gmail.com> 2 siblings, 0 replies; 45+ messages in thread From: Max Nikulin @ 2022-04-15 17:23 UTC (permalink / raw) To: Paul Eggert; +Cc: emacs-orgmode, 54764 On 15/04/2022 05:46, Paul Eggert wrote: > On 4/14/22 06:19, Max Nikulin wrote: > >> date-time + "America/Los_Angeles" input should not be reduced to >> timezone offset in the output. > > It depends on the application. For some applications (e.g., generating > "Date:" lines in email), it is entirely correct to output a timestamp > like "14 Apr 2022 15:16:04 -0700", thus losing the fact that the > timestamp was generated with TZ="America/Los_Angeles". However if you are storing future events bound to wall time then namely time zone identifier should have precedence. A new rule may be issued between scheduling event and the time it will happen. It is terrible feeling when it is necessary to guess if a web site stores TZ offset or its identifier and in the latter case whether its administrators updated tzinfo. It is better to store location of event since a time zone may be split and time transition may apply only to a part of the original zone. Actually I meant another case. Some representation is got for a time moment and it is necessary to get local time for another time moment. Time zone identifier or an object with internal representation allow to get correct offset for second moment of time. It should be possible to specify whether a function call is isolated conversion or further calculations will follow. >> Zone internal object or identifier is important for calculation of >> other date-time values based on the origin value. > > Again, that depends on the application. It's typically wrong to store an > old timestamp in a form like "1950-07-01 00:00 Europe/Lisbon", because > there is no standard for what "Europe/Lisbon" means. If you update your > copy of TZDB, or interpret such a timestamp on another computer, that > can change the interpretation of such a timestamp. In this particular > case, a change in TZDB release 2021b altered the interpretation of this > old timestamp because we discovered that DST was observed in 1950 in > Portugal. Just identifier may be ambiguous around DST transition. So timezone abbreviations are ambiguous per se but when identifiers are known they may be still necessary to resolve uncertainties for backward time shifts. At certain moment the Olson DB started to use "+04" abbreviations instead of letters for transitions unrelated to daylight saving time. > If you want to keep the TZDB identifier for advice about how to > interpret dates relative to a timestamp, that's fine. But you should > keep the UT offset in addition to the TZDB identifier, if you want your > app to be fully accurate and useful. For example, you should store > "1950-07-01 00:00:00 +0000 Europe/Lisbon" for a timestamp generated by > TZDB release 2021a, so that when you interpret the timestamp in release > 2021b you'll have an idea of what you're dealing with. And WET/WEST gets another bit of info in addition to numerical offset. >> I hope, they may work without explicitly providing time zone offset to >> the input that anyway requires additional calculations. > > It doesn't require additional calculations on the Emacs Lisp user's > part. All you need to do is save the UT offset, and use it later. > There's so little overhead to this that it's not worth worrying about. I do not remember if it is possible at all to obtain using libc the period of constant time offset, when time shift value is valid. Sometimes it is necessary to recalculate offset. >> ±n hours may mean ±n*3600 seconds or time with same minutes and >> seconds values but hours value is changed by n even if a 30 min DST >> transition happens in between. > > Sorry, I don't understand what this sentence is intended to mean. Let's consider Australia/Lord_Howe with 30min backward DST shift at 2022-04-03 02:00. 8 hours from 2022-04-02 22:00 may mean 2022-04-03 06:00 for duration of the night shift (8:30 instead of usual 8:00). Some technological process requiring precisely 8 hours finishes at 05:30 in such case. So it is not equivalent to add 8 hours or 480 minutes. In the former case it is more convenient to increment particular field and adjust the result if it coincides with ambiguity/impossible range. In the latter case it is better to increment timestamp as seconds since the epoch and back to time fields (leaving aside leap seconds). >> `parse-time-string' has another set of problems. > > Sure, but that was just an example. You can write your own date parser. > The point is that when you save a localtime timestamp, you should save > its UT offset too, in whatever notation is appropriate. You wrote that "2021-01-31 23:30:00 +0300" is parsed correctly. My opinion is that when time zone is known to be Africa/Juba (system-wide setting, environment variable, or an argument of the parsing function) then "2021-01-31 23:30:00 CAT" and "2021-01-31 23:30:00 EAT" should be parsed correctly (and localized date-time formats should be parsed as well). For transitions without DST change there is no conventional text representation. >> UTC offset is another feature and implementing the hints I have tried >> to describe may require implementing from scratch full stack of time >> handling functions. > > I doubt whether that's a good idea. I've written that sort of code, and > it's a lot more work than one might think and it's notoriously difficult > to do it correctly. You have better things to do. Elisp implementation of date-time library is not in my TODO list. I just know that there are enough implementations already (and some of them may be/was buggy): - https://github.com/moment/moment-timezone/blob/develop/moment-timezone.js and currently browsers should have their own implementations - https://github.com/php/php-src/blob/master/ext/date/lib/parse_tz.c - https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/time/qtimezoneprivate_tz.cpp - https://github.com/HowardHinnant/date/blob/master/src/tz.cpp and I have heard of more libraries. There are a lot of corner cases, so "universal" rules will unavoidably fail. Flexible API may alleviate some cases. P.S. Once I noticed the following comment on stackoverflow: Cubbi Jun 12, 2012 at 22:26 > std::broken_promise is the best named identifier in the > standard library. And there is no std::atomic_future. https://stackoverflow.com/questions/11004273/what-is-stdpromise mktime(3) man page uses "broken-down time" term for struct tm. It explains why it is not unusual when code dealing with time is broken. ^ permalink raw reply [flat|nested] 45+ messages in thread
[parent not found: <5cd820d4-ae67-43d4-9e63-c284d51ff1e4@gmail.com>]
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones [not found] ` <5cd820d4-ae67-43d4-9e63-c284d51ff1e4@gmail.com> @ 2022-04-16 19:23 ` Paul Eggert 2022-04-19 2:02 ` Paul Eggert [not found] ` <507cc0a2-d2d9-4283-7cc2-0da3c6510ac8@cs.ucla.edu> 2 siblings, 0 replies; 45+ messages in thread From: Paul Eggert @ 2022-04-16 19:23 UTC (permalink / raw) To: Max Nikulin; +Cc: emacs-orgmode, 54764 On 4/15/22 10:23, Max Nikulin wrote: > if you are storing future events bound to wall time then namely > time zone identifier should have precedence. Although that would make sense for some applications it's not a good idea in general. For example, if you're scheduling a Zoom meeting you should save both, because other meeting participants may interpret strings like "Pacific/Apia" differently. > Just identifier may be ambiguous around DST transition. So timezone > abbreviations are ambiguous per se but when identifiers are known they > may be still necessary to resolve uncertainties for backward time > shifts. At certain moment the Olson DB started to use "+04" > abbreviations instead of letters for transitions unrelated to daylight > saving time. Yes, timezone names like "Europe/Lisbon" are ambiguous during fallback transitions, or if the meaning of "Europe/Lisbon" changes. This is why one should also save UT offsets when generating localtime timestamps. Around five years ago I changed TZDB to numeric use time zone abbreviations like "+04" instead of my inventions like "GET", because I wanted TZDB to follow existing practice, not invent it. A nice side effect is that numeric abbreviations are unambiguous. (Besides, even _I_ couldn't remember what "GET" meant. :-) > And WET/WEST gets another bit of info in addition to numerical offset. That info is meant only for users; I wouldn't rely on it for calculations because those abbreviations are ambiguous. It could well be, for example that the meaning of "PST" in the United States will change in the near future. > I do not remember if it is possible at all to obtain using libc the > period of constant time offset, when time shift value is valid. > Sometimes it is necessary to recalculate offset. Sorry, I don't understand this point. One can easily recalculate the UT offset in Emacs Lisp by generating the desired timestamp and calling decode-time on it. You surely are talking about something else, but I don't know what it is. > You wrote that "2021-01-31 23:30:00 +0300" is parsed correctly. My > opinion is that when time zone is known to be Africa/Juba (system-wide > setting, environment variable, or an argument of the parsing function) > then "2021-01-31 23:30:00 CAT" and "2021-01-31 23:30:00 EAT" should be > parsed correctly (and localized date-time formats should be parsed as > well). That's not possible in general, since the two abbreviations can be the same. Traditionally in Australia, for example, "CST" meant both "Central Standard Time" and "Central Summer Time", and there are probably still apps that use the equivalent of TZ="CST-9:30CST,M10.1.0,M4.1.0/3" which does precisely that. It's hardly ever a good idea to rely on time zone abbreviations as they're too often ambiguous. It's much better to use UT offsets. When generating a localtime timestamp, one should always output its UT offset (in addition to any other advisory info you might want to output). And if you do that, many of the abovementioned problems are easily solved. ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones [not found] ` <5cd820d4-ae67-43d4-9e63-c284d51ff1e4@gmail.com> 2022-04-16 19:23 ` Paul Eggert @ 2022-04-19 2:02 ` Paul Eggert 2022-04-19 5:50 ` Eli Zaretskii ` (3 more replies) [not found] ` <507cc0a2-d2d9-4283-7cc2-0da3c6510ac8@cs.ucla.edu> 2 siblings, 4 replies; 45+ messages in thread From: Paul Eggert @ 2022-04-19 2:02 UTC (permalink / raw) To: Max Nikulin; +Cc: emacs-orgmode, 54764 [-- Attachment #1: Type: text/plain, Size: 2957 bytes --] Here are the main points I see about making timestamps work better in Org mode, and related patches to how Emacs handles timestamps. * Max would like encode-time to treat a list (SS MM HH DD MM YYYY) as if it were (SS MM HH DD MM YYYY nil -1 nil), as that would be more convenient for how Org mode deals with ambiguous local timestamps. This is relatively easy to add for Emacs 29+, and is done in first of the attached proposed patches to Emacs master. * As I understand it, Max would like a function that emulate Emacs 29 encode-time with one argument, even if running in Emacs versions back to Emacs 25. I suppose such a function would also need to implement Emacs 27+ encode-time's support for subsecond resolution. E.g., (org-encode-time '((44604411568 . 1000000000) 55 0 19 4 2022 - -1 t)) should return (1650329744604411568 . 1000000000) even in Emacs 25 and 26. * There are three other Emacs timestamp changes I should mention that might be relevant to Org mode: ** 1. Although in Emacs 28 functions like (time-convert nil t) return a timestamp resolution of 1 ns, in Emacs 29 I plan to fix them so that they return the system clock resolution, which is often coarser than 1 ns. This is done in the 4th attached patch. ** 2. In Emacs 29 format-time-string should support a new format "%-N" which outputs only enough digits for the system clock resolution. (This is the same as GNU "date".) This is done in the 5th attached patch. ** 3. Emacs 29 current-time and related functions should generate a (TICKS . HZ) timestamp instead of the old (HIGH LOW MICROSECS PICOSECS) timestamp. This change has been planned for some time; it was announced in Emacs 27. As far as I know Org mode is already ready for this change but I thought I'd mention it again here. This is done in the last attached patch. * My last topic in this email is Max's request for a feature that I'm not planning to put into Emacs 29 as it'll require more thought. This addresses the problem where your TZ is "Africa/Juba" and you want to encode a timestamp like "2021-01-31 23:30" which is ambiguous since at 24:00 that day Juba moved standard time back by an hour. Unfortunately the underlying C mktime function does not allow disambiguation in the rare situation where standard time moves further west of Greenwich. Addressing this problem would require rewriting mktime from scratch in Elisp, or using heuristics that would occasionally fail, or (my favorite) extending glibc mktime to treat tm_isdst values other than -1,0,1 to support disambiguating such timestamps. In the meantime, one can disambiguate such timestamps in Elisp by using numeric time zones, e.g.: (format-time-string "%F %T %z" (encode-time '(0 30 23 31 1 2021 - -1 TZ)) "Africa/Juba") yields "2021-01-31 23:30:00 +0200" if TZ is 7200, and "2021-01-31 23:30:00 +0300" if TZ is 10800. And perhaps this is the right way to go in the long run anyway. [-- Attachment #2: 0001-Support-encode-time-list-s-m-h-D-M-Y.patch --] [-- Type: text/x-patch, Size: 4727 bytes --] From 3d02a8e1192a782a16ffdee4940612f69a12629f Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Mon, 18 Apr 2022 13:08:26 -0700 Subject: [PATCH 1/6] Support (encode-time (list s m h D M Y)) * src/timefns.c (Fencode_time): Add support for a 6-elt list arg. Requested by Max Nikulin for Org (bug#54764). * test/src/timefns-tests.el (encode-time-alternate-apis): New test. --- doc/lispref/os.texi | 5 +++++ etc/NEWS | 5 +++++ src/timefns.c | 21 +++++++++++++++------ test/src/timefns-tests.el | 9 +++++++++ 4 files changed, 34 insertions(+), 6 deletions(-) diff --git a/doc/lispref/os.texi b/doc/lispref/os.texi index cabae08970..bfcd51318e 100644 --- a/doc/lispref/os.texi +++ b/doc/lispref/os.texi @@ -1660,6 +1660,11 @@ Time Conversion handle situations like this you can use a numeric @var{zone} to disambiguate instead. +The first argument can also be a list @code{(@var{second} @var{minute} +@var{hour} @var{day} @var{month} @var{year})}, which is treated like +the list @code{(@var{second} @var{minute} @var{hour} @var{day} +@var{month} @var{year} nil -1 nil)}. + As an obsolescent calling convention, this function can be given six or more arguments. The first six arguments @var{second}, @var{minute}, @var{hour}, @var{day}, @var{month}, and @var{year} diff --git a/etc/NEWS b/etc/NEWS index 3e7788277d..c5a136ea68 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -1950,6 +1950,11 @@ For example, '(time-add nil '(1 . 1000))' no longer warns that the '(1 . 1000)' acts like '(1000 . 1000000)'. This warning, which was a temporary transition aid for Emacs 27, has served its purpose. ++++ +** 'encode-time' now also accepts a 6-element list with just time and date. +(encode-time (list SECOND MINUTE HOUR DAY MONTH YEAR)) is now short for +(encode-time (list SECOND MINUTE HOUR DAY MONTH YEAR nil -1 nil)). + +++ ** 'date-to-time' now assumes earliest values if its argument lacks month, day, or time. For example, (date-to-time "2021-12-04") now diff --git a/src/timefns.c b/src/timefns.c index 7a4a7075ed..b0b84a438c 100644 --- a/src/timefns.c +++ b/src/timefns.c @@ -1620,6 +1620,9 @@ DEFUN ("encode-time", Fencode_time, Sencode_time, 1, MANY, 0, saving time, nil for standard time, and -1 to cause the daylight saving flag to be guessed. +TIME can also be a list (SECOND MINUTE HOUR DAY MONTH YEAR), which is +equivalent to (SECOND MINUTE HOUR DAY MONTH YEAR nil -1 nil). + As an obsolescent calling convention, if this function is called with 6 or more arguments, the first 6 arguments are SECOND, MINUTE, HOUR, DAY, MONTH, and YEAR, and specify the components of a decoded time. @@ -1645,7 +1648,7 @@ DEFUN ("encode-time", Fencode_time, Sencode_time, 1, MANY, 0, if (nargs == 1) { Lisp_Object tail = a; - for (int i = 0; i < 9; i++, tail = XCDR (tail)) + for (int i = 0; i < 6; i++, tail = XCDR (tail)) CHECK_CONS (tail); secarg = XCAR (a); a = XCDR (a); minarg = XCAR (a); a = XCDR (a); @@ -1653,11 +1656,17 @@ DEFUN ("encode-time", Fencode_time, Sencode_time, 1, MANY, 0, mdayarg = XCAR (a); a = XCDR (a); monarg = XCAR (a); a = XCDR (a); yeararg = XCAR (a); a = XCDR (a); - a = XCDR (a); - Lisp_Object dstflag = XCAR (a); a = XCDR (a); - zone = XCAR (a); - if (SYMBOLP (dstflag) && !FIXNUMP (zone) && !CONSP (zone)) - tm.tm_isdst = !NILP (dstflag); + if (! NILP (a)) + { + CHECK_CONS (a); + a = XCDR (a); + CHECK_CONS (a); + Lisp_Object dstflag = XCAR (a); a = XCDR (a); + CHECK_CONS (a); + zone = XCAR (a); + if (SYMBOLP (dstflag) && !FIXNUMP (zone) && !CONSP (zone)) + tm.tm_isdst = !NILP (dstflag); + } } else if (nargs < 6) xsignal2 (Qwrong_number_of_arguments, Qencode_time, make_fixnum (nargs)); diff --git a/test/src/timefns-tests.el b/test/src/timefns-tests.el index e7c464472d..08d06f27d9 100644 --- a/test/src/timefns-tests.el +++ b/test/src/timefns-tests.el @@ -225,6 +225,15 @@ encode-time-dst-numeric-zone (encode-time '(29 31 17 30 4 2019 2 t 7200)) '(23752 27217)))) +(ert-deftest encode-time-alternate-apis () + (let* ((time '(30 30 12 15 6 1970)) + (time-1 (append time '(nil -1 nil))) + (etime (encode-time time))) + (should (time-equal-p etime (encode-time time-1))) + (should (time-equal-p etime (apply #'encode-time time))) + (should (time-equal-p etime (apply #'encode-time time-1))) + (should (time-equal-p etime (apply #'encode-time (append time '(nil))))))) + (ert-deftest float-time-precision () (should (= (float-time '(0 1 0 4025)) 1.000000004025)) (should (= (float-time '(1000000004025 . 1000000000000)) 1.000000004025)) -- 2.35.1 [-- Attachment #3: 0002-Refactor-to-simplify-clock-resolution-support.patch --] [-- Type: text/x-patch, Size: 4332 bytes --] From 68bd5fdbb6a94fd90575e2e2199365a0c2b9db68 Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Mon, 18 Apr 2022 13:08:26 -0700 Subject: [PATCH 2/6] Refactor to simplify clock resolution support * src/timefns.c (timespec_mpz, timespec_ticks): New args HZ, RES. All callers changed. (timespec_clockres_to_lisp): New function, extended from timespec_to_lisp. (timespec_to_lisp): Use it. (make_lisp_time_clockres): New function, extended from make_lisp_time. (make_lisp_time): Use it. --- src/timefns.c | 61 ++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 43 insertions(+), 18 deletions(-) diff --git a/src/timefns.c b/src/timefns.c index b0b84a438c..3f8efadf54 100644 --- a/src/timefns.c +++ b/src/timefns.c @@ -482,30 +482,37 @@ mpz_set_time (mpz_t rop, time_t t) mpz_set_uintmax (rop, t); } -/* Store into mpz[0] a clock tick count for T, assuming a - TIMESPEC_HZ-frequency clock. Use mpz[1] as a temp. */ +/* Store into mpz[0] a clock tick count for T, assuming a HZ-frequency + clock with resolution RES ns (so that HZ*RES = TIMESPEC_HZ). + Use mpz[1] as a temp. */ static void -timespec_mpz (struct timespec t) +timespec_mpz (struct timespec t, long int hz, long int res) { - /* mpz[0] = sec * TIMESPEC_HZ + nsec. */ - mpz_set_ui (mpz[0], t.tv_nsec); + /* mpz[0] = sec * hz + nsec / res. */ + mpz_set_ui (mpz[0], t.tv_nsec / res); mpz_set_time (mpz[1], t.tv_sec); - mpz_addmul_ui (mpz[0], mpz[1], TIMESPEC_HZ); + mpz_addmul_ui (mpz[0], mpz[1], hz); } -/* Convert T to a Lisp integer counting TIMESPEC_HZ ticks. */ +/* Convert T to a Lisp integer counting HZ ticks each consisting + of RES ns. */ static Lisp_Object -timespec_ticks (struct timespec t) +timespec_ticks (struct timespec t, long int hz, long int res) { + eassume (0 <= t.tv_nsec && t.tv_nsec < TIMESPEC_HZ); + eassume (0 < hz && hz <= TIMESPEC_HZ); + eassume (0 < res && res <= TIMESPEC_HZ); + eassume (hz * res == TIMESPEC_HZ); + /* For speed, use intmax_t arithmetic if it will do. */ intmax_t accum; if (FASTER_TIMEFNS - && !INT_MULTIPLY_WRAPV (t.tv_sec, TIMESPEC_HZ, &accum) - && !INT_ADD_WRAPV (t.tv_nsec, accum, &accum)) + && !INT_MULTIPLY_WRAPV (t.tv_sec, hz, &accum) + && !INT_ADD_WRAPV (t.tv_nsec / res, accum, &accum)) return make_int (accum); /* Fall back on bignum arithmetic. */ - timespec_mpz (t); + timespec_mpz (t, hz, res); return make_integer_mpz (); } @@ -565,9 +572,26 @@ lisp_time_seconds (struct lisp_time t) return make_integer_mpz (); } -/* Convert T to a Lisp timestamp. */ +/* Return (TICKS . LISPHZ) for time T, assuming a LISPHZ,HZ,RES clock. */ +static Lisp_Object +timespec_clockres_to_lisp (struct timespec t, Lisp_Object lisphz, + long int hz, long int res) +{ + return Fcons (timespec_ticks (t, hz, res), lisphz); +} + +/* Return (TICKS . HZ) for time T, assuming a clock resolution of 1 ns. */ Lisp_Object -make_lisp_time (struct timespec t) +timespec_to_lisp (struct timespec t) +{ + return timespec_clockres_to_lisp (t, timespec_hz, TIMESPEC_HZ, 1); +} + +/* Convert T to a Lisp timestamp, assuming a LISPHZ or HZ-frequency + clock with resolution RES ns (so that HZ*RES = TIMESPEC_HZ). */ +static Lisp_Object +make_lisp_time_clockres (struct timespec t, Lisp_Object lisphz, + long int hz, long int res) { if (CURRENT_TIME_LIST) { @@ -577,14 +601,14 @@ make_lisp_time (struct timespec t) make_fixnum (ns / 1000), make_fixnum (ns % 1000 * 1000)); } else - return timespec_to_lisp (t); + return timespec_clockres_to_lisp (t, lisphz, hz, res); } -/* Return (TICKS . HZ) for time T. */ +/* Convert T to a Lisp timestamp. */ Lisp_Object -timespec_to_lisp (struct timespec t) +make_lisp_time (struct timespec t) { - return Fcons (timespec_ticks (t), timespec_hz); + return make_lisp_time_clockres (t, timespec_hz, TIMESPEC_HZ, 1); } /* Return NUMERATOR / DENOMINATOR, rounded to the nearest double. @@ -747,7 +771,8 @@ decode_time_components (enum timeform form, } case TIMEFORM_NIL: - return decode_ticks_hz (timespec_ticks (current_timespec ()), + return decode_ticks_hz (timespec_ticks (current_timespec (), + TIMESPEC_HZ, 1), timespec_hz, result, dresult); default: -- 2.35.1 [-- Attachment #4: 0003-Add-Gnulib-gettime-res-module.patch --] [-- Type: text/x-patch, Size: 5226 bytes --] From 8c25372709d256d83858be454987137dc202b725 Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Mon, 18 Apr 2022 13:08:27 -0700 Subject: [PATCH 3/6] Add Gnulib gettime-res module * admin/merge-gnulib (GNULIB_MODULES): Add gettime-res. * lib/gettime-res.c: New file, copied from Gnulib. * lib/gnulib.mk.in, m4/gnulib-comp.m4: Regenerate. --- admin/merge-gnulib | 3 +- lib/gettime-res.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++ lib/gnulib.mk.in | 9 ++++++ m4/gnulib-comp.m4 | 3 ++ 4 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 lib/gettime-res.c diff --git a/admin/merge-gnulib b/admin/merge-gnulib index ea3d23686f..0279d1f99d 100755 --- a/admin/merge-gnulib +++ b/admin/merge-gnulib @@ -37,7 +37,8 @@ GNULIB_MODULES= fchmodat fcntl fcntl-h fdopendir file-has-acl filemode filename filevercmp flexmember fpieee free-posix fstatat fsusage fsync futimens - getloadavg getopt-gnu getrandom gettime gettimeofday gitlog-to-changelog + getloadavg getopt-gnu getrandom + gettime gettime-res gettimeofday gitlog-to-changelog ieee754-h ignore-value intprops largefile libgmp lstat manywarnings memmem-simple mempcpy memrchr minmax mkostemp mktime nanosleep nproc nstrftime diff --git a/lib/gettime-res.c b/lib/gettime-res.c new file mode 100644 index 0000000000..611f83ad27 --- /dev/null +++ b/lib/gettime-res.c @@ -0,0 +1,78 @@ +/* Get the system clock resolution. + + Copyright 2021-2022 Free Software Foundation, Inc. + + This file is free software: you can redistribute it and/or modify + it under the terms of the GNU Lesser General Public License as + published by the Free Software Foundation, either version 3 of the + License, or (at your option) any later version. + + This file is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with this program. If not, see <https://www.gnu.org/licenses/>. */ + +/* Written by Paul Eggert. */ + +#include <config.h> + +#include "timespec.h" + +static long int _GL_ATTRIBUTE_CONST +gcd (long int a, long int b) +{ + while (b != 0) + { + long int r = a % b; + a = b; + b = r; + } + return a; +} + +/* Return the system time resolution in nanoseconds. */ + +long int +gettime_res (void) +{ + struct timespec res; +#if defined CLOCK_REALTIME && HAVE_CLOCK_GETRES + clock_getres (CLOCK_REALTIME, &res); +#elif defined HAVE_TIMESPEC_GETRES + timespec_getres (&res, TIME_UTC); +#else + /* Guess high and let the later code deduce better. */ + res.tv_sec = 1; + res.tv_nsec = 0; +#endif + + /* On all Gnulib platforms the following calculations do not overflow. */ + + long int hz = TIMESPEC_HZ; + long int r = hz * res.tv_sec + res.tv_nsec; + + /* On some platforms, clock_getres (CLOCK_REALTIME, ...) yields a + too-large resolution, under the mistaken theory that it should + return the timer interval. For example, on AIX 7.2 POWER8 + clock_getres yields 10 ms even though clock_gettime yields 1 µs + resolution. Work around the problem with high probability by + trying clock_gettime several times and observing the resulting + bounds on resolution. */ + for (int i = 0; 1 < r && i < 32; i++) + { + struct timespec now = current_timespec (); + r = gcd (r, now.tv_nsec ? now.tv_nsec : hz); + } + + return r; +} + +/* + * Hey Emacs! + * Local Variables: + * coding: utf-8 + * End: + */ diff --git a/lib/gnulib.mk.in b/lib/gnulib.mk.in index bbb05fdba5..fbc199faaf 100644 --- a/lib/gnulib.mk.in +++ b/lib/gnulib.mk.in @@ -114,6 +114,7 @@ # getopt-gnu \ # getrandom \ # gettime \ +# gettime-res \ # gettimeofday \ # gitlog-to-changelog \ # ieee754-h \ @@ -2188,6 +2189,14 @@ libgnu_a_SOURCES += gettime.c endif ## end gnulib module gettime +## begin gnulib module gettime-res +ifeq (,$(OMIT_GNULIB_MODULE_gettime-res)) + +libgnu_a_SOURCES += gettime-res.c + +endif +## end gnulib module gettime-res + ## begin gnulib module gettimeofday ifeq (,$(OMIT_GNULIB_MODULE_gettimeofday)) diff --git a/m4/gnulib-comp.m4 b/m4/gnulib-comp.m4 index fb5f1b52a4..a04ec44be9 100644 --- a/m4/gnulib-comp.m4 +++ b/m4/gnulib-comp.m4 @@ -112,6 +112,7 @@ AC_DEFUN # Code from module getrandom: # Code from module gettext-h: # Code from module gettime: + # Code from module gettime-res: # Code from module gettimeofday: # Code from module gitlog-to-changelog: # Code from module group-member: @@ -371,6 +372,7 @@ AC_DEFUN [test $HAVE_GETRANDOM = 0 || test $REPLACE_GETRANDOM = 1]) gl_SYS_RANDOM_MODULE_INDICATOR([getrandom]) gl_GETTIME + gl_GETTIME_RES gl_FUNC_GETTIMEOFDAY gl_CONDITIONAL([GL_COND_OBJ_GETTIMEOFDAY], [test $HAVE_GETTIMEOFDAY = 0 || test $REPLACE_GETTIMEOFDAY = 1]) @@ -1270,6 +1272,7 @@ AC_DEFUN lib/getopt_int.h lib/getrandom.c lib/gettext.h + lib/gettime-res.c lib/gettime.c lib/gettimeofday.c lib/gl_openssl.h -- 2.35.1 [-- Attachment #5: 0004-Use-CLOCK_REALTIME-resolution-for-TICKS-.-HZ.patch --] [-- Type: text/x-patch, Size: 8623 bytes --] From 9c42f5f9ebc5b4e5d7bd0c33a0ce7de27c27d893 Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Mon, 18 Apr 2022 13:08:27 -0700 Subject: [PATCH 4/6] Use CLOCK_REALTIME resolution for (TICKS . HZ) This gives Elisp code a clearer view of timestamps, for uses like (time-convert nil t). * src/emacs.c (main): Call init_timefns earlier. * src/keyboard.c (Fcurrent_idle_time): * src/sysdep.c (Fcurrent_time): Use make_lisp_realtime instead of make_lisp_time, for realtime timestamps (e.g., they come from gettime). * src/sysdep.c (make_lisp_s_us): Also define if HAVE_GETRUSAGE. Simplify by calling make_lisp_time_clockres. (Fget_internal_run_time): Fix resolution by calling make_lisp_s_us. * src/timefns.c (realtime_lisphz, realtime_hz, realtime_res): New static vars. (init_time_res): New function. (init_timefns): Initialize the new vars (make_lisp_time_clockres): Now extern. (make_lisp_realtime): New function. --- etc/NEWS | 6 ++++++ src/emacs.c | 7 ++++--- src/fileio.c | 5 ++++- src/keyboard.c | 4 ++-- src/sysdep.c | 14 ++++++-------- src/systime.h | 3 +++ src/timefns.c | 36 +++++++++++++++++++++++++++++++++--- 7 files changed, 58 insertions(+), 17 deletions(-) diff --git a/etc/NEWS b/etc/NEWS index c5a136ea68..38317fef03 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -1950,6 +1950,12 @@ For example, '(time-add nil '(1 . 1000))' no longer warns that the '(1 . 1000)' acts like '(1000 . 1000000)'. This warning, which was a temporary transition aid for Emacs 27, has served its purpose. ++++ +** (TICKS . HZ) timestamps now report system clock resolution. +For example, the macOS system clock resolution is 1 μs so +(time-convert nil t) now acts like (time-convert nil 1000000). +Formerly, time-convert used a clock resolution of 1 ns regardless. + +++ ** 'encode-time' now also accepts a 6-element list with just time and date. (encode-time (list SECOND MINUTE HOUR DAY MONTH YEAR)) is now short for diff --git a/src/emacs.c b/src/emacs.c index 3100852b2c..15a4a25770 100644 --- a/src/emacs.c +++ b/src/emacs.c @@ -1894,6 +1894,10 @@ main (int argc, char **argv) init_signals (); + /* This calls putenv and so must precede init_process_emacs. + It must also precede init_window_once, which creates timestamps. */ + init_timefns (); + noninteractive1 = noninteractive; /* Perform basic initializations (not merely interning symbols). */ @@ -2383,9 +2387,6 @@ main (int argc, char **argv) init_charset (); - /* This calls putenv and so must precede init_process_emacs. */ - init_timefns (); - init_editfns (); /* These two call putenv. */ diff --git a/src/fileio.c b/src/fileio.c index c418036fc6..2a857d10dd 100644 --- a/src/fileio.c +++ b/src/fileio.c @@ -5840,7 +5840,10 @@ DEFUN ("set-visited-file-modtime", Fset_visited_file_modtime, mtime = make_timespec (0, UNKNOWN_MODTIME_NSECS - flag); } else - mtime = lisp_time_argument (time_flag); + { + mtime = lisp_time_argument (time_flag); + /* FIXME: Do not assume mtime resolution is 1 ns. */ + } current_buffer->modtime = mtime; current_buffer->modtime_size = -1; diff --git a/src/keyboard.c b/src/keyboard.c index 19c8fdf1dc..3eb0694eda 100644 --- a/src/keyboard.c +++ b/src/keyboard.c @@ -4668,8 +4668,8 @@ DEFUN ("current-idle-time", Fcurrent_idle_time, Scurrent_idle_time, 0, 0, 0, (void) { if (timespec_valid_p (timer_idleness_start_time)) - return make_lisp_time (timespec_sub (current_timespec (), - timer_idleness_start_time)); + return make_lisp_realtime (timespec_sub (current_timespec (), + timer_idleness_start_time)); return Qnil; } diff --git a/src/sysdep.c b/src/sysdep.c index 36636d0ed5..b15d5c359e 100644 --- a/src/sysdep.c +++ b/src/sysdep.c @@ -3169,16 +3169,14 @@ list_system_processes (void) #endif /* !defined (WINDOWSNT) */ -#if defined __FreeBSD__ || defined DARWIN_OS || defined __OpenBSD__ +#if (defined HAVE_GETRUSAGE \ + || defined __FreeBSD__ || defined DARWIN_OS || defined __OpenBSD__) static Lisp_Object make_lisp_s_us (time_t s, long us) { - Lisp_Object sec = make_int (s); - Lisp_Object usec = make_fixnum (us); - Lisp_Object hz = make_fixnum (1000000); - Lisp_Object ticks = CALLN (Fplus, CALLN (Ftimes, sec, hz), usec); - return Ftime_convert (Fcons (ticks, hz), Qnil); + return make_lisp_time_clockres (make_timespec (s, us * 1000), + make_fixnum (1000000), 1000000, 1000); } #endif @@ -4238,7 +4236,7 @@ DEFUN ("get-internal-run-time", Fget_internal_run_time, Sget_internal_run_time, #ifdef HAVE_GETRUSAGE struct rusage usage; time_t secs; - int usecs; + long int usecs; if (getrusage (RUSAGE_SELF, &usage) < 0) /* This shouldn't happen. What action is appropriate? */ @@ -4252,7 +4250,7 @@ DEFUN ("get-internal-run-time", Fget_internal_run_time, Sget_internal_run_time, usecs -= 1000000; secs++; } - return make_lisp_time (make_timespec (secs, usecs * 1000)); + return make_lisp_s_us (secs, usecs); #else /* ! HAVE_GETRUSAGE */ #ifdef WINDOWSNT return w32_get_internal_run_time (); diff --git a/src/systime.h b/src/systime.h index 75088bd4a6..419da80826 100644 --- a/src/systime.h +++ b/src/systime.h @@ -86,7 +86,10 @@ timespec_valid_p (struct timespec t) /* defined in timefns.c */ extern struct timeval make_timeval (struct timespec) ATTRIBUTE_CONST; +extern Lisp_Object make_lisp_realtime (struct timespec); extern Lisp_Object make_lisp_time (struct timespec); +extern Lisp_Object make_lisp_time_clockres (struct timespec, Lisp_Object, + long int, long int); extern Lisp_Object timespec_to_lisp (struct timespec); extern bool list4_to_timespec (Lisp_Object, Lisp_Object, Lisp_Object, Lisp_Object, struct timespec *); diff --git a/src/timefns.c b/src/timefns.c index 3f8efadf54..0d0a8a5922 100644 --- a/src/timefns.c +++ b/src/timefns.c @@ -77,12 +77,17 @@ Copyright (C) 1985-1987, 1989, 1993-2022 Free Software Foundation, Inc. enum { CURRENT_TIME_LIST = true }; #endif +/* Clock resolution of struct timespec. */ #if FIXNUM_OVERFLOW_P (1000000000) static Lisp_Object timespec_hz; #else # define timespec_hz make_fixnum (TIMESPEC_HZ) #endif +/* Clock resolution of realtime, in various forms. HZ*RES == TIMESPEC_HZ. */ +static Lisp_Object realtime_lisphz; +static long int realtime_hz, realtime_res; + #define TRILLION 1000000000000 #if FIXNUM_OVERFLOW_P (TRILLION) static Lisp_Object trillion; @@ -302,9 +307,24 @@ tzlookup (Lisp_Object zone, bool settz) return new_tz; } +static void +init_time_res (void) +{ + realtime_res = gettime_res (); + eassume (0 < realtime_res); + if (TIMESPEC_HZ < realtime_res) + realtime_res = TIMESPEC_HZ; + realtime_hz = TIMESPEC_HZ / realtime_res; + realtime_lisphz = make_int (realtime_hz); +} + void init_timefns (void) { + pdumper_do_now_and_after_load (init_time_res); + if (FIXNUM_OVERFLOW_P (TIMESPEC_HZ) && !initialized) + staticpro (&realtime_lisphz); + #ifdef HAVE_UNEXEC /* A valid but unlikely setting for the TZ environment variable. It is OK (though a bit slower) if the user chooses this value. */ @@ -589,7 +609,7 @@ timespec_to_lisp (struct timespec t) /* Convert T to a Lisp timestamp, assuming a LISPHZ or HZ-frequency clock with resolution RES ns (so that HZ*RES = TIMESPEC_HZ). */ -static Lisp_Object +Lisp_Object make_lisp_time_clockres (struct timespec t, Lisp_Object lisphz, long int hz, long int res) { @@ -604,13 +624,23 @@ make_lisp_time_clockres (struct timespec t, Lisp_Object lisphz, return timespec_clockres_to_lisp (t, lisphz, hz, res); } -/* Convert T to a Lisp timestamp. */ +/* Convert T to a Lisp timestamp, assuming full resolution. */ Lisp_Object make_lisp_time (struct timespec t) { return make_lisp_time_clockres (t, timespec_hz, TIMESPEC_HZ, 1); } +/* Convert T to a Lisp timestamp, assuming gettime resolution. */ +Lisp_Object +make_lisp_realtime (struct timespec t) +{ + if (NILP (realtime_lisphz)) + abort (); + return make_lisp_time_clockres (t, realtime_lisphz, + realtime_hz, realtime_res); +} + /* Return NUMERATOR / DENOMINATOR, rounded to the nearest double. Arguments must be Lisp integers, and DENOMINATOR must be positive. */ static double @@ -1799,7 +1829,7 @@ DEFUN ("current-time", Fcurrent_time, Scurrent_time, 0, 0, 0, the current time in seconds. */) (void) { - return make_lisp_time (current_timespec ()); + return make_lisp_realtime (current_timespec ()); } DEFUN ("current-time-string", Fcurrent_time_string, Scurrent_time_string, -- 2.35.1 [-- Attachment #6: 0005-Support-format-time-string-N-like-date.patch --] [-- Type: text/x-patch, Size: 7510 bytes --] From 2a8467a5c48a5b7296bce13ccaee01fb80b53704 Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Mon, 18 Apr 2022 13:08:27 -0700 Subject: [PATCH 5/6] =?UTF-8?q?Support=20format-time-string=20%-N=20like?= =?UTF-8?q?=20=E2=80=98date=E2=80=99?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit format-time-string now supports "%-N" to mean "do not pad past timestamp resolution". This is compatible with GNU 'date'. * src/timefns.c (hz_width, replace_percent_minus_N): New functions. (Fformat_time_string): Use them to support %-N. * test/src/timefns-tests.el (format-time-string-%-N): New test. (format-time-string-padding-minimal-deletes-unneeded-zeros): Adjust test to match new behavior. --- doc/lispref/os.texi | 3 +++ etc/NEWS | 5 ++++ src/timefns.c | 53 +++++++++++++++++++++++++++++++++++---- test/src/timefns-tests.el | 12 +++++++-- 4 files changed, 66 insertions(+), 7 deletions(-) diff --git a/doc/lispref/os.texi b/doc/lispref/os.texi index bfcd51318e..30ece4aeff 100644 --- a/doc/lispref/os.texi +++ b/doc/lispref/os.texi @@ -1897,6 +1897,9 @@ Time Parsing @samp{_} pads with blanks, @samp{-} suppresses padding, @samp{^} upper-cases letters, and @samp{#} reverses the case of letters. +As a special case, @samp{%-N} suppresses padding past the resolution +of @var{time}. For example, if @var{time} is @code{(12345020 . 1000)}, +@samp{%-N} stands for @samp{020}. You can also specify the field width and type of padding for any of these @samp{%}-sequences. This works as in @code{printf}: you write diff --git a/etc/NEWS b/etc/NEWS index 38317fef03..d9635cce46 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -1956,6 +1956,11 @@ For example, the macOS system clock resolution is 1 μs so (time-convert nil t) now acts like (time-convert nil 1000000). Formerly, time-convert used a clock resolution of 1 ns regardless. ++++ +** 'format-time-string' no longer pads %-N excessively. +Insteads, it pads to the resolution of the timestamp when the +timestamp resolution is greater than 1 ns. + +++ ** 'encode-time' now also accepts a 6-element list with just time and date. (encode-time (list SECOND MINUTE HOUR DAY MONTH YEAR)) is now short for diff --git a/src/timefns.c b/src/timefns.c index 0d0a8a5922..a6cc9c9f05 100644 --- a/src/timefns.c +++ b/src/timefns.c @@ -1430,6 +1430,42 @@ format_time_string (char const *format, ptrdiff_t formatlen, return result; } +/* Yield the number of decimal digits needed to output a time with the + clock frequency HZ (0 < HZ <= TIMESPEC_HZ / 10), without losing info. + But if HZ is 1, yield 1. 0 < result < LOG10_TIMESPEC_HZ. */ + +static int +hz_width (int hz) +{ + int i = 0; + for (int r = 1; r < hz; r *= 10) + i++; + return i | !i; +} + +/* Modify FORMAT, of length FORMATLEN and with FORMAT[FORMATLEN] == '\0', + in place, replacing each "%-N" with "%9N", "%6N", or whatever + number of digits is appropriate for min (HZ, TIMESPEC_HZ). + HZ is a positive integer. */ + +static void +replace_percent_minus_N (char *format, ptrdiff_t formatlen, Lisp_Object hz) +{ + for (char *f = format; f < format + formatlen; f++) + if (f[0] == '%') + { + if (f[1] == '-' && f[2] == 'N') + { + f[1] = (FIXNUMP (hz) && XFIXNUM (hz) <= TIMESPEC_HZ / 10 + ? '0' + hz_width (XFIXNUM (hz)) + : '9'); + f += 2; + } + else + f += f[1] == '%'; + } +} + DEFUN ("format-time-string", Fformat_time_string, Sformat_time_string, 1, 3, 0, doc: /* Use FORMAT-STRING to format the time value TIME. A time value that is omitted or nil stands for the current time, @@ -1484,7 +1520,7 @@ DEFUN ("format-time-string", Fformat_time_string, Sformat_time_string, 1, 3, 0, A %-sequence can contain optional flags, field width, and a modifier (in that order) after the `%'. The flags are: -`-' Do not pad the field. +`-' Do not pad the field. %-N means to not pad past TIME's resolution. `_' Pad with spaces. `0' Pad with zeros. `+' Pad with zeros and put `+' before nonnegative year numbers with >4 digits. @@ -1508,14 +1544,21 @@ DEFUN ("format-time-string", Fformat_time_string, Sformat_time_string, 1, 3, 0, usage: (format-time-string FORMAT-STRING &optional TIME ZONE) */) (Lisp_Object format_string, Lisp_Object timeval, Lisp_Object zone) { - struct timespec t = lisp_time_argument (timeval); + struct lisp_time lt = lisp_time_struct (timeval, 0); + struct timespec t = lisp_to_timespec (lt); + if (! timespec_valid_p (t)) + time_overflow (); struct tm tm; - CHECK_STRING (format_string); + /* Convert FORMAT_STRING to the locale's encoding, and modify + any %-N formats in the copy to be the system clock resolution. */ format_string = code_convert_string_norecord (format_string, Vlocale_coding_system, 1); - return format_time_string (SSDATA (format_string), SBYTES (format_string), - t, zone, &tm); + char *format = SSDATA (format_string); + ptrdiff_t formatlen = SBYTES (format_string); + replace_percent_minus_N (format, formatlen, lt.hz); + + return format_time_string (format, formatlen, t, zone, &tm); } DEFUN ("decode-time", Fdecode_time, Sdecode_time, 0, 3, 0, diff --git a/test/src/timefns-tests.el b/test/src/timefns-tests.el index 08d06f27d9..0d79152f8a 100644 --- a/test/src/timefns-tests.el +++ b/test/src/timefns-tests.el @@ -128,7 +128,7 @@ format-time-string-with-bignum-on-32-bit (ert-deftest format-time-string-padding-minimal-deletes-unneeded-zeros () (let ((ref-time (encode-time '((123450 . 1000000) 0 0 15 2 2000 - - t)))) (should (equal (format-time-string "%-:::z" ref-time "FJT-12") "+12")) - (should (equal (format-time-string "%-N" ref-time t) "12345")) + (should (equal (format-time-string "%-N" ref-time t) "123450")) (should (equal (format-time-string "%-6N" ref-time t) "12345")) (should (equal (format-time-string "%-m" ref-time t) "2")))) ;not "02" @@ -137,7 +137,7 @@ format-time-string-padding-minimal-retains-needed-zeros (should (equal (format-time-string "%-z" ref-time "IST-5:30") "+530")) (should (equal (format-time-string "%-4z" ref-time "IST-5:30") "+530")) (should (equal (format-time-string "%4z" ref-time "IST-5:30") "+530")) - (should (equal (format-time-string "%-N" ref-time t) "00345")) + (should (equal (format-time-string "%-N" ref-time t) "003450")) (should (equal (format-time-string "%-3N" ref-time t) "003")) (should (equal (format-time-string "%3N" ref-time t) "003")) (should (equal (format-time-string "%-m" ref-time t) "10")) ;not "1" @@ -165,6 +165,14 @@ format-time-string-padding-zeros-adds-on-insignificant-side (should (equal (format-time-string "%9N" ref-time t) "123000000")) (should (equal (format-time-string "%6N" ref-time t) "123000")))) +(ert-deftest format-time-string-%-N () + (should (equal (format-time-string "%-N" '(0 . 10)) "0")) + (should (equal (format-time-string "%-N" '(0 . 11)) "00")) + (should (equal (format-time-string "%-N" '(0 . 100)) "00")) + (should (equal (format-time-string "%-N" '(0 . 101)) "000")) + (should (equal (format-time-string "%-N" '(0 . 100000000)) "00000000")) + (should (equal (format-time-string "%-N" '(0 . 100000001)) "000000000")) + (should (equal (format-time-string "%-N" '(0 . 1000000000)) "000000000"))) (ert-deftest time-equal-p-nil-nil () (should (time-equal-p nil nil))) -- 2.35.1 [-- Attachment #7: 0006-Use-TICKS-.-HZ-for-current-time-etc.patch --] [-- Type: text/x-patch, Size: 6915 bytes --] From c8b1fbb36aa0dee050dd7eea5fed9760c410a541 Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Mon, 18 Apr 2022 13:08:27 -0700 Subject: [PATCH 6/6] Use (TICKS . HZ) for current-time etc. * src/timefns.c (CURRENT_TIME_LIST): Change default to false. All documentation changed. --- doc/lispintro/emacs-lisp-intro.texi | 6 +++--- doc/lispref/files.texi | 14 +++++++------- doc/lispref/intro.texi | 2 +- doc/lispref/os.texi | 15 +++++++-------- etc/NEWS | 8 ++++++++ src/timefns.c | 26 ++++++++++++-------------- 6 files changed, 38 insertions(+), 33 deletions(-) diff --git a/doc/lispintro/emacs-lisp-intro.texi b/doc/lispintro/emacs-lisp-intro.texi index 466d7f0e60..afaed10cdf 100644 --- a/doc/lispintro/emacs-lisp-intro.texi +++ b/doc/lispintro/emacs-lisp-intro.texi @@ -15343,9 +15343,9 @@ Files List 100 @end group @group -(20615 27034 579989 697000) -(17905 55681 0 0) -(20615 26327 734791 805000) +(1351051674579989697 . 1000000000) +(1173477761000000000 . 1000000000) +(1351050967734791805 . 1000000000) 13188 "-rw-r--r--" @end group diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi index d8b55b114a..4394f64a32 100644 --- a/doc/lispref/files.texi +++ b/doc/lispref/files.texi @@ -1423,9 +1423,9 @@ File Attributes @group (file-attributes "files.texi" 'string) @result{} (nil 1 "lh" "users" - (20614 64019 50040 152000) - (20000 23 0 0) - (20614 64555 902289 872000) + (1351023123050040152 . 1000000000) + (1310720023000000000 . 1000000000) + (1351023659902289872 . 1000000000) 122295 "-rw-rw-rw-" t 6473924464520138 1014478468) @@ -1449,13 +1449,13 @@ File Attributes @item "users" is in the group with name @samp{users}. -@item (20614 64019 50040 152000) +@item (1351023123050040152 . 1000000000) was last accessed on October 23, 2012, at 20:12:03.050040152 UTC. -@item (20000 23 0 0) -was last modified on July 15, 2001, at 08:53:43 UTC. +@item (1310720023000000000 . 1000000000) +was last modified on July 15, 2001, at 08:53:43.000000000 UTC. -@item (20614 64555 902289 872000) +@item (1351023659902289872 . 1000000000) last had its status changed on October 23, 2012, at 20:20:59.902289872 UTC. @item 122295 diff --git a/doc/lispref/intro.texi b/doc/lispref/intro.texi index 5afd2f4ecf..d1a3fef7a4 100644 --- a/doc/lispref/intro.texi +++ b/doc/lispref/intro.texi @@ -503,7 +503,7 @@ Version Info @example @group emacs-build-time - @result{} (20614 63694 515336 438000) + @result{} (1650228902637038831 . 1000000000) @end group @end example @end defvar diff --git a/doc/lispref/os.texi b/doc/lispref/os.texi index 30ece4aeff..ae551b0f71 100644 --- a/doc/lispref/os.texi +++ b/doc/lispref/os.texi @@ -1359,9 +1359,6 @@ Time of Day @tex $high \times 2^{16} + low + micro \times 10^{-6} + pico \times 10^{-12}$. @end tex -In some cases, functions may default to returning two- or -three-element lists, with omitted @var{micro} and @var{pico} -components defaulting to zero. On all current machines @var{pico} is a multiple of 1000, but this may change as higher-resolution clocks become available. @end itemize @@ -1415,11 +1412,13 @@ Time of Day @defun current-time This function returns the current time as a Lisp timestamp. -Although the timestamp takes the form @code{(@var{high} @var{low} -@var{micro} @var{pico})} in the current Emacs release, this is -planned to change in a future Emacs version. You can use the -@code{time-convert} function to convert a timestamp to some other -form. @xref{Time Conversion}. +The timestamp has the form @code{(@var{ticks} . @var{hz})} where +@var{ticks} counts clock ticks and @var{hz} is the clock ticks per second. + +In Emacs 28 and earlier, the returned timestamp had the list form +@code{(@var{high} @var{low} @var{usec} @var{psec})}. You can use +@code{(time-convert nil 'list)} to return the current time in this +older form. @xref{Time Conversion}. @end defun @defun float-time &optional time diff --git a/etc/NEWS b/etc/NEWS index d9635cce46..c0b44effc6 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -202,6 +202,14 @@ speakers of the Tamil language. To get back the previous behavior, use the new 'tamil-itrans-digits' and 'tamil-inscript-digits' input methods instead. ++++ +** current-time and related functions now yield (TICKS . HZ) timestamps. +Previously they yielded timestamps of the forms (HI LO US PS), (HI LO +US) or (HI LO), which were less regular and less efficient and which +lacked information about clock resolution. This long-planned change +was documented in Emacs 27. To convert a timestamp X to the old +4-element list form, you can use (time-convert X 'list). + \f * Changes in Emacs 29.1 diff --git a/src/timefns.c b/src/timefns.c index a6cc9c9f05..286d5eb2b8 100644 --- a/src/timefns.c +++ b/src/timefns.c @@ -69,12 +69,11 @@ Copyright (C) 1985-1987, 1989, 1993-2022 Free Software Foundation, Inc. # define FASTER_TIMEFNS 1 #endif -/* Although current-time etc. generate list-format timestamps - (HI LO US PS), the plan is to change these functions to generate - frequency-based timestamps (TICKS . HZ) in a future release. - To try this now, compile with -DCURRENT_TIME_LIST=0. */ +/* current-time etc. generate (TICKS . HZ) timestamps. + To change that to the old 4-element list format (HI LO US PS), + compile with -DCURRENT_TIME_LIST=1. */ #ifndef CURRENT_TIME_LIST -enum { CURRENT_TIME_LIST = true }; +enum { CURRENT_TIME_LIST = false }; #endif /* Clock resolution of struct timespec. */ @@ -1861,15 +1860,14 @@ DEFUN ("time-convert", Ftime_convert, Stime_convert, 1, 2, 0, DEFUN ("current-time", Fcurrent_time, Scurrent_time, 0, 0, 0, doc: /* Return the current time, as the number of seconds since 1970-01-01 00:00:00. -The time is returned as a list of integers (HIGH LOW USEC PSEC). -HIGH has the most significant bits of the seconds, while LOW has the -least significant 16 bits. USEC and PSEC are the microsecond and -picosecond counts. - -In a future Emacs version, the format of the returned timestamp is -planned to change. Use `time-convert' if you need a particular -timestamp form; for example, (time-convert nil \\='integer) returns -the current time in seconds. */) +The time is returned as a pair of integers (TICKS . HZ), where TICKS +counts clock ticks and HZ is the clock ticks per second. + +In Emacs 28 and earlier, the returned timestamp had the form (HIGH LOW +USEC PSEC), where HIGH is the most significant bits of the seconds, +LOW the least significant 16 bits, and USEC and PSEC are the +microsecond and picosecond counts. Use \(time-convert nil \\='list) +if you need this older timestamp form. */) (void) { return make_lisp_realtime (current_timespec ()); -- 2.35.1 ^ permalink raw reply related [flat|nested] 45+ messages in thread
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones 2022-04-19 2:02 ` Paul Eggert @ 2022-04-19 5:50 ` Eli Zaretskii [not found] ` <83tuapvcxs.fsf@gnu.org> ` (2 subsequent siblings) 3 siblings, 0 replies; 45+ messages in thread From: Eli Zaretskii @ 2022-04-19 5:50 UTC (permalink / raw) To: Paul Eggert; +Cc: manikulin, emacs-orgmode, 54764 > Date: Mon, 18 Apr 2022 19:02:03 -0700 > From: Paul Eggert <eggert@cs.ucla.edu> > Cc: emacs-orgmode@gnu.org, 54764@debbugs.gnu.org > > From 8c25372709d256d83858be454987137dc202b725 Mon Sep 17 00:00:00 2001 > From: Paul Eggert <eggert@cs.ucla.edu> > Date: Mon, 18 Apr 2022 13:08:27 -0700 > Subject: [PATCH 3/6] Add Gnulib gettime-res module > > * admin/merge-gnulib (GNULIB_MODULES): Add gettime-res. > * lib/gettime-res.c: New file, copied from Gnulib. > * lib/gnulib.mk.in, m4/gnulib-comp.m4: Regenerate. Is this known to support MS-Windows correctly? If so, can you show how to test that, or tell where I can find the results of such testing that someone else did? If MS-Windows isn't supported by that, I would ask to add such support before this is installed. Thanks. ^ permalink raw reply [flat|nested] 45+ messages in thread
[parent not found: <83tuapvcxs.fsf@gnu.org>]
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones [not found] ` <83tuapvcxs.fsf@gnu.org> @ 2022-04-19 22:22 ` Paul Eggert 2022-04-20 7:23 ` Eli Zaretskii [not found] ` <83fsm8tdzl.fsf@gnu.org> 0 siblings, 2 replies; 45+ messages in thread From: Paul Eggert @ 2022-04-19 22:22 UTC (permalink / raw) To: Eli Zaretskii; +Cc: manikulin, emacs-orgmode, 54764 [-- Attachment #1: Type: text/plain, Size: 847 bytes --] On 4/18/22 22:50, Eli Zaretskii wrote: >> * admin/merge-gnulib (GNULIB_MODULES): Add gettime-res. >> * lib/gettime-res.c: New file, copied from Gnulib. >> * lib/gnulib.mk.in, m4/gnulib-comp.m4: Regenerate. > Is this known to support MS-Windows correctly? I haven't tested it on that platform, though I expect it to work since it relies only on current_timespec and Emacs already uses that. I just now added some test cases to Gnulib for it; see the patch in the first attachment. You can try these tests in your environment by running './gnulib-tool --test gettime-res' in the Gnulib source directory. Or you can save time by running './configure; make check' in the directory represented by the second attachment, which is a compressed tarball containing the output of './gnulib-tool --create-testdir --dir test-gettime-res gettime-res'. [-- Attachment #2: 0001-gettime-res-add-tests.patch --] [-- Type: text/x-patch, Size: 4055 bytes --] From f67c0cfb02c920bd89f490c7790f991db45a0bc5 Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Tue, 19 Apr 2022 15:13:09 -0700 Subject: [PATCH] gettime-res: add tests * modules/gettime-res-tests, tests/test-gettime-res.c: New files. --- ChangeLog | 5 +++ modules/gettime-res-tests | 12 ++++++ tests/test-gettime-res.c | 89 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 106 insertions(+) create mode 100644 modules/gettime-res-tests create mode 100644 tests/test-gettime-res.c diff --git a/ChangeLog b/ChangeLog index 1e238d14e9..9bab736be4 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2022-04-19 Paul Eggert <eggert@cs.ucla.edu> + + gettime-res: add tests + * modules/gettime-res-tests, tests/test-gettime-res.c: New files. + 2022-04-16 Paul Eggert <eggert@cs.ucla.edu> verify: port to Mac OS 10.7.5 diff --git a/modules/gettime-res-tests b/modules/gettime-res-tests new file mode 100644 index 0000000000..b169f1c187 --- /dev/null +++ b/modules/gettime-res-tests @@ -0,0 +1,12 @@ +Files: +tests/signature.h +tests/test-gettime-res.c + +Depends-on: + +configure.ac: + +Makefile.am: +TESTS += test-gettime-res +check_PROGRAMS += test-gettime-res +test_gettime_res_LDADD = $(LDADD) $(LIB_CLOCK_GETTIME) diff --git a/tests/test-gettime-res.c b/tests/test-gettime-res.c new file mode 100644 index 0000000000..0b13f93ec6 --- /dev/null +++ b/tests/test-gettime-res.c @@ -0,0 +1,89 @@ +/* + * Copyright 2022 Free Software Foundation, Inc. + * Written by Paul Eggert. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <https://www.gnu.org/licenses/>. */ + +#include <config.h> + +#include <timespec.h> + +#include <stdio.h> + +int +main (void) +{ + long int res = gettime_res (); + printf ("gettime_res returned %ld ns\n", res); + + if (res <= 0) + { + fprintf (stderr, "gettime_res value %ld not positive\n", res); + return 1; + } + + if (res < TIMESPEC_HZ) + { + if (TIMESPEC_HZ % res != 0) + { + fprintf (stderr, + ("gettime_res value %ld ns is smaller than %d" + " but does not divide it\n"), + res, TIMESPEC_HZ); + return 1; + } + } + else + { + if (res % TIMESPEC_HZ != 0) + { + fprintf (stderr, + ("gettime_res value %ld ns is larger than %d" + " but is not a multiple of it\n"), + res, TIMESPEC_HZ); + return 1; + } + } + + int saw_res = 0; + + for (int i = 0; i < 100000; i++) + { + struct timespec t = current_timespec (); + if (res < TIMESPEC_HZ + ? t.tv_nsec % res != 0 + : t.tv_nsec != 0 || t.tv_sec % (res / TIMESPEC_HZ) != 0) + { + fprintf (stderr, + ("current_timespec returned %lld.%09ld which is not" + " a multiple of the resolution, %ld ns\n"), + (long long) t.tv_sec, t.tv_nsec, res); + return 1; + } + if (res < TIMESPEC_HZ + ? (t.tv_nsec / res % 2 != 0 + || t.tv_nsec / res % 5 != 0) + : (t.tv_sec / (res / TIMESPEC_HZ) % 2 != 0 + || t.tv_sec / (res / TIMESPEC_HZ) % 5 != 0)) + saw_res = 1; + } + + if (saw_res == 0) + fprintf (stderr, + ("warning: all timestamps had coarser" + " resolution than %ld ns\n"), + res); + + return 0; +} -- 2.35.1 [-- Attachment #3: test-gettime-res.tgz --] [-- Type: application/x-compressed-tar, Size: 438013 bytes --] ^ permalink raw reply related [flat|nested] 45+ messages in thread
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones 2022-04-19 22:22 ` Paul Eggert @ 2022-04-20 7:23 ` Eli Zaretskii [not found] ` <83fsm8tdzl.fsf@gnu.org> 1 sibling, 0 replies; 45+ messages in thread From: Eli Zaretskii @ 2022-04-20 7:23 UTC (permalink / raw) To: Paul Eggert; +Cc: manikulin, emacs-orgmode, 54764 > Date: Tue, 19 Apr 2022 15:22:29 -0700 > Cc: manikulin@gmail.com, emacs-orgmode@gnu.org, 54764@debbugs.gnu.org > From: Paul Eggert <eggert@cs.ucla.edu> > > On 4/18/22 22:50, Eli Zaretskii wrote: > >> * admin/merge-gnulib (GNULIB_MODULES): Add gettime-res. > >> * lib/gettime-res.c: New file, copied from Gnulib. > >> * lib/gnulib.mk.in, m4/gnulib-comp.m4: Regenerate. > > Is this known to support MS-Windows correctly? > > I haven't tested it on that platform, though I expect it to work since > it relies only on current_timespec and Emacs already uses that. > > I just now added some test cases to Gnulib for it; see the patch in the > first attachment. You can try these tests in your environment by running > './gnulib-tool --test gettime-res' in the Gnulib source directory. Or > you can save time by running './configure; make check' in the directory > represented by the second attachment, which is a compressed tarball > containing the output of './gnulib-tool --create-testdir --dir > test-gettime-res gettime-res'. Thanks, the test-gettime-res test says "gettime_res returned 625000 ns", which is a strange number: it doesn't fit any MS-Windows system time resolution figure I know about. Do you happen to know what does this number represent, and why it is the result of gettime-res.c when it runs on MS-Windows? AFAIK, the basic resolution of MS-Windows time stamps is 100 ns, so using the above much larger number seems to hint at some significant loss of information. If the goal of this future changeset is to make Emacs time stamps more fine-grained, it would be a shame not to have the 100-ns resolution on MS-Windows. ^ permalink raw reply [flat|nested] 45+ messages in thread
[parent not found: <83fsm8tdzl.fsf@gnu.org>]
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones [not found] ` <83fsm8tdzl.fsf@gnu.org> @ 2022-04-20 18:19 ` Paul Eggert 2022-04-20 18:41 ` Eli Zaretskii 2022-04-23 14:35 ` Bernhard Voelker 0 siblings, 2 replies; 45+ messages in thread From: Paul Eggert @ 2022-04-20 18:19 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-orgmode, manikulin, Gnulib bugs, 54764 [-- Attachment #1: Type: text/plain, Size: 1125 bytes --] On 4/20/22 00:23, Eli Zaretskii wrote: >> Date: Tue, 19 Apr 2022 15:22:29 -0700 > Thanks, the test-gettime-res test says "gettime_res returned 625000 > ns", which is a strange number: it doesn't fit any MS-Windows system > time resolution figure I know about. Do you happen to know what does > this number represent, and why it is the result of gettime-res.c when > it runs on MS-Windows? It comes from current_timespec samples taken by gettime_res. Evidently something is going wrong, either in gettime_res or in current_timespec. I stared at the code a bit and see one possible problem, which I fixed by installing the attached patch into Gnulib. I then generated a new test-gettime-res.tgz compressed tarball (also attached); could you give it a try? This tarball is the result of running ./gnulib-tool as before, except I added an extra print statement executed if you pass an extra argument to that test program. So if you run the test and it's still outputting an outlandish value for the resolution, please run the command: gltests/test-gettime-res x and let's take a look at its (long) debugging output. [-- Attachment #2: 0001-gettime-res-more-robust-sampling.patch --] [-- Type: text/x-patch, Size: 2501 bytes --] From 6dc5c1be733cbfbcecaa44ceab070ea4724a1ba0 Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Wed, 20 Apr 2022 10:42:51 -0700 Subject: [PATCH] gettime-res: more-robust sampling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * lib/gettime-res.c (gettime_res): If adjacent timestamps are identical search for a differing timestamp. Also, stop collecting samples thereafter since they surely won’t help. --- ChangeLog | 7 +++++++ lib/gettime-res.c | 19 +++++++++++++++++-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index fee51331c8..4b39a6a443 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2022-04-20 Paul Eggert <eggert@cs.ucla.edu> + + gettime-res: more-robust sampling + * lib/gettime-res.c (gettime_res): If adjacent timestamps are + identical search for a differing timestamp. Also, stop collecting + samples thereafter since they surely won’t help. + 2022-04-19 Paul Eggert <eggert@cs.ucla.edu> Port _GL_HAS_C_ATTRIBUTE to pedantic gcc -std=c99 diff --git a/lib/gettime-res.c b/lib/gettime-res.c index 611f83ad27..bb4d0b191d 100644 --- a/lib/gettime-res.c +++ b/lib/gettime-res.c @@ -53,6 +53,8 @@ gettime_res (void) long int hz = TIMESPEC_HZ; long int r = hz * res.tv_sec + res.tv_nsec; + struct timespec earlier; + earlier.tv_nsec = -1; /* On some platforms, clock_getres (CLOCK_REALTIME, ...) yields a too-large resolution, under the mistaken theory that it should @@ -61,9 +63,22 @@ gettime_res (void) resolution. Work around the problem with high probability by trying clock_gettime several times and observing the resulting bounds on resolution. */ - for (int i = 0; 1 < r && i < 32; i++) + int nsamples = 32; + for (int i = 0; 1 < r && i < nsamples; i++) { - struct timespec now = current_timespec (); + /* If successive timestamps disagree the clock resolution must + be small, so exit the inner loop to check this sample. + Otherwise, arrange for the outer loop to exit but continue + the inner-loop search for a differing timestamp sample. */ + struct timespec now; + for (;; i = nsamples) + { + now = current_timespec (); + if (earlier.tv_nsec != now.tv_nsec || earlier.tv_sec != now.tv_sec) + break; + } + earlier = now; + r = gcd (r, now.tv_nsec ? now.tv_nsec : hz); } -- 2.35.1 [-- Attachment #3: test-gettime-res.tgz --] [-- Type: application/x-compressed-tar, Size: 438972 bytes --] ^ permalink raw reply related [flat|nested] 45+ messages in thread
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones 2022-04-20 18:19 ` Paul Eggert @ 2022-04-20 18:41 ` Eli Zaretskii 2022-04-20 19:01 ` Paul Eggert [not found] ` <4e41671c-fae8-61c4-845c-4c7ba4317e88@cs.ucla.edu> 2022-04-23 14:35 ` Bernhard Voelker 1 sibling, 2 replies; 45+ messages in thread From: Eli Zaretskii @ 2022-04-20 18:41 UTC (permalink / raw) To: Paul Eggert; +Cc: emacs-orgmode, manikulin, bug-gnulib, 54764 > Date: Wed, 20 Apr 2022 11:19:29 -0700 > Cc: manikulin@gmail.com, emacs-orgmode@gnu.org, 54764@debbugs.gnu.org, > Gnulib bugs <bug-gnulib@gnu.org> > From: Paul Eggert <eggert@cs.ucla.edu> > > > Thanks, the test-gettime-res test says "gettime_res returned 625000 > > ns", which is a strange number: it doesn't fit any MS-Windows system > > time resolution figure I know about. Do you happen to know what does > > this number represent, and why it is the result of gettime-res.c when > > it runs on MS-Windows? > > It comes from current_timespec samples taken by gettime_res. Evidently > something is going wrong, either in gettime_res or in current_timespec. > > I stared at the code a bit and see one possible problem, which I fixed > by installing the attached patch into Gnulib. I then generated a new > test-gettime-res.tgz compressed tarball (also attached); could you give > it a try? > > This tarball is the result of running ./gnulib-tool as before, except I > added an extra print statement executed if you pass an extra argument to > that test program. So if you run the test and it's still outputting an > outlandish value for the resolution, please run the command: > > gltests/test-gettime-res x > > and let's take a look at its (long) debugging output. I get the same result, and moreover I see no differences between this and the previous tarball, and no printouts in test-gettime-res.c. Did you perhaps attach the wrong tarball this time? Thanks. ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones 2022-04-20 18:41 ` Eli Zaretskii @ 2022-04-20 19:01 ` Paul Eggert [not found] ` <4e41671c-fae8-61c4-845c-4c7ba4317e88@cs.ucla.edu> 1 sibling, 0 replies; 45+ messages in thread From: Paul Eggert @ 2022-04-20 19:01 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-orgmode, manikulin, bug-gnulib, 54764 [-- Attachment #1: Type: text/plain, Size: 522 bytes --] On 4/20/22 11:41, Eli Zaretskii wrote: > I get the same result, and moreover I see no differences between this > and the previous tarball, and no printouts in test-gettime-res.c. Did > you perhaps attach the wrong tarball this time? That's odd, as I get the different result (i.e., with debugging info) when I use that tarball, a copy of which I got off the email archive (so I know I sent it :-). Please see the attached shell transcript for exactly how I got the different result, starting from the email archive. [-- Attachment #2: test-gettime-res-shell-transcript.txt.gz --] [-- Type: application/gzip, Size: 412620 bytes --] ^ permalink raw reply [flat|nested] 45+ messages in thread
[parent not found: <4e41671c-fae8-61c4-845c-4c7ba4317e88@cs.ucla.edu>]
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones [not found] ` <4e41671c-fae8-61c4-845c-4c7ba4317e88@cs.ucla.edu> @ 2022-04-20 19:14 ` Eli Zaretskii [not found] ` <83fsm7sh2s.fsf@gnu.org> 1 sibling, 0 replies; 45+ messages in thread From: Eli Zaretskii @ 2022-04-20 19:14 UTC (permalink / raw) To: Paul Eggert; +Cc: emacs-orgmode, manikulin, bug-gnulib, 54764 > Date: Wed, 20 Apr 2022 12:01:27 -0700 > Cc: manikulin@gmail.com, emacs-orgmode@gnu.org, 54764@debbugs.gnu.org, > bug-gnulib@gnu.org > From: Paul Eggert <eggert@cs.ucla.edu> > > > I get the same result, and moreover I see no differences between this > > and the previous tarball, and no printouts in test-gettime-res.c. Did > > you perhaps attach the wrong tarball this time? > > That's odd, as I get the different result (i.e., with debugging info) > when I use that tarball, a copy of which I got off the email archive (so > I know I sent it :-). Please see the attached shell transcript for > exactly how I got the different result, starting from the email archive. Sorry, my bad. The result is the same, but I do get printouts. What do you want to know or see from there? ^ permalink raw reply [flat|nested] 45+ messages in thread
[parent not found: <83fsm7sh2s.fsf@gnu.org>]
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones [not found] ` <83fsm7sh2s.fsf@gnu.org> @ 2022-04-20 19:23 ` Paul Eggert [not found] ` <e4cc58ca-51f9-395e-05f5-5f06cb9d439d@cs.ucla.edu> 1 sibling, 0 replies; 45+ messages in thread From: Paul Eggert @ 2022-04-20 19:23 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-orgmode, manikulin, bug-gnulib, 54764 On 4/20/22 12:14, Eli Zaretskii wrote: > Sorry, my bad. The result is the same, but I do get printouts. What > do you want to know or see from there? I want to see what the current_timespec's resolution is, which we should be able to tell from the debugging output. For example, on my Solaris 10 sparc platform the command 'gltests/test-gettime-res x' outputs: gettime_res returned 200 ns time = 1650482432.256445600 time = 1650482432.256460600 time = 1650482432.256464400 time = 1650482432.256468200 time = 1650482432.256471400 time = 1650482432.256474600 time = 1650482432.256478000 time = 1650482432.256481200 time = 1650482432.256484800 ... and these timestamps say that with very high probability current_timespec's clock resolution is indeed 200 ns. ^ permalink raw reply [flat|nested] 45+ messages in thread
[parent not found: <e4cc58ca-51f9-395e-05f5-5f06cb9d439d@cs.ucla.edu>]
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones [not found] ` <e4cc58ca-51f9-395e-05f5-5f06cb9d439d@cs.ucla.edu> @ 2022-04-20 19:30 ` Eli Zaretskii 2022-04-21 0:11 ` Paul Eggert [not found] ` <33fb24fb-282b-cc13-a597-e7b63f19982d@cs.ucla.edu> 0 siblings, 2 replies; 45+ messages in thread From: Eli Zaretskii @ 2022-04-20 19:30 UTC (permalink / raw) To: Paul Eggert; +Cc: emacs-orgmode, manikulin, bug-gnulib, 54764 > Date: Wed, 20 Apr 2022 12:23:43 -0700 > Cc: manikulin@gmail.com, emacs-orgmode@gnu.org, 54764@debbugs.gnu.org, > bug-gnulib@gnu.org > From: Paul Eggert <eggert@cs.ucla.edu> > > On 4/20/22 12:14, Eli Zaretskii wrote: > > Sorry, my bad. The result is the same, but I do get printouts. What > > do you want to know or see from there? > > I want to see what the current_timespec's resolution is, which we should > be able to tell from the debugging output. For example, on my Solaris 10 > sparc platform the command 'gltests/test-gettime-res x' outputs: > > gettime_res returned 200 ns > time = 1650482432.256445600 > time = 1650482432.256460600 > time = 1650482432.256464400 > time = 1650482432.256468200 > time = 1650482432.256471400 > time = 1650482432.256474600 > time = 1650482432.256478000 > time = 1650482432.256481200 > time = 1650482432.256484800 > ... > > and these timestamps say that with very high probability > current_timespec's clock resolution is indeed 200 ns. I see the time samples change in jumps of 15 msec. Which is expected on MS-Windows, given the scheduler time tick, but what does that have to do with the system's time resolution? And how is the 0.625 msec number reported by the program obtained from those samples? ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones 2022-04-20 19:30 ` Eli Zaretskii @ 2022-04-21 0:11 ` Paul Eggert [not found] ` <33fb24fb-282b-cc13-a597-e7b63f19982d@cs.ucla.edu> 1 sibling, 0 replies; 45+ messages in thread From: Paul Eggert @ 2022-04-21 0:11 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-orgmode, manikulin, bug-gnulib, 54764 On 4/20/22 12:30, Eli Zaretskii wrote: > I see the time samples change in jumps of 15 msec. Could you give the first part of the output? I would like to see what the the samples are jumping from and to, and how often they jump. Something like the following is what I'd hope to see from the first lines of the output of 'gllib/test-gettime-res x'. What are you seeing? gettime_res returned 15625000 ns time = 1650496481.515625000 time = 1650496481.531250000 time = 1650496481.546875000 time = 1650496481.562500000 time = 1650496481.578125000 time = 1650496481.593750000 time = 1650496481.609375000 time = 1650496481.625000000 time = 1650496481.640625000 time = 1650496481.656250000 > Which is expected > on MS-Windows, given the scheduler time tick, but what does that have > to do with the system's time resolution? The resolution of Elisp's (time-convert nil t) is determined by the smallest nonzero gap between timestamps that are returned by C's current_timespec. This is the system time resolution as far as Elisp is concerned, because Elisp cannot return the current time at a finer resolution than what current_timespec gives it. This resolution is not necessarily the same as the time resolution of the motherboard clock, OS high-res timestamp, file timestamps, etc. > And how is the 0.625 msec > number reported by the program obtained from those samples? Use the largest resolution R ns consistent with the samples, such that 1000000000 is an integer multiple of R so that timestamp computations are exact. ^ permalink raw reply [flat|nested] 45+ messages in thread
[parent not found: <33fb24fb-282b-cc13-a597-e7b63f19982d@cs.ucla.edu>]
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones [not found] ` <33fb24fb-282b-cc13-a597-e7b63f19982d@cs.ucla.edu> @ 2022-04-21 6:44 ` Eli Zaretskii [not found] ` <83y1zzq6kd.fsf@gnu.org> 1 sibling, 0 replies; 45+ messages in thread From: Eli Zaretskii @ 2022-04-21 6:44 UTC (permalink / raw) To: Paul Eggert; +Cc: emacs-orgmode, manikulin, bug-gnulib, 54764 > Date: Wed, 20 Apr 2022 17:11:34 -0700 > Cc: manikulin@gmail.com, emacs-orgmode@gnu.org, 54764@debbugs.gnu.org, > bug-gnulib@gnu.org > From: Paul Eggert <eggert@cs.ucla.edu> > > On 4/20/22 12:30, Eli Zaretskii wrote: > > > I see the time samples change in jumps of 15 msec. > > Could you give the first part of the output? I would like to see what > the the samples are jumping from and to, and how often they jump. That "first part", as I understand what you wanted, would be too long and tedious to examine, as the value changes once every 5500 lines. So I've modified the test program to print the time only when it changes, and here's the output: gettime_res returned 625000 ns time = 1650522863.038750000 time = 1650522863.054375000 time = 1650522863.070000000 time = 1650522863.085625000 time = 1650522863.101250000 time = 1650522863.116875000 time = 1650522863.132500000 time = 1650522863.148125000 time = 1650522863.163750000 time = 1650522863.179375000 time = 1650522863.195000000 time = 1650522863.210625000 time = 1650522863.226250000 time = 1650522863.241875000 time = 1650522863.257500000 time = 1650522863.273125000 time = 1650522863.288750000 time = 1650522863.304375000 time = 1650522863.320000000 time = 1650522863.335625000 time = 1650522863.351250000 time = 1650522863.366875000 time = 1650522863.382500000 time = 1650522863.398125000 time = 1650522863.413750000 > > Which is expected > > on MS-Windows, given the scheduler time tick, but what does that have > > to do with the system's time resolution? > > The resolution of Elisp's (time-convert nil t) is determined by the > smallest nonzero gap between timestamps that are returned by C's > current_timespec. This is the system time resolution as far as Elisp is > concerned, because Elisp cannot return the current time at a finer > resolution than what current_timespec gives it. This resolution is not > necessarily the same as the time resolution of the motherboard clock, OS > high-res timestamp, file timestamps, etc. Then I think I don't understand the purpose of this measurement, as applied to Emacs Lisp. For example, you say that this is unrelated to file timestamps, but don't we use time values for file timestamps? And for Windows, all this does is measure the "resolution" of the Gnulib emulation of timespec functions on MS-Windows, it tells nothing about the real resolution of the system time values. More generally, if the "time resolution" determined by this procedure is different between two systems, does it mean that two time values that are 'equal' on one of them could be NOT 'equal' on another? And if so, wouldn't that mean Emacs Lisp programs will be inherently not portable? IOW, how do you intend to incorporate this "time resolution" into Emacs Lisp, and what will be affected by that value? ^ permalink raw reply [flat|nested] 45+ messages in thread
[parent not found: <83y1zzq6kd.fsf@gnu.org>]
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones [not found] ` <83y1zzq6kd.fsf@gnu.org> @ 2022-04-21 15:30 ` Max Nikulin 2022-04-21 15:58 ` Eli Zaretskii 2022-04-21 23:56 ` Paul Eggert [not found] ` <aa2bc0a0-1bec-37ff-919d-c20fcdfdab68@cs.ucla.edu> 2 siblings, 1 reply; 45+ messages in thread From: Max Nikulin @ 2022-04-21 15:30 UTC (permalink / raw) To: Eli Zaretskii, Paul Eggert; +Cc: bug-gnulib, 54764 Eli Zaretskii, Wed, 20 Apr 2022 22:30:21 > I see the time samples change in jumps of 15 msec. Which is expected > on MS-Windows, given the scheduler time tick, Why do you expect such value? Clock resolution generally has a little common with timers. Does it mean some protection against timing attacks? I have no experience with windows-specific code, but quick search gives GetSystemTimePreciseAsFileTime that should provide timestamps with fine granularity https://docs.microsoft.com/en-us/windows/win32/api/sysinfoapi/nf-sysinfoapi-getsystemtimepreciseasfiletime On 21/04/2022 13:44, Eli Zaretskii wrote: > > gettime_res returned 625000 ns > time = 1650522863.038750000 > time = 1650522863.054375000 > > Then I think I don't understand the purpose of this measurement, as > applied to Emacs Lisp. For example, you say that this is unrelated to > file timestamps, but don't we use time values for file timestamps? It might mean that Emacs already have some problems unrelated to the suggested patches. > And for Windows, all this does is measure the "resolution" of the > Gnulib emulation of timespec functions on MS-Windows, it tells nothing > about the real resolution of the system time values. > > More generally, if the "time resolution" determined by this procedure > is different between two systems, does it mean that two time values > that are 'equal' on one of them could be NOT 'equal' on another? And > if so, wouldn't that mean Emacs Lisp programs will be inherently not > portable? > > IOW, how do you intend to incorporate this "time resolution" into > Emacs Lisp, and what will be affected by that value? If I got the idea of the patches correctly, it should be a different low level representation for the same numbers. However I do not like too coarse clock resolution reported by the current implementation. P.S. I have removed emacs-orgmode list since this part of discussion is unrelated to Org. ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones 2022-04-21 15:30 ` Max Nikulin @ 2022-04-21 15:58 ` Eli Zaretskii 2022-04-21 17:23 ` Max Nikulin 0 siblings, 1 reply; 45+ messages in thread From: Eli Zaretskii @ 2022-04-21 15:58 UTC (permalink / raw) To: Max Nikulin; +Cc: eggert, bug-gnulib, 54764 > Date: Thu, 21 Apr 2022 22:30:21 +0700 > Cc: bug-gnulib@gnu.org, 54764@debbugs.gnu.org > From: Max Nikulin <manikulin@gmail.com> > > Eli Zaretskii, Wed, 20 Apr 2022 22:30:21 > > I see the time samples change in jumps of 15 msec. Which is expected > > on MS-Windows, given the scheduler time tick, > > Why do you expect such value? It is a well-known fact that the system timer tick happens 64 times a second on MS-Windows. > Clock resolution generally has a little common with timers. I agree, but that's something you should tell/ask Paul, not myself. > I have no experience with windows-specific code, but quick search gives > GetSystemTimePreciseAsFileTime that should provide timestamps with fine > granularity > https://docs.microsoft.com/en-us/windows/win32/api/sysinfoapi/nf-sysinfoapi-getsystemtimepreciseasfiletime That API exists only since Windows 8. More importantly, the Gnulib implementation of current_timespec doesn't use it. ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones 2022-04-21 15:58 ` Eli Zaretskii @ 2022-04-21 17:23 ` Max Nikulin 2022-04-21 18:46 ` Eli Zaretskii 0 siblings, 1 reply; 45+ messages in thread From: Max Nikulin @ 2022-04-21 17:23 UTC (permalink / raw) To: Eli Zaretskii; +Cc: eggert, bug-gnulib, 54764 On 21/04/2022 22:58, Eli Zaretskii wrote: >> Date: Thu, 21 Apr 2022 22:30:21 +0700 >> From: Max Nikulin >> >> Eli Zaretskii, Wed, 20 Apr 2022 22:30:21 >>> I see the time samples change in jumps of 15 msec. Which is expected >>> on MS-Windows, given the scheduler time tick, >> >> Why do you expect such value? > > It is a well-known fact that the system timer tick happens 64 times a > second on MS-Windows. You may notice similar coarse scheduler timer on Linux as well. General purpose PC nowadays may have higher scheduler frequency, but I saw a case when usleep(1) caused 20ms pause while there was no problem to get higher precision form system clock. >> Clock resolution generally has a little common with timers. > > I agree, but that's something you should tell/ask Paul, not myself. It may be deeper, e.g. additional call during initialization or extra privileges may be required. >> I have no experience with windows-specific code, but quick search gives >> GetSystemTimePreciseAsFileTime that should provide timestamps with fine >> granularity >> https://docs.microsoft.com/en-us/windows/win32/api/sysinfoapi/nf-sysinfoapi-getsystemtimepreciseasfiletime > > That API exists only since Windows 8. I suspect that it means just that MS does not support anything prior to Windows 8 any more. XP is mentioned in connection to this function: https://www.lochan.org/2005/keith-cl/useful/win32time.html Writing about possible intentional hiding of high precision clock I was assuming latest windows versions, I believe, even Windows 8 is too old for such modern tricks. > More importantly the Gnulib > implementation of current_timespec doesn't use it. Even if it is not used yet, is it intentional design decision or just an issue that should be fixed? I am unsure what function clock_gettime/timespec_get/gettimeofday is actually used on windows platform and how it is implemented. It seems, only coarse clock is currently available in Emacs on Windows due to usage of current_timespec. Are there any known problem with 16ms resolution? ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones 2022-04-21 17:23 ` Max Nikulin @ 2022-04-21 18:46 ` Eli Zaretskii 0 siblings, 0 replies; 45+ messages in thread From: Eli Zaretskii @ 2022-04-21 18:46 UTC (permalink / raw) To: Max Nikulin; +Cc: eggert, bug-gnulib, 54764 > Date: Fri, 22 Apr 2022 00:23:01 +0700 > Cc: eggert@cs.ucla.edu, bug-gnulib@gnu.org, 54764@debbugs.gnu.org > From: Max Nikulin <manikulin@gmail.com> > > >> Clock resolution generally has a little common with timers. > > > > I agree, but that's something you should tell/ask Paul, not myself. > > It may be deeper, e.g. additional call during initialization or extra > privileges may be required. No, that's not the case here. > >> I have no experience with windows-specific code, but quick search gives > >> GetSystemTimePreciseAsFileTime that should provide timestamps with fine > >> granularity > >> https://docs.microsoft.com/en-us/windows/win32/api/sysinfoapi/nf-sysinfoapi-getsystemtimepreciseasfiletime > > > > That API exists only since Windows 8. > > I suspect that it means just that MS does not support anything prior to > Windows 8 any more. XP is mentioned in connection to this function: > https://www.lochan.org/2005/keith-cl/useful/win32time.html It doesn't matter to us, because we do support older versions. > Writing about possible intentional hiding of high precision clock I was > assuming latest windows versions, I believe, even Windows 8 is too old > for such modern tricks. There's no problem having high-resolution time stamps in Emacs on all versions of Windows since Windows 2000, but before we implement something like that, we should understand the goal and the effects on Lisp programs. Which is why I asked Paul questions that need to be answered before we talk about the implementation. > > More importantly the Gnulib > > implementation of current_timespec doesn't use it. > > Even if it is not used yet, is it intentional design decision or just an > issue that should be fixed? I am unsure what function > clock_gettime/timespec_get/gettimeofday is actually used on windows > platform and how it is implemented. We use gettimeofdate, because the more modern clock_gettime requires linking against pthreads in the MinGW64 builds, and we don't want that additional dependency. But since clock_gettime produces the same 15.6 msec granularity, we don't lose anything by using a slightly older interface. > It seems, only coarse clock is currently available in Emacs on Windows > due to usage of current_timespec. Yes. > Are there any known problem with 16ms resolution? Not that I know of, but please wait for the discussion with Paul to come to its conclusion. ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones [not found] ` <83y1zzq6kd.fsf@gnu.org> 2022-04-21 15:30 ` Max Nikulin @ 2022-04-21 23:56 ` Paul Eggert [not found] ` <aa2bc0a0-1bec-37ff-919d-c20fcdfdab68@cs.ucla.edu> 2 siblings, 0 replies; 45+ messages in thread From: Paul Eggert @ 2022-04-21 23:56 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-orgmode, manikulin, bug-gnulib, 54764 What appears to be happening here is that the MS-Windows native timestamp resolution is 1/64th of a second, and your system's clock is offset by 0.0075 s from an integer boundary. I.e., the timestamps in increasing order are: ... 1650522862 + 62/64 + 0.0075 = 1650522862.976250 1650522862 + 63/64 + 0.0075 = 1650522862.991875 1650522863 + 0/64 + 0.0075 = 1650522863.007500 1650522863 + 1/64 + 0.0075 = 1650522863.023125 1650522863 + 2/64 + 0.0075 = 1650522863.038750 ... and the system clock never returns a timestamp on an integer boundary (i.e., tv_nsec is never zero). We have two options to express this as Emacs timestamps: (1) We can keep information about resolution but lose information about time, by using a resolution of 15.625 ms (i.e., 1/64 s) and truncating timestamps to the nearest 1/64 second. This would generate the following (TICKS . HZ) timestamps: ... (105633463230 . 64) = 1650522862 + 62/64 = 1650522862.968750 (105633463231 . 64) = 1650522862 + 63/64 = 1650522862.984375 (105633463232 . 64) = 1650522863 + 0/64 = 1650522863.000000 (105633463233 . 64) = 1650522863 + 1/64 = 1650522863.015625 (105633463234 . 64) = 1650522863 + 2/64 = 1650522863.031250 ... (2) We can keep information about time but lose information about the resolution, by using a resolution of 0.625 ms (i.e., HZ = 1000000000 / 625000 = 16000). (We use 0.625 ms because it is the coarsest resolution that does not lose time info.) This would generate the following (TICKS . HZ) timestamps: ... (2640836580762 . 1600) = 1650522862 + 1562/1600 = 1650522862.976250 (2640836580762 . 1600) = 1650522862 + 1587/1600 = 1650522862.991875 (2640836580762 . 1600) = 1650522863 + 12/1600 = 1650522863.007500 (2640836580762 . 1600) = 1650522863 + 37/1600 = 1650522863.023125 (2640836580762 . 1600) = 1650522863 + 62/1600 = 1650522863.038750 ... The patch does (2), and this explains the "gettime_res returned 625000 ns" in your output. It shouldn't be hard to change the patch to do (1), if desired. I doubt whether users will care one way or the other. > don't we use time values for file timestamps? Yes, but file timestamps should use the resolution of the file system, which in general is different from the resolution of the system clock. That's a separate matter, which would be the subject of a separate patch. For now we can stick with what we already have in that department. > And for Windows, all this does is measure the "resolution" of the > Gnulib emulation of timespec functions on MS-Windows, it tells nothing > about the real resolution of the system time values. If Emacs Lisp code (which currently is based on the Gnulib code) can see only (say) 1-microsecond timestamps, then it doesn't matter that the underlying system clock has (say) 1-nanosecond precision. Of course it would be better for Emacs to see the system timestamp resolution, and if we can get the time of day on MS-Windows to a precision better than 1/64 second then I assume Emacs should do that. Once it does, the patch should calculate a higher HZ value to tell users about the improved resolution. > if the "time resolution" determined by this procedure > is different between two systems, does it mean that two time values > that are 'equal' on one of them could be NOT 'equal' on another? Sure, but the traditional (HIGH LOW MICROSEC PICOSEC) representation has the same issue. For example, if A's clock has 1 ms resolution and B's clock has 10 ms resolution, A's (time-convert nil 'list) called twice would return (say) the two timestamps (25184 64239 1000 0) and (25184 64239 1001 0) at the same moments that B's calls would return (25184 64239 1000 0) twice. A would say that the two timestamps differ; B would say they're the same. This sort of disagreement is inherent to how timestamp resolution works. It doesn't matter whether the timestamps are represented by (HIGH LOW MICROSEC PICOSEC) or by (TICKS . HZ); users will run into the same problem in both cases. ^ permalink raw reply [flat|nested] 45+ messages in thread
[parent not found: <aa2bc0a0-1bec-37ff-919d-c20fcdfdab68@cs.ucla.edu>]
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones [not found] ` <aa2bc0a0-1bec-37ff-919d-c20fcdfdab68@cs.ucla.edu> @ 2022-04-22 5:01 ` Eli Zaretskii 0 siblings, 0 replies; 45+ messages in thread From: Eli Zaretskii @ 2022-04-22 5:01 UTC (permalink / raw) To: Paul Eggert; +Cc: emacs-orgmode, manikulin, bug-gnulib, 54764 > Date: Thu, 21 Apr 2022 16:56:25 -0700 > Cc: manikulin@gmail.com, emacs-orgmode@gnu.org, 54764@debbugs.gnu.org, > bug-gnulib@gnu.org > From: Paul Eggert <eggert@cs.ucla.edu> > > What appears to be happening here is that the MS-Windows native > timestamp resolution is 1/64th of a second, and your system's clock is > offset by 0.0075 s from an integer boundary. I.e., the timestamps in > increasing order are: > > ... > 1650522862 + 62/64 + 0.0075 = 1650522862.976250 > 1650522862 + 63/64 + 0.0075 = 1650522862.991875 > 1650522863 + 0/64 + 0.0075 = 1650522863.007500 > 1650522863 + 1/64 + 0.0075 = 1650522863.023125 > 1650522863 + 2/64 + 0.0075 = 1650522863.038750 > ... > > and the system clock never returns a timestamp on an integer boundary > (i.e., tv_nsec is never zero). > > We have two options to express this as Emacs timestamps: > > (1) We can keep information about resolution but lose information about > time, by using a resolution of 15.625 ms (i.e., 1/64 s) and truncating > timestamps to the nearest 1/64 second. This would generate the > following (TICKS . HZ) timestamps: > > ... > (105633463230 . 64) = 1650522862 + 62/64 = 1650522862.968750 > (105633463231 . 64) = 1650522862 + 63/64 = 1650522862.984375 > (105633463232 . 64) = 1650522863 + 0/64 = 1650522863.000000 > (105633463233 . 64) = 1650522863 + 1/64 = 1650522863.015625 > (105633463234 . 64) = 1650522863 + 2/64 = 1650522863.031250 > ... > > (2) We can keep information about time but lose information about the > resolution, by using a resolution of 0.625 ms (i.e., HZ = 1000000000 / > 625000 = 16000). (We use 0.625 ms because it is the coarsest resolution > that does not lose time info.) This would generate the following (TICKS > . HZ) timestamps: > > ... > (2640836580762 . 1600) = 1650522862 + 1562/1600 = 1650522862.976250 > (2640836580762 . 1600) = 1650522862 + 1587/1600 = 1650522862.991875 > (2640836580762 . 1600) = 1650522863 + 12/1600 = 1650522863.007500 > (2640836580762 . 1600) = 1650522863 + 37/1600 = 1650522863.023125 > (2640836580762 . 1600) = 1650522863 + 62/1600 = 1650522863.038750 > ... > > The patch does (2), and this explains the "gettime_res returned 625000 > ns" in your output. > > It shouldn't be hard to change the patch to do (1), if desired. I doubt > whether users will care one way or the other. These are very fine details of the implementation, which we can get back to later. I would like first to discuss the more general issue of basing the design on such tests, and on the notion of "clock resolution" as expressed by these tests. TBH, what you propose makes no sense to me for now, and for some reason you didn't answer my more general questions about that, but instead preferred to respond only to their secondary aspects. At this point, I object to any changes in this area until we discuss the more general aspects of this design and decide whether we agree with it. Such a discussion should be on emacs-devel, so I move this there; please continue the discussion there. Thanks. ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones 2022-04-20 18:19 ` Paul Eggert 2022-04-20 18:41 ` Eli Zaretskii @ 2022-04-23 14:35 ` Bernhard Voelker 1 sibling, 0 replies; 45+ messages in thread From: Bernhard Voelker @ 2022-04-23 14:35 UTC (permalink / raw) To: Paul Eggert, Eli Zaretskii; +Cc: Gnulib bugs, manikulin, emacs-orgmode, 54764 On 4/20/22 20:19, Paul Eggert wrote: > diff --git a/lib/gettime-res.c b/lib/gettime-res.c > index 611f83ad27..bb4d0b191d 100644 > --- a/lib/gettime-res.c > +++ b/lib/gettime-res.c > @@ -53,6 +53,8 @@ gettime_res (void) > > long int hz = TIMESPEC_HZ; > long int r = hz * res.tv_sec + res.tv_nsec; > + struct timespec earlier; > + earlier.tv_nsec = -1; > > /* On some platforms, clock_getres (CLOCK_REALTIME, ...) yields a > too-large resolution, under the mistaken theory that it should > @@ -61,9 +63,22 @@ gettime_res (void) > resolution. Work around the problem with high probability by > trying clock_gettime several times and observing the resulting > bounds on resolution. */ > - for (int i = 0; 1 < r && i < 32; i++) > + int nsamples = 32; > + for (int i = 0; 1 < r && i < nsamples; i++) > { > - struct timespec now = current_timespec (); > + /* If successive timestamps disagree the clock resolution must > + be small, so exit the inner loop to check this sample. > + Otherwise, arrange for the outer loop to exit but continue > + the inner-loop search for a differing timestamp sample. */ > + struct timespec now; > + for (;; i = nsamples) > + { > + now = current_timespec (); > + if (earlier.tv_nsec != now.tv_nsec || earlier.tv_sec != now.tv_sec) > + break; > + } > + earlier = now; > + > r = gcd (r, now.tv_nsec ? now.tv_nsec : hz); > } lib/gettime-res.c: In function 'gettime_res': lib/gettime-res.c:77:46: error: 'earlier.tv_sec' may be used uninitialized in this function \ [-Werror=maybe-uninitialized] 77 | if (earlier.tv_nsec != now.tv_nsec || earlier.tv_sec != now.tv_sec) | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ We know that earlier.tv_sec is set when tv_nsec is set, but the compiler does not, obviously. Considering the nested loops, I'd say initializing tv_sec doesn't harm performance-wise. Have a nice day, Berny ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones 2022-04-19 2:02 ` Paul Eggert 2022-04-19 5:50 ` Eli Zaretskii [not found] ` <83tuapvcxs.fsf@gnu.org> @ 2022-04-20 15:07 ` Max Nikulin [not found] ` <25d90a4b-f47d-01b4-2bfe-9951e97fe676@gmail.com> 3 siblings, 0 replies; 45+ messages in thread From: Max Nikulin @ 2022-04-20 15:07 UTC (permalink / raw) To: Paul Eggert; +Cc: emacs-orgmode, 54764 On 19/04/2022 09:02, Paul Eggert wrote: > Here are the main points I see about making timestamps work better in > Org mode, and related patches to how Emacs handles timestamps. > > * Max would like encode-time to treat a list (SS MM HH DD MM YYYY) as if > it were (SS MM HH DD MM YYYY nil -1 nil), Thank you, Paul, it is exactly what I had in mind when I created this bug. > as that would be more > convenient for how Org mode deals with ambiguous local timestamps. This > is relatively easy to add for Emacs 29+, and is done in first of the > attached proposed patches to Emacs master. I would say that Org just does not care concerning ambiguous local time. Anyway there are other similar cases besides DST. > * As I understand it, Max would like a function that emulate Emacs 29 > encode-time with one argument, even if running in Emacs versions back to > Emacs 25. I suppose such a function would also need to implement Emacs > 27+ encode-time's support for subsecond resolution. E.g., > (org-encode-time '((44604411568 . 1000000000) 55 0 19 4 2022 - -1 t)) > should return (1650329744604411568 . 1000000000) even in Emacs 25 and 26. I am just afraid of possibility of recurrent attempts to modernize time-related code within Emacs including Org code in a such way that can not be directly ported to the Org repository. Discrepancy of the code increases maintenance burden. The main purpose of a compatibility wrapper is to prevent grep-driven refactoring. Another point of the helper function is to allow to remove from Emacs support confusing old-style `encode-time' arguments with ignored DST value. In Org timestamps are often built from scratch, so separate argument are still convenient. Org timestamps have minute precision, even seconds are trimmed. So at least explicitly I did not ask for subsecond timestamps. I admit however that they might emerge in some code paths. Notice that nobody from Org developers & maintainers commented the patch demonstrating the idea of such wrapper. > * My last topic in this email is Max's request for a feature that I'm > not planning to put into Emacs 29 as it'll require more thought. This > addresses the problem where your TZ is "Africa/Juba" and you want to > encode a timestamp like "2021-01-31 23:30" which is ambiguous since at > 24:00 that day Juba moved standard time back by an hour. Unfortunately > the underlying C mktime function does not allow disambiguation in the > rare situation where standard time moves further west of Greenwich. > Addressing this problem would require rewriting mktime from scratch in > Elisp, or using heuristics that would occasionally fail, or (my > favorite) extending glibc mktime to treat tm_isdst values other than > -1,0,1 to support disambiguating such timestamps. I do not urge such changes. I have not checked if mktime is a part of POSIX and C standard. If it is so, I am not in favor of adding more values for the tm_isdst field since they are not related to DST. I started this branch of discussion to convince Paul that requirement of 9 fields is not really encourage more correct usage of `encode-time' in comparison to 6 values. More convenient interface for processing of local time moments requires significant amount of work, maybe some prototypes. It should be considered separately from this bug. I still believe that optional DST and ZONE values is an improvement of the `encode-time' interface with no real drawbacks. It minimizes the chance of passing nil as "no DST" when actual value is unknown and developers are not ready to add a bunch of code to determine proper TZ offset for each case of time transition. ^ permalink raw reply [flat|nested] 45+ messages in thread
[parent not found: <25d90a4b-f47d-01b4-2bfe-9951e97fe676@gmail.com>]
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones [not found] ` <25d90a4b-f47d-01b4-2bfe-9951e97fe676@gmail.com> @ 2022-04-20 18:29 ` Paul Eggert 2022-04-25 15:30 ` Max Nikulin 0 siblings, 1 reply; 45+ messages in thread From: Paul Eggert @ 2022-04-20 18:29 UTC (permalink / raw) To: Max Nikulin; +Cc: emacs-orgmode, 54764 On 4/20/22 08:07, Max Nikulin wrote: > I have not checked if mktime is a part of > POSIX and C standard. mktime is part of both the C standard and POSIX. POSIX extends the C standard by saying that time_t is an integer type (the C standard allows time_t to be a floating-point type) and that time_t counts non-leap seconds since the Epoch (the C standard doesn't say what time_t counts, thought it implies that it counts seconds from some origin, which may not be 1970). > I still believe that optional DST and ZONE values is an improvement of > the `encode-time' interface with no real drawbacks. Yes, that's the direction we're headed. ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones 2022-04-20 18:29 ` Paul Eggert @ 2022-04-25 15:30 ` Max Nikulin 2022-04-25 15:37 ` Paul Eggert [not found] ` <4bd57f21-0660-fce0-d796-08c534402340@cs.ucla.edu> 0 siblings, 2 replies; 45+ messages in thread From: Max Nikulin @ 2022-04-25 15:30 UTC (permalink / raw) To: Paul Eggert; +Cc: emacs-orgmode, 54764 On 21/04/2022 01:29, Paul Eggert wrote: > On 4/20/22 08:07, Max Nikulin wrote: > >> I still believe that optional DST and ZONE values is an improvement of >> the `encode-time' interface with no real drawbacks. > > Yes, that's the direction we're headed. Paul, a week has passed since you posted the patch series. Unlike the changes related to timestamp representation, nobody argued concerning 6 element list argument of `encode-time': [PATCH 1/6] Support (encode-time (list s m h D M Y)) Unless I have missed something, this patch can be pushed without other ones and it will be a step forward. ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones 2022-04-25 15:30 ` Max Nikulin @ 2022-04-25 15:37 ` Paul Eggert [not found] ` <4bd57f21-0660-fce0-d796-08c534402340@cs.ucla.edu> 1 sibling, 0 replies; 45+ messages in thread From: Paul Eggert @ 2022-04-25 15:37 UTC (permalink / raw) To: Max Nikulin; +Cc: emacs-orgmode, 54764 Yes, I plan to omit the patches that were objected to, and install the rest. Once that's done you should be good to go for Org. (Alas my workstation died over the weekend, but I should have things up and running again soon...) ^ permalink raw reply [flat|nested] 45+ messages in thread
[parent not found: <4bd57f21-0660-fce0-d796-08c534402340@cs.ucla.edu>]
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones [not found] ` <4bd57f21-0660-fce0-d796-08c534402340@cs.ucla.edu> @ 2022-04-25 19:49 ` Paul Eggert 2022-04-30 11:22 ` Max Nikulin 0 siblings, 1 reply; 45+ messages in thread From: Paul Eggert @ 2022-04-25 19:49 UTC (permalink / raw) To: Max Nikulin; +Cc: 54764-done, emacs-orgmode On 4/25/22 08:37, Paul Eggert wrote: > Yes, I plan to omit the patches that were objected to, and install the > rest. Once that's done you should be good to go for Org. (Alas my > workstation died over the weekend, but I should have things up and > running again soon...) Got my workstation up, installed the patches into Emacs master, and am closing the Emacs bug report. I'll be happy to review the revised org-encode-time implementation, whenever you think it could use a review. (Sorry, I've lost track of what the proposal is.) ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones 2022-04-25 19:49 ` Paul Eggert @ 2022-04-30 11:22 ` Max Nikulin 2022-05-01 2:32 ` Paul Eggert 0 siblings, 1 reply; 45+ messages in thread From: Max Nikulin @ 2022-04-30 11:22 UTC (permalink / raw) To: 54764, eggert; +Cc: orgmode On 26/04/2022 02:49, Paul Eggert wrote: > On 4/25/22 08:37, Paul Eggert wrote: > > I'll be happy to review the revised org-encode-time implementation, > whenever you think it could use a review. (Sorry, I've lost track of > what the proposal is.) I suspended my activity due to discussions of other changes and waiting for commits related to your fixes of `org-parse-time-string' and `org-store-link' that do not require introducing of `org-encode-time-1'. I mean excerpts from https://debbugs.gnu.org/cgi/bugreport.cgi?att=1;msg=10;bug=54764;filename=0001-Improve-Org-usage-of-timestamps.patch I posted a corrected version of my `org-encode-time' macro, but I did not add you to Cc (I sent reply through news.gmane.io), and it has no special case to check whether `encode-time' supports 6 elements list argument: Max Nikulin to emacs-orgmode. [DRAFT][PATCH v2] org-encode-time compatibility and convenience helper. Sun, 24 Apr 2022 18:34:40 +0700. https://list.orgmode.org/t43cki$ct$1@ciao.gmane.io In my drafts I have the following changes in tests related to `org-parse-time-string': diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el index 6aecc3af8..551d17d64 100644 --- a/testing/lisp/test-org.el +++ b/testing/lisp/test-org.el @@ -268,15 +268,15 @@ (ert-deftest test-org/org-parse-time-string () "Test `org-parse-time-string'." (should (equal (org-parse-time-string "2012-03-29 16:40") - '(0 40 16 29 3 2012 nil nil nil))) + '(0 40 16 29 3 2012 nil -1 nil))) (should (equal (org-parse-time-string "[2012-03-29 16:40]") - '(0 40 16 29 3 2012 nil nil nil))) + '(0 40 16 29 3 2012 nil -1 nil))) (should (equal (org-parse-time-string "<2012-03-29 16:40>") - '(0 40 16 29 3 2012 nil nil nil))) + '(0 40 16 29 3 2012 nil -1 nil))) (should (equal (org-parse-time-string "<2012-03-29>") - '(0 0 0 29 3 2012 nil nil nil))) + '(0 0 0 29 3 2012 nil -1 nil))) (should (equal (org-parse-time-string "<2012-03-29>" t) - '(0 nil nil 29 3 2012 nil nil nil)))) + '(0 nil nil 29 3 2012 nil -1 nil)))) (ert-deftest test-org/closest-date () "Test `org-closest-date' specifications." ^ permalink raw reply related [flat|nested] 45+ messages in thread
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones 2022-04-30 11:22 ` Max Nikulin @ 2022-05-01 2:32 ` Paul Eggert 2022-05-01 17:15 ` Max Nikulin 0 siblings, 1 reply; 45+ messages in thread From: Paul Eggert @ 2022-05-01 2:32 UTC (permalink / raw) To: Max Nikulin; +Cc: orgmode, 54764 [-- Attachment #1: Type: text/plain, Size: 826 bytes --] On 4/30/22 04:22, Max Nikulin wrote: > > I posted a corrected version of my `org-encode-time' macro, but I did > not add you to Cc (I sent reply through news.gmane.io), and it has no > special case to check whether `encode-time' supports 6 elements list > argument: Thanks, I looked at that and have a couple of questions. As I understand it, org-encode-time is intended to be a compatibility function like org-newline-and-indent and org-string-distance. Those are in org-compat.el, so I assumed org-encode-time would be there too. Also, if the intent is to emulate Emacs 29 encode-time, can't we do something like the attached instead? The idea is to implement Emacs 29 encode-time both on pre-29 Emacs (that don't support lists of length 6) and post-29 Emacs (which might drop support for the obsolescent form). [-- Attachment #2: 0001-org-encode-time-compatibility-function.patch --] [-- Type: text/x-patch, Size: 1213 bytes --] From 2f44ee7524e5b2e53f912cff1276f7817495c657 Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Sat, 30 Apr 2022 19:27:15 -0700 Subject: [PATCH] org-encode-time compatibility function * lisp/org/org-compat.el (org-encode-time): New function. --- lisp/org/org-compat.el | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lisp/org/org-compat.el b/lisp/org/org-compat.el index 3e394fbab1..0a0025fa0d 100644 --- a/lisp/org/org-compat.el +++ b/lisp/org/org-compat.el @@ -144,6 +144,16 @@ org-file-has-changed-p--hash-table (defun org-time-convert-to-list (time) (seconds-to-time (float-time time)))) +(if (ignore-errors (encode-time '(0 0 0 1 1 1971))) + (if (ignore-errors (encode-time 0 0 0 1 1 1971)) + (defalias 'org-encode-time #'encode-time) + (defun org-encode-time (time &rest args) + (encode-time (if args (cons time args) time)))) + (defun org-encode-time (time &rest args) + (if args + (apply #'encode-time time args) + (apply #'encode-time time)))) + ;; `newline-and-indent' did not take a numeric argument before 27.1. (if (version< emacs-version "27") (defsubst org-newline-and-indent (&optional _arg) -- 2.34.1 ^ permalink raw reply related [flat|nested] 45+ messages in thread
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones 2022-05-01 2:32 ` Paul Eggert @ 2022-05-01 17:15 ` Max Nikulin 0 siblings, 0 replies; 45+ messages in thread From: Max Nikulin @ 2022-05-01 17:15 UTC (permalink / raw) To: Paul Eggert; +Cc: orgmode, 54764 On 01/05/2022 09:32, Paul Eggert wrote: > On 4/30/22 04:22, Max Nikulin wrote: >> >> I posted a corrected version of my `org-encode-time' macro, but I did >> not add you to Cc (I sent reply through news.gmane.io), and it has no >> special case to check whether `encode-time' supports 6 elements list >> argument: > As I understand it, org-encode-time is intended to be a compatibility > function like org-newline-and-indent and org-string-distance. Those are > in org-compat.el, so I assumed org-encode-time would be there too. Maybe you are right. I believe that it should do a bit more than just ensure compatibility. An additional goal is to avoid pitfall with list vs. separate arguments result discrepancy: (format-time-string "%F %T %z %Z" (encode-time 0 30 23 31 3 2022 nil nil "Europe/Madrid") "Europe/Madrid") "2022-03-31 23:30:00 +0200 CEST" (format-time-string "%F %T %z %Z" (encode-time '(0 30 23 31 3 2022 nil nil "Europe/Madrid")) "Europe/Madrid") "2022-04-01 00:30:00 +0200 CEST" > Also, if the intent is to emulate Emacs 29 encode-time, can't we do > something like the attached instead? The idea is to implement Emacs 29 > encode-time both on pre-29 Emacs (that don't support lists of length 6) > and post-29 Emacs (which might drop support for the obsolescent form). > +(if (ignore-errors (encode-time '(0 0 0 1 1 1971))) > + (if (ignore-errors (encode-time 0 0 0 1 1 1971)) > + (defalias 'org-encode-time #'encode-time) > + (defun org-encode-time (time &rest args) > + (encode-time (if args (cons time args) time)))) > + (defun org-encode-time (time &rest args) > + (if args > + (apply #'encode-time time args) > + (apply #'encode-time time)))) 1. I would prefer macro since it works at compile or load time, so runtime impact is minimal. 2. Your variant may be fixed, but currently I do not like behavior for Emacs-27. Compare encode-time and org-encode-time with new calling style: (format-time-string "%F %T %z %Z" (encode-time '(0 30 23 31 3 2022 nil nil "Europe/Madrid")) "Europe/Madrid") "2022-04-01 00:30:00 +0200 CEST" (format-time-string "%F %T %z %Z" (org-encode-time '(0 30 23 31 3 2022 nil nil "Europe/Madrid")) "Europe/Madrid") "2022-03-31 23:30:00 +0200 CEST" ^ permalink raw reply [flat|nested] 45+ messages in thread
[parent not found: <507cc0a2-d2d9-4283-7cc2-0da3c6510ac8@cs.ucla.edu>]
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones [not found] ` <507cc0a2-d2d9-4283-7cc2-0da3c6510ac8@cs.ucla.edu> @ 2022-04-21 16:59 ` Max Nikulin 0 siblings, 0 replies; 45+ messages in thread From: Max Nikulin @ 2022-04-21 16:59 UTC (permalink / raw) To: Paul Eggert; +Cc: emacs-orgmode, 54764 On 17/04/2022 02:23, Paul Eggert wrote: > On 4/15/22 10:23, Max Nikulin wrote: > >> if you are storing future events bound to wall time then namely time >> zone identifier should have precedence. > > Although that would make sense for some applications it's not a good > idea in general. For example, if you're scheduling a Zoom meeting you > should save both, because other meeting participants may interpret > strings like "Pacific/Apia" differently. I would say that in such cases there is a primary time zone for such event and secondary time zones of other participants. Time transitions in the primary time zone (unless it is UTC that is the reference) affect participants from all other time zones. If some secondary time zone is changed then it affects only wall time in this particular time zone. So primary timezone and offsets in all time zones should be stored for user convenience and to figure out which notification should be sent after introducing new rules for some time zones. >> Just identifier may be ambiguous around DST transition. So timezone >> abbreviations are ambiguous per se but when identifiers are known they >> may be still necessary to resolve uncertainties for backward time >> shifts. At certain moment the Olson DB started to use "+04" >> abbreviations instead of letters for transitions unrelated to daylight >> saving time. > > Yes, timezone names like "Europe/Lisbon" are ambiguous during fallback > transitions, or if the meaning of "Europe/Lisbon" changes. This is why > one should also save UT offsets when generating localtime timestamps. Before/after time transition around the date may be more meaningful for people. Local tradition may use other reference than Greenwich. > Around five years ago I changed TZDB to numeric use time zone > abbreviations like "+04" instead of my inventions like "GET", because I > wanted TZDB to follow existing practice, not invent it. A nice side > effect is that numeric abbreviations are unambiguous. (Besides, even _I_ > couldn't remember what "GET" meant. :-) Numerical abbreviation broke parsers in stable linux distribution, e.g. a patch for Qt required in addition to tzdata update. I do not remember details, but removed old-style abbreviations caused some problems as well. I may be wrong concerning usage of such abbreviation in the postgres parser and earlier generated text timestamps. On the other hand an abbreviation for a timezone with evolved offset significantly contributes to uncertainties and does not help to resolve ambiguity around time shift. >> And WET/WEST gets another bit of info in addition to numerical offset. > > That info is meant only for users; I wouldn't rely on it for > calculations because those abbreviations are ambiguous. It could well > be, for example that the meaning of "PST" in the United States will > change in the near future. I agree that abbreviations are ambiguous when considered globally. When constrained to particular location (time zone) and moment of time, they may provide additional bit of information that is more convenient for users and enough to resolve ambiguity. It is not a general rule, sometimes uncertainty remains even when abbreviation is known. >> I do not remember if it is possible at all to obtain using libc the >> period of constant time offset, when time shift value is valid. >> Sometimes it is necessary to recalculate offset. > > Sorry, I don't understand this point. One can easily recalculate the UT > offset in Emacs Lisp by generating the desired timestamp and calling > decode-time on it. You surely are talking about something else, but I > don't know what it is. Let's assume Europe/Lisbon timezone. `decode-time' for today gives just +0100. It tells nothing if I need to process some thousands of timestamps for yesterday and past week. If some function returns "+0100 for given timestamp and the same offset is valid for Europe/Lisbon since Sun Mar 27 01:00:00 2022 UT = Sun Mar 27 02:00:00 2022 WEST till Sun Oct 30 00:59:59 2022 UT = Sun Oct 30 01:59:59 2022 WEST" then I can process whole bunch without any worry concerning non-existing or ambiguous time, extra or lost hour in time intervals. mktime should have all this information during intermediate calculations but it does not expose it to callers. Interface of mktime is suitable for conversion of isolated timestamps. For bunch of related data there is enough room for optimizing. >> You wrote that "2021-01-31 23:30:00 +0300" is parsed correctly. My >> opinion is that when time zone is known to be Africa/Juba (system-wide >> setting, environment variable, or an argument of the parsing function) >> then "2021-01-31 23:30:00 CAT" and "2021-01-31 23:30:00 EAT" should be >> parsed correctly (and localized date-time formats should be parsed as >> well). > > That's not possible in general, since the two abbreviations can be the > same. Traditionally in Australia, for example, "CST" meant both "Central > Standard Time" and "Central Summer Time", and there are probably still > apps that use the equivalent of TZ="CST-9:30CST,M10.1.0,M4.1.0/3" which > does precisely that. They should still have some way to disambiguate whether local time precedes transition or follows it in various schedules: night trains, buses, flights. However it might be just "*" and a footnote. > It's hardly ever a good idea to rely on time zone abbreviations as > they're too often ambiguous. It's much better to use UT offsets. When > generating a localtime timestamp, one should always output its UT offset > (in addition to any other advisory info you might want to output). And > if you do that, many of the abovementioned problems are easily solved. There is no general rule suitable for all cases. In some cases it is more convenient to store timestamps as seconds since epoch. However there are cases when it is fragile: dates without time (e.g. birth date in documents, not for astrology) or future events. Actually input data should be clearly marked to distinguish from guessed or derived values. If wall time is what exactly known then UTC offset is secondary data. When presented to users, UTC offset may sometimes add unnecessary noise with no real value. I do not dispute that UTC offset is important, I am just trying to say that sometimes it may be inconvenient in usage. To build agenda view aside from DST and over transitions and assuming no travel across time zones, all calculations may be performed without inspecting of UTC offset. ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones [not found] ` <149de00f-115b-5367-414f-c7700ef8966b@cs.ucla.edu> 2022-04-09 11:36 ` Max Nikulin 2022-04-13 14:40 ` Max Nikulin @ 2022-04-13 15:12 ` Max Nikulin 2022-04-16 16:26 ` Max Nikulin [not found] ` <2d57e59b-f971-483b-ad65-e0c5ff7883e8@gmail.com> 4 siblings, 0 replies; 45+ messages in thread From: Max Nikulin @ 2022-04-13 15:12 UTC (permalink / raw) To: Paul Eggert, 54764; +Cc: emacs-orgmode On 09/04/2022 14:52, Paul Eggert wrote: > 0001-Improve-Org-usage-of-timestamps.patch > > From 094345e10ad45e06f7b32e2f8017592210f43463 Mon Sep 17 00:00:00 2001 > From: Paul Eggert <eggert@cs.ucla.edu> > Date: Sat, 9 Apr 2022 00:17:09 -0700 > Subject: [PATCH] Improve Org usage of timestamps > MIME-Version: 1.0 > Content-Type: text/plain; charset=UTF-8 > Content-Transfer-Encoding: 8bit > > The main thing is to follow the (encode-time X) convention where > X’s DST component of nil means standard time, -1 means unknown. I am marking this thread as a patch for https://updates.orgmode.org "Use -1 (guess) instead of nil (no) for DST value in encode-time arguments" From my point of view the changes quoted below are most important ones in the patch and they should be committed to the Org repository. Preferably a unit test for time zones should be added for `org-parse-time-string'. I omitted changes to improve code style, they should be applied as well, I just consider them as having less priority. I am in doubt concerning `org-encode-time-1' part. > diff --git a/lisp/org/ol.el b/lisp/org/ol.el > index a03d85f618..fe6e97e928 100644 > --- a/lisp/org/ol.el > +++ b/lisp/org/ol.el > @@ -1575,9 +1575,7 @@ org-store-link > (setq link > (format-time-string > (car org-time-stamp-formats) > - (apply 'encode-time > - (list 0 0 0 (nth 1 cd) (nth 0 cd) (nth 2 cd) > - nil nil nil)))) > + (encode-time 0 0 0 (nth 1 cd) (nth 0 cd) (nth 2 cd)))) > (org-link-store-props :type "calendar" :date cd))) > > ((eq major-mode 'w3-mode) It allows to avoid a pitfall with nil as DST value. > diff --git a/lisp/org/org-macs.el b/lisp/org/org-macs.el > index b10725bd52..0916da89ac 100644 > --- a/lisp/org/org-macs.el > +++ b/lisp/org/org-macs.el > @@ -1242,7 +1242,7 @@ org-parse-time-string > (string-to-number (match-string 4 s)) > (string-to-number (match-string 3 s)) > (string-to-number (match-string 2 s)) > - nil nil nil)) > + nil -1 nil)) It definitely must be changed in the Org code. > diff --git a/lisp/org/org.el b/lisp/org/org.el > index d656a51591..1bceb0f53a 100644 > --- a/lisp/org/org.el > +++ b/lisp/org/org.el > @@ -14334,7 +14334,7 @@ org-read-date-analyze > (setq year (nth 5 org-defdecode)) > (setq org-read-date-analyze-forced-year t)))) > (setq org-read-date-analyze-futurep futurep) > - (list second minute hour day month year))) > + (list second minute hour day month year nil -1 nil))) > > (defvar parse-time-weekdays) > (defun org-read-date-get-relative (s today default) Unsure concerning adding extra elements. I would prefer change of `encode-time' in future and a compatibility macro as in the following for a while. Max Nikulin [DRAFT][PATCH] org-encode-time compatibility and convenience helper. Mon, 11 Apr 2022 22:22:48 +0700. https://lsit.orgmode.org/7f4ea652-7d22-fb61-f873-5e92f078c9e6@gmail.com ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones [not found] ` <149de00f-115b-5367-414f-c7700ef8966b@cs.ucla.edu> ` (2 preceding siblings ...) 2022-04-13 15:12 ` Max Nikulin @ 2022-04-16 16:26 ` Max Nikulin [not found] ` <2d57e59b-f971-483b-ad65-e0c5ff7883e8@gmail.com> 4 siblings, 0 replies; 45+ messages in thread From: Max Nikulin @ 2022-04-16 16:26 UTC (permalink / raw) To: Paul Eggert, 54764; +Cc: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 501 bytes --] On 09/04/2022 14:52, Paul Eggert wrote: > On 4/7/22 05:37, Max Nikulin wrote: > >> Daylight saving time field matters only as a list component and >> ignored as a separate argument (by the way, it should be stressed in >> the docstring). > > Do you have a wording suggestion? (The doc string already covers the > topic concisely; however, conciseness is not always a virtue. :-) Feel free to shorten the added fragment, to change the wording, or to use your variant instead. See the attachment. [-- Attachment #2: 0001-Stress-difference-of-new-and-old-ways-to-call-encode.patch --] [-- Type: text/x-patch, Size: 3304 bytes --] From 42a20494e9f1461f7166c922452028548796bd16 Mon Sep 17 00:00:00 2001 From: Max Nikulin <manikulin@gmail.com> Date: Sat, 16 Apr 2022 23:19:10 +0700 Subject: [PATCH] Stress difference of new and old ways to call `encode-time' * doc/lispref/os.texi (Time Conversion): Add a warning that blind changing of code calling `encode-time' to use single list instead of multiple values may cause deferred bugs since it is common to use nil for ignored arguments such as DST in the old calling convention. * src/timefns.c (encode-time): Mention the warning added to the elisp reference in the docstring. Refactoring related to `encode-time' caused (bug#54731), so it is better to make apparent the difference between the recommended and the obsolescent ways to call the function. More details concerning the purpose and limitations of the DST field are added after discussion with Paul Eggert in (bug#54764). --- doc/lispref/os.texi | 19 +++++++++++++++++++ src/timefns.c | 4 ++++ 2 files changed, 23 insertions(+) diff --git a/doc/lispref/os.texi b/doc/lispref/os.texi index 4ee893f860..b7b41f97ab 100644 --- a/doc/lispref/os.texi +++ b/doc/lispref/os.texi @@ -1707,6 +1707,25 @@ the latter to the former as follows: You can perform simple date arithmetic by using out-of-range values for @var{seconds}, @var{minutes}, @var{hour}, @var{day}, and @var{month}; for example, day 0 means the day preceding the given month. + +The old and the new styles to call @code{encode-time} with the same +values of time fields may give different results. While modernizing +code that uses obsolescent calling convention, ensure that the list +argument contains 9 elements. Pay special attention that the @code{dst} +field does not use @code{nil} expecting that actual value will be +guessed, pass @samp{-1} instead. During normalizing of values to +correct state of daylight saving time users may get time shift and even +wrong date. It may take months to discover such problem. When +called with multiple arguments, the function ignores equivalent of the +@code{dst} value and @samp{-1} is effectively used. The new way to call +@code{encode-time} has an advantage that it is possible to resolve +ambiguity around backward time shift by passing @code{nil} or @code{t}. +Unfortunately there are enough cases across the world when a particular +area is moved to another time zone with no change of daylight saving +time state. @code{encode-time} may signal an error in response to +@code{t} passed as @code{dst}. You have to pass @code{zone} explicitly +as time offset in such case if default ambiguity resolution is not +acceptable. @end defun @node Time Parsing diff --git a/src/timefns.c b/src/timefns.c index b061be0a78..9af89a512d 100644 --- a/src/timefns.c +++ b/src/timefns.c @@ -1631,6 +1631,10 @@ convention, DST and ZONE default to -1 and nil respectively. Years before 1970 are not guaranteed to work. On some systems, year values as low as 1901 do work. +See Info node `(elisp)Time Conversion' for description of a pitfall +that can be faced during migration from the obsolescent to the new +calling convention due to unconscious usage of nil for the DST argument. + usage: (encode-time TIME &rest OBSOLESCENT-ARGUMENTS) */) (ptrdiff_t nargs, Lisp_Object *args) { -- 2.25.1 ^ permalink raw reply related [flat|nested] 45+ messages in thread
[parent not found: <2d57e59b-f971-483b-ad65-e0c5ff7883e8@gmail.com>]
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones [not found] ` <2d57e59b-f971-483b-ad65-e0c5ff7883e8@gmail.com> @ 2022-04-17 1:58 ` Paul Eggert [not found] ` <3624beb8-71fd-924e-a065-74d0034ed351@cs.ucla.edu> 1 sibling, 0 replies; 45+ messages in thread From: Paul Eggert @ 2022-04-17 1:58 UTC (permalink / raw) To: Max Nikulin; +Cc: emacs-orgmode, 54764 [-- Attachment #1: Type: text/plain, Size: 375 bytes --] On 4/16/22 09:26, Max Nikulin wrote: > Feel free to shorten the added fragment, to change the wording, or to > use your variant instead. See the attachment. Thanks, I installed that and then installed the attached, which merges that with some documentation improvements that I drafted based on this thread. It is a messy area but I hope the documentation is clearer now. [-- Attachment #2: 0001-Document-encode-time-caveats.patch --] [-- Type: text/x-patch, Size: 13979 bytes --] From f1ba92448d1e573640547c68d9bed89fe5c43da0 Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Sat, 16 Apr 2022 18:48:51 -0700 Subject: [PATCH] Document encode-time caveats MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * doc/lispref/os.texi (Time of Day, Time Conversion): Move the warnings about DST being -1 to closer to where DST is discussed, and reword and improve the discussions and warnings. Be more precise about years before 1969 (possible west of UTC) vs the Epoch. Mention some problems due to leap seconds, leap years, daylight saving transitions, and time zone changes. Modernize discussion of OS timestamp range. Prefer secular ‘BCE’ to religious ‘BC’. Omit discussion of decoded-time-add and make-decoded-time, as they are in a library and are not always available; instead, mention the library. Warn about common mistakes when doing simple date arithmetic. * src/timefns.c (Fencode_time): In doc string, mention date arithmetic and tighten up the wording a bit. --- doc/lispref/os.texi | 153 +++++++++++++++++++------------------------- src/timefns.c | 16 ++--- 2 files changed, 73 insertions(+), 96 deletions(-) diff --git a/doc/lispref/os.texi b/doc/lispref/os.texi index 66689f43a9..8366689640 100644 --- a/doc/lispref/os.texi +++ b/doc/lispref/os.texi @@ -1303,10 +1303,16 @@ Time of Day @cindex Lisp timestamp @cindex timestamp, Lisp +@cindex Coordinated Universal Time +@cindex Universal Time +@cindex UTC +@cindex leap seconds Many functions like @code{current-time} and @code{file-attributes} return @dfn{Lisp timestamp} values that count seconds, and that can represent absolute time by counting seconds since the @dfn{epoch} of -1970-01-01 00:00:00 UTC. +1970-01-01 00:00:00 UTC (Coordinated Universal Time). Typically these +counts ignore leap seconds; however, GNU and some other operating +systems can be configured to count leap seconds. Although traditionally Lisp timestamps were integer pairs, their form has evolved and programs ordinarily should not depend on the @@ -1367,8 +1373,8 @@ Time of Day Some of these conversions rely on operating system functions that limit the range of possible time values, and signal an error such as @samp{"Specified time is not representable"} if the -limits are exceeded. For instance, a system may not support years -before 1970, or years before 1901, or years far in the future. +limits are exceeded. For instance, a system might not support +timestamps before the epoch, or years far in the future. You can convert a time value into a human-readable string using @code{format-time-string}, into a Lisp timestamp using @code{time-convert}, and into other forms using @@ -1434,11 +1440,11 @@ Time Zone Rules which is a platform-dependent default time zone. The set of supported @env{TZ} strings is system-dependent. GNU and -many other systems support the tzdata database, e.g., +many other systems support TZDB timezones, e.g., @samp{"America/New_York"} specifies the time zone and daylight saving time history for locations near New York City. GNU and most other systems support POSIX-style @env{TZ} strings, e.g., -@samp{"EST+5EDT,M4.1.0/2,M10.5.0/2"} specifies the rules used in New +@samp{"EST5EDT,M4.1.0,M10.5.0"} specifies the rules used in New York from 1987 through 2006. All systems support the string @samp{"UTC0"} meaning Universal Time. @@ -1490,18 +1496,20 @@ Time Conversion These functions convert time values (@pxref{Time of Day}) to Lisp timestamps, or into calendrical information and vice versa. - Many 32-bit operating systems are limited to system times containing -32 bits of information in their seconds component; these systems -typically handle only the times from 1901-12-13 20:45:52 through -2038-01-19 03:14:07 Universal Time. However, 64-bit and some 32-bit operating -systems have larger seconds components, and can represent times far in -the past or future. - - Calendrical conversion functions always use the Gregorian calendar, even -for dates before the Gregorian calendar was introduced. Year numbers -count the number of years since the year 1 BC, and do not skip zero + Many operating systems use 64-bit signed integers to count seconds, +and can represent times far in the past or future. However, some are +more limited. For example, old-fashioned operating systems that use +32-bit signed integers typically handle only times from 1901-12-13 +20:45:52 through 2038-01-19 03:14:07 Universal Time. + + Calendrical conversion functions use the Gregorian calendar even for +dates before the Gregorian calendar was introduced, and for dates in +the far distant past or future for which the Gregorian calendar +is wildly inaccurate and disagrees with common practice in scientific fields +like astronomy and paleontology, which use Julian-calendar year lengths. +Year numbers count since the year 1 BCE, and do not skip zero as traditional Gregorian years do; for example, the year number -@minus{}37 represents the Gregorian year 38 BC@. +@minus{}37 represents the Gregorian year 38 BCE@. @defun time-convert time &optional form This function converts a time value into a Lisp timestamp. @@ -1620,53 +1628,6 @@ Time Conversion @code{decoded-time-month}, @code{decoded-time-year}, @code{decoded-time-weekday}, @code{decoded-time-dst} and @code{decoded-time-zone} accessors can be used. - -For instance, to increase the year in a decoded time, you could say: - -@lisp -(setf (decoded-time-year decoded-time) - (+ (decoded-time-year decoded-time) 4)) -@end lisp - -Also see the following function. - -@end defun - -@defun decoded-time-add time delta -This function takes a decoded time structure and adds @var{delta} -(also a decoded time structure) to it. Elements in @var{delta} that -are @code{nil} are ignored. - -For instance, if you want ``same time next month'', you -could say: - -@lisp -(let ((time (decode-time nil nil t)) - (delta (make-decoded-time :month 2))) - (encode-time (decoded-time-add time delta))) -@end lisp - -If this date doesn't exist (if you're running this on January 31st, -for instance), then the date will be shifted back until you get a -valid date (which will be February 28th or 29th, depending). - -Fields are added in a most to least significant order, so if the -adjustment described above happens, it happens before adding days, -hours, minutes or seconds. - -The values in @var{delta} can be negative to subtract values instead. - -The return value is a decoded time structure. -@end defun - -@defun make-decoded-time &key second minute hour day month year dst zone -Return a decoded time structure with only the given keywords filled -out, leaving the rest @code{nil}. For instance, to get a structure -that represents ``two months'', you could say: - -@lisp -(make-decoded-time :month 2) -@end lisp @end defun @defun encode-time time &rest obsolescent-arguments @@ -1676,9 +1637,21 @@ Time Conversion Ordinarily the first argument is a list @code{(@var{second} @var{minute} @var{hour} @var{day} @var{month} @var{year} @var{ignored} @var{dst} @var{zone})} that specifies a -decoded time in the style of @code{decode-time}, so that -@code{(encode-time (decode-time ...))} works. For the meanings of -these list members, see the table under @code{decode-time}. +decoded time in the style of @code{decode-time}. For the meanings of +these list elements, see the table under @code{decode-time}. +In particular, @var{dst} says how to interpret timestamps during a +daylight saving fallback when timestamps are repeated. +If @var{dst} is @minus{}1, the DST value is guessed; if it +is @code{t} or @code{nil} the timestamp with that DST value +is returned, with an error signaled if no such timestamp exists. +Unfortunately a @var{dst} value of @code{t} or @code{nil} does not +disambiguate timestamps duplicated when a TZDB-based timezone moves +further west of Greenwich, such as disambiguating the two +standard-time timestamps 2020-12-27 01:30 when @var{zone} is +@samp{"Europe/Volgograd"}, which at 02:00 that day changed +standard time from 4 to 3 hours east of Greenwich; if you need to +handle situations like this you can use a numeric @var{zone} to +disambiguate instead. As an obsolescent calling convention, this function can be given six or more arguments. The first six arguments @var{second}, @@ -1687,14 +1660,18 @@ Time Conversion than six arguments the @emph{last} argument is used as @var{zone} and any other extra arguments are ignored, so that @code{(apply #'encode-time (decode-time ...))} works. In this obsolescent -convention, @var{zone} defaults to the current time zone rule -(@pxref{Time Zone Rules}), and @var{dst} is treated as if it was -@minus{}1. +convention, @var{dst} is @minus{}1 and @var{zone} defaults to the +current time zone rule (@pxref{Time Zone Rules}). +When modernizing an obsolescent caller, ensure that the more-modern +list equivalent contains 9 elements with a a @code{dst} element that +is @minus{}1, not @code{nil}. Year numbers less than 100 are not treated specially. If you want them to stand for years above 1900, or years above 2000, you must alter them yourself before you call @code{encode-time}. The operating system limits the range of time and zone values. +However, timestamps ranging from the epoch to the near future are +always supported. The @code{encode-time} function acts as a rough inverse to @code{decode-time}. For example, you can pass the output of @@ -1707,25 +1684,27 @@ Time Conversion You can perform simple date arithmetic by using out-of-range values for @var{seconds}, @var{minutes}, @var{hour}, @var{day}, and @var{month}; for example, day 0 means the day preceding the given month. +Take care when doing so, as it is common for this to fail in some cases. +For example: + +@lisp +;; Try to compute the time four years from now. +;; Watch out; this might not work as expected. +(let ((time (decode-time))) + (setf (decoded-time-year time) + (+ (decoded-time-year time) 4)) + time) +@end lisp -The old and the new styles to call @code{encode-time} with the same -values of time fields may give different results. While modernizing -code that uses obsolescent calling convention, ensure that the list -argument contains 9 elements. Pay special attention that the @code{dst} -field does not use @code{nil} expecting that actual value will be -guessed, pass @samp{-1} instead. During normalizing of values to -correct state of daylight saving time users may get time shift and even -wrong date. It may take months to discover such problem. When -called with multiple arguments, the function ignores equivalent of the -@code{dst} value and @samp{-1} is effectively used. The new way to call -@code{encode-time} has an advantage that it is possible to resolve -ambiguity around backward time shift by passing @code{nil} or @code{t}. -Unfortunately there are enough cases across the world when a particular -area is moved to another time zone with no change of daylight saving -time state. @code{encode-time} may signal an error in response to -@code{t} passed as @code{dst}. You have to pass @code{zone} explicitly -as time offset in such case if default ambiguity resolution is not -acceptable. +@noindent +Unfortunately, this code might not work as expected if the resulting +time is invalid due to daylight saving transitions, time zone changes, +or missing leap days or leap seconds. For example, if executed on +February 29, 2096 this code yields a nonexistent date because 2100 is +not a leap year. To avoid some (though not all) of the problem, you +can base calculations on the middle of the affected unit, e.g., start +at July 1 when adding years. Alternatively, you can use the +@file{calendar} and @file{time-date} libraries. @end defun @node Time Parsing diff --git a/src/timefns.c b/src/timefns.c index 9af89a512d..7a4a7075ed 100644 --- a/src/timefns.c +++ b/src/timefns.c @@ -1609,11 +1609,11 @@ check_tm_member (Lisp_Object obj, int offset) DEFUN ("encode-time", Fencode_time, Sencode_time, 1, MANY, 0, doc: /* Convert TIME to a timestamp. -TIME is a list (SECOND MINUTE HOUR DAY MONTH YEAR IGNORED DST ZONE). +TIME is a list (SECOND MINUTE HOUR DAY MONTH YEAR IGNORED DST ZONE) in the style of `decode-time', so that (encode-time (decode-time ...)) works. In this list, ZONE can be nil for Emacs local time, t for Universal Time, `wall' for system wall clock time, or a string as in the TZ -environment variable. It can also be a list (as from +environment variable. ZONE can also be a list (as from `current-time-zone') or an integer (as from `decode-time') applied without consideration for daylight saving time. If ZONE specifies a time zone with daylight-saving transitions, DST is t for daylight @@ -1626,14 +1626,12 @@ DEFUN ("encode-time", Fencode_time, Sencode_time, 1, MANY, 0, If there are more than 6 arguments the *last* argument is used as ZONE and any other extra arguments are ignored, so that (apply #\\='encode-time (decode-time ...)) works. In this obsolescent -convention, DST and ZONE default to -1 and nil respectively. +convention, DST is -1 and ZONE defaults to nil. -Years before 1970 are not guaranteed to work. On some systems, -year values as low as 1901 do work. - -See Info node `(elisp)Time Conversion' for description of a pitfall -that can be faced during migration from the obsolescent to the new -calling convention due to unconscious usage of nil for the DST argument. +The range of supported years is at least 1970 to the near future. +Out-of-range values for SECOND through MONTH are brought into range +via date arithmetic. This can be tricky especially when combined with +DST; see Info node `(elisp)Time Conversion' for details and caveats. usage: (encode-time TIME &rest OBSOLESCENT-ARGUMENTS) */) (ptrdiff_t nargs, Lisp_Object *args) -- 2.32.0 ^ permalink raw reply related [flat|nested] 45+ messages in thread
[parent not found: <3624beb8-71fd-924e-a065-74d0034ed351@cs.ucla.edu>]
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones [not found] ` <3624beb8-71fd-924e-a065-74d0034ed351@cs.ucla.edu> @ 2022-04-20 16:56 ` Max Nikulin 2022-04-20 19:17 ` Paul Eggert 0 siblings, 1 reply; 45+ messages in thread From: Max Nikulin @ 2022-04-20 16:56 UTC (permalink / raw) To: Paul Eggert; +Cc: orgmode, 54764 On 17/04/2022 08:58, Paul Eggert wrote: > Thanks, I installed that and then installed the attached, which merges > that with some documentation improvements that I drafted based on this > thread. Thank you for further editing of docs. Please, fix a typo. > diff --git a/doc/lispref/os.texi b/doc/lispref/os.texi > index 66689f43a9..8366689640 100644 > --- a/doc/lispref/os.texi > +++ b/doc/lispref/os.texi > > @@ -1687,14 +1660,18 @@ Time Conversion > than six arguments the @emph{last} argument is used as @var{zone} and > any other extra arguments are ignored, so that @code{(apply > #'encode-time (decode-time ...))} works. In this obsolescent > -convention, @var{zone} defaults to the current time zone rule > -(@pxref{Time Zone Rules}), and @var{dst} is treated as if it was > -@minus{}1. > +convention, @var{dst} is @minus{}1 and @var{zone} defaults to the > +current time zone rule (@pxref{Time Zone Rules}). > +When modernizing an obsolescent caller, ensure that the more-modern > +list equivalent contains 9 elements with a a @code{dst} element that ^^^ A typo: double "a". > +is @minus{}1, not @code{nil}. > > +@lisp > +;; Try to compute the time four years from now. > +;; Watch out; this might not work as expected. > +(let ((time (decode-time))) > + (setf (decoded-time-year time) > + (+ (decoded-time-year time) 4)) > + time) > +@end lisp > +@noindent > +Unfortunately, this code might not work as expected if the resulting > +time is invalid due to daylight saving transitions, time zone changes, > +or missing leap days or leap seconds. For example, if executed on > +February 29, 2096 this code yields a nonexistent date because 2100 is > +not a leap year. To avoid some (though not all) of the problem, you > +can base calculations on the middle of the affected unit, e.g., start > +at July 1 when adding years. If I get your idea correctly then "January, 31" + "1 month" should be more impressive as impossible date. Year 2096 is too far in future. I am unsure concerning expectation. Overflow arithmetic is described above and e.g. JavaScript normalizes Date object in a similar fashion. The special point is that elisp decoded time requires explicit normalization however and 2100 is a good example that updating of any field may "break" the date. > Alternatively, you can use the > +@file{calendar} and @file{time-date} libraries. A remark loosely related to your patch. Earlier you mentioned missed midnight due to time transition and suggested to use calendrical functions in Org. I can not figure out which elisp function can help to determine wall time for Aug 1 start of day in Cairo: Africa/Cairo Thu Jul 31 21:59:59 2014 UT = Thu Jul 31 23:59:59 2014 EET isdst=0 gmtoff=7200 Africa/Cairo Thu Jul 31 22:00:00 2014 UT = Fri Aug 1 01:00:00 2014 EEST isdst=1 gmtoff=10800 input: 2014-08-01 Africa/Cairo (timezone may be implicit as the system one) expected output: 01:00:00 ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones 2022-04-20 16:56 ` Max Nikulin @ 2022-04-20 19:17 ` Paul Eggert 0 siblings, 0 replies; 45+ messages in thread From: Paul Eggert @ 2022-04-20 19:17 UTC (permalink / raw) To: Max Nikulin; +Cc: orgmode, 54764 [-- Attachment #1: Type: text/plain, Size: 1934 bytes --] On 4/20/22 09:56, Max Nikulin wrote: > A typo: double "a". Thanks, fixed in the attached which I installed in master. > If I get your idea correctly then "January, 31" + "1 month" should be > more impressive as impossible date. Thanks, good idea; also in the attached patch. > I can not figure out which elisp function can help to > determine wall time for Aug 1 start of day in Cairo: > > Africa/Cairo Thu Jul 31 21:59:59 2014 UT = Thu Jul 31 23:59:59 2014 EET > isdst=0 gmtoff=7200 > Africa/Cairo Thu Jul 31 22:00:00 2014 UT = Fri Aug 1 01:00:00 2014 > EEST isdst=1 gmtoff=10800 > > input: 2014-08-01 Africa/Cairo > (timezone may be implicit as the system one) > expected output: 01:00:00 Given mktime's limitations there's no trivial way to do this for arbitrary timestamps, since 00:00 doesn't exist in Cairo that day. Worse, in some locations near the International Date Line entire days do not exist, because at 00:00 they advanced the clocks forward 24 hours in order to move the date line. It sounds like you're asking for a function that, given a date, yields the first broken-down timestamp on or after 00:00 of that date. For something like that, I'd use encode-time on 00:00 of that date to get a timestamp T, and then use time-add and decode-time to decode T-86400 seconds, T, and T+86400 seconds, and if the decoded times all look fine then return (decode-time T). If not (i.e., their UTC offsets differ, or T's decoded time is not 00:00 on the correct date) I'd use binary search to find discontinuities between T-86400 and T+86400 and look next to those discontinuities to find timestamps closer to what you want. Of course this is not ideal - but it's similar to what many mktime implementations do internally, and it's also similar to what Emacs's cal-dst already does (maybe you can look there for ideas), so you'd be in good company. [-- Attachment #2: 0001-More-encode-time-pitfall-doc-fixes.patch --] [-- Type: text/x-patch, Size: 2651 bytes --] From f98c3f4426fecf794f47f27aebe1f3b854fb1bfd Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Wed, 20 Apr 2022 12:03:19 -0700 Subject: [PATCH] More encode-time pitfall doc fixes * doc/lispref/os.texi (Time Conversion): Improve discussion of encode-time pitfalls based on comments by Max Nikulin (Bug#54764#63). --- doc/lispref/os.texi | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/doc/lispref/os.texi b/doc/lispref/os.texi index cabae08970..4138dab09f 100644 --- a/doc/lispref/os.texi +++ b/doc/lispref/os.texi @@ -1670,7 +1670,7 @@ Time Conversion convention, @var{dst} is @minus{}1 and @var{zone} defaults to the current time zone rule (@pxref{Time Zone Rules}). When modernizing an obsolescent caller, ensure that the more-modern -list equivalent contains 9 elements with a a @code{dst} element that +list equivalent contains 9 elements with a @code{dst} element that is @minus{}1, not @code{nil}. Year numbers less than 100 are not treated specially. If you want them @@ -1695,22 +1695,28 @@ Time Conversion For example: @lisp -;; Try to compute the time four years from now. +;; Try to compute the time one month from now. ;; Watch out; this might not work as expected. (let ((time (decode-time))) - (setf (decoded-time-year time) - (+ (decoded-time-year time) 4)) + (setf (decoded-time-month time) + (+ (decoded-time-month time) 1)) time) @end lisp @noindent Unfortunately, this code might not work as expected if the resulting -time is invalid due to daylight saving transitions, time zone changes, +time is invalid due to month length differences, +daylight saving transitions, time zone changes, or missing leap days or leap seconds. For example, if executed on -February 29, 2096 this code yields a nonexistent date because 2100 is -not a leap year. To avoid some (though not all) of the problem, you +January 30 this code yields a nonexistent date February 30, +which @code{encode-time} would adjust to early March. +Similarly, adding four years to February 29, 2096 would yield the +nonexistent date February 29, 2100; and adding one hour to 01:30 on +March 13, 2022 in New York would yield a timestamp 02:30 that does not +exist because clocks sprang forward from 02:00 to 03:00 that day. +To avoid some (though not all) of the problem, you can base calculations on the middle of the affected unit, e.g., start -at July 1 when adding years. Alternatively, you can use the +at the 15th of the month when adding months. Alternatively, you can use the @file{calendar} and @file{time-date} libraries. @end defun -- 2.32.0 ^ permalink raw reply related [flat|nested] 45+ messages in thread
[parent not found: <handler.54764.D54764.165091617725815.notifdone@debbugs.gnu.org>]
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones [not found] ` <handler.54764.D54764.165091617725815.notifdone@debbugs.gnu.org> @ 2022-04-25 21:16 ` Glenn Morris 2022-04-26 1:18 ` Paul Eggert 0 siblings, 1 reply; 45+ messages in thread From: Glenn Morris @ 2022-04-25 21:16 UTC (permalink / raw) To: Paul Eggert; +Cc: Max Nikulin, 54764 It seems that eg (days-to-time 7) now returns a cons rather than a two-element list. I don't know if this matters, but it means the (admittedly silly) custom type of gnus-html-image-cache-ttl is no longer valid. Ref: https://hydra.nixos.org/build/174776067 There is this comment in days-to-time: ;; Traditionally, this returned a two-element list if DAYS was an integer. ;; Keep that tradition if time-convert outputs timestamps in list form. ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones 2022-04-25 21:16 ` Glenn Morris @ 2022-04-26 1:18 ` Paul Eggert 0 siblings, 0 replies; 45+ messages in thread From: Paul Eggert @ 2022-04-26 1:18 UTC (permalink / raw) To: Glenn Morris; +Cc: Max Nikulin, 54764 [-- Attachment #1: Type: text/plain, Size: 634 bytes --] On 4/25/22 14:16, Glenn Morris wrote: > the > (admittedly silly) custom type of gnus-html-image-cache-ttl is no longer > valid. Ref: https://hydra.nixos.org/build/174776067 Thanks for pointing this out. I installed the attached patch to pacify the test. The updated FIXME comment for gnus-html-image-cache-ttl suggests that a better approach would be for that TTL value to be a count of seconds, which is what TTL values normally are, and which is what I think gnus-html-image-cache-ttl's value would have been in the first place except that Emacs didn't have bignums way back when. I proposed a patch to do that (Bug#55117). [-- Attachment #2: 0001-Pacify-misc-test-custom-opts.patch --] [-- Type: text/x-patch, Size: 1095 bytes --] From d6c7054ff5e1e87904353ddd73aecfead4321a7a Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Mon, 25 Apr 2022 17:49:23 -0700 Subject: [PATCH] Pacify misc/test-custom-opts * lisp/gnus/gnus-html.el (gnus-html-image-cache-ttl): Also allow it to be a cons of integers. --- lisp/gnus/gnus-html.el | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lisp/gnus/gnus-html.el b/lisp/gnus/gnus-html.el index 45f1e6099e..8b2200af54 100644 --- a/lisp/gnus/gnus-html.el +++ b/lisp/gnus/gnus-html.el @@ -46,8 +46,9 @@ gnus-html-image-cache-ttl :group 'gnus-art ;; FIXME hardly the friendliest type. The allowed value is actually ;; any time value, but we are assuming no-one cares about USEC and - ;; PSEC here. It would be better to eg make it a number of minutes. - :type '(list integer integer)) + ;; PSEC here. It would be better to make it a number of seconds. + :type '(choice (cons integer integer) + (list integer integer))) (defcustom gnus-html-image-automatic-caching t "Whether automatically cache retrieve images." -- 2.35.1 ^ permalink raw reply related [flat|nested] 45+ messages in thread
end of thread, other threads:[~2022-05-01 17:15 UTC | newest] Thread overview: 45+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-04-07 12:37 bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones Max Nikulin 2022-04-09 7:52 ` Paul Eggert [not found] ` <149de00f-115b-5367-414f-c7700ef8966b@cs.ucla.edu> 2022-04-09 11:36 ` Max Nikulin 2022-04-13 14:40 ` Max Nikulin 2022-04-13 18:35 ` Paul Eggert 2022-04-14 13:19 ` Max Nikulin [not found] ` <4a23f3a4-fe8f-d396-49d8-10034803be63@gmail.com> 2022-04-14 22:46 ` Paul Eggert [not found] ` <52fb10fb-892a-f273-3be8-28793f27e204@cs.ucla.edu> 2022-04-15 2:14 ` Tim Cross 2022-04-15 17:23 ` Max Nikulin [not found] ` <5cd820d4-ae67-43d4-9e63-c284d51ff1e4@gmail.com> 2022-04-16 19:23 ` Paul Eggert 2022-04-19 2:02 ` Paul Eggert 2022-04-19 5:50 ` Eli Zaretskii [not found] ` <83tuapvcxs.fsf@gnu.org> 2022-04-19 22:22 ` Paul Eggert 2022-04-20 7:23 ` Eli Zaretskii [not found] ` <83fsm8tdzl.fsf@gnu.org> 2022-04-20 18:19 ` Paul Eggert 2022-04-20 18:41 ` Eli Zaretskii 2022-04-20 19:01 ` Paul Eggert [not found] ` <4e41671c-fae8-61c4-845c-4c7ba4317e88@cs.ucla.edu> 2022-04-20 19:14 ` Eli Zaretskii [not found] ` <83fsm7sh2s.fsf@gnu.org> 2022-04-20 19:23 ` Paul Eggert [not found] ` <e4cc58ca-51f9-395e-05f5-5f06cb9d439d@cs.ucla.edu> 2022-04-20 19:30 ` Eli Zaretskii 2022-04-21 0:11 ` Paul Eggert [not found] ` <33fb24fb-282b-cc13-a597-e7b63f19982d@cs.ucla.edu> 2022-04-21 6:44 ` Eli Zaretskii [not found] ` <83y1zzq6kd.fsf@gnu.org> 2022-04-21 15:30 ` Max Nikulin 2022-04-21 15:58 ` Eli Zaretskii 2022-04-21 17:23 ` Max Nikulin 2022-04-21 18:46 ` Eli Zaretskii 2022-04-21 23:56 ` Paul Eggert [not found] ` <aa2bc0a0-1bec-37ff-919d-c20fcdfdab68@cs.ucla.edu> 2022-04-22 5:01 ` Eli Zaretskii 2022-04-23 14:35 ` Bernhard Voelker 2022-04-20 15:07 ` Max Nikulin [not found] ` <25d90a4b-f47d-01b4-2bfe-9951e97fe676@gmail.com> 2022-04-20 18:29 ` Paul Eggert 2022-04-25 15:30 ` Max Nikulin 2022-04-25 15:37 ` Paul Eggert [not found] ` <4bd57f21-0660-fce0-d796-08c534402340@cs.ucla.edu> 2022-04-25 19:49 ` Paul Eggert 2022-04-30 11:22 ` Max Nikulin 2022-05-01 2:32 ` Paul Eggert 2022-05-01 17:15 ` Max Nikulin [not found] ` <507cc0a2-d2d9-4283-7cc2-0da3c6510ac8@cs.ucla.edu> 2022-04-21 16:59 ` Max Nikulin 2022-04-13 15:12 ` Max Nikulin 2022-04-16 16:26 ` Max Nikulin [not found] ` <2d57e59b-f971-483b-ad65-e0c5ff7883e8@gmail.com> 2022-04-17 1:58 ` Paul Eggert [not found] ` <3624beb8-71fd-924e-a065-74d0034ed351@cs.ucla.edu> 2022-04-20 16:56 ` Max Nikulin 2022-04-20 19:17 ` Paul Eggert [not found] ` <handler.54764.D54764.165091617725815.notifdone@debbugs.gnu.org> 2022-04-25 21:16 ` Glenn Morris 2022-04-26 1:18 ` Paul Eggert
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.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).