git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: git-2.13.0: log --date=format:%z not working

From
Jeff King <peff@peff.net>
Date
Jun 2, 2017, 18:35 UTC
Message-ID
<20170602183504.ii7arq2ssxgwgyxr@sigill.intra.peff.net>
In-Reply-To
<72b001fc-80e7-42b9-bd9d-87621da7978a@web.de>
On Fri, Jun 02, 2017 at 07:25:43PM +0200, René Scharfe wrote:
Show 24 quoted lines
> Am 02.06.2017 um 05:08 schrieb Jeff King:
> > In theory the solution is:
> > 
> >    1. Start using localtime() instead of gmtime() with an adjustment when
> >       we are converting to the local timezone (i.e., format-local). We
> >       should be able to do this portably.
> > 
> >       This is easy to do, and it's better than handling %z ourselves,
> >       because it makes %Z work, too.
> > 
> >    2. When showing the author's timezone, do some trickery to set the
> >       program's timezone, then use localtime(), then restore the program
> >       timezone.
> > 
> >       I couldn't get this to work reliably. And anyway, we'd still have
> >       nothing to put in %Z since we don't have a timezone name at all in
> >       the git objects. We just have "+0400" or whatever.
> > 
> > So I don't see a portable way to make (2) work.
> 
> We could create a strftime wrapper that also takes a time zone offset,
> with platform-specific implementations.  Is it worth the effort?
> 
> What reliability issues did you run into?

My patch is below for reference. The issue is that we have to stuff a name into $TZ that the system libc will parse into something sensible. Just setting it to "%+05d" doesn't work at all. glibc at least seems to accept names like FOO+4, but:

  - I have no idea if that's portable
  - it only allows single-hour offsets, so +0330 is out. There might be
    some way to represent that, but I'm not sure if it's portable
    (FOO+0300 doesn't seem to work even on glibc).
  - that sets %Z to "FOO", which is obviously nonsense
Show 15 quoted lines
> > If we do handle "%z" ourselves (either always or for just the one case),
> > what should the matching %Z say? Right now (and I think with René's
> > patch) it says GMT, which is actively misleading. We should probably
> > replace it with the same text as "%z". That's not quite what the user
> > wanted, but at least it's accurate.
> 
> On Linux "%z %Z" is expanded to "+0200 CEST" for me, while on Windows I
> get "Mitteleurop▒ische Sommerzeit Mitteleurop▒ische Sommerzeit".  (That
> "▒" is probably supposed to be an "ä".)  POSIX requires  +hhmm or -hhmm
> format for %z, and for %Z is to be "Replaced by the timezone name or
> abbreviation".
> 
> I'd say "GMT+0200" etc. is a nice enough timezone name, i.e. having %Z
> resolve to the same as %z plus a literal prefix of "GMT" should at least
> not be wrong.

I thought that, too, but I think it is wrong based on my understanding of how $TZ is parsed. There something like "EDT-4" means "call this EDT, and by the way it is 4 hours behind GMT".

So what you're proposing isn't wrong per se, but your notation means something totally different than what similar-looking notation looks like on the $TZ end, which is bound to create confusion.

> Alternatively we could have a lookup table mapping a few typical offsets
> to timezone names, but e.g. handling daylight saving times would
> probably be too hard (when did that part of the world switch in the
> given year?  north or south of the equator?)..

Right, I don't think the mapping of zone to offset is reversible, because many zones map to the same offset. If I tell you I'm in -0500, even just in the US that could mean Eastern Standard Time (winter, no DST) or Central Daylight Time (summer, DST). Not to mention that other political entities in the same longitude have their own zones which do DST at different times (or were even established as zones at different times; historical dates need to use the zones as they were at that time).

Show 16 quoted lines
> > As far as the patch itself goes, I'm disappointed to lose the automatic
> > "%" handling for all of the other callers. But I suspect the boilerplate
> > involved in any solution that lets callers choose whether or not to use
> > it would end up being longer than just handling it in each caller.
> 
> Actually I felt uneasy when you added that forced %% handling because it
> put a policy into an otherwise neutral interpreter function.  I just had
> no practical argument against it -- until now.
> 
> I'd rather see strbuf_expand also lose the hard-coded percent sign, but
> again I don't have an actual user for such a flexibility (yet).
> 
> Perhaps we should add a fully neutral strbuf_expand_core (or whatever),
> make strbuf_expand a wrapper with hard-coded % and %% handling and use
> the core function in the strftime wrapper.  Except that the function is
> not easily stackable.  Hmm..

Right, that's the boilerplate trickiness I was referring to. It's probably not worth the effort.

Anyway, here's my patch. I've been testing it with:
  ./git log --format='%ai%n%ad%n' --date=format:'%Y-%m-%d %H:%M:%S %z (%Z)'

which lets you compare a variety of commits with the existing formatting routine.

---
diff --git a/date.c b/date.c
index 63fa99685..a6214172e 100644
--- a/date.c
+++ b/date.c
@@ -196,6 +196,34 @@ const char *show_date(timestamp_t time, int tz, const struct date_mode *mode)
 		return timebuf.buf;
 	}
 
+	if (mode->type == DATE_STRFTIME) {
+		char *orig_tz;
+		time_t t = time;
+
+		if (!mode->local) {
+			char *new_tz = xstrfmt("GIT%+d", -tz / 100);
+			orig_tz = xstrdup_or_null(getenv("TZ"));
+			setenv("TZ", new_tz, 1);
+			free(new_tz);
+			tzset();
+		}
+
+		tm = localtime(&t);
+		strbuf_reset(&timebuf);
+		strbuf_addftime(&timebuf, mode->strftime_fmt, tm);
+
+		if (!mode->local) {
+			if (orig_tz)
+				setenv("TZ", orig_tz, 1);
+			else
+				unsetenv("TZ");
+			free(orig_tz);
+			tzset();
+		}
+
+		return timebuf.buf;
+	}
+
 	if (mode->local)
 		tz = local_tzoffset(time);
 
@@ -245,8 +273,6 @@ const char *show_date(timestamp_t time, int tz, const struct date_mode *mode)
 			weekday_names[tm->tm_wday], tm->tm_mday,
 			month_names[tm->tm_mon], tm->tm_year + 1900,
 			tm->tm_hour, tm->tm_min, tm->tm_sec, tz);
-	else if (mode->type == DATE_STRFTIME)
-		strbuf_addftime(&timebuf, mode->strftime_fmt, tm);
 	else
 		strbuf_addf(&timebuf, "%.3s %.3s %d %02d:%02d:%02d %d%c%+05d",
 				weekday_names[tm->tm_wday],
Previous: René ScharfeNext: Ulrich Mueller
Message 10 of 44 in “git-2.13.0: log --date=format:%z not working”
  1. Ulrich MuellerMay 26, 2017
  2. Ævar Arnfjörð BjarmasonMay 27, 2017
  3. Jeff KingMay 27, 2017
  4. Ævar Arnfjörð BjarmasonMay 28, 2017
  5. Junio C HamanoMay 29, 2017
  6. René ScharfeMay 28, 2017
  7. Junio C HamanoJun 2, 2017
  8. Jeff KingJun 2, 2017
  9. René ScharfeJun 2, 2017
  10. Jeff KingJun 2, 2017
  11. Ulrich MuellerJun 2, 2017
  12. Jeff KingJun 2, 2017
  13. Ulrich MuellerJun 2, 2017
  14. Jeff KingJun 2, 2017
  15. strbuf: let strbuf_addftime handle %z and %Z itselfRené Scharfe, Jun 3, 2017
  16. Ulrich MuellerJun 3, 2017
  17. René ScharfeJun 3, 2017
  18. Jeff KingJun 7, 2017
  19. date: use localtime() for "-local" time formatsJeff King, Jun 7, 2017
  20. René ScharfeJun 11, 2017
  21. Junio C HamanoJun 12, 2017
  22. Ævar Arnfjörð BjarmasonJun 12, 2017
  23. Ulrich MuellerJun 12, 2017
  24. Ævar Arnfjörð BjarmasonJun 12, 2017
  25. Junio C HamanoJun 12, 2017
  26. Jeff KingJun 12, 2017
  27. Ævar Arnfjörð BjarmasonJun 12, 2017
  28. Jeff KingJun 12, 2017
  29. Linus TorvaldsJun 13, 2017
  30. René ScharfeJun 12, 2017
  31. Ævar Arnfjörð BjarmasonJun 13, 2017
  32. Ulrich MuellerJun 13, 2017
  33. René ScharfeJun 12, 2017
  34. Jeff KingJun 12, 2017
  35. strbuf: let strbuf_addftime handle %z and %Z itselfRené Scharfe, Jun 15, 2017
  36. Ulrich MuellerJun 15, 2017
  37. René ScharfeJun 15, 2017
  38. strbuf: let strbuf_addftime handle %z and %Z itselfRené Scharfe, Jun 15, 2017
  39. Jeff KingJun 15, 2017
  40. 1/2 t0006: check --date=format zone offsetsJeff King, Jun 15, 2017
  41. 2/2 date: use localtime() for "-local" time formatsJeff King, Jun 15, 2017
  42. René ScharfeJun 15, 2017
  43. Junio C HamanoJun 15, 2017
  44. Jeff KingJun 16, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.