bug#22302: 25.1.50; time-stamp ignores time-stamp-time-zone

bug#22302: 25.1.50; time-stamp ignores time-stamp-time-zone

[Harald Hanche-Olsen]

Run emacs -Q and insert this text:
Time-stamp: <>

Then do:
M-: (require 'time-stamp) RET
M-x set-variable RET time-stamp-time-zone RET "UTC" RET
M-x time-stamp

Expected result: A UTC time stamp is inserted.
Actual result: A time stamp in the local time zone is inserted.

My analysis:
time-stamp uses time-stamp-string to compute the string to insert.
And that calls:
      (format-time-string (time-stamp-string-preprocess ts-format)
                          nil time-stamp-time-zone)
However, time-stamp-string-preprocess does all the work,
leaving nothing for format-time-string to do.
And it does so without consulting time-stamp-time-zone.

Admittedly, the doc string for time-stamp (no longer?) mentions time-stamp-time-zone.
However, the variable does exist, and its doc string indicates that it will be used by time-stamp.

– Harald

In GNU Emacs 25.1.50.1 (x86_64-apple-darwin15.2.0, NS appkit-1404.34 Version 10.11.2 (Build 15C50))
 of 2015-12-17
Repository revision: 23b5c22703eeee7b4fe6608ce12ffe3b87794933
Windowing system distributor 'Apple', version 10.3.1404
Configured using:
 'configure --with-ns'

Configured features:
RSVG IMAGEMAGICK NOTIFY ACL GNUTLS LIBXML2 ZLIB TOOLKIT_SCROLL_BARS NS

Important settings:
  value of $LC_CTYPE: en_US.UTF-8
  locale-coding-system: utf-8-unix

bug#22302: 25.1.50; time-stamp ignores time-stamp-time-zone

[Glenn Morris]

Harald Hanche-Olsen wrote:

> Run emacs -Q and insert this text:
> Time-stamp: <>
>
> Then do:
> M-: (require 'time-stamp) RET
> M-x set-variable RET time-stamp-time-zone RET "UTC" RET
> M-x time-stamp
>
> Expected result: A UTC time stamp is inserted.
> Actual result: A time stamp in the local time zone is inserted.

This is due to af32fa956. Paul, could you have a look please?

bug#22302: 25.1.50; time-stamp ignores time-stamp-time-zone

[Paul Eggert]

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

On 01/11/2016 08:40 PM, Glenn Morris wrote:
> Harald Hanche-Olsen wrote:
>
>> Run emacs -Q and insert this text:
>> Time-stamp: <>
>>
>> Then do:
>> M-: (require 'time-stamp) RET
>> M-x set-variable RET time-stamp-time-zone RET "UTC" RET
>> M-x time-stamp
>>
>> Expected result: A UTC time stamp is inserted.
>> Actual result: A time stamp in the local time zone is inserted.
> This is due to af32fa956. Paul, could you have a look please?

Thanks for reporting this. I fixed the bug by installing the attached 
patch to emacs-25.

By the way, the value "UTC" is not portable to non-GNU systems, and 
POSIX says that in theory it could cause Emacs to crash. The value 
should be "UTC0" or (in Emacs 25) t. I see that some of Emacs's own 
source files have this issue, and plan to look into that.

[-- Attachment #2: 0001-Fix-time-stamp-time-zone-bugs-introduced-in-July.patch --]
[-- Type: text/x-patch, Size: 7956 bytes --]

From 92f27b6928b99b75630cda663c4a726c0b8ec0bf Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Tue, 12 Jan 2016 09:09:27 -0800
Subject: [PATCH] Fix time-stamp-time-zone bugs introduced in July

This fixes a bug introduced when the July changes to
format-time-string installed, as the changes were not
correctly handled in this module (Bug#22302).
Also, document time stamp time zones.
* lisp/time-stamp.el (time-stamp-time-zone): Document values better.
(time-stamp--format): New private function.
(time-stamp-string, time-stamp-string-preprocess)
(time-stamp-do-number): Use it.
* doc/emacs/files.texi (Time Stamps): Mention time zones.
* doc/misc/autotype.texi (Timestamps): Document time-stamp-time-zone.
---
 doc/emacs/files.texi   |  7 ++++---
 doc/misc/autotype.texi |  8 +++++---
 lisp/time-stamp.el     | 48 ++++++++++++++++++++++++++----------------------
 3 files changed, 35 insertions(+), 28 deletions(-)

diff --git a/doc/emacs/files.texi b/doc/emacs/files.texi
index bfa55d3..3d5562d 100644
--- a/doc/emacs/files.texi
+++ b/doc/emacs/files.texi
@@ -881,9 +881,10 @@ Time Stamps
 @code{before-save-hook} (@pxref{Hooks}).  When you save the file, this
 function then automatically updates the time stamp with the current
 date and time.  You can also use the command @kbd{M-x time-stamp} to
-update the time stamp manually.  For other customizations, see the
-Custom group @code{time-stamp}.  Note that the time stamp is formatted
-according to your locale setting (@pxref{Environment}).
+update the time stamp manually.  By default the time stamp is
+formatted according to your locale setting (@pxref{Environment}) and
+time zone (@pxref{Time of Day,,, elisp, The Emacs Lisp Reference
+Manual}).  For customizations, see the Custom group @code{time-stamp}.
 
 @node Reverting
 @section Reverting a Buffer
diff --git a/doc/misc/autotype.texi b/doc/misc/autotype.texi
index 839782a..6bdbd34 100644
--- a/doc/misc/autotype.texi
+++ b/doc/misc/autotype.texi
@@ -531,15 +531,17 @@ Timestamps
 customization buffer.
 
 @vindex time-stamp-active
+@findex time-stamp-toggle-active
 @vindex time-stamp-format
-@vindex time-stamp-start
+@vindex time-stamp-time-zone
 The time stamp is updated only if the customizable variable
 @code{time-stamp-active} is on, which it is by default; the command
 @code{time-stamp-toggle-active} can be used to toggle it.  The format of
-the time stamp is set by the customizable variable
-@code{time-stamp-format}.
+the time stamp is set by the customizable variables
+@code{time-stamp-format} and @code{time-stamp-time-zone}.
 
 @vindex time-stamp-line-limit
+@vindex time-stamp-start
 @vindex time-stamp-end
 @vindex time-stamp-count
 @vindex time-stamp-inserts-lines
diff --git a/lisp/time-stamp.el b/lisp/time-stamp.el
index 46c993e..dffd590 100644
--- a/lisp/time-stamp.el
+++ b/lisp/time-stamp.el
@@ -121,9 +121,12 @@ time-stamp-old-format-warn
   :group 'time-stamp)
 
 (defcustom time-stamp-time-zone nil
-  "If non-nil, a string naming the timezone to be used by \\[time-stamp].
-Format is the same as that used by the environment variable TZ on your system."
-  :type '(choice (const nil) string)
+  "The time zone to be used by \\[time-stamp].
+Its format is that of the ZONE argument of the `format-time-string' function,"
+  :type '(choice (const :tag "Emacs local time" nil)
+                 (const :tag "Universal Time" t)
+                 (const :tag "system wall clock time" wall)
+                 (string :tag "TZ environment variable value"))
   :group 'time-stamp
   :version "20.1")
 ;;;###autoload(put 'time-stamp-time-zone 'safe-local-variable 'string-or-null-p)
@@ -412,6 +415,8 @@ time-stamp-toggle-active
 	  (> (prefix-numeric-value arg) 0)))
   (message "time-stamp is now %s." (if time-stamp-active "active" "off")))
 
+(defun time-stamp--format (format time)
+  (format-time-string format time time-stamp-time-zone))
 
 (defun time-stamp-string (&optional ts-format)
   "Generate the new string to be inserted by \\[time-stamp].
@@ -420,8 +425,7 @@ time-stamp-string
   (or ts-format
       (setq ts-format time-stamp-format))
   (if (stringp ts-format)
-      (format-time-string (time-stamp-string-preprocess ts-format)
-                          nil time-stamp-time-zone)
+      (time-stamp--format (time-stamp-string-preprocess ts-format) nil)
     ;; handle version 1 compatibility
     (cond ((or (eq time-stamp-old-format-warn 'error)
 	       (and (eq time-stamp-old-format-warn 'ask)
@@ -515,32 +519,32 @@ time-stamp-string-preprocess
 	  "%%")
 	 ((eq cur-char ?a)		;day of week
 	  (if change-case
-	      (format-time-string "%#a" time)
+	      (time-stamp--format "%#a" time)
 	    (or alt-form (not (string-equal field-width ""))
 		(time-stamp-conv-warn "%a" "%:a"))
 	    (if (and alt-form (not (string-equal field-width "")))
 		""			;discourage "%:3a"
-	      (format-time-string "%A" time))))
+	      (time-stamp--format "%A" time))))
 	 ((eq cur-char ?A)
 	  (if alt-form
-	      (format-time-string "%A" time)
+	      (time-stamp--format "%A" time)
 	    (or change-case (not (string-equal field-width ""))
 		(time-stamp-conv-warn "%A" "%#A"))
-	    (format-time-string "%#A" time)))
+	    (time-stamp--format "%#A" time)))
 	 ((eq cur-char ?b)		;month name
 	  (if change-case
-	      (format-time-string "%#b" time)
+	      (time-stamp--format "%#b" time)
 	    (or alt-form (not (string-equal field-width ""))
 		(time-stamp-conv-warn "%b" "%:b"))
 	    (if (and alt-form (not (string-equal field-width "")))
 		""			;discourage "%:3b"
-	    (format-time-string "%B" time))))
+	    (time-stamp--format "%B" time))))
 	 ((eq cur-char ?B)
 	  (if alt-form
-	      (format-time-string "%B" time)
+	      (time-stamp--format "%B" time)
 	    (or change-case (not (string-equal field-width ""))
 		(time-stamp-conv-warn "%B" "%#B"))
-	    (format-time-string "%#B" time)))
+	    (time-stamp--format "%#B" time)))
 	 ((eq cur-char ?d)		;day of month, 1-31
 	  (time-stamp-do-number cur-char alt-form field-width time))
 	 ((eq cur-char ?H)		;hour, 0-23
@@ -554,27 +558,27 @@ time-stamp-string-preprocess
 	 ((eq cur-char ?p)		;am or pm
 	  (or change-case
 	      (time-stamp-conv-warn "%p" "%#p"))
-	  (format-time-string "%#p" time))
+	  (time-stamp--format "%#p" time))
 	 ((eq cur-char ?P)		;AM or PM
-	  (format-time-string "%p" time))
+	  (time-stamp--format "%p" time))
 	 ((eq cur-char ?S)		;seconds, 00-60
 	  (time-stamp-do-number cur-char alt-form field-width time))
 	 ((eq cur-char ?w)		;weekday number, Sunday is 0
-	  (format-time-string "%w" time))
+	  (time-stamp--format "%w" time))
 	 ((eq cur-char ?y)		;year
 	  (or alt-form (not (string-equal field-width ""))
 	      (time-stamp-conv-warn "%y" "%:y"))
-	  (string-to-number (format-time-string "%Y" time)))
+	  (string-to-number (time-stamp--format "%Y" time)))
 	 ((eq cur-char ?Y)		;4-digit year, new style
-	  (string-to-number (format-time-string "%Y" time)))
+	  (string-to-number (time-stamp--format "%Y" time)))
 	 ((eq cur-char ?z)		;time zone lower case
 	  (if change-case
 	      ""			;discourage %z variations
-	    (format-time-string "%#Z" time)))
+	    (time-stamp--format "%#Z" time)))
 	 ((eq cur-char ?Z)
 	  (if change-case
-	      (format-time-string "%#Z" time)
-	    (format-time-string "%Z" time)))
+	      (time-stamp--format "%#Z" time)
+	    (time-stamp--format "%Z" time)))
 	 ((eq cur-char ?f)		;buffer-file-name, base name only
 	  (if buffer-file-name
 	      (file-name-nondirectory buffer-file-name)
@@ -634,7 +638,7 @@ time-stamp-do-number
 			       (format "%%:%c" format-char)))
     (if (and alt-form (not (string-equal field-width "")))
 	""				;discourage "%:2d" and the like
-      (string-to-number (format-time-string format-string time)))))
+      (string-to-number (time-stamp--format format-string time)))))
 
 (defvar time-stamp-conversion-warn t
   "Warn about soon-to-be-unsupported forms in `time-stamp-format'.
-- 
2.5.0

bug#22302: 25.1.50; time-stamp ignores time-stamp-time-zone

[Glenn Morris]

Paul Eggert wrote:

> By the way, the value "UTC" is not portable to non-GNU systems, and
> POSIX says that in theory it could cause Emacs to crash.

Well, that sure sounds like a recipe for trouble.
Please make it so that perfectly reasonable user-input can't crash Emacs.

> The value should be "UTC0" or (in Emacs 25) t. 

I would guess that very few people are going to use "UTC0" instead of
"UTC". Can't say I blame them.

bug#22302: 25.1.50; time-stamp ignores time-stamp-time-zone

[Paul Eggert]

On 01/12/2016 09:47 AM, Glenn Morris wrote:
> Please make it so that perfectly reasonable user-input can't crash Emacs.

Sorry, I take it back, in rereading the spec POSIX says merely that the 
behavior is unspecified when TZ="UTC", which I understand to mean that 
localtime_r should not crash so we don't need to worry about that in 
Emacs. However, the user might not get Universal Time, so "UTC0" is 
still safer.

This is not an issue for operating systems like GNU/Linux, FreeBSD, OS 
X, and Solaris, where "UTC" means the same thing as "UTC0", because 
there's a file with a name like /usr/share/zoneinfo/UTC that does the 
right thing. I don't know what MS-Windows does with TZ="UTC" but I 
expect it works OK there too, otherwise we would have gotten bug reports 
before now (as this issue would come up even in older Emacs versions). I 
am mostly worried about obscure operating systems like HP-UX that do not 
use the tz database and which are not much tested by GNU Emacs developers.

bug#22302: 25.1.50; time-stamp ignores time-stamp-time-zone

[Eli Zaretskii]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Tue, 12 Jan 2016 12:19:10 -0800
> Cc: 22302-done@debbugs.gnu.org, Harald Hanche-Olsen <hanche@math.ntnu.no>
> 
> I don't know what MS-Windows does with TZ="UTC"

Nothing.  It doesn't understand that.  The MS-Windows time routines
need an explicit offset from UTC to be able to account for the
timezone.  So unless you say UTC-0, nothing will happen.

bug#22302: 25.1.50; time-stamp ignores time-stamp-time-zone

[Paul Eggert]

On 01/12/2016 12:51 PM, Eli Zaretskii wrote:
>>   don't know what MS-Windows does with TZ="UTC"
> Nothing.  It doesn't understand that.  The MS-Windows time routines
> need an explicit offset from UTC to be able to account for the
> timezone.  So unless you say UTC-0, nothing will happen.

In that case I'm a bit puzzled. Before today, lines like this:

# time-stamp-time-zone: "UTC"

appeared in files like build-aux/gitlog-to-changelog. Emacs implements 
these lines by temporarily setting the TZ environment variable to "UTC". 
If that setting is ineffective on MS-Windows, presumably any relevant 
time stamps were generated in local time instead, which is not wanted.

Perhaps no MS-Windows users noticed, or none of them bothered to file a 
bug report.

Or perhaps "nothing will happen" means "no offset from UTC will be 
applied", which means that unrecognized TZ settings act like UTC; this 
is what tzcode does.

Or it could be something else, though I don't know what.

bug#22302: 25.1.50; time-stamp ignores time-stamp-time-zone

[Eli Zaretskii]

> Cc: rgm@gnu.org, 22302@debbugs.gnu.org, hanche@math.ntnu.no
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Tue, 12 Jan 2016 13:29:23 -0800
> 
> On 01/12/2016 12:51 PM, Eli Zaretskii wrote:
> >>   don't know what MS-Windows does with TZ="UTC"
> > Nothing.  It doesn't understand that.  The MS-Windows time routines
> > need an explicit offset from UTC to be able to account for the
> > timezone.  So unless you say UTC-0, nothing will happen.
> 
> In that case I'm a bit puzzled. Before today, lines like this:
> 
> # time-stamp-time-zone: "UTC"
> 
> appeared in files like build-aux/gitlog-to-changelog. Emacs implements 
> these lines by temporarily setting the TZ environment variable to "UTC". 
> If that setting is ineffective on MS-Windows, presumably any relevant 
> time stamps were generated in local time instead, which is not wanted.
> 
> Perhaps no MS-Windows users noticed, or none of them bothered to file a 
> bug report.
> 
> Or perhaps "nothing will happen" means "no offset from UTC will be 
> applied", which means that unrecognized TZ settings act like UTC; this 
> is what tzcode does.

I see that what I said was inaccurate: the MS-Windows runtime indeed
doesn't interpret the 3-letter TZ name, but if the offset is missing,
it is taken as zero.  So "UTC" will indeed be interpreted as "UTC+0"
(and so will "EDT" and any other "XXX" with no offset).

Of course, currently this doesn't work in time-stamp, for the reasons
described in the original bug report.  But format-time-string does
behave on Windows as described above.

Sorry for any confusion I could cause.