:format:%s for date fields seems to be shifted by timezone

Hello!

my current timezone is UTC+0200.

I create a test repo, add one commit and create a tag:

Now:

$ git for-each-ref --format="%(taggerdate:format:%s)" refs/tags 1588504146

$ date +"%s" 1588500585

$ git for-each-ref --format="%(taggerdate:raw)" refs/tags 1588500546 +0200

Somehow %(taggerdate:format:%s) gives a Unix timestamp which is one hour in future and it is different than what ` date +"%s"` gives around approximately the same time the tag was created.

clime

On Sun, May 03, 2020 at 12:15:14PM +0200, clime wrote:

It's caused by strftime() being clever with DST. Try this:

  git commit --date=@1559361600 --allow-empty -m summer
  git commit --date=@1577854800 --allow-empty -m winter
  git log --format=%ad --date=unix >unix
  git log --format=%ad --date=format:%s >strftime
  diff -u unix strftime

We get the winter time right, but the summer time wrong.

The issue is that strftime() takes a broken-down "struct tm", not a unix time_t. We have all of the right values for hour/minute/etc there, so using "format:%H:%M:%S" prints what you'd expect. But we never set the "isdst" field, so when it tries to convert back to unix time, it applies a one-hour offset (if it's "summer" in your local timezone).

Unfortunately I don't think we can solve this easily. If we were operating completely in your local timezone, then we would have gotten that "struct tm" from localtime(), and its isdst field would be set properly. And indeed, if you use "--date=format-local:%s", the problem goes away.

But when we're formatting in the original author's timezone, which is the default, we have no idea if they were in dst then or not. We only know their offset-to-gmt, so we munge the time_t ourselves and use gmtime().

So there are a few reasons I think this is the best we can do:

 - the full timezone information literally isn't there in Git; we might
   know the author was in +0200, but we don't know if they were
   observing DST, or if they were simply in a timezone further east.

 - even if we had a zone, there's no system function to convert a time_t
   to a tm in an arbitrary timezone (hence the gmtime() hack above;
   we've tried playing games with $TZ and tzset(), but it's awkward and
   unportable)

 - likewise, strftime() is doing the reverse conversion using the local
   timezone anyway, which would be wrong.

So my advice is not to use "%s" (which isn't portable anyway). Use "--date=unix" or "%(taggerdate:unix)".

-Peff

On Mon, 4 May 2020 at 17:43, Jeff King <peff@peff.net> wrote:

Hello Jeff,

what about just printing the raw timestamp from either commit or tag, i.e. avoiding any conversion for format:%s?

Best regards! clime

On Wed, May 06, 2020 at 08:03:52PM +0200, clime wrote:

We don't parse the "%s" ourselves at all. We feed everything after the colon to the system strftime(), and that function only takes a broken-down "struct tm".

That said, we do already intercept "%z" for similar reasons in strbuf_addftime(). So it would probably be possible to declare that we don't respect system "%s" and just handle it ourselves. It would require either massaging the "tm" back to a time_t, or changing the signature to take a redundant time_t (and not all callers might have that).

So if somebody wants to work on a patch in that direction, we could see how ugly it ends up being. But I am reasonably happy with "don't do that" as a solution in the meantime.

-Peff