{"thread":{"id":"46051","subject":"git-2.13.0: log --date=format:%z not working","startedAt":"2017-05-26T18:33:40Z","lastAt":"2017-06-16T12:19:05Z","messageCount":44,"participants":["Ulrich Mueller","Ævar Arnfjörð Bjarmason","Jeff King","René Scharfe","Junio C Hamano","Linus Torvalds"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"320830","messageId":"22824.29946.305300.380299@a1i15.kph.uni-mainz.de","threadId":"46051","inReplyTo":null,"subject":"git-2.13.0: log --date=format:%z not working","fromName":"Ulrich Mueller","fromEmail":"ulm@gentoo.org","sentAt":"2017-05-26T18:33:30Z","receivedAt":"2017-05-26T18:33:40Z","isPatch":false,"sender":{"key":"ulm@gentoo.org","avatar":null},"body":"The following commands work as expected (using commit b06d364310\nin the git://git.kernel.org/pub/scm/git/git.git repo as test case):\n\n$ export TZ=Europe/Berlin\n$ git --no-pager log -1 --pretty=\"%ad\" --date=iso b06d364310\n2017-05-09 23:26:02 +0900\n$ git --no-pager log -1 --pretty=\"%ad\" --date=iso-local b06d364310\n2017-05-09 16:26:02 +0200\n\nHowever, if I use explicit format: then the output of the time zone is\nwrong:\n\n$ git --no-pager log -1 --pretty=\"%ad\" --date=\"format:%F %T %z\" b06d364310\n2017-05-09 23:26:02 +0000\n$ git --no-pager log -1 --pretty=\"%ad\" --date=\"format-local:%F %T %z\" b06d364310\n2017-05-09 16:26:02 +0000\n\nI would expect the output to be the same as in the first two examples.\n"},{"id":"320871","messageId":"CACBZZX6t0Q9AJ4cpnG298tf5Las-YpmvvJXgLNNYMszikLvtMQ@mail.gmail.com","threadId":"46051","inReplyTo":"22824.29946.305300.380299@a1i15.kph.uni-mainz.de","subject":"Re: git-2.13.0: log --date=format:%z not working","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-27T16:57:08Z","receivedAt":"2017-05-27T16:57:37Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Fri, May 26, 2017 at 8:33 PM, Ulrich Mueller wrote:\n> The following commands work as expected (using commit b06d364310\n> in the git://git.kernel.org/pub/scm/git/git.git repo as test case):\n>\n> $ export TZ=Europe/Berlin\n> $ git --no-pager log -1 --pretty=\"%ad\" --date=iso b06d364310\n> 2017-05-09 23:26:02 +0900\n> $ git --no-pager log -1 --pretty=\"%ad\" --date=iso-local b06d364310\n> 2017-05-09 16:26:02 +0200\n>\n> However, if I use explicit format: then the output of the time zone is\n> wrong:\n>\n> $ git --no-pager log -1 --pretty=\"%ad\" --date=\"format:%F %T %z\" b06d364310\n> 2017-05-09 23:26:02 +0000\n> $ git --no-pager log -1 --pretty=\"%ad\" --date=\"format-local:%F %T %z\" b06d364310\n> 2017-05-09 16:26:02 +0000\n>\n> I would expect the output to be the same as in the first two examples.\n\nThis patch solves half of your problem, i.e. makes the latter\nformat-local case work:\n\ndiff --git a/date.c b/date.c\nindex 63fa99685e..469306ebf5 100644\n--- a/date.c\n+++ b/date.c\n@@ -70,6 +70,12 @@ static struct tm *time_to_tm(timestamp_t time, int tz)\n return gmtime(&t);\n }\n\n+static struct tm *time_to_local_tm(timestamp_t time, int tz)\n+{\n+ time_t t = gm_time_t(time, tz);\n+ return localtime(&t);\n+}\n+\n /*\n * What value of \"tz\" was in effect back then at \"time\" in the\n * local timezone?\n@@ -214,10 +220,14 @@ const char *show_date(timestamp_t time, int tz,\nconst struct date_mode *mode)\n return timebuf.buf;\n }\n\n- tm = time_to_tm(time, tz);\n- if (!tm) {\n- tm = time_to_tm(0, 0);\n- tz = 0;\n+ if (mode->type == DATE_STRFTIME) {\n+ tm = time_to_local_tm(time, tz);\n+ } else {\n+ tm = time_to_tm(time, tz);\n+ if (!tm) {\n+ tm = time_to_tm(0, 0);\n+ tz = 0;\n+ }\n }\n\n strbuf_reset(&timebuf);\n\nThere's another test which breaks if we just s/gmtime/localtime/g. As\nfar as I can tell to make the non-local case work we'd need to do a\nwhole dance where we set the TZ variable to e.g. UTC$offset, then call\nstrftime(), then call it again. Maybe there's some way to just specify\nthe tz offset, but I didn't find any in a quick skimming of time.h.\n"},{"id":"320874","messageId":"20170527214611.suohggo226tvmbgt@sigill.intra.peff.net","threadId":"46051","inReplyTo":"CACBZZX6t0Q9AJ4cpnG298tf5Las-YpmvvJXgLNNYMszikLvtMQ@mail.gmail.com","subject":"Re: git-2.13.0: log --date=format:%z not working","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-05-27T21:46:11Z","receivedAt":"2017-05-27T21:46:21Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, May 27, 2017 at 06:57:08PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> There's another test which breaks if we just s/gmtime/localtime/g. As\n> far as I can tell to make the non-local case work we'd need to do a\n> whole dance where we set the TZ variable to e.g. UTC$offset, then call\n> strftime(), then call it again. Maybe there's some way to just specify\n> the tz offset, but I didn't find any in a quick skimming of time.h.\n\nThere isn't. At least on _some_ platforms, the zone information is\nembedded in \"struct tm\" and stored by gmtime() and localtime(), but the\nfields aren't publicly accessible. Which is why your patch worked for\nformat-local (it swaps out gmtime() for localtime() which sets those\nfields behind the scenes). But:\n\n - I'm not sure that's guaranteed by the standard; strftime() might get\n its zone information elsewhere (if it needs to reliably distinguish\n between gmtime() and localtime() results it has to at least set a\n bit in the \"struct tm\", but that bit may not be the full zone info).\n\n - Even if it does work, you're stuck with only the local timezone. In\n theory you could temporarily tweak the process's timezone, call\n localtime(), and then tweak it back. I was never able to get that to\n work (links below).\n\nOn glibc, at least, you can access the zone fields in \"struct tm\" by\ncompiling with _DEFAULT_SOURCE.\n\nSo I think the best we could do is probably to have a feature macro like\nTM_HAS_GMTOFF, and set tm->tm_gmtoff and tm->tm_zone on platforms that\nsupport it. I'm not sure what we'd put in the latter, though; we don't\nactually have the timezone name at all (we just have \"+0200\" or whatever\nwe parsed from the git object, but that would be better than nothing).\n\nThat leaves other platforms still broken, but like I said, I don't think\nthere's a portable solution.\n\nHere are some links to past explorations:\n\n http://public-inbox.org/git/20160208152858.GA17226@sigill.intra.peff.net/\n\n http://public-inbox.org/git/87vb2d37ea.fsf@web.de/\n\n-Peff\n"},{"id":"320882","messageId":"CACBZZX6zG0z90coXz_Xzqvdp+9LpJ4RE2dAJggfLL-nd0nkWVQ@mail.gmail.com","threadId":"46051","inReplyTo":"20170527214611.suohggo226tvmbgt@sigill.intra.peff.net","subject":"Re: git-2.13.0: log --date=format:%z not working","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-28T10:29:22Z","receivedAt":"2017-05-28T10:29:48Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sat, May 27, 2017 at 11:46 PM, Jeff King wrote:\n> On Sat, May 27, 2017 at 06:57:08PM +0200, Ævar Arnfjörð Bjarmason wrote:\n>\n>> There's another test which breaks if we just s/gmtime/localtime/g. As\n>> far as I can tell to make the non-local case work we'd need to do a\n>> whole dance where we set the TZ variable to e.g. UTC$offset, then call\n>> strftime(), then call it again. Maybe there's some way to just specify\n>> the tz offset, but I didn't find any in a quick skimming of time.h.\n>\n> There isn't. At least on _some_ platforms, the zone information is\n> embedded in \"struct tm\" and stored by gmtime() and localtime(), but the\n> fields aren't publicly accessible. Which is why your patch worked for\n> format-local (it swaps out gmtime() for localtime() which sets those\n> fields behind the scenes). But:\n>\n> - I'm not sure that's guaranteed by the standard; strftime() might get\n> its zone information elsewhere (if it needs to reliably distinguish\n> between gmtime() and localtime() results it has to at least set a\n> bit in the \"struct tm\", but that bit may not be the full zone info).\n>\n> - Even if it does work, you're stuck with only the local timezone. In\n> theory you could temporarily tweak the process's timezone, call\n> localtime(), and then tweak it back. I was never able to get that to\n> work (links below).\n>\n> On glibc, at least, you can access the zone fields in \"struct tm\" by\n> compiling with _DEFAULT_SOURCE.\n>\n> So I think the best we could do is probably to have a feature macro like\n> TM_HAS_GMTOFF, and set tm->tm_gmtoff and tm->tm_zone on platforms that\n> support it. I'm not sure what we'd put in the latter, though; we don't\n> actually have the timezone name at all (we just have \"+0200\" or whatever\n> we parsed from the git object, but that would be better than nothing).\n>\n> That leaves other platforms still broken, but like I said, I don't think\n> there's a portable solution.\n>\n> Here are some links to past explorations:\n>\n> http://public-inbox.org/git/20160208152858.GA17226@sigill.intra.peff.net/\n>\n> http://public-inbox.org/git/87vb2d37ea.fsf@web.de/\n\nThere's a third and possibly least shitty option that isn't covered in\nthose threads; We could just make a pass over the strftime format\nourselves and replace %z and %Z with the timezone (as done for\nDATE_ISO8601_STRICT in date.c), then hand the rest off to strftime().\n\nI'm not going to pursue it though, Ulrich, are you maybe interested in\nhacking on that?\n"},{"id":"320885","messageId":"0a56f99e-aaa4-17ea-245a-12897ba08dbb@web.de","threadId":"46051","inReplyTo":"20170527214611.suohggo226tvmbgt@sigill.intra.peff.net","subject":"Re: git-2.13.0: log --date=format:%z not working","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-05-28T11:43:19Z","receivedAt":"2017-05-28T11:47:56Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 27.05.2017 um 23:46 schrieb Jeff King:\n> On Sat, May 27, 2017 at 06:57:08PM +0200, Ævar Arnfjörð Bjarmason wrote:\n> \n>> There's another test which breaks if we just s/gmtime/localtime/g. As\n>> far as I can tell to make the non-local case work we'd need to do a\n>> whole dance where we set the TZ variable to e.g. UTC$offset, then call\n>> strftime(), then call it again. Maybe there's some way to just specify\n>> the tz offset, but I didn't find any in a quick skimming of time.h.\n> \n> There isn't.\nRight. We could handle %z internally, though. %Z would be harder (left\nas an exercise for readers..).\n\nFirst we'd have to undo 0a0416a3 (strbuf_expand: convert \"%%\" to \"%\"),\nthough, in order to give full control back to strbuf_expand callbacks.\n\n2-pack patch:\n\n--- >8 ---\n builtin/cat-file.c | 5 +++++\n convert.c | 1 +\n daemon.c | 3 +++\n date.c | 2 +-\n ll-merge.c | 5 +++--\n pretty.c | 3 +++\n strbuf.c | 39 ++++++++++++++++++++++++++++++---------\n strbuf.h | 11 +++++------\n t/t6006-rev-list-format.sh | 12 ++++++++++++\n 9 files changed, 63 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 1890d7a639..9cf2e4291d 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -252,6 +252,11 @@ static size_t expand_format(struct strbuf *sb, const char *start, void *data)\n {\n \tconst char *end;\n \n+\tif (*start == '%') {\n+\t\tstrbuf_addch(sb, '%');\n+\t\treturn 1;\n+\t}\n+\n \tif (*start != '(')\n \t\treturn 0;\n \tend = strchr(start + 1, ')');\ndiff --git a/convert.c b/convert.c\nindex 8d652bf27c..8336fff694 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -399,6 +399,7 @@ static int filter_buffer_or_fd(int in, int out, void *data)\n \tstruct strbuf path = STRBUF_INIT;\n \tstruct strbuf_expand_dict_entry dict[] = {\n \t\t{ \"f\", NULL, },\n+\t\t{ \"%\", \"%\" },\n \t\t{ NULL, NULL, },\n \t};\n \ndiff --git a/daemon.c b/daemon.c\nindex ac7181a483..191fac2716 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -148,6 +148,9 @@ static size_t expand_path(struct strbuf *sb, const char *placeholder, void *ctx)\n \tcase 'D':\n \t\tstrbuf_addstr(sb, context->directory);\n \t\treturn 1;\n+\tcase '%':\n+\t\tstrbuf_addch(sb, '%');\n+\t\treturn 1;\n \t}\n \treturn 0;\n }\ndiff --git a/date.c b/date.c\nindex 63fa99685e..d16a88eea5 100644\n--- a/date.c\n+++ b/date.c\n@@ -246,7 +246,7 @@ const char *show_date(timestamp_t time, int tz, const struct date_mode *mode)\n \t\t\tmonth_names[tm->tm_mon], tm->tm_year + 1900,\n \t\t\ttm->tm_hour, tm->tm_min, tm->tm_sec, tz);\n \telse if (mode->type == DATE_STRFTIME)\n-\t\tstrbuf_addftime(&timebuf, mode->strftime_fmt, tm);\n+\t\tstrbuf_addftime(&timebuf, mode->strftime_fmt, tm, tz);\n \telse\n \t\tstrbuf_addf(&timebuf, \"%.3s %.3s %d %02d:%02d:%02d %d%c%+05d\",\n \t\t\t\tweekday_names[tm->tm_wday],\ndiff --git a/ll-merge.c b/ll-merge.c\nindex ac0d4a5d78..e6202c7397 100644\n--- a/ll-merge.c\n+++ b/ll-merge.c\n@@ -172,7 +172,7 @@ static int ll_ext_merge(const struct ll_merge_driver *fn,\n {\n \tchar temp[4][50];\n \tstruct strbuf cmd = STRBUF_INIT;\n-\tstruct strbuf_expand_dict_entry dict[6];\n+\tstruct strbuf_expand_dict_entry dict[7];\n \tstruct strbuf path_sq = STRBUF_INIT;\n \tconst char *args[] = { NULL, NULL };\n \tint status, fd, i;\n@@ -185,7 +185,8 @@ static int ll_ext_merge(const struct ll_merge_driver *fn,\n \tdict[2].placeholder = \"B\"; dict[2].value = temp[2];\n \tdict[3].placeholder = \"L\"; dict[3].value = temp[3];\n \tdict[4].placeholder = \"P\"; dict[4].value = path_sq.buf;\n-\tdict[5].placeholder = NULL; dict[5].value = NULL;\n+\tdict[5].placeholder = \"%\"; dict[5].value = \"%\";\n+\tdict[6].placeholder = NULL; dict[6].value = NULL;\n \n \tif (fn->cmdline == NULL)\n \t\tdie(\"custom merge driver %s lacks command line.\", fn->name);\ndiff --git a/pretty.c b/pretty.c\nindex 587d48371b..56872bfa25 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1428,6 +1428,9 @@ static size_t format_commit_item(struct strbuf *sb, /* in UTF-8 */\n \t} magic = NO_MAGIC;\n \n \tswitch (placeholder[0]) {\n+\tcase '%':\n+\t\tstrbuf_addch(sb, '%');\n+\t\treturn 1;\n \tcase '-':\n \t\tmagic = DEL_LF_BEFORE_EMPTY;\n \t\tbreak;\ndiff --git a/strbuf.c b/strbuf.c\nindex 00457940cf..206dff6037 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -309,12 +309,6 @@ void strbuf_expand(struct strbuf *sb, const char *format, expand_fn_t fn,\n \t\t\tbreak;\n \t\tformat = percent + 1;\n \n-\t\tif (*format == '%') {\n-\t\t\tstrbuf_addch(sb, '%');\n-\t\t\tformat++;\n-\t\t\tcontinue;\n-\t\t}\n-\n \t\tconsumed = fn(sb, format, context);\n \t\tif (consumed)\n \t\t\tformat += consumed;\n@@ -785,7 +779,28 @@ char *xstrfmt(const char *fmt, ...)\n \treturn ret;\n }\n \n-void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm)\n+struct date_format_context {\n+\tint tz;\n+};\n+\n+static size_t expand_date_format(struct strbuf *sb, const char *placeholder,\n+\t\t\t\t void *ctx)\n+{\n+\tstruct date_format_context *c = ctx;\n+\n+\tswitch (placeholder[0]) {\n+\tcase '%':\n+\t\tstrbuf_addstr(sb, \"%%\");\n+\t\treturn 1;\n+\tcase 'z':\n+\t\tstrbuf_addf(sb, \"%+05d\", c->tz);\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n+void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm,\n+\t\t int tz)\n {\n \tsize_t hint = 128;\n \tsize_t len;\n@@ -794,7 +809,10 @@ void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm)\n \t\treturn;\n \n \tstrbuf_grow(sb, hint);\n-\tlen = strftime(sb->buf + sb->len, sb->alloc - sb->len, fmt, tm);\n+\tif (strstr(fmt, \"%z\"))\n+\t\tlen = 0;\n+\telse\n+\t\tlen = strftime(sb->buf + sb->len, sb->alloc - sb->len, fmt, tm);\n \n \tif (!len) {\n \t\t/*\n@@ -805,7 +823,10 @@ void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm)\n \t\t * character before returning.\n \t\t */\n \t\tstruct strbuf munged_fmt = STRBUF_INIT;\n-\t\tstrbuf_addf(&munged_fmt, \"%s \", fmt);\n+\t\tstruct date_format_context ctx;\n+\t\tctx.tz = tz;\n+\t\tstrbuf_expand(&munged_fmt, fmt, expand_date_format, &ctx);\n+\t\tstrbuf_addch(&munged_fmt, ' ');\n \t\twhile (!len) {\n \t\t\thint *= 2;\n \t\t\tstrbuf_grow(sb, hint);\ndiff --git a/strbuf.h b/strbuf.h\nindex 80047b1bb7..fc4c796a2b 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -281,10 +281,6 @@ extern void strbuf_adddup(struct strbuf *sb, size_t pos, size_t len);\n * the length of the placeholder recognized and `strbuf_expand()` skips\n * over it.\n *\n- * The format `%%` is automatically expanded to a single `%` as a quoting\n- * mechanism; callers do not need to handle the `%` placeholder themselves,\n- * and the callback function will not be invoked for this placeholder.\n- *\n * All other characters (non-percent and not skipped ones) are copied\n * verbatim to the strbuf. If the callback returned zero, meaning that the\n * placeholder is unknown, then the percent sign is copied, too.\n@@ -339,9 +335,12 @@ __attribute__((format (printf,2,0)))\n extern void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap);\n \n /**\n- * Add the time specified by `tm`, as formatted by `strftime`.\n+ * Add the time specified by `tm` and `tz`, as formatted by `strftime`.\n+ * The time zone offset is specified like hhmm, so e.g. -600 means six\n+ * hours west of Greenwich.\n */\n-extern void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm);\n+extern void strbuf_addftime(struct strbuf *sb, const char *fmt,\n+\t\t\t const struct tm *tm, int tz);\n \n /**\n * Read a given size of data from a FILE* pointer to the buffer.\ndiff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh\nindex a1dcdb81d7..dc0bed8d90 100755\n--- a/t/t6006-rev-list-format.sh\n+++ b/t/t6006-rev-list-format.sh\n@@ -483,4 +483,16 @@ test_expect_success 'unused %G placeholders are passed through' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'date format \"%F %T %z\" is the same as iso' '\n+\tgit log -1 --format=\"%ad\" --date=iso >expect &&\n+\tgit log -1 --format=\"%ad\" --date=\"format:%F %T %z\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'date format \"%%z\" expands to percent zed' '\n+\techo \"%z\" >expect &&\n+\tgit log -1 --format=\"%ad\" --date=\"format:%%z\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n"},{"id":"320912","messageId":"xmqqh904a3gt.fsf@gitster.mtv.corp.google.com","threadId":"46051","inReplyTo":"CACBZZX6zG0z90coXz_Xzqvdp+9LpJ4RE2dAJggfLL-nd0nkWVQ@mail.gmail.com","subject":"Re: git-2.13.0: log --date=format:%z not working","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-29T00:53:06Z","receivedAt":"2017-05-29T00:53:15Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n>>\n>> Here are some links to past explorations:\n>>\n>> http://public-inbox.org/git/20160208152858.GA17226@sigill.intra.peff.net/\n>>\n>> http://public-inbox.org/git/87vb2d37ea.fsf@web.de/\n>\n> There's a third and possibly least shitty option that isn't covered in\n> those threads; We could just make a pass over the strftime format\n> ourselves and replace %z and %Z with the timezone (as done for\n> DATE_ISO8601_STRICT in date.c), then hand the rest off to strftime().\n\nI do not know about %Z but certainly for %z that sounds the least\nbad.\n\nIn a nearby message René seems to have come up with the same idea,\ntoo ;-)\n"},{"id":"321381","messageId":"xmqq1sr3161p.fsf@gitster.mtv.corp.google.com","threadId":"46051","inReplyTo":"0a56f99e-aaa4-17ea-245a-12897ba08dbb@web.de","subject":"Re: git-2.13.0: log --date=format:%z not working","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-06-02T02:23:30Z","receivedAt":"2017-06-02T02:23:37Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> Am 27.05.2017 um 23:46 schrieb Jeff King:\n>> On Sat, May 27, 2017 at 06:57:08PM +0200, Ævar Arnfjörð Bjarmason wrote:\n>> \n>>> There's another test which breaks if we just s/gmtime/localtime/g. As\n>>> far as I can tell to make the non-local case work we'd need to do a\n>>> whole dance where we set the TZ variable to e.g. UTC$offset, then call\n>>> strftime(), then call it again. Maybe there's some way to just specify\n>>> the tz offset, but I didn't find any in a quick skimming of time.h.\n>> \n>> There isn't.\n> Right. We could handle %z internally, though. %Z would be harder (left\n> as an exercise for readers..).\n>\n> First we'd have to undo 0a0416a3 (strbuf_expand: convert \"%%\" to \"%\"),\n> though, in order to give full control back to strbuf_expand callbacks.\n>\n> 2-pack patch:\n\nI think the list concensus is that handling %z ourselves like this\none does is the best we can do portably.\n\nAnybody wants to wrap this up into a patch with log message?\n\nThanks.\n\n\n>\n> --- >8 ---\n> builtin/cat-file.c | 5 +++++\n> convert.c | 1 +\n> daemon.c | 3 +++\n> date.c | 2 +-\n> ll-merge.c | 5 +++--\n> pretty.c | 3 +++\n> strbuf.c | 39 ++++++++++++++++++++++++++++++---------\n> strbuf.h | 11 +++++------\n> t/t6006-rev-list-format.sh | 12 ++++++++++++\n> 9 files changed, 63 insertions(+), 18 deletions(-)\n>\n> diff --git a/builtin/cat-file.c b/builtin/cat-file.c\n> index 1890d7a639..9cf2e4291d 100644\n> --- a/builtin/cat-file.c\n> +++ b/builtin/cat-file.c\n> @@ -252,6 +252,11 @@ static size_t expand_format(struct strbuf *sb, const char *start, void *data)\n> {\n> \tconst char *end;\n> \n> +\tif (*start == '%') {\n> +\t\tstrbuf_addch(sb, '%');\n> +\t\treturn 1;\n> +\t}\n> +\n> \tif (*start != '(')\n> \t\treturn 0;\n> \tend = strchr(start + 1, ')');\n> diff --git a/convert.c b/convert.c\n> index 8d652bf27c..8336fff694 100644\n> --- a/convert.c\n> +++ b/convert.c\n> @@ -399,6 +399,7 @@ static int filter_buffer_or_fd(int in, int out, void *data)\n> \tstruct strbuf path = STRBUF_INIT;\n> \tstruct strbuf_expand_dict_entry dict[] = {\n> \t\t{ \"f\", NULL, },\n> +\t\t{ \"%\", \"%\" },\n> \t\t{ NULL, NULL, },\n> \t};\n> \n> diff --git a/daemon.c b/daemon.c\n> index ac7181a483..191fac2716 100644\n> --- a/daemon.c\n> +++ b/daemon.c\n> @@ -148,6 +148,9 @@ static size_t expand_path(struct strbuf *sb, const char *placeholder, void *ctx)\n> \tcase 'D':\n> \t\tstrbuf_addstr(sb, context->directory);\n> \t\treturn 1;\n> +\tcase '%':\n> +\t\tstrbuf_addch(sb, '%');\n> +\t\treturn 1;\n> \t}\n> \treturn 0;\n> }\n> diff --git a/date.c b/date.c\n> index 63fa99685e..d16a88eea5 100644\n> --- a/date.c\n> +++ b/date.c\n> @@ -246,7 +246,7 @@ const char *show_date(timestamp_t time, int tz, const struct date_mode *mode)\n> \t\t\tmonth_names[tm->tm_mon], tm->tm_year + 1900,\n> \t\t\ttm->tm_hour, tm->tm_min, tm->tm_sec, tz);\n> \telse if (mode->type == DATE_STRFTIME)\n> -\t\tstrbuf_addftime(&timebuf, mode->strftime_fmt, tm);\n> +\t\tstrbuf_addftime(&timebuf, mode->strftime_fmt, tm, tz);\n> \telse\n> \t\tstrbuf_addf(&timebuf, \"%.3s %.3s %d %02d:%02d:%02d %d%c%+05d\",\n> \t\t\t\tweekday_names[tm->tm_wday],\n> diff --git a/ll-merge.c b/ll-merge.c\n> index ac0d4a5d78..e6202c7397 100644\n> --- a/ll-merge.c\n> +++ b/ll-merge.c\n> @@ -172,7 +172,7 @@ static int ll_ext_merge(const struct ll_merge_driver *fn,\n> {\n> \tchar temp[4][50];\n> \tstruct strbuf cmd = STRBUF_INIT;\n> -\tstruct strbuf_expand_dict_entry dict[6];\n> +\tstruct strbuf_expand_dict_entry dict[7];\n> \tstruct strbuf path_sq = STRBUF_INIT;\n> \tconst char *args[] = { NULL, NULL };\n> \tint status, fd, i;\n> @@ -185,7 +185,8 @@ static int ll_ext_merge(const struct ll_merge_driver *fn,\n> \tdict[2].placeholder = \"B\"; dict[2].value = temp[2];\n> \tdict[3].placeholder = \"L\"; dict[3].value = temp[3];\n> \tdict[4].placeholder = \"P\"; dict[4].value = path_sq.buf;\n> -\tdict[5].placeholder = NULL; dict[5].value = NULL;\n> +\tdict[5].placeholder = \"%\"; dict[5].value = \"%\";\n> +\tdict[6].placeholder = NULL; dict[6].value = NULL;\n> \n> \tif (fn->cmdline == NULL)\n> \t\tdie(\"custom merge driver %s lacks command line.\", fn->name);\n> diff --git a/pretty.c b/pretty.c\n> index 587d48371b..56872bfa25 100644\n> --- a/pretty.c\n> +++ b/pretty.c\n> @@ -1428,6 +1428,9 @@ static size_t format_commit_item(struct strbuf *sb, /* in UTF-8 */\n> \t} magic = NO_MAGIC;\n> \n> \tswitch (placeholder[0]) {\n> +\tcase '%':\n> +\t\tstrbuf_addch(sb, '%');\n> +\t\treturn 1;\n> \tcase '-':\n> \t\tmagic = DEL_LF_BEFORE_EMPTY;\n> \t\tbreak;\n> diff --git a/strbuf.c b/strbuf.c\n> index 00457940cf..206dff6037 100644\n> --- a/strbuf.c\n> +++ b/strbuf.c\n> @@ -309,12 +309,6 @@ void strbuf_expand(struct strbuf *sb, const char *format, expand_fn_t fn,\n> \t\t\tbreak;\n> \t\tformat = percent + 1;\n> \n> -\t\tif (*format == '%') {\n> -\t\t\tstrbuf_addch(sb, '%');\n> -\t\t\tformat++;\n> -\t\t\tcontinue;\n> -\t\t}\n> -\n> \t\tconsumed = fn(sb, format, context);\n> \t\tif (consumed)\n> \t\t\tformat += consumed;\n> @@ -785,7 +779,28 @@ char *xstrfmt(const char *fmt, ...)\n> \treturn ret;\n> }\n> \n> -void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm)\n> +struct date_format_context {\n> +\tint tz;\n> +};\n> +\n> +static size_t expand_date_format(struct strbuf *sb, const char *placeholder,\n> +\t\t\t\t void *ctx)\n> +{\n> +\tstruct date_format_context *c = ctx;\n> +\n> +\tswitch (placeholder[0]) {\n> +\tcase '%':\n> +\t\tstrbuf_addstr(sb, \"%%\");\n> +\t\treturn 1;\n> +\tcase 'z':\n> +\t\tstrbuf_addf(sb, \"%+05d\", c->tz);\n> +\t\treturn 1;\n> +\t}\n> +\treturn 0;\n> +}\n> +\n> +void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm,\n> +\t\t int tz)\n> {\n> \tsize_t hint = 128;\n> \tsize_t len;\n> @@ -794,7 +809,10 @@ void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm)\n> \t\treturn;\n> \n> \tstrbuf_grow(sb, hint);\n> -\tlen = strftime(sb->buf + sb->len, sb->alloc - sb->len, fmt, tm);\n> +\tif (strstr(fmt, \"%z\"))\n> +\t\tlen = 0;\n> +\telse\n> +\t\tlen = strftime(sb->buf + sb->len, sb->alloc - sb->len, fmt, tm);\n> \n> \tif (!len) {\n> \t\t/*\n> @@ -805,7 +823,10 @@ void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm)\n> \t\t * character before returning.\n> \t\t */\n> \t\tstruct strbuf munged_fmt = STRBUF_INIT;\n> -\t\tstrbuf_addf(&munged_fmt, \"%s \", fmt);\n> +\t\tstruct date_format_context ctx;\n> +\t\tctx.tz = tz;\n> +\t\tstrbuf_expand(&munged_fmt, fmt, expand_date_format, &ctx);\n> +\t\tstrbuf_addch(&munged_fmt, ' ');\n> \t\twhile (!len) {\n> \t\t\thint *= 2;\n> \t\t\tstrbuf_grow(sb, hint);\n> diff --git a/strbuf.h b/strbuf.h\n> index 80047b1bb7..fc4c796a2b 100644\n> --- a/strbuf.h\n> +++ b/strbuf.h\n> @@ -281,10 +281,6 @@ extern void strbuf_adddup(struct strbuf *sb, size_t pos, size_t len);\n> * the length of the placeholder recognized and `strbuf_expand()` skips\n> * over it.\n> *\n> - * The format `%%` is automatically expanded to a single `%` as a quoting\n> - * mechanism; callers do not need to handle the `%` placeholder themselves,\n> - * and the callback function will not be invoked for this placeholder.\n> - *\n> * All other characters (non-percent and not skipped ones) are copied\n> * verbatim to the strbuf. If the callback returned zero, meaning that the\n> * placeholder is unknown, then the percent sign is copied, too.\n> @@ -339,9 +335,12 @@ __attribute__((format (printf,2,0)))\n> extern void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap);\n> \n> /**\n> - * Add the time specified by `tm`, as formatted by `strftime`.\n> + * Add the time specified by `tm` and `tz`, as formatted by `strftime`.\n> + * The time zone offset is specified like hhmm, so e.g. -600 means six\n> + * hours west of Greenwich.\n> */\n> -extern void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm);\n> +extern void strbuf_addftime(struct strbuf *sb, const char *fmt,\n> +\t\t\t const struct tm *tm, int tz);\n> \n> /**\n> * Read a given size of data from a FILE* pointer to the buffer.\n> diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh\n> index a1dcdb81d7..dc0bed8d90 100755\n> --- a/t/t6006-rev-list-format.sh\n> +++ b/t/t6006-rev-list-format.sh\n> @@ -483,4 +483,16 @@ test_expect_success 'unused %G placeholders are passed through' '\n> \ttest_cmp expect actual\n> '\n> \n> +test_expect_success 'date format \"%F %T %z\" is the same as iso' '\n> +\tgit log -1 --format=\"%ad\" --date=iso >expect &&\n> +\tgit log -1 --format=\"%ad\" --date=\"format:%F %T %z\" >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'date format \"%%z\" expands to percent zed' '\n> +\techo \"%z\" >expect &&\n> +\tgit log -1 --format=\"%ad\" --date=\"format:%%z\" >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> test_done\n"},{"id":"321412","messageId":"72b001fc-80e7-42b9-bd9d-87621da7978a@web.de","threadId":"46051","inReplyTo":"20170602030825.hdpbaisn54d4fi4n@sigill.intra.peff.net","subject":"Re: git-2.13.0: log --date=format:%z not working","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-06-02T17:25:43Z","receivedAt":"2017-06-02T17:26:23Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 02.06.2017 um 05:08 schrieb Jeff King:\n> In theory the solution is:\n> \n> 1. Start using localtime() instead of gmtime() with an adjustment when\n> we are converting to the local timezone (i.e., format-local). We\n> should be able to do this portably.\n> \n> This is easy to do, and it's better than handling %z ourselves,\n> because it makes %Z work, too.\n> \n> 2. When showing the author's timezone, do some trickery to set the\n> program's timezone, then use localtime(), then restore the program\n> timezone.\n> \n> I couldn't get this to work reliably. And anyway, we'd still have\n> nothing to put in %Z since we don't have a timezone name at all in\n> the git objects. We just have \"+0400\" or whatever.\n> \n> So I don't see a portable way to make (2) work.\n\nWe could create a strftime wrapper that also takes a time zone offset,\nwith platform-specific implementations. Is it worth the effort?\n\nWhat reliability issues did you run into?\n\n> But it seems a shame\n> that %Z does not work for case (1) with René's patch.\n> \n> I guess we could do (1) for the local cases and then handle \"%z\"\n> ourselves otherwise. That sounds even _more_ confusing, but it at least\n> gets the most cases right.\n> \n> If we do handle \"%z\" ourselves (either always or for just the one case),\n> what should the matching %Z say? Right now (and I think with René's\n> patch) it says GMT, which is actively misleading. We should probably\n> replace it with the same text as \"%z\". That's not quite what the user\n> wanted, but at least it's accurate.\n\nOn Linux \"%z %Z\" is expanded to \"+0200 CEST\" for me, while on Windows I\nget \"Mitteleurop▒ische Sommerzeit Mitteleurop▒ische Sommerzeit\". (That\n\"▒\" is probably supposed to be an \"ä\".) POSIX requires +hhmm or -hhmm\nformat for %z, and for %Z is to be \"Replaced by the timezone name or\nabbreviation\".\n\nI'd say \"GMT+0200\" etc. is a nice enough timezone name, i.e. having %Z\nresolve to the same as %z plus a literal prefix of \"GMT\" should at least\nnot be wrong.\n\nAlternatively we could have a lookup table mapping a few typical offsets\nto timezone names, but e.g. handling daylight saving times would\nprobably be too hard (when did that part of the world switch in the\ngiven year? north or south of the equator?)..\n\n> As far as the patch itself goes, I'm disappointed to lose the automatic\n> \"%\" handling for all of the other callers. But I suspect the boilerplate\n> involved in any solution that lets callers choose whether or not to use\n> it would end up being longer than just handling it in each caller.\n\nActually I felt uneasy when you added that forced %% handling because it\nput a policy into an otherwise neutral interpreter function. I just had\nno practical argument against it -- until now.\n\nI'd rather see strbuf_expand also lose the hard-coded percent sign, but\nagain I don't have an actual user for such a flexibility (yet).\n\nPerhaps we should add a fully neutral strbuf_expand_core (or whatever),\nmake strbuf_expand a wrapper with hard-coded % and %% handling and use\nthe core function in the strftime wrapper. Except that the function is\nnot easily stackable. Hmm..\n\nRené\n"},{"id":"321425","messageId":"20170602183504.ii7arq2ssxgwgyxr@sigill.intra.peff.net","threadId":"46051","inReplyTo":"72b001fc-80e7-42b9-bd9d-87621da7978a@web.de","subject":"Re: git-2.13.0: log --date=format:%z not working","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-06-02T18:35:04Z","receivedAt":"2017-06-02T19:35:20Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 02, 2017 at 07:25:43PM +0200, René Scharfe wrote:\n\n> Am 02.06.2017 um 05:08 schrieb Jeff King:\n> > In theory the solution is:\n> > \n> > 1. Start using localtime() instead of gmtime() with an adjustment when\n> > we are converting to the local timezone (i.e., format-local). We\n> > should be able to do this portably.\n> > \n> > This is easy to do, and it's better than handling %z ourselves,\n> > because it makes %Z work, too.\n> > \n> > 2. When showing the author's timezone, do some trickery to set the\n> > program's timezone, then use localtime(), then restore the program\n> > timezone.\n> > \n> > I couldn't get this to work reliably. And anyway, we'd still have\n> > nothing to put in %Z since we don't have a timezone name at all in\n> > the git objects. We just have \"+0400\" or whatever.\n> > \n> > So I don't see a portable way to make (2) work.\n> \n> We could create a strftime wrapper that also takes a time zone offset,\n> with platform-specific implementations. Is it worth the effort?\n> \n> What reliability issues did you run into?\n\nMy patch is below for reference. The issue is that we have to stuff a\nname into $TZ that the system libc will parse into something sensible.\nJust setting it to \"%+05d\" doesn't work at all. glibc at least seems to\naccept names like FOO+4, but:\n\n - I have no idea if that's portable\n\n - it only allows single-hour offsets, so +0330 is out. There might be\n some way to represent that, but I'm not sure if it's portable\n (FOO+0300 doesn't seem to work even on glibc).\n\n - that sets %Z to \"FOO\", which is obviously nonsense\n\n> > If we do handle \"%z\" ourselves (either always or for just the one case),\n> > what should the matching %Z say? Right now (and I think with René's\n> > patch) it says GMT, which is actively misleading. We should probably\n> > replace it with the same text as \"%z\". That's not quite what the user\n> > wanted, but at least it's accurate.\n> \n> On Linux \"%z %Z\" is expanded to \"+0200 CEST\" for me, while on Windows I\n> get \"Mitteleurop▒ische Sommerzeit Mitteleurop▒ische Sommerzeit\". (That\n> \"▒\" is probably supposed to be an \"ä\".) POSIX requires +hhmm or -hhmm\n> format for %z, and for %Z is to be \"Replaced by the timezone name or\n> abbreviation\".\n> \n> I'd say \"GMT+0200\" etc. is a nice enough timezone name, i.e. having %Z\n> resolve to the same as %z plus a literal prefix of \"GMT\" should at least\n> not be wrong.\n\nI thought that, too, but I think it is wrong based on my understanding\nof how $TZ is parsed. There something like \"EDT-4\" means \"call this EDT,\nand by the way it is 4 hours behind GMT\".\n\nSo what you're proposing isn't wrong per se, but your notation means\nsomething totally different than what similar-looking notation looks\nlike on the $TZ end, which is bound to create confusion.\n\n> Alternatively we could have a lookup table mapping a few typical offsets\n> to timezone names, but e.g. handling daylight saving times would\n> probably be too hard (when did that part of the world switch in the\n> given year? north or south of the equator?)..\n\nRight, I don't think the mapping of zone to offset is reversible,\nbecause many zones map to the same offset. If I tell you I'm in -0500,\neven just in the US that could mean Eastern Standard Time (winter, no\nDST) or Central Daylight Time (summer, DST). Not to mention that other\npolitical entities in the same longitude have their own zones which do\nDST at different times (or were even established as zones at different\ntimes; historical dates need to use the zones as they were at that\ntime).\n\n> > As far as the patch itself goes, I'm disappointed to lose the automatic\n> > \"%\" handling for all of the other callers. But I suspect the boilerplate\n> > involved in any solution that lets callers choose whether or not to use\n> > it would end up being longer than just handling it in each caller.\n> \n> Actually I felt uneasy when you added that forced %% handling because it\n> put a policy into an otherwise neutral interpreter function. I just had\n> no practical argument against it -- until now.\n> \n> I'd rather see strbuf_expand also lose the hard-coded percent sign, but\n> again I don't have an actual user for such a flexibility (yet).\n> \n> Perhaps we should add a fully neutral strbuf_expand_core (or whatever),\n> make strbuf_expand a wrapper with hard-coded % and %% handling and use\n> the core function in the strftime wrapper. Except that the function is\n> not easily stackable. Hmm..\n\nRight, that's the boilerplate trickiness I was referring to. It's\nprobably not worth the effort.\n\nAnyway, here's my patch. I've been testing it with:\n\n ./git log --format='%ai%n%ad%n' --date=format:'%Y-%m-%d %H:%M:%S %z (%Z)'\n\nwhich lets you compare a variety of commits with the existing formatting\nroutine.\n\n---\ndiff --git a/date.c b/date.c\nindex 63fa99685..a6214172e 100644\n--- a/date.c\n+++ b/date.c\n@@ -196,6 +196,34 @@ const char *show_date(timestamp_t time, int tz, const struct date_mode *mode)\n \t\treturn timebuf.buf;\n \t}\n \n+\tif (mode->type == DATE_STRFTIME) {\n+\t\tchar *orig_tz;\n+\t\ttime_t t = time;\n+\n+\t\tif (!mode->local) {\n+\t\t\tchar *new_tz = xstrfmt(\"GIT%+d\", -tz / 100);\n+\t\t\torig_tz = xstrdup_or_null(getenv(\"TZ\"));\n+\t\t\tsetenv(\"TZ\", new_tz, 1);\n+\t\t\tfree(new_tz);\n+\t\t\ttzset();\n+\t\t}\n+\n+\t\ttm = localtime(&t);\n+\t\tstrbuf_reset(&timebuf);\n+\t\tstrbuf_addftime(&timebuf, mode->strftime_fmt, tm);\n+\n+\t\tif (!mode->local) {\n+\t\t\tif (orig_tz)\n+\t\t\t\tsetenv(\"TZ\", orig_tz, 1);\n+\t\t\telse\n+\t\t\t\tunsetenv(\"TZ\");\n+\t\t\tfree(orig_tz);\n+\t\t\ttzset();\n+\t\t}\n+\n+\t\treturn timebuf.buf;\n+\t}\n+\n \tif (mode->local)\n \t\ttz = local_tzoffset(time);\n \n@@ -245,8 +273,6 @@ const char *show_date(timestamp_t time, int tz, const struct date_mode *mode)\n \t\t\tweekday_names[tm->tm_wday], tm->tm_mday,\n \t\t\tmonth_names[tm->tm_mon], tm->tm_year + 1900,\n \t\t\ttm->tm_hour, tm->tm_min, tm->tm_sec, tz);\n-\telse if (mode->type == DATE_STRFTIME)\n-\t\tstrbuf_addftime(&timebuf, mode->strftime_fmt, tm);\n \telse\n \t\tstrbuf_addf(&timebuf, \"%.3s %.3s %d %02d:%02d:%02d %d%c%+05d\",\n \t\t\t\tweekday_names[tm->tm_wday],\n\n"},{"id":"321446","messageId":"20170602030825.hdpbaisn54d4fi4n@sigill.intra.peff.net","threadId":"46051","inReplyTo":"xmqq1sr3161p.fsf@gitster.mtv.corp.google.com","subject":"Re: git-2.13.0: log --date=format:%z not working","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-06-02T03:08:25Z","receivedAt":"2017-06-02T21:55:25Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 02, 2017 at 11:23:30AM +0900, Junio C Hamano wrote:\n\n> René Scharfe writes:\n> \n> > Am 27.05.2017 um 23:46 schrieb Jeff King:\n> >> On Sat, May 27, 2017 at 06:57:08PM +0200, Ævar Arnfjörð Bjarmason wrote:\n> >> \n> >>> There's another test which breaks if we just s/gmtime/localtime/g. As\n> >>> far as I can tell to make the non-local case work we'd need to do a\n> >>> whole dance where we set the TZ variable to e.g. UTC$offset, then call\n> >>> strftime(), then call it again. Maybe there's some way to just specify\n> >>> the tz offset, but I didn't find any in a quick skimming of time.h.\n> >> \n> >> There isn't.\n> > Right. We could handle %z internally, though. %Z would be harder (left\n> > as an exercise for readers..).\n> >\n> > First we'd have to undo 0a0416a3 (strbuf_expand: convert \"%%\" to \"%\"),\n> > though, in order to give full control back to strbuf_expand callbacks.\n> >\n> > 2-pack patch:\n> \n> I think the list concensus is that handling %z ourselves like this\n> one does is the best we can do portably.\n\nI've actually been turning it over in my head. This feels hacky, but I'm\nnot sure if we can do better.\n\nIn theory the solution is:\n\n 1. Start using localtime() instead of gmtime() with an adjustment when\n we are converting to the local timezone (i.e., format-local). We\n should be able to do this portably.\n\n This is easy to do, and it's better than handling %z ourselves,\n because it makes %Z work, too.\n\n 2. When showing the author's timezone, do some trickery to set the\n program's timezone, then use localtime(), then restore the program\n timezone.\n\n I couldn't get this to work reliably. And anyway, we'd still have\n nothing to put in %Z since we don't have a timezone name at all in\n the git objects. We just have \"+0400\" or whatever.\n\nSo I don't see a portable way to make (2) work. But it seems a shame\nthat %Z does not work for case (1) with René's patch.\n\nI guess we could do (1) for the local cases and then handle \"%z\"\nourselves otherwise. That sounds even _more_ confusing, but it at least\ngets the most cases right.\n\nIf we do handle \"%z\" ourselves (either always or for just the one case),\nwhat should the matching %Z say? Right now (and I think with René's\npatch) it says GMT, which is actively misleading. We should probably\nreplace it with the same text as \"%z\". That's not quite what the user\nwanted, but at least it's accurate.\n\n> > --- >8 ---\n> > builtin/cat-file.c | 5 +++++\n> > convert.c | 1 +\n> > daemon.c | 3 +++\n> > date.c | 2 +-\n> > ll-merge.c | 5 +++--\n> > pretty.c | 3 +++\n> > strbuf.c | 39 ++++++++++++++++++++++++++++++---------\n> > strbuf.h | 11 +++++------\n> > t/t6006-rev-list-format.sh | 12 ++++++++++++\n> > 9 files changed, 63 insertions(+), 18 deletions(-)\n\nAs far as the patch itself goes, I'm disappointed to lose the automatic\n\"%\" handling for all of the other callers. But I suspect the boilerplate\ninvolved in any solution that lets callers choose whether or not to use\nit would end up being longer than just handling it in each caller.\n\n-Peff\n"},{"id":"321447","messageId":"22833.57584.108133.30274@a1i15.kph.uni-mainz.de","threadId":"46051","inReplyTo":"20170602183504.ii7arq2ssxgwgyxr@sigill.intra.peff.net","subject":"Re: git-2.13.0: log --date=format:%z not working","fromName":"Ulrich Mueller","fromEmail":"ulm@gentoo.org","sentAt":"2017-06-02T22:04:32Z","receivedAt":"2017-06-02T22:04:45Z","isPatch":false,"sender":{"key":"ulm@gentoo.org","avatar":null},"body":">>>>> On Fri, 2 Jun 2017, Jeff King wrote:\n\n> On Fri, Jun 02, 2017 at 07:25:43PM +0200, René Scharfe wrote:\n>> On Linux \"%z %Z\" is expanded to \"+0200 CEST\" for me, while on Windows I\n>> get \"Mitteleurop▒ische Sommerzeit Mitteleurop▒ische Sommerzeit\". (That\n>> \"▒\" is probably supposed to be an \"ä\".) POSIX requires +hhmm or -hhmm\n>> format for %z, and for %Z is to be \"Replaced by the timezone name or\n>> abbreviation\".\n\nActually, the POSIX definition for %Z continues: \"or by no bytes if no\ntimezone information exists.\" So also returning an empty string would\nbe compliant (but maybe not very helpful).\n\n>> I'd say \"GMT+0200\" etc. is a nice enough timezone name, i.e. having %Z\n>> resolve to the same as %z plus a literal prefix of \"GMT\" should at least\n>> not be wrong.\n\n> I thought that, too, but I think it is wrong based on my understanding\n> of how $TZ is parsed. There something like \"EDT-4\" means \"call this EDT,\n> and by the way it is 4 hours behind GMT\".\n\n> So what you're proposing isn't wrong per se, but your notation means\n> something totally different than what similar-looking notation looks\n> like on the $TZ end, which is bound to create confusion.\n\nI agree that GMT+0200 could be misleading. But what about resolving %Z\nthe same as %z in the case of the author's time zone, as was suggested\nearlier? It is supposed to be human-readable output, or do we expect\nthat someone would use the %Z output and e.g. plug it back into their\nTZ?\n\n>> Alternatively we could have a lookup table mapping a few typical offsets\n>> to timezone names, but e.g. handling daylight saving times would\n>> probably be too hard (when did that part of the world switch in the\n>> given year? north or south of the equator?)..\n\nIMHO maintaining such a local table of timezones won't fly.\n\n> Right, I don't think the mapping of zone to offset is reversible,\n> because many zones map to the same offset. If I tell you I'm in -0500,\n> even just in the US that could mean Eastern Standard Time (winter, no\n> DST) or Central Daylight Time (summer, DST). Not to mention that other\n> political entities in the same longitude have their own zones which do\n> DST at different times (or were even established as zones at different\n> times; historical dates need to use the zones as they were at that\n> time).\n\nSame here, my +0200 offset could be anything of CAT, CEST, EET, IST,\nSAST, or WAST, according to IANA timezone data. It's a one-directional\nmapping, and there's no way to get the author's /etc/localtime info\n(or whatever its equivalent is on other systems) back from the offset\nstored in the commit. A timezone name may not even exist at all for a\ngiven [+-]hhmm offset.\n\nUlrich\n"},{"id":"321452","messageId":"20170602223003.6etkdnnogb2jmoh3@sigill.intra.peff.net","threadId":"46051","inReplyTo":"22833.57584.108133.30274@a1i15.kph.uni-mainz.de","subject":"Re: git-2.13.0: log --date=format:%z not working","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-06-02T22:30:04Z","receivedAt":"2017-06-02T22:30:11Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jun 03, 2017 at 12:04:32AM +0200, Ulrich Mueller wrote:\n\n> Actually, the POSIX definition for %Z continues: \"or by no bytes if no\n> timezone information exists.\" So also returning an empty string would\n> be compliant (but maybe not very helpful).\n> [...]\n> I agree that GMT+0200 could be misleading. But what about resolving %Z\n> the same as %z in the case of the author's time zone, as was suggested\n> earlier? It is supposed to be human-readable output, or do we expect\n> that someone would use the %Z output and e.g. plug it back into their\n> TZ?\n\nYeah, I think these are the only real contenders: an empty string, or\n\"+0200\" (which _isn't_ confusing, because it doesn't have the\nabbreviation in front of it, so it pretty clearly is an offset from\nGMT).\n\nI don't have a preference between the other two.\n\nThe remaining question is whether we want to care about preserving the\nsystem %Z for the local-timezone case.\n\n-Peff\n"},{"id":"321453","messageId":"22833.60191.771422.3111@a1i15.kph.uni-mainz.de","threadId":"46051","inReplyTo":"20170602223003.6etkdnnogb2jmoh3@sigill.intra.peff.net","subject":"Re: git-2.13.0: log --date=format:%z not working","fromName":"Ulrich Mueller","fromEmail":"ulm@gentoo.org","sentAt":"2017-06-02T22:47:59Z","receivedAt":"2017-06-02T22:48:09Z","isPatch":false,"sender":{"key":"ulm@gentoo.org","avatar":null},"body":">>>>> On Fri, 2 Jun 2017, Jeff King wrote:\n\n> The remaining question is whether we want to care about preserving the\n> system %Z for the local-timezone case.\n\nNo strong preference here. Maybe go for consistency, and have %Z\nalways return the same format (either empty, or same as %z). That\nwould at least prevent surprises when users switch from format-local\nto format.\n\nUlrich\n"},{"id":"321454","messageId":"20170602225148.drkl7obwhzypgjtr@sigill.intra.peff.net","threadId":"46051","inReplyTo":"22833.60191.771422.3111@a1i15.kph.uni-mainz.de","subject":"Re: git-2.13.0: log --date=format:%z not working","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-06-02T22:51:48Z","receivedAt":"2017-06-02T22:51:55Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jun 03, 2017 at 12:47:59AM +0200, Ulrich Mueller wrote:\n\n> >>>>> On Fri, 2 Jun 2017, Jeff King wrote:\n> \n> > The remaining question is whether we want to care about preserving the\n> > system %Z for the local-timezone case.\n> \n> No strong preference here. Maybe go for consistency, and have %Z\n> always return the same format (either empty, or same as %z). That\n> would at least prevent surprises when users switch from format-local\n> to format.\n\nIt also a lot easier to implement, which is nice.\n\nI agree on the least surprise thing, but the flipside of this is that\nGit's use of strftime will behave differently than other programs on the\nsystem (e.g., \"date +%Z\").\n\n-Peff\n"},{"id":"321477","messageId":"a8b789e6-d0cd-6d96-1bfb-ccc5bc174013@web.de","threadId":"46051","inReplyTo":"20170602225148.drkl7obwhzypgjtr@sigill.intra.peff.net","subject":"[PATCH] strbuf: let strbuf_addftime handle %z and %Z itself","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-06-03T10:40:34Z","receivedAt":"2017-06-03T10:41:03Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"There is no portable way to pass timezone information to strftime. Add\nparameters for timezone offset and name to strbuf_addftime and let it\nhandle the timezone-related format specifiers %z and %Z internally.\nCallers can opt out by passing NULL as timezone name.\n\nUse an empty string as timezone name in show_date (the only current\ncaller) for now because we only have the timezone offset in non-local\nmode. POSIX allows %Z to resolve to nothing in case of missing info.\n\nHelped-by: Ulrich Mueller \nHelped-by: Jeff King \nSigned-off-by: Rene Scharfe \n---\nDuplicates strbuf_expand to a certain extent, but not too badly, I\nthink. Leaves the door open for letting strftime handle the local\ncase.\n\n date.c | 2 +-\n strbuf.c | 42 ++++++++++++++++++++++++++++++++++++++----\n strbuf.h | 11 ++++++++---\n t/t6006-rev-list-format.sh | 12 ++++++++++++\n 4 files changed, 59 insertions(+), 8 deletions(-)\n\ndiff --git a/date.c b/date.c\nindex 63fa99685e..5580577334 100644\n--- a/date.c\n+++ b/date.c\n@@ -246,7 +246,7 @@ const char *show_date(timestamp_t time, int tz, const struct date_mode *mode)\n \t\t\tmonth_names[tm->tm_mon], tm->tm_year + 1900,\n \t\t\ttm->tm_hour, tm->tm_min, tm->tm_sec, tz);\n \telse if (mode->type == DATE_STRFTIME)\n-\t\tstrbuf_addftime(&timebuf, mode->strftime_fmt, tm);\n+\t\tstrbuf_addftime(&timebuf, mode->strftime_fmt, tm, tz, \"\");\n \telse\n \t\tstrbuf_addf(&timebuf, \"%.3s %.3s %d %02d:%02d:%02d %d%c%+05d\",\n \t\t\t\tweekday_names[tm->tm_wday],\ndiff --git a/strbuf.c b/strbuf.c\nindex 00457940cf..b880107370 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -785,14 +785,47 @@ char *xstrfmt(const char *fmt, ...)\n \treturn ret;\n }\n \n-void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm)\n+void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm,\n+\t\t int tz_offset, const char *tz_name)\n {\n+\tstruct strbuf munged_fmt = STRBUF_INIT;\n \tsize_t hint = 128;\n \tsize_t len;\n \n \tif (!*fmt)\n \t\treturn;\n \n+\t/*\n+\t * There is no portable way to pass timezone information to\n+\t * strftime, so we handle %z and %Z here.\n+\t */\n+\tif (tz_name) {\n+\t\tfor (;;) {\n+\t\t\tconst char *percent = strchrnul(fmt, '%');\n+\t\t\tstrbuf_add(&munged_fmt, fmt, percent - fmt);\n+\t\t\tif (!*percent)\n+\t\t\t\tbreak;\n+\t\t\tfmt = percent + 1;\n+\t\t\tswitch (*fmt) {\n+\t\t\tcase '%':\n+\t\t\t\tstrbuf_addstr(&munged_fmt, \"%%\");\n+\t\t\t\tfmt++;\n+\t\t\t\tbreak;\n+\t\t\tcase 'z':\n+\t\t\t\tstrbuf_addf(&munged_fmt, \"%+05d\", tz_offset);\n+\t\t\t\tfmt++;\n+\t\t\t\tbreak;\n+\t\t\tcase 'Z':\n+\t\t\t\tstrbuf_addstr(&munged_fmt, tz_name);\n+\t\t\t\tfmt++;\n+\t\t\t\tbreak;\n+\t\t\tdefault:\n+\t\t\t\tstrbuf_addch(&munged_fmt, '%');\n+\t\t\t}\n+\t\t}\n+\t\tfmt = munged_fmt.buf;\n+\t}\n+\n \tstrbuf_grow(sb, hint);\n \tlen = strftime(sb->buf + sb->len, sb->alloc - sb->len, fmt, tm);\n \n@@ -804,17 +837,18 @@ void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm)\n \t\t * output contains at least one character, and then drop the extra\n \t\t * character before returning.\n \t\t */\n-\t\tstruct strbuf munged_fmt = STRBUF_INIT;\n-\t\tstrbuf_addf(&munged_fmt, \"%s \", fmt);\n+\t\tif (fmt != munged_fmt.buf)\n+\t\t\tstrbuf_addstr(&munged_fmt, fmt);\n+\t\tstrbuf_addch(&munged_fmt, ' ');\n \t\twhile (!len) {\n \t\t\thint *= 2;\n \t\t\tstrbuf_grow(sb, hint);\n \t\t\tlen = strftime(sb->buf + sb->len, sb->alloc - sb->len,\n \t\t\t\t munged_fmt.buf, tm);\n \t\t}\n-\t\tstrbuf_release(&munged_fmt);\n \t\tlen--; /* drop munged space */\n \t}\n+\tstrbuf_release(&munged_fmt);\n \tstrbuf_setlen(sb, sb->len + len);\n }\n \ndiff --git a/strbuf.h b/strbuf.h\nindex 80047b1bb7..39d5836abd 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -339,9 +339,14 @@ __attribute__((format (printf,2,0)))\n extern void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap);\n \n /**\n- * Add the time specified by `tm`, as formatted by `strftime`.\n- */\n-extern void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm);\n+ * Add the time specified by `tm`, as formatted by `strftime`. `tz_offset`\n+ * and `tz_name` are used to expand %z and %Z internally, unless `tz_name`\n+ * is NULL. `tz_offset` is in decimal hhmm format, e.g. -600 means six\n+ * hours west of Greenwich.\n+ */\n+extern void strbuf_addftime(struct strbuf *sb, const char *fmt,\n+\t\t\t const struct tm *tm, int tz_offset,\n+\t\t\t const char *tz_name);\n \n /**\n * Read a given size of data from a FILE* pointer to the buffer.\ndiff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh\nindex a1dcdb81d7..dc0bed8d90 100755\n--- a/t/t6006-rev-list-format.sh\n+++ b/t/t6006-rev-list-format.sh\n@@ -483,4 +483,16 @@ test_expect_success 'unused %G placeholders are passed through' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'date format \"%F %T %z\" is the same as iso' '\n+\tgit log -1 --format=\"%ad\" --date=iso >expect &&\n+\tgit log -1 --format=\"%ad\" --date=\"format:%F %T %z\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'date format \"%%z\" expands to percent zed' '\n+\techo \"%z\" >expect &&\n+\tgit log -1 --format=\"%ad\" --date=\"format:%%z\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.13.0\n"},{"id":"321478","messageId":"22834.46566.700197.257204@a1i15.kph.uni-mainz.de","threadId":"46051","inReplyTo":"a8b789e6-d0cd-6d96-1bfb-ccc5bc174013@web.de","subject":"Re: [PATCH] strbuf: let strbuf_addftime handle %z and %Z itself","fromName":"Ulrich Mueller","fromEmail":"ulm@gentoo.org","sentAt":"2017-06-03T13:13:10Z","receivedAt":"2017-06-03T13:13:21Z","isPatch":true,"sender":{"key":"ulm@gentoo.org","avatar":null},"body":">>>>> On Sat, 3 Jun 2017, René Scharfe wrote:\n\n> +\t\t\tcase 'Z':\n> +\t\t\t\tstrbuf_addstr(&munged_fmt, tz_name);\n\nIs it guaranteed that tz_name cannot contain a percent sign itself?\n\nUlrich\n"},{"id":"321481","messageId":"52a998be-edbc-e8fb-afa9-e0c7ab682425@web.de","threadId":"46051","inReplyTo":"22834.46566.700197.257204@a1i15.kph.uni-mainz.de","subject":"Re: [PATCH] strbuf: let strbuf_addftime handle %z and %Z itself","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-06-03T16:20:17Z","receivedAt":"2017-06-03T16:20:46Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 03.06.2017 um 15:13 schrieb Ulrich Mueller:\n>>>>>> On Sat, 3 Jun 2017, René Scharfe wrote:\n> \n>> +\t\t\tcase 'Z':\n>> +\t\t\t\tstrbuf_addstr(&munged_fmt, tz_name);\n> \n> Is it guaranteed that tz_name cannot contain a percent sign itself?\n\nCurrently yes, because the only caller passes an empty string.\n\nThe fact that tz_name is subject to expansion by strftime could be\nmentioned explicitly in strbuf.h. I'm not sure if that's a desirable\nproperty, but it allows callers to expand %z internally and %Z using\nstrftime.\n\nRené\n"},{"id":"321663","messageId":"20170607081729.6pz5yo2hmp4fwuas@sigill.intra.peff.net","threadId":"46051","inReplyTo":"a8b789e6-d0cd-6d96-1bfb-ccc5bc174013@web.de","subject":"Re: [PATCH] strbuf: let strbuf_addftime handle %z and %Z itself","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-06-07T08:17:29Z","receivedAt":"2017-06-07T08:17:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jun 03, 2017 at 12:40:34PM +0200, René Scharfe wrote:\n\n> There is no portable way to pass timezone information to strftime. Add\n> parameters for timezone offset and name to strbuf_addftime and let it\n> handle the timezone-related format specifiers %z and %Z internally.\n> Callers can opt out by passing NULL as timezone name.\n> \n> Use an empty string as timezone name in show_date (the only current\n> caller) for now because we only have the timezone offset in non-local\n> mode. POSIX allows %Z to resolve to nothing in case of missing info.\n\nThis direction looks good to me overall. It's not pretty, but I think\nit's the least-bad option.\n\n> ---\n> Duplicates strbuf_expand to a certain extent, but not too badly, I\n> think. Leaves the door open for letting strftime handle the local\n> case.\n\nI guess you'd plan to do that like this in the caller:\n\n if (date->local)\n\ttz_name = NULL;\n else\n\ttz_name = \"\";\n\nand then your strftime() doesn't do any %z expansion when tz_name is\nNULL.\n\nI was thinking that we would need to have it take the actual time_t, and\nthen it would be able to do the tzset/localtime dance itself. But since\nI don't think we're planning to do that (if anything we'd just handle\nthe normal localtime() case), the complication it would add to the\ninterface isn't worth it.\n\n> -void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm)\n> +void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm,\n> +\t\t int tz_offset, const char *tz_name)\n> {\n> +\tstruct strbuf munged_fmt = STRBUF_INIT;\n\nNow we're doing two types of munging: sometimes handling %z, and\nsometimes the extra-space hack. I had to read through it carefully to\nmake sure we handle all cases correctly, but I think it works.\n\nIn particular, I worried about us setting \"fmt\" to munged_fmt.buf for\nthe %z case, and then later adding the extra space to it for the\nzero-length hack, which might reallocate, leaving \"fmt\" pointing to\nunallocated memory. But it's OK because at that point we never touch the\noriginal \"fmt\" again.\n\n> /**\n> - * Add the time specified by `tm`, as formatted by `strftime`.\n> - */\n> -extern void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm);\n> + * Add the time specified by `tm`, as formatted by `strftime`. `tz_offset`\n> + * and `tz_name` are used to expand %z and %Z internally, unless `tz_name`\n> + * is NULL. `tz_offset` is in decimal hhmm format, e.g. -600 means six\n> + * hours west of Greenwich.\n> + */\n> +extern void strbuf_addftime(struct strbuf *sb, const char *fmt,\n> +\t\t\t const struct tm *tm, int tz_offset,\n> +\t\t\t const char *tz_name);\n\nGood, documentation (the diff order put the implementation first so I\nscratched my head for a moment before realizing you had already\ndescribed it).\n\n-Peff\n"},{"id":"321664","messageId":"20170607091307.2vh2eevgaljxu7ha@sigill.intra.peff.net","threadId":"46051","inReplyTo":"20170607081729.6pz5yo2hmp4fwuas@sigill.intra.peff.net","subject":"[PATCH] date: use localtime() for \"-local\" time formats","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-06-07T09:13:08Z","receivedAt":"2017-06-07T09:13:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 07, 2017 at 04:17:29AM -0400, Jeff King wrote:\n\n> > Duplicates strbuf_expand to a certain extent, but not too badly, I\n> > think. Leaves the door open for letting strftime handle the local\n> > case.\n> \n> I guess you'd plan to do that like this in the caller:\n> \n> if (date->local)\n> \ttz_name = NULL;\n> else\n> \ttz_name = \"\";\n> \n> and then your strftime() doesn't do any %z expansion when tz_name is\n> NULL.\n\nAnd here's a patch that handles the local case.\n\n-- >8 --\nSubject: [PATCH] date: use localtime() for \"-local\" time formats\n\nWhen we convert seconds-since-epochs timestamps into a\nbroken-down \"struct tm\", we do so by adjusting the timestamp\naccording to the correct timezone and then using gmtime() to\nbreak down the result. This means that the resulting struct\n\"knows\" that it's in GMT, even though the time it represents\nis adjusted for a different zone. The fields where it stores\nthis data are not portably accessible, so we have no way to\noverride them to tell them the real zone info.\n\nFor the most part, this works. Our date-formatting routines\ndon't pay attention to these inaccessible fields, and use\nthe the same tz info we provided for adjustment. The one\nexception is when we call strftime(), whose %z and %Z\nformats reveal this hidden timezone data.\n\nWe can't make this work in the general case, as there's no\nportable function for setting an arbitrary timezone. But for\nthe special case of the \"-local\" formats, we can just skip\nthe adjustment and use localtime() instead of gmtime(). This\nmakes --date=format-local:%Z work correctly, showing the\nlocal timezone instead of an empty string.\n\nThis patch adds three tests:\n\n 1. We check that format:%Z returns an empty string. This\n isn't what we'd want ideally, but it's important to\n confirm that it doesn't produce nonsense (like GMT when\n we are formatting another zone entirely).\n\n 2. We check that format-local:%Z produces \"UTC\", which is\n the value of $TZ set by test-lib.sh.\n\n 3. We check that format-local actually produces the\n correct time for zones other than UTC. If we made a\n mistake in the adjustment logic (say, applying the tz\n adjustment even though we are about to call\n localtime()), it wouldn't show up in the second test,\n because the offset for UTC is 0.\n\n We use the EST5 zone, which is already used elsewhere\n in the script, so is assumed to be available\n everywhere.\n\n However, this test _doesn't_ check %Z. That expansion\n produces an abbreviation which may not be portable\n across systems (on my system it expands as just \"EST\").\n Technically \"UTC\" could suffer from the same problem,\n but presumably it's universal enough to be relied upon.\n\nSigned-off-by: Jeff King \n---\n date.c | 14 ++++++++++++--\n t/t0006-date.sh | 20 ++++++++++++++++++--\n 2 files changed, 30 insertions(+), 4 deletions(-)\n\ndiff --git a/date.c b/date.c\nindex 558057733..1fd6d6637 100644\n--- a/date.c\n+++ b/date.c\n@@ -70,6 +70,12 @@ static struct tm *time_to_tm(timestamp_t time, int tz)\n \treturn gmtime(&t);\n }\n \n+static struct tm *time_to_tm_local(timestamp_t time)\n+{\n+\ttime_t t = time;\n+\treturn localtime(&t);\n+}\n+\n /*\n * What value of \"tz\" was in effect back then at \"time\" in the\n * local timezone?\n@@ -214,7 +220,10 @@ const char *show_date(timestamp_t time, int tz, const struct date_mode *mode)\n \t\treturn timebuf.buf;\n \t}\n \n-\ttm = time_to_tm(time, tz);\n+\tif (mode->local)\n+\t\ttm = time_to_tm_local(time);\n+\telse\n+\t\ttm = time_to_tm(time, tz);\n \tif (!tm) {\n \t\ttm = time_to_tm(0, 0);\n \t\ttz = 0;\n@@ -246,7 +255,8 @@ const char *show_date(timestamp_t time, int tz, const struct date_mode *mode)\n \t\t\tmonth_names[tm->tm_mon], tm->tm_year + 1900,\n \t\t\ttm->tm_hour, tm->tm_min, tm->tm_sec, tz);\n \telse if (mode->type == DATE_STRFTIME)\n-\t\tstrbuf_addftime(&timebuf, mode->strftime_fmt, tm, tz, \"\");\n+\t\tstrbuf_addftime(&timebuf, mode->strftime_fmt, tm, tz,\n+\t\t\t\tmode->local ? NULL : \"\");\n \telse\n \t\tstrbuf_addf(&timebuf, \"%.3s %.3s %d %02d:%02d:%02d %d%c%+05d\",\n \t\t\t\tweekday_names[tm->tm_wday],\ndiff --git a/t/t0006-date.sh b/t/t0006-date.sh\nindex 42d4ea61e..3be6692bb 100755\n--- a/t/t0006-date.sh\n+++ b/t/t0006-date.sh\n@@ -31,9 +31,18 @@ check_show () {\n \tformat=$1\n \ttime=$2\n \texpect=$3\n-\ttest_expect_success $4 \"show date ($format:$time)\" '\n+\tprereqs=$4\n+\tzone=$5\n+\ttest_expect_success $prereqs \"show date ($format:$time)\" '\n \t\techo \"$time -> $expect\" >expect &&\n-\t\ttest-date show:$format \"$time\" >actual &&\n+\t\t(\n+\t\t\tif test -n \"$zone\"\n+\t\t\tthen\n+\t\t\t\tTZ=$zone\n+\t\t\t\texport $TZ\n+\t\t\tfi &&\n+\t\t\ttest-date show:\"$format\" \"$time\" >actual\n+\t\t) &&\n \t\ttest_cmp expect actual\n \t'\n }\n@@ -51,6 +60,13 @@ check_show iso-local \"$TIME\" '2016-06-15 14:13:20 +0000'\n check_show raw-local \"$TIME\" '1466000000 +0000'\n check_show unix-local \"$TIME\" '1466000000'\n \n+check_show \"format:%Y-%m-%d %H:%M:%S %z (%Z)\" \"$TIME\" \\\n+\t '2016-06-15 16:13:20 +0200 ()'\n+check_show format-local:\"%Y-%m-%d %H:%M:%S %z (%Z)\" \"$TIME\" \\\n+\t '2016-06-15 14:13:20 +0000 (UTC)'\n+check_show format-local:\"%Y-%m-%d %H:%M:%S %z\" \"$TIME\" \\\n+\t '2016-06-15 09:13:20 -0500' '' EST5\n+\n # arbitrary time absurdly far in the future\n FUTURE=\"5758122296 -0400\"\n check_show iso \"$FUTURE\" \"2152-06-19 18:24:56 -0400\" TIME_IS_64BIT,TIME_T_IS_64BIT\n-- \n2.13.1.664.g1b5a21ec3\n\n"},{"id":"321937","messageId":"662a84da-8a66-3a37-d9d2-4ff8b5f996c3@web.de","threadId":"46051","inReplyTo":"20170607081729.6pz5yo2hmp4fwuas@sigill.intra.peff.net","subject":"Re: [PATCH] strbuf: let strbuf_addftime handle %z and %Z itself","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-06-11T17:36:47Z","receivedAt":"2017-06-11T17:37:25Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 07.06.2017 um 10:17 schrieb Jeff King:\n> On Sat, Jun 03, 2017 at 12:40:34PM +0200, René Scharfe wrote:\n>> Duplicates strbuf_expand to a certain extent, but not too badly, I\n>> think. Leaves the door open for letting strftime handle the local\n>> case.\n> \n> I guess you'd plan to do that like this in the caller:\n> \n> if (date->local)\n> \ttz_name = NULL;\n> else\n> \ttz_name = \"\";\n> \n> and then your strftime() doesn't do any %z expansion when tz_name is\n> NULL.\n\nYes, or you could look up a time zone name somewhere else -- except we\ndon't have a way to do that, at least for now.\n\n> I was thinking that we would need to have it take the actual time_t, and\n> then it would be able to do the tzset/localtime dance itself. But since\n> I don't think we're planning to do that (if anything we'd just handle\n> the normal localtime() case), the complication it would add to the\n> interface isn't worth it.\n\nA caller that really needs to do that can, and pass the result as a\nstring. Not pretty, but at least it's a possibility.\n\nRené\n"},{"id":"321956","messageId":"xmqq37b5qly8.fsf@gitster.mtv.corp.google.com","threadId":"46051","inReplyTo":"662a84da-8a66-3a37-d9d2-4ff8b5f996c3@web.de","subject":"Re: [PATCH] strbuf: let strbuf_addftime handle %z and %Z itself","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-06-12T15:12:31Z","receivedAt":"2017-06-12T15:12:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> Am 07.06.2017 um 10:17 schrieb Jeff King:\n>> On Sat, Jun 03, 2017 at 12:40:34PM +0200, René Scharfe wrote:\n>>> Duplicates strbuf_expand to a certain extent, but not too badly, I\n>>> think. Leaves the door open for letting strftime handle the local\n>>> case.\n>>\n>> I guess you'd plan to do that like this in the caller:\n>>\n>> if (date->local)\n>> \ttz_name = NULL;\n>> else\n>> \ttz_name = \"\";\n>>\n>> and then your strftime() doesn't do any %z expansion when tz_name is\n>> NULL.\n>\n> Yes, or you could look up a time zone name somewhere else -- except we\n> don't have a way to do that, at least for now.\n\nIs that only \"for now\"? I have a feeling that it is fundamentally\nimpossible with the data we record. When GMTOFF 9:00 is the only\nthing we have for a timestamp, can we tell if we should label it as\nJST (aka Asia/Tokyo) or KST (aka Asia/Seoul)?\n"},{"id":"321962","messageId":"CACBZZX5ofJC70S09rfL_EMK2KWAoPCMun1eisi+CXeX=FSwy6Q@mail.gmail.com","threadId":"46051","inReplyTo":"xmqq37b5qly8.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] strbuf: let strbuf_addftime handle %z and %Z itself","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-06-12T16:16:28Z","receivedAt":"2017-06-12T16:16:56Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Mon, Jun 12, 2017 at 5:12 PM, Junio C Hamano wrote:\n> René Scharfe writes:\n>\n>> Am 07.06.2017 um 10:17 schrieb Jeff King:\n>>> On Sat, Jun 03, 2017 at 12:40:34PM +0200, René Scharfe wrote:\n>>>> Duplicates strbuf_expand to a certain extent, but not too badly, I\n>>>> think. Leaves the door open for letting strftime handle the local\n>>>> case.\n>>>\n>>> I guess you'd plan to do that like this in the caller:\n>>>\n>>> if (date->local)\n>>> tz_name = NULL;\n>>> else\n>>> tz_name = \"\";\n>>>\n>>> and then your strftime() doesn't do any %z expansion when tz_name is\n>>> NULL.\n>>\n>> Yes, or you could look up a time zone name somewhere else -- except we\n>> don't have a way to do that, at least for now.\n>\n> Is that only \"for now\"? I have a feeling that it is fundamentally\n> impossible with the data we record. When GMTOFF 9:00 is the only\n> thing we have for a timestamp, can we tell if we should label it as\n> JST (aka Asia/Tokyo) or KST (aka Asia/Seoul)?\n\nIt's obviously not perfect for all the reasons mentioned in this\nthread, but we already have a timezone->offset mapping in the\ntimezone_names variable in date.c, a good enough solution might be to\nsimply reverse that lookup when formatting %Z\n\nOf course we can never know if you were in Tokyo or Seul from the info\nin the commit object, but we don't need to, it's enough that we just\nemit JST for +0900 and anyone reading the output has at least some\nidea what +0900 maps to.\n\nWe could also simply replace \"%Z\" with the empty string, as the the\nPOSIX strftime() documentation allows for:\nhttp://pubs.opengroup.org/onlinepubs/009695399/functions/strftime.html\n(\"Replaced by the timezone name or abbreviation, or by no bytes if no\ntimezone information exists.\").\n"},{"id":"321964","messageId":"22846.51138.555606.729612@a1i15.kph.uni-mainz.de","threadId":"46051","inReplyTo":"CACBZZX5ofJC70S09rfL_EMK2KWAoPCMun1eisi+CXeX=FSwy6Q@mail.gmail.com","subject":"Re: [PATCH] strbuf: let strbuf_addftime handle %z and %Z itself","fromName":"Ulrich Mueller","fromEmail":"ulm@gentoo.org","sentAt":"2017-06-12T16:56:34Z","receivedAt":"2017-06-12T16:56:51Z","isPatch":true,"sender":{"key":"ulm@gentoo.org","avatar":null},"body":">>>>> On Mon, 12 Jun 2017, Ævar Arnfjörð Bjarmason wrote:\n\n> On Mon, Jun 12, 2017 at 5:12 PM, Junio C Hamano wrote:\n>> René Scharfe writes:\n>>> Yes, or you could look up a time zone name somewhere else -- except we\n>>> don't have a way to do that, at least for now.\n>> \n>> Is that only \"for now\"? I have a feeling that it is fundamentally\n>> impossible with the data we record. When GMTOFF 9:00 is the only\n>> thing we have for a timestamp, can we tell if we should label it as\n>> JST (aka Asia/Tokyo) or KST (aka Asia/Seoul)?\n\n> It's obviously not perfect for all the reasons mentioned in this\n> thread, but we already have a timezone->offset mapping in the\n> timezone_names variable in date.c, a good enough solution might be to\n> simply reverse that lookup when formatting %Z\n\nThat cannot work, because there is no unique mapping from offset to\ntimezone name. For a given offset there may be several timezone names,\nor no name at all. (For example, if I decide to commit with my local\nmean time which has an offset of +0033 then there won't be any\ntimezone name at all.)\n\n> Of course we can never know if you were in Tokyo or Seul from the info\n> in the commit object, but we don't need to, it's enough that we just\n> emit JST for +0900 and anyone reading the output has at least some\n> idea what +0900 maps to.\n\nPlease don't. Outputting invented information for something that\nreally isn't in the data is worse than outputting no information at\nall.\n\n> We could also simply replace \"%Z\" with the empty string, as the the\n> POSIX strftime() documentation allows for:\n> http://pubs.opengroup.org/onlinepubs/009695399/functions/strftime.html\n> (\"Replaced by the timezone name or abbreviation, or by no bytes if no\n> timezone information exists.\").\n\nIMHO the two viable possibilities are the empty string, or the same\ninformation as for %z. But this has been said before in this thread.\n\ntzdata2017b has the following data in its \"etcetera\" file, the last\ncolumn being the timezone name:\n\n Zone\t Etc/GMT \t 0\t- GMT\n\n Zone\t Etc/GMT-14\t 14\t- +14\n Zone\t Etc/GMT-13\t 13\t- +13\n [...]\n Zone\t Etc/GMT+12\t -12\t- -12\n\nThis would be at least some rationale for converting %Z to [+-]hhmm,\ni.e., the same as for %z. (One could even think about [+-]hh if the\nminutes part is zero, but this already looks like over-engineering\nto me.)\n\nUlrich\n"},{"id":"321965","messageId":"d7e3abd9-0fac-8336-de34-dbd9aad39bd9@web.de","threadId":"46051","inReplyTo":"CACBZZX5ofJC70S09rfL_EMK2KWAoPCMun1eisi+CXeX=FSwy6Q@mail.gmail.com","subject":"Re: [PATCH] strbuf: let strbuf_addftime handle %z and %Z itself","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-06-12T16:58:13Z","receivedAt":"2017-06-12T16:59:01Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 12.06.2017 um 18:16 schrieb Ævar Arnfjörð Bjarmason:\n> On Mon, Jun 12, 2017 at 5:12 PM, Junio C Hamano wrote:\n>> René Scharfe writes:\n>>\n>>> Am 07.06.2017 um 10:17 schrieb Jeff King:\n>>>> On Sat, Jun 03, 2017 at 12:40:34PM +0200, René Scharfe wrote:\n>>>>> Duplicates strbuf_expand to a certain extent, but not too badly, I\n>>>>> think. Leaves the door open for letting strftime handle the local\n>>>>> case.\n>>>>\n>>>> I guess you'd plan to do that like this in the caller:\n>>>>\n>>>> if (date->local)\n>>>> tz_name = NULL;\n>>>> else\n>>>> tz_name = \"\";\n>>>>\n>>>> and then your strftime() doesn't do any %z expansion when tz_name is\n>>>> NULL.\n>>>\n>>> Yes, or you could look up a time zone name somewhere else -- except we\n>>> don't have a way to do that, at least for now.\n>>\n>> Is that only \"for now\"? I have a feeling that it is fundamentally\n>> impossible with the data we record. When GMTOFF 9:00 is the only\n>> thing we have for a timestamp, can we tell if we should label it as\n>> JST (aka Asia/Tokyo) or KST (aka Asia/Seoul)?\n\nWe could track the time zone on commit, e.g. in a new header. I doubt\nthat the need for showing time zone names will be strong enough to go to\nthat length, though.\n\n> It's obviously not perfect for all the reasons mentioned in this\n> thread, but we already have a timezone->offset mapping in the\n> timezone_names variable in date.c, a good enough solution might be to\n> simply reverse that lookup when formatting %Z\n> \n> Of course we can never know if you were in Tokyo or Seul from the info\n> in the commit object, but we don't need to, it's enough that we just\n> emit JST for +0900 and anyone reading the output has at least some\n> idea what +0900 maps to.\n\nI suspect that there will be different opinions, for cultural and\npolitical reasons.\n\n> We could also simply replace \"%Z\" with the empty string, as the the\n> POSIX strftime() documentation allows for:\n> http://pubs.opengroup.org/onlinepubs/009695399/functions/strftime.html\n> (\"Replaced by the timezone name or abbreviation, or by no bytes if no\n> timezone information exists.\").\n\nYes, that's what the patch does.\n\nRené\n"},{"id":"321968","messageId":"20170612173601.kpdkxb23r6shssl6@sigill.intra.peff.net","threadId":"46051","inReplyTo":"d7e3abd9-0fac-8336-de34-dbd9aad39bd9@web.de","subject":"Re: [PATCH] strbuf: let strbuf_addftime handle %z and %Z itself","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-06-12T17:36:01Z","receivedAt":"2017-06-12T17:36:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 12, 2017 at 06:58:13PM +0200, René Scharfe wrote:\n\n> > We could also simply replace \"%Z\" with the empty string, as the the\n> > POSIX strftime() documentation allows for:\n> > http://pubs.opengroup.org/onlinepubs/009695399/functions/strftime.html\n> > (\"Replaced by the timezone name or abbreviation, or by no bytes if no\n> > timezone information exists.\").\n> \n> Yes, that's what the patch does.\n\nYeah, after thinking about all that was said in this thread, the empty\nstring is the only thing that makes sense to me.\n\nIMHO, your patch as-is is a strict improvement. Mine on top (to handle\nthe \"-local\" cases) is optional, but I think worth doing. It makes the\nrule easy to understand and explain (\"-local\" makes %Z behave as\nexpected, and the non-local variants leave it blank because the\ninformation isn't present).\n\n-Peff\n"},{"id":"321971","messageId":"CACBZZX6AH2nEGPHMq6XOLDxr4SH9v-zT_YGovLXN_ZQ+fB345g@mail.gmail.com","threadId":"46051","inReplyTo":"22846.51138.555606.729612@a1i15.kph.uni-mainz.de","subject":"Re: [PATCH] strbuf: let strbuf_addftime handle %z and %Z itself","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-06-12T17:53:21Z","receivedAt":"2017-06-12T17:53:49Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Mon, Jun 12, 2017 at 6:56 PM, Ulrich Mueller wrote:\n>>>>>> On Mon, 12 Jun 2017, Ęvar Arnfjörš Bjarmason wrote:\n>\n>> On Mon, Jun 12, 2017 at 5:12 PM, Junio C Hamano wrote:\n>>> René Scharfe writes:\n>>>> Yes, or you could look up a time zone name somewhere else -- except we\n>>>> don't have a way to do that, at least for now.\n>>>\n>>> Is that only \"for now\"? I have a feeling that it is fundamentally\n>>> impossible with the data we record. When GMTOFF 9:00 is the only\n>>> thing we have for a timestamp, can we tell if we should label it as\n>>> JST (aka Asia/Tokyo) or KST (aka Asia/Seoul)?\n>\n>> It's obviously not perfect for all the reasons mentioned in this\n>> thread, but we already have a timezone->offset mapping in the\n>> timezone_names variable in date.c, a good enough solution might be to\n>> simply reverse that lookup when formatting %Z\n>\n> That cannot work, because there is no unique mapping from offset to\n> timezone name. For a given offset there may be several timezone names,\n> or no name at all. (For example, if I decide to commit with my local\n> mean time which has an offset of +0033 then there won't be any\n> timezone name at all.)\n\nYes, I understand that it's a 1:many not a 1:1 mapping. What I'm\nsaying is that %Z in the context of git-log means a human readable\ntimezone name, and we can just make a best-effort at guessing that by\npicking 1/many out of the 1:many mapping.\n\nThis obviously doesn't give us the user's timezone, but when you're\nbrowsing through git log you can get an approximate human-readable\nidea of what timezone someone was located in, and that's the whole\npoint of %Z in the context of how it would be used via git-log, isn't\nit?\n\n>> Of course we can never know if you were in Tokyo or Seul from the info\n>> in the commit object, but we don't need to, it's enough that we just\n>> emit JST for +0900 and anyone reading the output has at least some\n>> idea what +0900 maps to.\n>\n> Please don't. Outputting invented information for something that\n> really isn't in the data is worse than outputting no information at\n> all.\n\nIt's not invented information. %z being +0900 is the same thing as %Z\nbeing JST since +0900 == JST, just because some other things are also\n== +0900 that doesn't mean that JST is invalid or less useful than\n+0900 or \"\" for the purposes of human-readable output.\n"},{"id":"321977","messageId":"xmqqefupnkcs.fsf@gitster.mtv.corp.google.com","threadId":"46051","inReplyTo":"CACBZZX6AH2nEGPHMq6XOLDxr4SH9v-zT_YGovLXN_ZQ+fB345g@mail.gmail.com","subject":"Re: [PATCH] strbuf: let strbuf_addftime handle %z and %Z itself","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-06-12T18:15:15Z","receivedAt":"2017-06-12T18:15:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> On Mon, Jun 12, 2017 at 6:56 PM, Ulrich Mueller wrote:\n>\n>> Please don't. Outputting invented information for something that\n>> really isn't in the data is worse than outputting no information at\n>> all.\n>\n> It's not invented information. %z being +0900 is the same thing as %Z\n> being JST since +0900 == JST, just because some other things are also\n> == +0900 that doesn't mean that JST is invalid or less useful than\n> +0900 or \"\" for the purposes of human-readable output.\n\nUlrich is right. The issue is not if JST is a possibly correct\nanswer.\n\nGMT offset alone does not tell us anything about which one of the\nzones that happen to share the same value is associated with the\ncommitter (or author) time, and picking one at random to pretend\nthat we know more than we actually do is a disservice to the users.\n\n"},{"id":"321978","messageId":"20170612182045.z4d37ph5uqqhwmas@sigill.intra.peff.net","threadId":"46051","inReplyTo":"CACBZZX6AH2nEGPHMq6XOLDxr4SH9v-zT_YGovLXN_ZQ+fB345g@mail.gmail.com","subject":"Re: [PATCH] strbuf: let strbuf_addftime handle %z and %Z itself","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-06-12T18:20:45Z","receivedAt":"2017-06-12T18:20:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 12, 2017 at 07:53:21PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> >> Of course we can never know if you were in Tokyo or Seul from the info\n> >> in the commit object, but we don't need to, it's enough that we just\n> >> emit JST for +0900 and anyone reading the output has at least some\n> >> idea what +0900 maps to.\n> >\n> > Please don't. Outputting invented information for something that\n> > really isn't in the data is worse than outputting no information at\n> > all.\n> \n> It's not invented information. %z being +0900 is the same thing as %Z\n> being JST since +0900 == JST, just because some other things are also\n> == +0900 that doesn't mean that JST is invalid or less useful than\n> +0900 or \"\" for the purposes of human-readable output.\n\nI think the main problem is that the mapping isn't just \"JST->+0900\".\nIt's a set of rules that depend on the specific time being converted. So\nit's true that at some time t, +0900 may mean JST or KST or whatever.\nBut at a different time, those time zones will map to a different\noffset.\n\nSo I think at best this is semantically confusing (the author was not in\nJST, but just happened to be in a time zone that maps to the same thing\nat that moment, and any attempt to extrapolate their zone at another\ntime is going to be wrong). And at worst the output of \"git log\" is\ngoing to show the same user flip-flopping between zones as their commits\nmove around with respect to the (politically determined) zone changes.\nFor instance in my time zone I'd shift between EST and CST twice per\nyear due to DST. My real time zone is EST5EDT, or better still,\nAmerica/New_York (I don't live there, but the Olson database carves up\nregions based on the most populous city in the zone).\n\n-Peff\n"},{"id":"321982","messageId":"CACBZZX5OQc45fUyDVayE89rkT=+8m5S4efSXCAbCy7Upme5zLA@mail.gmail.com","threadId":"46051","inReplyTo":"20170612182045.z4d37ph5uqqhwmas@sigill.intra.peff.net","subject":"Re: [PATCH] strbuf: let strbuf_addftime handle %z and %Z itself","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-06-12T19:02:32Z","receivedAt":"2017-06-12T19:02:59Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Mon, Jun 12, 2017 at 8:20 PM, Jeff King wrote:\n> On Mon, Jun 12, 2017 at 07:53:21PM +0200, Ævar Arnfjörð Bjarmason wrote:\n>\n>> >> Of course we can never know if you were in Tokyo or Seul from the info\n>> >> in the commit object, but we don't need to, it's enough that we just\n>> >> emit JST for +0900 and anyone reading the output has at least some\n>> >> idea what +0900 maps to.\n>> >\n>> > Please don't. Outputting invented information for something that\n>> > really isn't in the data is worse than outputting no information at\n>> > all.\n>>\n>> It's not invented information. %z being +0900 is the same thing as %Z\n>> being JST since +0900 == JST, just because some other things are also\n>> == +0900 that doesn't mean that JST is invalid or less useful than\n>> +0900 or \"\" for the purposes of human-readable output.\n>\n> I think the main problem is that the mapping isn't just \"JST->+0900\".\n> It's a set of rules that depend on the specific time being converted. So\n> it's true that at some time t, +0900 may mean JST or KST or whatever.\n> But at a different time, those time zones will map to a different\n> offset.\n\nTo the extent that it doesn't map correctly (e.g. due to summer time)\nthat's already a bug/understood limitation in the fuzzy TZ parser in\ndate.c, isn't it (dating back to Linus's \"In my world, it's always\nsummer\" comment in 89967023da).\n\n> So I think at best this is semantically confusing (the author was not in\n> JST, but just happened to be in a time zone that maps to the same thing\n> at that moment, and any attempt to extrapolate their zone at another\n> time is going to be wrong). And at worst the output of \"git log\" is\n> going to show the same user flip-flopping between zones as their commits\n> move around with respect to the (politically determined) zone changes.\n> For instance in my time zone I'd shift between EST and CST twice per\n> year due to DST. My real time zone is EST5EDT, or better still,\n> America/New_York (I don't live there, but the Olson database carves up\n> regions based on the most populous city in the zone).\n\nI think everyone in this thread disagrees with what I think makes\nsense for %Z, and I'm not going to argue the point, I just thought I'd\nchime in because it seemed to me that the discussion had missed the\nmapping we had in the timezone_names variable.\n\nBut FWIW and just to (I think fairly) summarize, here's where we differ.\n\nYou (and others) think that unless we can actually show the user's\ntime zone as it appeared on their machine %Z shouldn't show anything\nat all, since it would be confusing. Fair enough.\n\nI only ever use the time offset info to quickly make a mental note of\n\"oh +0200, this guy's in Europe\", or \"oh -0400 America East\". Having\nany info at all for %Z would allow me to easily replace that already\nbuggy mapping that exists in my head right now with something that's\nat least a bit more accurate, e.g. I remember that +0900 is Japan, but\nI can't now offhand recall what timezones India is at.\n\nNow, obviously we can't know whether to translate e.g. -0400 to New\nYork or Santiago, but for the purposes of human readable output I\nthought it would be more useful to guess than show nothing at all. So\nfor that purpose the most useful output of %Z *for me* would be just\nshowing a string with the 3 most notable cities/areas, weighted for\nshowing locations on different continents, e.g.:\n\n +0000 = Iceland, West Africa, Ittoqqortoormiit\n +0100 = London, Lisbon, Kinshasa\n ...\n +0900 = Tokyo, Seul, Manokwari\n ....\n -0900 = San Francisco, Vancouver, Tijuana\n ....\n -0600 = Denver, Managua, Calgary\n\nThen I could run:\n\n git log --date=format-local:\"%Y[...](%Z)\"\n\nAnd get output like:\n\n commit 826c06412e (HEAD -> master, origin/master, origin/HEAD)\n Author: Junio C Hamano \n Date: Fri Jun 2 15:07:36 2017 +0900 (San Francisco, Vancouver,\nTijuana etc.)\n\n Fifth batch for 2.14\n [...]\n commit 30d005c020\n Author: Jeff King \n Date: Fri May 19 08:59:34 2017 -0400 (New York, Havana, Santiago etc.)\n\n diff: use blob path for blob/file diffs\n\nWhich gives me a pretty good idea of where the people who are making\nmy colleges / collaborators who are making commits all over the world\nare located, for the purposes of reinforcing the abstract numeric\nmapping with a best-guess at what the location might be, or at least\nsomething that's close to the actual location.\n\nI'd definitely use a feature like that, and could hack it up on top of\nthe current patches if there wasn't vehement opposition to it. Maybe\nthe above examples change someone's mind, I can't think of a case\nwhere someone's relying on %Z now for date-local, or at least cases\nwhere something like the above wouldn't be more useful in practice\nthan just an empty string, but if nobody agrees so be it :)\n"},{"id":"321991","messageId":"20170612211022.k7t2ywngrdcrkehn@sigill.intra.peff.net","threadId":"46051","inReplyTo":"CACBZZX5OQc45fUyDVayE89rkT=+8m5S4efSXCAbCy7Upme5zLA@mail.gmail.com","subject":"Re: [PATCH] strbuf: let strbuf_addftime handle %z and %Z itself","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-06-12T21:10:22Z","receivedAt":"2017-06-12T21:10:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 12, 2017 at 09:02:32PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> > I think the main problem is that the mapping isn't just \"JST->+0900\".\n> > It's a set of rules that depend on the specific time being converted. So\n> > it's true that at some time t, +0900 may mean JST or KST or whatever.\n> > But at a different time, those time zones will map to a different\n> > offset.\n> \n> To the extent that it doesn't map correctly (e.g. due to summer time)\n> that's already a bug/understood limitation in the fuzzy TZ parser in\n> date.c, isn't it (dating back to Linus's \"In my world, it's always\n> summer\" comment in 89967023da).\n\nTo be honest, I find that whole timezone_names[] array pretty gross. It\ngets basic stuff wrong like:\n\n $ t/helper/test-date parse '2017-02-01 00:00:00 PST8PDT'\n 2017-02-01 00:00:00 PST8PDT -> 2017-02-01 00:00:00 -0700\n\nThat should be -0800 because it's winter (though the bigger problem is\nthat our parser is too stupid and just sees \"PDT\" at the end and guesses\nit's summer).\n\nI suspect nobody has complained because we generally encourage real\n\"-0800\" names when specifying zones. Which is really just punting the\nproblem to whoever is generating the timestamp, but it works in\npractice. And we use real localtime() for handling things in the current\nzone, which is usually what's responsible for generating any such time.\nSo just using the local zone, both of these are right (notice the zone\ndifference in the summer):\n\n $ TZ=PST8PDT t/helper/test-date parse '2017-02-01 00:00:00'\n 2017-02-01 00:00:00 -> 2017-02-01 00:00:00 -0800\n\n $ TZ=PST8PDT t/helper/test-date parse '2017-06-01 00:00:00'\n 2017-06-01 00:00:00 -> 2017-06-01 00:00:00 -0700\n\nIt would be nice to fix that. In theory strptime is the solution, but\ncomments in date.c indicate that it doesn't work well (and I can well\nbelieve that). Presumably there is some not-terrible date library out\nthere that really can parse/format in arbitrary time-zones (perl's\nDateTime is pleasant to use, for example). But I don't know of a C\nlibrary offhand (and unless we can ship it with the source, I'd guess\nthat the extra dependency isn't worth it).\n\n> I think everyone in this thread disagrees with what I think makes\n> sense for %Z, and I'm not going to argue the point, I just thought I'd\n> chime in because it seemed to me that the discussion had missed the\n> mapping we had in the timezone_names variable.\n\nI did actually forget we had it, though I don't think it changes my\nopinion.\n\n> But FWIW and just to (I think fairly) summarize, here's where we differ.\n> \n> You (and others) think that unless we can actually show the user's\n> time zone as it appeared on their machine %Z shouldn't show anything\n> at all, since it would be confusing. Fair enough.\n> \n> I only ever use the time offset info to quickly make a mental note of\n> \"oh +0200, this guy's in Europe\", or \"oh -0400 America East\". Having\n> any info at all for %Z would allow me to easily replace that already\n> buggy mapping that exists in my head right now with something that's\n> at least a bit more accurate, e.g. I remember that +0900 is Japan, but\n> I can't now offhand recall what timezones India is at.\n\nI can't either. But nor could I tell you which timezone abbreviations\nthey use. ;)\n\n> Author: Jeff King \n> Date: Fri May 19 08:59:34 2017 -0400 (New York, Havana, Santiago etc.)\n\nBut I don't live in any of those places. :)\n\nI see your point, though. I wouldn't want that on, but I wouldn't have\nan objection to turning that into a configurable feature. But I'm not\nsure we would just want it plumbed through to %Z. It seems like you'd\npotentially want it on for any dates that show the numeric zone.\n\nThe biggest headache would be managing the reverse-mapping to cities. It\nwould be nice if there were some way to access the system zone database,\nbut I doubt there's a portable way to do so. And besides, it would\nprobably come up with way too many options (e.g., even just in the US\n-0500 encompasses not just what we normally think of as Eastern Time,\nbut lots of little localities who do or do not handle DST; we obviously\nwouldn't want to mention them, but I don't know how we'd limit the tz\ndatabase entries to only \"you know, the important ones\").\n\n-Peff\n"},{"id":"322030","messageId":"c7532160-53eb-a0c5-94e6-483aadf0cfc5@web.de","threadId":"46051","inReplyTo":"CACBZZX5OQc45fUyDVayE89rkT=+8m5S4efSXCAbCy7Upme5zLA@mail.gmail.com","subject":"Re: [PATCH] strbuf: let strbuf_addftime handle %z and %Z itself","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-06-12T22:31:44Z","receivedAt":"2017-06-12T22:32:07Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 12.06.2017 um 21:02 schrieb Ævar Arnfjörð Bjarmason:\n> I only ever use the time offset info to quickly make a mental note of\n> \"oh +0200, this guy's in Europe\", or \"oh -0400 America East\". Having\n> any info at all for %Z would allow me to easily replace that already\n> buggy mapping that exists in my head right now with something that's\n> at least a bit more accurate, e.g. I remember that +0900 is Japan, but\n> I can't now offhand recall what timezones India is at.\n> \n> Now, obviously we can't know whether to translate e.g. -0400 to New\n> York or Santiago, but for the purposes of human readable output I\n> thought it would be more useful to guess than show nothing at all. So\n> for that purpose the most useful output of %Z *for me* would be just\n> showing a string with the 3 most notable cities/areas, weighted for\n> showing locations on different continents, e.g.:\n> \n> +0000 = Iceland, West Africa, Ittoqqortoormiit\n> +0100 = London, Lisbon, Kinshasa\n> ...\n> +0900 = Tokyo, Seul, Manokwari\n> ....\n> -0900 = San Francisco, Vancouver, Tijuana\n> ....\n> -0600 = Denver, Managua, Calgary\n> \n> Then I could run:\n> \n> git log --date=format-local:\"%Y[...](%Z)\"\n> \n> And get output like:\n> \n> commit 826c06412e (HEAD -> master, origin/master, origin/HEAD)\n> Author: Junio C Hamano \n> Date: Fri Jun 2 15:07:36 2017 +0900 (San Francisco, Vancouver,\n> Tijuana etc.)\n> \n> Fifth batch for 2.14\n> [...]\n> commit 30d005c020\n> Author: Jeff King \n> Date: Fri May 19 08:59:34 2017 -0400 (New York, Havana, Santiago etc.)\n> \n> diff: use blob path for blob/file diffs\n> \n> Which gives me a pretty good idea of where the people who are making\n> my colleges / collaborators who are making commits all over the world\n> are located, for the purposes of reinforcing the abstract numeric\n> mapping with a best-guess at what the location might be, or at least\n> something that's close to the actual location.\n\nHalf the time this would be off by a zone in areas that use daylight\nsaving time, or you'd need to determine when DST starts and ends, but\nsince that depends on the exact time zone it will be tricky.\n\nYou could use military time zones, which are nice and easy to convert:\nAlpha (UTC+1) to Mike (UTC+12) (Juliet is skipped), November (UTC-1) to \nYankee (UTC-12), and Zulu time zone (UTC+0). Downside: Most civilians\ndon't use them. :) Also there are no names for zones that have an\noffset of a fraction of an hour.\n\n> I'd definitely use a feature like that, and could hack it up on top of\n> the current patches if there wasn't vehement opposition to it. Maybe\n> the above examples change someone's mind, I can't think of a case\n> where someone's relying on %Z now for date-local, or at least cases\n> where something like the above wouldn't be more useful in practice\n> than just an empty string, but if nobody agrees so be it :)\n\nPersonally I don't have a use for time information; dates alone would\nsuffice for me -- so I certainly won't hold you back. But I don't see\nhow it could be done.\n\nRené\n"},{"id":"322049","messageId":"CA+55aFz5TC5XWWEZkf=e3rJZFwvQJoFyvWSaNSTiQZM3-yiOKQ@mail.gmail.com","threadId":"46051","inReplyTo":"20170612211022.k7t2ywngrdcrkehn@sigill.intra.peff.net","subject":"Re: [PATCH] strbuf: let strbuf_addftime handle %z and %Z itself","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2017-06-13T06:23:43Z","receivedAt":"2017-06-13T06:23:51Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Mon, Jun 12, 2017 at 2:10 PM, Jeff King wrote:\n>\n> I suspect nobody has complained because we generally encourage real\n> \"-0800\" names when specifying zones.\n\nThat's what any sane person uses, and it's what SMTP requiries.\n\nThe timezone names are a (bad) joke. If a human can't understand them\n(and people don't), there's no point in thinking that a computer\nshould either.\n\nSo yes, timezones should strive very much to be numeric. The names are\nan incomprehensible mess. Don't rely on them, and try to avoid using\nthem.\n\n Linus\n"},{"id":"322064","messageId":"CACBZZX4w1OKYZ2uiBSB7S1hO7tUzqvfWskmCJmMmtBebNRDPCQ@mail.gmail.com","threadId":"46051","inReplyTo":"c7532160-53eb-a0c5-94e6-483aadf0cfc5@web.de","subject":"Re: [PATCH] strbuf: let strbuf_addftime handle %z and %Z itself","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-06-13T10:16:01Z","receivedAt":"2017-06-13T10:16:41Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, Jun 13, 2017 at 12:31 AM, René Scharfe wrote:\n> Am 12.06.2017 um 21:02 schrieb Ævar Arnfjörð Bjarmason:\n>>\n>> I only ever use the time offset info to quickly make a mental note of\n>> \"oh +0200, this guy's in Europe\", or \"oh -0400 America East\". Having\n>> any info at all for %Z would allow me to easily replace that already\n>> buggy mapping that exists in my head right now with something that's\n>> at least a bit more accurate, e.g. I remember that +0900 is Japan, but\n>> I can't now offhand recall what timezones India is at.\n>>\n>> Now, obviously we can't know whether to translate e.g. -0400 to New\n>> York or Santiago, but for the purposes of human readable output I\n>> thought it would be more useful to guess than show nothing at all. So\n>> for that purpose the most useful output of %Z *for me* would be just\n>> showing a string with the 3 most notable cities/areas, weighted for\n>> showing locations on different continents, e.g.:\n>>\n>> +0000 = Iceland, West Africa, Ittoqqortoormiit\n>> +0100 = London, Lisbon, Kinshasa\n>> ...\n>> +0900 = Tokyo, Seul, Manokwari\n>> ....\n>> -0900 = San Francisco, Vancouver, Tijuana\n>> ....\n>> -0600 = Denver, Managua, Calgary\n>>\n>> Then I could run:\n>>\n>> git log --date=format-local:\"%Y[...](%Z)\"\n>>\n>> And get output like:\n>>\n>> commit 826c06412e (HEAD -> master, origin/master, origin/HEAD)\n>> Author: Junio C Hamano \n>> Date: Fri Jun 2 15:07:36 2017 +0900 (San Francisco, Vancouver,\n>> Tijuana etc.)\n>>\n>> Fifth batch for 2.14\n>> [...]\n>> commit 30d005c020\n>> Author: Jeff King \n>> Date: Fri May 19 08:59:34 2017 -0400 (New York, Havana, Santiago\n>> etc.)\n>>\n>> diff: use blob path for blob/file diffs\n>>\n>> Which gives me a pretty good idea of where the people who are making\n>> my colleges / collaborators who are making commits all over the world\n>> are located, for the purposes of reinforcing the abstract numeric\n>> mapping with a best-guess at what the location might be, or at least\n>> something that's close to the actual location.\n>\n> Half the time this would be off by a zone in areas that use daylight\n> saving time, or you'd need to determine when DST starts and ends, but\n> since that depends on the exact time zone it will be tricky.\n\nYes, not to beat this point to death, but to clarify since we still\nseem to be talking about different things:\n\nI'm not saying it isn't fuzzy and inaccurate, I'm saying there's a\nuse-case for human readable output where saying e.g. \"somewhere New\nYork-ish\" is more useful than nothing, even given summer time I'm\nstill going to know approximately what timezone it is, more so than\nthe offset number.\n\nAnd that is, I would argue, the best thing to do when the user is via\n%Z asking for a human-readable timezone-ish given a commit object,\nwhich only has the offset.\n\n> You could use military time zones, which are nice and easy to convert:\n> Alpha (UTC+1) to Mike (UTC+12) (Juliet is skipped), November (UTC-1) to\n> Yankee (UTC-12), and Zulu time zone (UTC+0). Downside: Most civilians\n> don't use them. :) Also there are no names for zones that have an\n> offset of a fraction of an hour.\n\nYes I think if we're going down the road of providing some\nhuman-readable version of the offset this is near-useless since almost\nnobody using Git is going to know these by heart.\n\n>> I'd definitely use a feature like that, and could hack it up on top of\n>> the current patches if there wasn't vehement opposition to it. Maybe\n>> the above examples change someone's mind, I can't think of a case\n>> where someone's relying on %Z now for date-local, or at least cases\n>> where something like the above wouldn't be more useful in practice\n>> than just an empty string, but if nobody agrees so be it :)\n>\n> Personally I don't have a use for time information; dates alone would\n> suffice for me -- so I certainly won't hold you back. But I don't see\n> how it could be done.\n\nIt could be done by accepting that you're not going to get a perfect\nsolution but one that's good enough to aid the humans currently\nreading the currently only numeric output.\n\nMy use-case for this is having colleges all over the world who create\ncommits using different time offsets. It's simply easier to at a\nglance realize what office someone is in if \"git log\" is printing city\nnames in/near those timezones, even if none of those cities are where\nthose people are located.\n"},{"id":"322065","messageId":"22847.48875.297447.531147@a1i15.kph.uni-mainz.de","threadId":"46051","inReplyTo":"c7532160-53eb-a0c5-94e6-483aadf0cfc5@web.de","subject":"Re: [PATCH] strbuf: let strbuf_addftime handle %z and %Z itself","fromName":"Ulrich Mueller","fromEmail":"ulm@gentoo.org","sentAt":"2017-06-13T10:31:07Z","receivedAt":"2017-06-13T10:31:23Z","isPatch":true,"sender":{"key":"ulm@gentoo.org","avatar":null},"body":">>>>> On Tue, 13 Jun 2017, René Scharfe wrote:\n\n> Am 12.06.2017 um 21:02 schrieb Ævar Arnfjörð Bjarmason:\n>> Which gives me a pretty good idea of where the people who are making\n>> my colleges / collaborators who are making commits all over the world\n>> are located, for the purposes of reinforcing the abstract numeric\n>> mapping with a best-guess at what the location might be, or at least\n>> something that's close to the actual location.\n\n> Half the time this would be off by a zone in areas that use daylight\n> saving time, or you'd need to determine when DST starts and ends, but\n> since that depends on the exact time zone it will be tricky.\n\nAnd sometimes it would be impossible since DST rules differ between\nhemispheres. For example, there is Europe/Berlin vs Africa/Windhoek,\nor America/Halifax vs America/Santiago.\n\n> You could use military time zones, which are nice and easy to convert:\n> Alpha (UTC+1) to Mike (UTC+12) (Juliet is skipped), November (UTC-1) to \n> Yankee (UTC-12), and Zulu time zone (UTC+0). Downside: Most civilians\n> don't use them. :)\n\nYeah, that would ensure complete confusion. :) You'd have \"Quebec\"\nfor -0400 whereas the city of Quebec is in zone -0500. Similar for\n\"Lima\" denoting +1100.\n\n> Also there are no names for zones that have an offset of a fraction\n> of an hour.\n\nThere are also zones like Pacific/Tongatapu which have +1300 (and +1400\nin summer).\n\nAll in all I think that Jeff's suggestion makes most sense: Expand %Z\nto the timezone name for the -local case, when the name is readily\navailable. Otherwise, expand it to the empty string.\n\nUlrich\n"},{"id":"322301","messageId":"d578d104-79c6-6c07-db4d-3e4ccb0c0dd9@web.de","threadId":"46051","inReplyTo":"a8b789e6-d0cd-6d96-1bfb-ccc5bc174013@web.de","subject":"[PATCH v2] strbuf: let strbuf_addftime handle %z and %Z itself","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-06-15T08:46:07Z","receivedAt":"2017-06-15T08:46:42Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"There is no portable way to pass timezone information to strftime. Add\nparameters for timezone offset and name to strbuf_addftime and let it\nhandle the timezone-related format specifiers %z and %Z internally.\n\nCallers can opt out for %Z by passing NULL as timezone name. %z is\nalways handled internally -- this helps on Windows, where strftime would\nexpand it to a timezone name (same as %Z), in violation of POSIX.\nModifiers are not handled, e.g. %Ez is still passed to strftime.\n\nUse an empty string as timezone name in show_date (the only current\ncaller) for now because we only have the timezone offset in non-local\nmode. POSIX allows %Z to resolve to an empty string in case of missing\ninformation.\n\nHelped-by: Ulrich Mueller \nHelped-by: Jeff King \nSigned-off-by: Rene Scharfe \n---\nChanges from v1:\n- Always handle %z internally.\n- Move tests from t6006 to t0006, as that's a more appropriate place.\n- Changed tests to only use %%, %z and %Z to avoid incompatibilities.\n- Tested on mingw (applies there with patch and some fuzz).\n\n date.c | 2 +-\n strbuf.c | 41 +++++++++++++++++++++++++++++++++++++----\n strbuf.h | 11 ++++++++---\n t/t0006-date.sh | 6 ++++++\n 4 files changed, 52 insertions(+), 8 deletions(-)\n\ndiff --git a/date.c b/date.c\nindex 63fa99685e..5580577334 100644\n--- a/date.c\n+++ b/date.c\n@@ -246,7 +246,7 @@ const char *show_date(timestamp_t time, int tz, const struct date_mode *mode)\n \t\t\tmonth_names[tm->tm_mon], tm->tm_year + 1900,\n \t\t\ttm->tm_hour, tm->tm_min, tm->tm_sec, tz);\n \telse if (mode->type == DATE_STRFTIME)\n-\t\tstrbuf_addftime(&timebuf, mode->strftime_fmt, tm);\n+\t\tstrbuf_addftime(&timebuf, mode->strftime_fmt, tm, tz, \"\");\n \telse\n \t\tstrbuf_addf(&timebuf, \"%.3s %.3s %d %02d:%02d:%02d %d%c%+05d\",\n \t\t\t\tweekday_names[tm->tm_wday],\ndiff --git a/strbuf.c b/strbuf.c\nindex 00457940cf..be3b9e37b1 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -785,14 +785,48 @@ char *xstrfmt(const char *fmt, ...)\n \treturn ret;\n }\n \n-void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm)\n+void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm,\n+\t\t int tz_offset, const char *tz_name)\n {\n+\tstruct strbuf munged_fmt = STRBUF_INIT;\n \tsize_t hint = 128;\n \tsize_t len;\n \n \tif (!*fmt)\n \t\treturn;\n \n+\t/*\n+\t * There is no portable way to pass timezone information to\n+\t * strftime, so we handle %z and %Z here.\n+\t */\n+\tfor (;;) {\n+\t\tconst char *percent = strchrnul(fmt, '%');\n+\t\tstrbuf_add(&munged_fmt, fmt, percent - fmt);\n+\t\tif (!*percent)\n+\t\t\tbreak;\n+\t\tfmt = percent + 1;\n+\t\tswitch (*fmt) {\n+\t\tcase '%':\n+\t\t\tstrbuf_addstr(&munged_fmt, \"%%\");\n+\t\t\tfmt++;\n+\t\t\tbreak;\n+\t\tcase 'z':\n+\t\t\tstrbuf_addf(&munged_fmt, \"%+05d\", tz_offset);\n+\t\t\tfmt++;\n+\t\t\tbreak;\n+\t\tcase 'Z':\n+\t\t\tif (tz_name) {\n+\t\t\t\tstrbuf_addstr(&munged_fmt, tz_name);\n+\t\t\t\tfmt++;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\t/* FALLTHROUGH */\n+\t\tdefault:\n+\t\t\tstrbuf_addch(&munged_fmt, '%');\n+\t\t}\n+\t}\n+\tfmt = munged_fmt.buf;\n+\n \tstrbuf_grow(sb, hint);\n \tlen = strftime(sb->buf + sb->len, sb->alloc - sb->len, fmt, tm);\n \n@@ -804,17 +838,16 @@ void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm)\n \t\t * output contains at least one character, and then drop the extra\n \t\t * character before returning.\n \t\t */\n-\t\tstruct strbuf munged_fmt = STRBUF_INIT;\n-\t\tstrbuf_addf(&munged_fmt, \"%s \", fmt);\n+\t\tstrbuf_addch(&munged_fmt, ' ');\n \t\twhile (!len) {\n \t\t\thint *= 2;\n \t\t\tstrbuf_grow(sb, hint);\n \t\t\tlen = strftime(sb->buf + sb->len, sb->alloc - sb->len,\n \t\t\t\t munged_fmt.buf, tm);\n \t\t}\n-\t\tstrbuf_release(&munged_fmt);\n \t\tlen--; /* drop munged space */\n \t}\n+\tstrbuf_release(&munged_fmt);\n \tstrbuf_setlen(sb, sb->len + len);\n }\n \ndiff --git a/strbuf.h b/strbuf.h\nindex 80047b1bb7..39d5836abd 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -339,9 +339,14 @@ __attribute__((format (printf,2,0)))\n extern void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap);\n \n /**\n- * Add the time specified by `tm`, as formatted by `strftime`.\n- */\n-extern void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm);\n+ * Add the time specified by `tm`, as formatted by `strftime`. `tz_offset`\n+ * and `tz_name` are used to expand %z and %Z internally, unless `tz_name`\n+ * is NULL. `tz_offset` is in decimal hhmm format, e.g. -600 means six\n+ * hours west of Greenwich.\n+ */\n+extern void strbuf_addftime(struct strbuf *sb, const char *fmt,\n+\t\t\t const struct tm *tm, int tz_offset,\n+\t\t\t const char *tz_name);\n \n /**\n * Read a given size of data from a FILE* pointer to the buffer.\ndiff --git a/t/t0006-date.sh b/t/t0006-date.sh\nindex 42d4ea61ef..71082008f0 100755\n--- a/t/t0006-date.sh\n+++ b/t/t0006-date.sh\n@@ -51,6 +51,12 @@ check_show iso-local \"$TIME\" '2016-06-15 14:13:20 +0000'\n check_show raw-local \"$TIME\" '1466000000 +0000'\n check_show unix-local \"$TIME\" '1466000000'\n \n+check_show 'format:%z' \"$TIME\" '+0200'\n+check_show 'format-local:%z' \"$TIME\" '+0000'\n+check_show 'format:%Z' \"$TIME\" ''\n+check_show 'format:%%z' \"$TIME\" '%z'\n+check_show 'format-local:%%z' \"$TIME\" '%z'\n+\n # arbitrary time absurdly far in the future\n FUTURE=\"5758122296 -0400\"\n check_show iso \"$FUTURE\" \"2152-06-19 18:24:56 -0400\" TIME_IS_64BIT,TIME_T_IS_64BIT\n-- \n2.13.0\n\n"},{"id":"322311","messageId":"22850.28477.715057.441985@a1i15.kph.uni-mainz.de","threadId":"46051","inReplyTo":"d578d104-79c6-6c07-db4d-3e4ccb0c0dd9@web.de","subject":"Re: [PATCH v2] strbuf: let strbuf_addftime handle %z and %Z itself","fromName":"Ulrich Mueller","fromEmail":"ulm@gentoo.org","sentAt":"2017-06-15T11:27:57Z","receivedAt":"2017-06-15T11:28:09Z","isPatch":true,"sender":{"key":"ulm@gentoo.org","avatar":null},"body":">>>>> On Thu, 15 Jun 2017, René Scharfe wrote:\n\n> Callers can opt out for %Z by passing NULL as timezone name. %z is\n> always handled internally -- this helps on Windows, where strftime would\n> expand it to a timezone name (same as %Z), in violation of POSIX.\n> Modifiers are not handled, e.g. %Ez is still passed to strftime.\n\nPOSIX would also allow other things, like a field width:\nhttp://pubs.opengroup.org/onlinepubs/9699919799/functions/strftime.html\n\n$ date '+%8z'\n+0000200\n\n(But I believe that's not very useful, and supporting it might require\nduplicating much of strftime's code.)\n\n> Changes from v1:\n> - Always handle %z internally.\n\nMinor nitpick: Shouldn't the comment in strbuf.h be updated to reflect\nthat change?\n\n> + * Add the time specified by `tm`, as formatted by `strftime`. `tz_offset`\n> + * and `tz_name` are used to expand %z and %Z internally, unless `tz_name`\n> + * is NULL. `tz_offset` is in decimal hhmm format, e.g. -600 means six\n> + * hours west of Greenwich.\n\nUlrich\n"},{"id":"322316","messageId":"02cf275a-8eaa-49b9-7c2a-f98424088e1e@web.de","threadId":"46051","inReplyTo":"22850.28477.715057.441985@a1i15.kph.uni-mainz.de","subject":"Re: [PATCH v2] strbuf: let strbuf_addftime handle %z and %Z itself","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-06-15T12:28:22Z","receivedAt":"2017-06-15T12:28:57Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 15.06.2017 um 13:27 schrieb Ulrich Mueller:\n>>>>>> On Thu, 15 Jun 2017, René Scharfe wrote:\n> \n>> Callers can opt out for %Z by passing NULL as timezone name. %z is\n>> always handled internally -- this helps on Windows, where strftime would\n>> expand it to a timezone name (same as %Z), in violation of POSIX.\n>> Modifiers are not handled, e.g. %Ez is still passed to strftime.\n> \n> POSIX would also allow other things, like a field width:\n> http://pubs.opengroup.org/onlinepubs/9699919799/functions/strftime.html\n> \n> $ date '+%8z'\n> +0000200\n> \n> (But I believe that's not very useful, and supporting it might require\n> duplicating much of strftime's code.)\n\nWindows doesn't support that (unsurprisingly), but it accepts %#z,\nwhich does the same as %z. Let's wait for someone to request support\nfor modifiers and just document the behavior for now.\n\n>> Changes from v1:\n>> - Always handle %z internally.\n> \n> Minor nitpick: Shouldn't the comment in strbuf.h be updated to reflect\n> that change?\n> \n>> + * Add the time specified by `tm`, as formatted by `strftime`. `tz_offset`\n>> + * and `tz_name` are used to expand %z and %Z internally, unless `tz_name`\n>> + * is NULL. `tz_offset` is in decimal hhmm format, e.g. -600 means six\n>> + * hours west of Greenwich.\n\nYes, it should. Thanks for paying attention! :)\n\nRené\n"},{"id":"322317","messageId":"5f398cf6-2bc3-f999-acfa-0ef6d7a43435@web.de","threadId":"46051","inReplyTo":"d578d104-79c6-6c07-db4d-3e4ccb0c0dd9@web.de","subject":"[PATCH v3] strbuf: let strbuf_addftime handle %z and %Z itself","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-06-15T12:29:53Z","receivedAt":"2017-06-15T12:30:18Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"There is no portable way to pass timezone information to strftime. Add\nparameters for timezone offset and name to strbuf_addftime and let it\nhandle the timezone-related format specifiers %z and %Z internally.\n\nCallers can opt out for %Z by passing NULL as timezone name. %z is\nalways handled internally -- this helps on Windows, where strftime would\nexpand it to a timezone name (same as %Z), in violation of POSIX.\nModifiers are not handled, e.g. %Ez is still passed to strftime.\n\nUse an empty string as timezone name in show_date (the only current\ncaller) for now because we only have the timezone offset in non-local\nmode. POSIX allows %Z to resolve to an empty string in case of missing\ninformation.\n\nHelped-by: Ulrich Mueller \nHelped-by: Jeff King \nSigned-off-by: Rene Scharfe \n---\nChanges from v3:\n- Updated developer documentation in strbuf.h.\n- Added short note to user documentation.\n\n Documentation/rev-list-options.txt | 3 ++-\n date.c | 2 +-\n strbuf.c | 41 ++++++++++++++++++++++++++++++++++----\n strbuf.h | 10 ++++++++--\n t/t0006-date.sh | 6 ++++++\n 5 files changed, 54 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex a46f70c2b1..34ae2553f1 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -768,7 +768,8 @@ timezone value.\n 1970). As with `--raw`, this is always in UTC and therefore `-local`\n has no effect.\n +\n-`--date=format:...` feeds the format `...` to your system `strftime`.\n+`--date=format:...` feeds the format `...` to your system `strftime`,\n+except for %z and %Z, which are handled internally.\n Use `--date=format:%c` to show the date in your system locale's\n preferred format. See the `strftime` manual for a complete list of\n format placeholders. When using `-local`, the correct syntax is\ndiff --git a/date.c b/date.c\nindex 63fa99685e..5580577334 100644\n--- a/date.c\n+++ b/date.c\n@@ -246,7 +246,7 @@ const char *show_date(timestamp_t time, int tz, const struct date_mode *mode)\n \t\t\tmonth_names[tm->tm_mon], tm->tm_year + 1900,\n \t\t\ttm->tm_hour, tm->tm_min, tm->tm_sec, tz);\n \telse if (mode->type == DATE_STRFTIME)\n-\t\tstrbuf_addftime(&timebuf, mode->strftime_fmt, tm);\n+\t\tstrbuf_addftime(&timebuf, mode->strftime_fmt, tm, tz, \"\");\n \telse\n \t\tstrbuf_addf(&timebuf, \"%.3s %.3s %d %02d:%02d:%02d %d%c%+05d\",\n \t\t\t\tweekday_names[tm->tm_wday],\ndiff --git a/strbuf.c b/strbuf.c\nindex 00457940cf..be3b9e37b1 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -785,14 +785,48 @@ char *xstrfmt(const char *fmt, ...)\n \treturn ret;\n }\n \n-void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm)\n+void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm,\n+\t\t int tz_offset, const char *tz_name)\n {\n+\tstruct strbuf munged_fmt = STRBUF_INIT;\n \tsize_t hint = 128;\n \tsize_t len;\n \n \tif (!*fmt)\n \t\treturn;\n \n+\t/*\n+\t * There is no portable way to pass timezone information to\n+\t * strftime, so we handle %z and %Z here.\n+\t */\n+\tfor (;;) {\n+\t\tconst char *percent = strchrnul(fmt, '%');\n+\t\tstrbuf_add(&munged_fmt, fmt, percent - fmt);\n+\t\tif (!*percent)\n+\t\t\tbreak;\n+\t\tfmt = percent + 1;\n+\t\tswitch (*fmt) {\n+\t\tcase '%':\n+\t\t\tstrbuf_addstr(&munged_fmt, \"%%\");\n+\t\t\tfmt++;\n+\t\t\tbreak;\n+\t\tcase 'z':\n+\t\t\tstrbuf_addf(&munged_fmt, \"%+05d\", tz_offset);\n+\t\t\tfmt++;\n+\t\t\tbreak;\n+\t\tcase 'Z':\n+\t\t\tif (tz_name) {\n+\t\t\t\tstrbuf_addstr(&munged_fmt, tz_name);\n+\t\t\t\tfmt++;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\t/* FALLTHROUGH */\n+\t\tdefault:\n+\t\t\tstrbuf_addch(&munged_fmt, '%');\n+\t\t}\n+\t}\n+\tfmt = munged_fmt.buf;\n+\n \tstrbuf_grow(sb, hint);\n \tlen = strftime(sb->buf + sb->len, sb->alloc - sb->len, fmt, tm);\n \n@@ -804,17 +838,16 @@ void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm)\n \t\t * output contains at least one character, and then drop the extra\n \t\t * character before returning.\n \t\t */\n-\t\tstruct strbuf munged_fmt = STRBUF_INIT;\n-\t\tstrbuf_addf(&munged_fmt, \"%s \", fmt);\n+\t\tstrbuf_addch(&munged_fmt, ' ');\n \t\twhile (!len) {\n \t\t\thint *= 2;\n \t\t\tstrbuf_grow(sb, hint);\n \t\t\tlen = strftime(sb->buf + sb->len, sb->alloc - sb->len,\n \t\t\t\t munged_fmt.buf, tm);\n \t\t}\n-\t\tstrbuf_release(&munged_fmt);\n \t\tlen--; /* drop munged space */\n \t}\n+\tstrbuf_release(&munged_fmt);\n \tstrbuf_setlen(sb, sb->len + len);\n }\n \ndiff --git a/strbuf.h b/strbuf.h\nindex 80047b1bb7..4559035c47 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -340,8 +340,14 @@ extern void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap);\n \n /**\n * Add the time specified by `tm`, as formatted by `strftime`.\n- */\n-extern void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm);\n+ * `tz_name` is used to expand %Z internally unless it's NULL.\n+ * `tz_offset` is in decimal hhmm format, e.g. -600 means six hours west\n+ * of Greenwich, and it's used to expand %z internally. However, tokens\n+ * with modifiers (e.g. %Ez) are passed to `strftime`.\n+ */\n+extern void strbuf_addftime(struct strbuf *sb, const char *fmt,\n+\t\t\t const struct tm *tm, int tz_offset,\n+\t\t\t const char *tz_name);\n \n /**\n * Read a given size of data from a FILE* pointer to the buffer.\ndiff --git a/t/t0006-date.sh b/t/t0006-date.sh\nindex 42d4ea61ef..71082008f0 100755\n--- a/t/t0006-date.sh\n+++ b/t/t0006-date.sh\n@@ -51,6 +51,12 @@ check_show iso-local \"$TIME\" '2016-06-15 14:13:20 +0000'\n check_show raw-local \"$TIME\" '1466000000 +0000'\n check_show unix-local \"$TIME\" '1466000000'\n \n+check_show 'format:%z' \"$TIME\" '+0200'\n+check_show 'format-local:%z' \"$TIME\" '+0000'\n+check_show 'format:%Z' \"$TIME\" ''\n+check_show 'format:%%z' \"$TIME\" '%z'\n+check_show 'format-local:%%z' \"$TIME\" '%z'\n+\n # arbitrary time absurdly far in the future\n FUTURE=\"5758122296 -0400\"\n check_show iso \"$FUTURE\" \"2152-06-19 18:24:56 -0400\" TIME_IS_64BIT,TIME_T_IS_64BIT\n-- \n2.13.0\n\n"},{"id":"322322","messageId":"20170615134958.mzmdmhonjsnconu2@sigill.intra.peff.net","threadId":"46051","inReplyTo":"5f398cf6-2bc3-f999-acfa-0ef6d7a43435@web.de","subject":"Re: [PATCH v3] strbuf: let strbuf_addftime handle %z and %Z itself","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-06-15T13:49:58Z","receivedAt":"2017-06-15T13:50:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 15, 2017 at 02:29:53PM +0200, René Scharfe wrote:\n\n> There is no portable way to pass timezone information to strftime. Add\n> parameters for timezone offset and name to strbuf_addftime and let it\n> handle the timezone-related format specifiers %z and %Z internally.\n> \n> Callers can opt out for %Z by passing NULL as timezone name. %z is\n> always handled internally -- this helps on Windows, where strftime would\n> expand it to a timezone name (same as %Z), in violation of POSIX.\n> Modifiers are not handled, e.g. %Ez is still passed to strftime.\n> \n> Use an empty string as timezone name in show_date (the only current\n> caller) for now because we only have the timezone offset in non-local\n> mode. POSIX allows %Z to resolve to an empty string in case of missing\n> information.\n> \n> Helped-by: Ulrich Mueller \n> Helped-by: Jeff King \n> Signed-off-by: Rene Scharfe \n> ---\n> Changes from v3:\n> - Updated developer documentation in strbuf.h.\n> - Added short note to user documentation.\n\nThis looks good to me overall.\n\n> diff --git a/t/t0006-date.sh b/t/t0006-date.sh\n> index 42d4ea61ef..71082008f0 100755\n> --- a/t/t0006-date.sh\n> +++ b/t/t0006-date.sh\n> @@ -51,6 +51,12 @@ check_show iso-local \"$TIME\" '2016-06-15 14:13:20 +0000'\n> check_show raw-local \"$TIME\" '1466000000 +0000'\n> check_show unix-local \"$TIME\" '1466000000'\n> \n> +check_show 'format:%z' \"$TIME\" '+0200'\n> +check_show 'format-local:%z' \"$TIME\" '+0000'\n> +check_show 'format:%Z' \"$TIME\" ''\n> +check_show 'format:%%z' \"$TIME\" '%z'\n> +check_show 'format-local:%%z' \"$TIME\" '%z'\n\nThese check that the zone output is correct, but I don't think we ever\ncheck that the value we feed to strftime is actually in the correct zone\nin the first place (i.e., that %H shows the correct time).\n\nI think that should go in a separate test from the %z/%Z handling, as\nthere are some subtleties.\n\nSo here are two patches on top of yours: more tests, and then the\nformat-local handling of %Z.\n\n [1/2]: t0006: check --date=format zone offsets\n [2/2]: date: use localtime() for \"-local\" time formats\n\n date.c | 14 ++++++++++++--\n t/t0006-date.sh | 10 ++++++++--\n 2 files changed, 20 insertions(+), 4 deletions(-)\n\n-Peff\n"},{"id":"322323","messageId":"20170615135121.jrrxng6ktjfe3wth@sigill.intra.peff.net","threadId":"46051","inReplyTo":"20170615134958.mzmdmhonjsnconu2@sigill.intra.peff.net","subject":"[PATCH 1/2] t0006: check --date=format zone offsets","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-06-15T13:51:22Z","receivedAt":"2017-06-15T13:51:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We already test that \"%z\" and \"%Z\" show the right thing, but\nwe don't actually check that the time we display is the\ncorrect one. Let's add two new tests:\n\n 1. Test that \"format:\" shows the time in the author's\n timezone, just like the other time formats.\n\n 2. Test that \"format-local:\" shows time in the local\n timezone. We don't want to use our normal UTC for this,\n because its offset is zero (so the result would be\n \"correct\" even if the code forgot to apply the offset\n or applied it in the wrong direction).\n\n We'll use the EST5 zone, which is already used\n elsewhere in the script (and so is assumed to be\n available everywhere).\n\nSigned-off-by: Jeff King \n---\n t/t0006-date.sh | 9 +++++++--\n 1 file changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t0006-date.sh b/t/t0006-date.sh\nindex 71082008f..9f81bec7a 100755\n--- a/t/t0006-date.sh\n+++ b/t/t0006-date.sh\n@@ -31,9 +31,11 @@ check_show () {\n \tformat=$1\n \ttime=$2\n \texpect=$3\n-\ttest_expect_success $4 \"show date ($format:$time)\" '\n+\tprereqs=$4\n+\tzone=$5\n+\ttest_expect_success $prereqs \"show date ($format:$time)\" '\n \t\techo \"$time -> $expect\" >expect &&\n-\t\ttest-date show:$format \"$time\" >actual &&\n+\t\tTZ=${zone:-$TZ} test-date show:\"$format\" \"$time\" >actual &&\n \t\ttest_cmp expect actual\n \t'\n }\n@@ -57,6 +59,9 @@ check_show 'format:%Z' \"$TIME\" ''\n check_show 'format:%%z' \"$TIME\" '%z'\n check_show 'format-local:%%z' \"$TIME\" '%z'\n \n+check_show 'format:%Y-%m-%d %H:%M:%S' \"$TIME\" '2016-06-15 16:13:20'\n+check_show 'format-local:%Y-%m-%d %H:%M:%S' \"$TIME\" '2016-06-15 09:13:20' '' EST5\n+\n # arbitrary time absurdly far in the future\n FUTURE=\"5758122296 -0400\"\n check_show iso \"$FUTURE\" \"2152-06-19 18:24:56 -0400\" TIME_IS_64BIT,TIME_T_IS_64BIT\n-- \n2.13.1.766.g6bea926c5\n\n"},{"id":"322324","messageId":"20170615135216.2jfsrjpicku6zxv3@sigill.intra.peff.net","threadId":"46051","inReplyTo":"20170615134958.mzmdmhonjsnconu2@sigill.intra.peff.net","subject":"[PATCH 2/2] date: use localtime() for \"-local\" time formats","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-06-15T13:52:17Z","receivedAt":"2017-06-15T13:52:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When we convert seconds-since-epochs timestamps into a\nbroken-down \"struct tm\", we do so by adjusting the timestamp\naccording to the known offset and then using gmtime() to\nbreak down the result. This means that the resulting struct\n\"knows\" that it's in GMT, even though the time it represents\nis adjusted for a different zone. The fields where it stores\nthis data are not portably accessible, so we have no way to\noverride them to tell them the real zone info.\n\nFor the most part, this works. Our date-formatting routines\ndon't pay attention to these inaccessible fields, and use\nthe same tz info we provided for adjustment. The one\nexception is when we call strftime(), whose %Z format\nreveals this hidden timezone data.\n\nWe solved that by always showing the empty string for %Z.\nThis is allowed by POSIX, but not very helpful to the user.\nWe can't make this work in the general case, as there's no\nportable function for setting an arbitrary timezone (and\nanyway, we don't have the zone name for the author zones,\nonly their offsets).\n\nBut for the special case of the \"-local\" formats, we can\njust skip the adjustment and use localtime() instead of\ngmtime(). This makes --date=format-local:%Z work correctly,\nshowing the local timezone instead of an empty string.\n\nThe new test checks the result for \"UTC\", our default\ntest-lib value for $TZ. Using something like EST5 might be\nmore interesting, but the actual zone string is\nsystem-dependent (for instance, on my system it expands to\njust EST). Hopefully \"UTC\" is vanilla enough that every\nsystem treats it the same.\n\nSigned-off-by: Jeff King \n---\nI don't have a Windows system to test this on, but from the output Dscho\nprovided earlier, I believe this should pass.\n\n date.c | 14 ++++++++++++--\n t/t0006-date.sh | 1 +\n 2 files changed, 13 insertions(+), 2 deletions(-)\n\ndiff --git a/date.c b/date.c\nindex 558057733..1fd6d6637 100644\n--- a/date.c\n+++ b/date.c\n@@ -70,6 +70,12 @@ static struct tm *time_to_tm(timestamp_t time, int tz)\n \treturn gmtime(&t);\n }\n \n+static struct tm *time_to_tm_local(timestamp_t time)\n+{\n+\ttime_t t = time;\n+\treturn localtime(&t);\n+}\n+\n /*\n * What value of \"tz\" was in effect back then at \"time\" in the\n * local timezone?\n@@ -214,7 +220,10 @@ const char *show_date(timestamp_t time, int tz, const struct date_mode *mode)\n \t\treturn timebuf.buf;\n \t}\n \n-\ttm = time_to_tm(time, tz);\n+\tif (mode->local)\n+\t\ttm = time_to_tm_local(time);\n+\telse\n+\t\ttm = time_to_tm(time, tz);\n \tif (!tm) {\n \t\ttm = time_to_tm(0, 0);\n \t\ttz = 0;\n@@ -246,7 +255,8 @@ const char *show_date(timestamp_t time, int tz, const struct date_mode *mode)\n \t\t\tmonth_names[tm->tm_mon], tm->tm_year + 1900,\n \t\t\ttm->tm_hour, tm->tm_min, tm->tm_sec, tz);\n \telse if (mode->type == DATE_STRFTIME)\n-\t\tstrbuf_addftime(&timebuf, mode->strftime_fmt, tm, tz, \"\");\n+\t\tstrbuf_addftime(&timebuf, mode->strftime_fmt, tm, tz,\n+\t\t\t\tmode->local ? NULL : \"\");\n \telse\n \t\tstrbuf_addf(&timebuf, \"%.3s %.3s %d %02d:%02d:%02d %d%c%+05d\",\n \t\t\t\tweekday_names[tm->tm_wday],\ndiff --git a/t/t0006-date.sh b/t/t0006-date.sh\nindex 9f81bec7a..7ac9466d5 100755\n--- a/t/t0006-date.sh\n+++ b/t/t0006-date.sh\n@@ -56,6 +56,7 @@ check_show unix-local \"$TIME\" '1466000000'\n check_show 'format:%z' \"$TIME\" '+0200'\n check_show 'format-local:%z' \"$TIME\" '+0000'\n check_show 'format:%Z' \"$TIME\" ''\n+check_show 'format-local:%Z' \"$TIME\" 'UTC'\n check_show 'format:%%z' \"$TIME\" '%z'\n check_show 'format-local:%%z' \"$TIME\" '%z'\n \n-- \n2.13.1.766.g6bea926c5\n"},{"id":"322358","messageId":"b79d1c3d-e43e-a82b-2d33-2283cb0aa5ef@web.de","threadId":"46051","inReplyTo":"20170615135216.2jfsrjpicku6zxv3@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] date: use localtime() for \"-local\" time formats","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-06-15T16:12:31Z","receivedAt":"2017-06-15T16:12:55Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 15.06.2017 um 15:52 schrieb Jeff King:\n> But for the special case of the \"-local\" formats, we can\n> just skip the adjustment and use localtime() instead of\n> gmtime(). This makes --date=format-local:%Z work correctly,\n> showing the local timezone instead of an empty string.\n\nDocumentation/rev-list-options.txt should be updated to mention that %Z\nis passed to strftime in the local case, no?\n\n> The new test checks the result for \"UTC\", our default\n> test-lib value for $TZ. Using something like EST5 might be\n> more interesting, but the actual zone string is\n> system-dependent (for instance, on my system it expands to\n> just EST). Hopefully \"UTC\" is vanilla enough that every\n> system treats it the same.\n> \n> Signed-off-by: Jeff King \n> ---\n> I don't have a Windows system to test this on, but from the output Dscho\n> provided earlier, I believe this should pass.\n\nThe first patch applies with some fuzz on master of Git for Windows, the\nsecond one applies cleanly. A \"typedef unsigned long timestamp_t;\" is\nrequired to compile it; such a fixup won't be needed for long, I guess.\nt0006 succeeds.\n\nRené\n"},{"id":"322403","messageId":"xmqqinjxylno.fsf@gitster.mtv.corp.google.com","threadId":"46051","inReplyTo":"b79d1c3d-e43e-a82b-2d33-2283cb0aa5ef@web.de","subject":"Re: [PATCH 2/2] date: use localtime() for \"-local\" time formats","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-06-15T21:40:43Z","receivedAt":"2017-06-15T21:40:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> Am 15.06.2017 um 15:52 schrieb Jeff King:\n>> But for the special case of the \"-local\" formats, we can\n>> just skip the adjustment and use localtime() instead of\n>> gmtime(). This makes --date=format-local:%Z work correctly,\n>> showing the local timezone instead of an empty string.\n>\n> Documentation/rev-list-options.txt should be updated to mention that %Z\n> is passed to strftime in the local case, no?\n>\n>> The new test checks the result for \"UTC\", our default\n>> test-lib value for $TZ. Using something like EST5 might be\n>> more interesting, but the actual zone string is\n>> system-dependent (for instance, on my system it expands to\n>> just EST). Hopefully \"UTC\" is vanilla enough that every\n>> system treats it the same.\n>>\n>> Signed-off-by: Jeff King \n>> ---\n>> I don't have a Windows system to test this on, but from the output Dscho\n>> provided earlier, I believe this should pass.\n>\n> The first patch applies with some fuzz on master of Git for Windows, the\n> second one applies cleanly. A \"typedef unsigned long timestamp_t;\" is\n> required to compile it; such a fixup won't be needed for long, I guess.\n> t0006 succeeds.\n\nThis time, I'll do s/timestamp_t/unsigned long/ while queuing ;-)\nOf course, it needs to be converted back to timestamp_t when the\ntopic is merged to a more recent integration branch that knows about\nthe type.\n\nThanks.\n"},{"id":"322456","messageId":"20170616121859.oas4v2ddleflufus@sigill.intra.peff.net","threadId":"46051","inReplyTo":"b79d1c3d-e43e-a82b-2d33-2283cb0aa5ef@web.de","subject":"Re: [PATCH 2/2] date: use localtime() for \"-local\" time formats","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-06-16T12:18:59Z","receivedAt":"2017-06-16T12:19:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 15, 2017 at 06:12:31PM +0200, René Scharfe wrote:\n\n> Am 15.06.2017 um 15:52 schrieb Jeff King:\n> > But for the special case of the \"-local\" formats, we can\n> > just skip the adjustment and use localtime() instead of\n> > gmtime(). This makes --date=format-local:%Z work correctly,\n> > showing the local timezone instead of an empty string.\n> \n> Documentation/rev-list-options.txt should be updated to mention that %Z\n> is passed to strftime in the local case, no?\n\nI wasn't sure if we wanted to get into that. Your documentation update\n(with an empty string) says only that they are \"handled internally\".\nWhile it is true that we aren't handling %Z internally anymore, the\npoint is that we try to do something sane which may or may not match\nwhat your system strftime() does. And that continues to be the case\nafter my patch.\n\nSo unless we are going to give the full breakdown of when %Z is empty\nand when it is not, I prefer to leave it unspecified.\n\n> > I don't have a Windows system to test this on, but from the output Dscho\n> > provided earlier, I believe this should pass.\n> \n> The first patch applies with some fuzz on master of Git for Windows, the\n> second one applies cleanly. A \"typedef unsigned long timestamp_t;\" is\n> required to compile it; such a fixup won't be needed for long, I guess.\n> t0006 succeeds.\n\nThanks for checking.\n\n-Peff\n"}]}