* Re: master da4e5f6: Simplify use of timestamps
2018-09-15 16:46 ` master da4e5f6: Simplify use of timestamps Glenn Morris
@ 2018-09-15 22:05 ` Paul Eggert
0 siblings, 0 replies; 2+ messages in thread
From: Paul Eggert @ 2018-09-15 22:05 UTC (permalink / raw)
To: Glenn Morris, emacs-devel
[-- Attachment #1: Type: text/plain, Size: 1904 bytes --]
Glenn Morris wrote:
> This causes test icalendar--create-uid to fail.
Argh, that test is fooling around with internal implementation details that it
has no business looking into. I fixed it to stop doing that by installing the
first attached patch.
> (BTW, make -j8 check takes 22 seconds here, so not a big burden to run
> after making a change.)
Yes, I normally run those tests, though they take 36 seconds on my older
machine. Don't know why I forgot to do it this time; sorry about that.
By the way, 'make check' routinely fails on my platform (Fedora 28 x86-64) on
master, and this is a bit discouraging. Does it fail for you?
lisp/progmodes/flymake-tests.log and lisp/epg-tests.log are common failures, but
there are others on occasion. It's getting so that I sometimes don't notice real
failures.
> I haven't looked at this in any detail, so this comment could be
> rubbish, but if this causes a change in the on-disk file names Gnus uses
> for maildir, that could be bad, no?
The timestamp is used as part of a nonce, as 'time' was set to (current-time) a
few lines earlier. So although the patch might change the on-disk file name (by
padding a microseconds count less than 100,000 with leading "0"s) I don't see
how that would cause trouble; anybody later needs to grab the file by its name
anyway, and will get that name either from this code or by looking at the
containing directory.
That being said, you're right that this is a change. And it's one that I
overlooked, because I didn't notice that the old code didn't pad with leading
"0"s. Since it's only a trivial simplification I undid it by installing the
second attached patch. Although this doesn't entirely revert the change, since
the old code had some unnecessary stuff about checking that (current-time)
returns a list of four items, it does make the file names byte-for-byte
equivalent to the old code.
[-- Attachment #2: 0001-Fix-icalendar-tests-to-match-new-behavior.patch --]
[-- Type: text/x-patch, Size: 1955 bytes --]
From 3937b5b52af3eb78101dc385ac8dfe7a36e2e624 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sat, 15 Sep 2018 14:10:49 -0700
Subject: [PATCH 1/2] Fix icalendar tests to match new behavior
* test/lisp/calendar/icalendar-tests.el (icalendar--create-uid):
Do not intrude into or rely upon undocumented internal
implementation details of icalendar--create-uid.
Problem reported by Glenn Morris in:
https://lists.gnu.org/r/emacs-devel/2018-09/msg00660.html
---
test/lisp/calendar/icalendar-tests.el | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/test/lisp/calendar/icalendar-tests.el b/test/lisp/calendar/icalendar-tests.el
index 2fecabcd75..617e886989 100644
--- a/test/lisp/calendar/icalendar-tests.el
+++ b/test/lisp/calendar/icalendar-tests.el
@@ -57,17 +57,16 @@ icalendar-tests--trim
(ert-deftest icalendar--create-uid ()
"Test for `icalendar--create-uid'."
- (let* ((icalendar-uid-format "xxx-%t-%c-%h-%u-%s")
+ (let* ((icalendar-uid-format "xxx-%c-%h-%u-%s")
(icalendar--uid-count 77)
(entry-full "30.06.1964 07:01 blahblah")
(hash (format "%d" (abs (sxhash entry-full))))
(contents "DTSTART:19640630T070100\nblahblah")
(username (or user-login-name "UNKNOWN_USER")))
- (cl-letf (((symbol-function 'current-time) (lambda () '(1 2 3))))
- (should (= 77 icalendar--uid-count))
- (should (string= (concat "xxx-123-77-" hash "-" username "-19640630")
- (icalendar--create-uid entry-full contents)))
- (should (= 78 icalendar--uid-count)))
+ (should (= 77 icalendar--uid-count))
+ (should (string= (concat "xxx-77-" hash "-" username "-19640630")
+ (icalendar--create-uid entry-full contents)))
+ (should (= 78 icalendar--uid-count))
(setq contents "blahblah")
(setq icalendar-uid-format "yyy%syyy")
(should (string= (concat "yyyDTSTARTyyy")
--
2.17.1
[-- Attachment #3: 0002-Go-back-to-old-method-for-nnmaildir-names.patch --]
[-- Type: text/x-patch, Size: 1176 bytes --]
From d2048949bcd5735c17b4a194fc75d16ba25907ca Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sat, 15 Sep 2018 15:00:54 -0700
Subject: [PATCH 2/2] Go back to old method for nnmaildir names
* lisp/gnus/nnmaildir.el (nnmaildir-request-accept-article):
Omit leading 0s after "M" in file name.
Problem reported by Glenn Morris in:
https://lists.gnu.org/r/emacs-devel/2018-09/msg00660.html
---
lisp/gnus/nnmaildir.el | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lisp/gnus/nnmaildir.el b/lisp/gnus/nnmaildir.el
index 48a470c746..fbabf573c4 100644
--- a/lisp/gnus/nnmaildir.el
+++ b/lisp/gnus/nnmaildir.el
@@ -1463,7 +1463,7 @@ nnmaildir-request-accept-article
(unless (string-equal nnmaildir--delivery-time file)
(setq nnmaildir--delivery-time file
nnmaildir--delivery-count 0))
- (setq file (concat file (format-time-string "M%6N" time)))
+ (setq file (concat file "M" (number-to-string (caddr time))))
(setq file (concat file nnmaildir--delivery-pid)
file (concat file "Q" (number-to-string nnmaildir--delivery-count))
file (concat file "." (nnmaildir--system-name))
--
2.17.1
^ permalink raw reply related [flat|nested] 2+ messages in thread