Re: [PATCH 0/3] fix local_tzoffset with far-in-future dates

Jeff King [off-list ref] writes:

I still don't know how that screwed-up timestamp got _into_
a commit, so perhaps there is another bug lurking.  I couldn't convince
git to parse anything beyond 2100, and committing with
GIT_AUTHOR_DATE='@5758122296 +0000' works just fine.

Interesting.  The weirdest I could come up with was with

    GIT_AUTHOR_DATE='@5758122296 -9999

which gets turned into the same timestamp but with -10039 timezone
(simply because 99 minutes is an hour and 39 minutes).

  [1/3]: t0006: rename test-date's "show" to "relative"
  [2/3]: t0006: test various date formats
  [3/3]: local_tzoffset: detect errors from tm_to_time_t

Thanks, will queue.

On Mon, Jun 20, 2016 at 03:11:23PM -0700, Junio C Hamano wrote:

Jeff King [off-list ref] writes:

quoted

I still don't know how that screwed-up timestamp got _into_
a commit, so perhaps there is another bug lurking.  I couldn't convince
git to parse anything beyond 2100, and committing with
GIT_AUTHOR_DATE='@5758122296 +0000' works just fine.

Interesting.  The weirdest I could come up with was with

    GIT_AUTHOR_DATE='@5758122296 -9999

which gets turned into the same timestamp but with -10039 timezone
(simply because 99 minutes is an hour and 39 minutes).

Yeah, as weird as that is, I think it's reasonable. We _could_ turn
nonsense timezones into "+0000". That doesn't necessarily help the user
much, but at least it's less bizarre than making a 46-year timezone
offset.

I also looked for other uses of tm_to_time_t without checking for an
error return. Most of them do check. The exception is datestamp(), but
is calling it on the output of localtime(time()), which should generally
be sensible.

-Peff

There are more strange things happening with dates.  One example is
that `git commit --date=@4102444799` produces a commit with the
correct author date "Thu Dec 31 15:59:59 2099 -0800" (for my local
timezone which is Americas/Los_Angeles), while `git commit
--date=@4102444800` produces a commit with "now" as author date, as
does any other larger number. `date --date=@4102444800` results in
"Thu Dec 31 16:00:00 PST 2099". So seems 2100-01-01T00:00:00Z is a
hard limit for git when using this format.

On Mon, Jun 20, 2016 at 3:21 PM, Jeff King [off-list ref] wrote:

On Mon, Jun 20, 2016 at 03:11:23PM -0700, Junio C Hamano wrote:

quoted

Jeff King [off-list ref] writes:

quoted

I still don't know how that screwed-up timestamp got _into_
a commit, so perhaps there is another bug lurking.  I couldn't convince
git to parse anything beyond 2100, and committing with
GIT_AUTHOR_DATE='@5758122296 +0000' works just fine.

Interesting.  The weirdest I could come up with was with

    GIT_AUTHOR_DATE='@5758122296 -9999

which gets turned into the same timestamp but with -10039 timezone
(simply because 99 minutes is an hour and 39 minutes).

Yeah, as weird as that is, I think it's reasonable. We _could_ turn
nonsense timezones into "+0000". That doesn't necessarily help the user
much, but at least it's less bizarre than making a 46-year timezone
offset.

I also looked for other uses of tm_to_time_t without checking for an
error return. Most of them do check. The exception is datestamp(), but
is calling it on the output of localtime(time()), which should generally
be sensible.

-Peff

On Mon, Jun 20, 2016 at 11:37:50PM -0700, Norbert Kiesel wrote:

There are more strange things happening with dates.  One example is
that `git commit --date=@4102444799` produces a commit with the
correct author date "Thu Dec 31 15:59:59 2099 -0800" (for my local
timezone which is Americas/Los_Angeles), while `git commit
--date=@4102444800` produces a commit with "now" as author date, as
does any other larger number. `date --date=@4102444800` results in
"Thu Dec 31 16:00:00 PST 2099". So seems 2100-01-01T00:00:00Z is a
hard limit for git when using this format.

Yes, I noticed that, too. I suspect it comes from the same source; the
date parser calls tm_to_time_t at some point which will refuse to handle
the date, and we fallback to something else. So certainly there is room
for improvement:

  1. We could handle a wider range of dates in tm_to_time_t(). This is
     essentially mktime(), but notice that mktime() was avoided for good
     reasons long ago, so any proposal to just move to that would need
     to figure out all those reasons and whether they are still valid.

  2. We should perhaps be flagging an error here instead of falling back
     to the current time. I suspect this is happening because --date
     falls back to approxidate() when we fail to parse the date (so you
     can say things like "--date=last.friday". Especially for cases with
     "@", which indicate that no approximate parsing is really required.

     Note that using GIT_AUTHOR_DATE _doesn't_ go through the date
     parser, but expects a raw time_t. So that does work for these
     far-future dates.

I'm not planning on working on either of these in the near term, but I'd
be happy to review patches if somebody else wants to.

-Peff