{"thread":{"id":"42672","subject":"[PATCH 2/3] t0006: test various date formats","startedAt":"2016-06-20T21:12:35Z","lastAt":"2016-06-21T12:18:28Z","messageCount":8,"participants":["Jeff King","Junio C Hamano","Norbert Kiesel"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"289707","messageId":"20160620211158.GA15521@sigill.intra.peff.net","threadId":"42672","inReplyTo":"20160620210901.GE3631@sigill.intra.peff.net","subject":"[PATCH 2/3] t0006: test various date formats","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-20T21:11:59Z","receivedAt":"2016-06-20T21:12:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We ended up testing some of these date formats throughout\nthe rest of the suite (e.g., via for-each-ref's\n\"$(authordate:...)\" format), but we never did so\nsystematically. t0006 is the right place for unit-testing of\nour date-handling code.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/helper/test-date.c | 26 ++++++++++++++++++++++++++\n t/t0006-date.sh      | 21 +++++++++++++++++++++\n 2 files changed, 47 insertions(+)\n\ndiff --git a/t/helper/test-date.c b/t/helper/test-date.c\nindex 8ebcded..d9ab360 100644\n--- a/t/helper/test-date.c\n+++ b/t/helper/test-date.c\n@@ -2,6 +2,7 @@\n \n static const char *usage_msg = \"\\n\"\n \"  test-date relative [time_t]...\\n\"\n+\"  test-date show:<format> [time_t]...\\n\"\n \"  test-date parse [date]...\\n\"\n \"  test-date approxidate [date]...\\n\";\n \n@@ -17,6 +18,29 @@ static void show_relative_dates(char **argv, struct timeval *now)\n \tstrbuf_release(&buf);\n }\n \n+static void show_dates(char **argv, const char *format)\n+{\n+\tstruct date_mode mode;\n+\n+\tparse_date_format(format, &mode);\n+\tfor (; *argv; argv++) {\n+\t\tchar *arg = *argv;\n+\t\ttime_t t;\n+\t\tint tz;\n+\n+\t\t/*\n+\t\t * Do not use our normal timestamp parsing here, as the point\n+\t\t * is to test the formatting code in isolation.\n+\t\t */\n+\t\tt = strtol(arg, &arg, 10);\n+\t\twhile (*arg == ' ')\n+\t\t\targ++;\n+\t\ttz = atoi(arg);\n+\n+\t\tprintf(\"%s -> %s\\n\", *argv, show_date(t, tz, &mode));\n+\t}\n+}\n+\n static void parse_dates(char **argv, struct timeval *now)\n {\n \tstruct strbuf result = STRBUF_INIT;\n@@ -63,6 +87,8 @@ int main(int argc, char **argv)\n \t\tusage(usage_msg);\n \tif (!strcmp(*argv, \"relative\"))\n \t\tshow_relative_dates(argv+1, &now);\n+\telse if (skip_prefix(*argv, \"show:\", &x))\n+\t\tshow_dates(argv+1, x);\n \telse if (!strcmp(*argv, \"parse\"))\n \t\tparse_dates(argv+1, &now);\n \telse if (!strcmp(*argv, \"approxidate\"))\ndiff --git a/t/t0006-date.sh b/t/t0006-date.sh\nindex fa05269..57033dd 100755\n--- a/t/t0006-date.sh\n+++ b/t/t0006-date.sh\n@@ -27,6 +27,27 @@ check_relative 630000000 '20 years ago'\n check_relative 31449600 '12 months ago'\n check_relative 62985600 '2 years ago'\n \n+check_show () {\n+\tformat=$1\n+\ttime=$2\n+\texpect=$3\n+\ttest_expect_${4:-success} \"show date ($format:$time)\" '\n+\t\techo \"$time -> $expect\" >expect &&\n+\t\ttest-date show:$format \"$time\" >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+}\n+\n+# arbitrary but sensible time for examples\n+TIME='1466000000 +0200'\n+check_show iso8601 \"$TIME\" '2016-06-15 16:13:20 +0200'\n+check_show iso8601-strict \"$TIME\" '2016-06-15T16:13:20+02:00'\n+check_show rfc2822 \"$TIME\" 'Wed, 15 Jun 2016 16:13:20 +0200'\n+check_show short \"$TIME\" '2016-06-15'\n+check_show default \"$TIME\" 'Wed Jun 15 16:13:20 2016 +0200'\n+check_show raw \"$TIME\" '1466000000 +0200'\n+check_show iso-local \"$TIME\" '2016-06-15 14:13:20 +0000'\n+\n check_parse() {\n \techo \"$1 -> $2\" >expect\n \ttest_expect_${4:-success} \"parse date ($1${3:+ TZ=$3})\" \"\n-- \n2.9.0.167.g9e4667c\n\n"},{"id":"289708","messageId":"20160620211413.GB15521@sigill.intra.peff.net","threadId":"42672","inReplyTo":"20160620210901.GE3631@sigill.intra.peff.net","subject":"[PATCH 3/3] local_tzoffset: detect errors from tm_to_time_t","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-20T21:14:14Z","receivedAt":"2016-06-20T21:14:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When we want to know the local timezone offset at a given\ntimestamp, we compute it by asking for localtime() at the\ngiven time, and comparing the offset to GMT at that time.\nHowever, there's some juggling between time_t and \"struct\ntm\" which happens, which involves calling our own\ntm_to_time_t().\n\nIf that function returns an error (e.g., because it only\nhandles dates up to the year 2099), it returns \"-1\", which\nwe treat as a time_t, and is clearly bogus, leading to\nbizarre timestamps (that seem to always adjust the time back\nto (time_t)(uint32_t)-1, in the year 2106).\n\nIt's not a good idea for local_tzoffset() to simply die\nhere; it would make it hard to run \"git log\" on a repository\nwith funny timestamps. Instead, let's just treat such cases\nas \"zero offset\".\n\nReported-by: Norbert Kiesel <nkiesel@gmail.com>\nSigned-off-by: Jeff King <peff@peff.net>\n---\n date.c          | 2 ++\n t/t0006-date.sh | 5 +++++\n 2 files changed, 7 insertions(+)\n\ndiff --git a/date.c b/date.c\nindex 7c9f769..4c7aa9b 100644\n--- a/date.c\n+++ b/date.c\n@@ -74,6 +74,8 @@ static int local_tzoffset(unsigned long time)\n \tlocaltime_r(&t, &tm);\n \tt_local = tm_to_time_t(&tm);\n \n+\tif (t_local == -1)\n+\t\treturn 0; /* error; just use +0000 */\n \tif (t_local < t) {\n \t\teastwest = -1;\n \t\toffset = t - t_local;\ndiff --git a/t/t0006-date.sh b/t/t0006-date.sh\nindex 57033dd..04ce535 100755\n--- a/t/t0006-date.sh\n+++ b/t/t0006-date.sh\n@@ -48,6 +48,11 @@ check_show default \"$TIME\" 'Wed Jun 15 16:13:20 2016 +0200'\n check_show raw \"$TIME\" '1466000000 +0200'\n check_show iso-local \"$TIME\" '2016-06-15 14:13:20 +0000'\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\"\n+check_show iso-local \"$FUTURE\" \"2152-06-19 22:24:56 +0000\"\n+\n check_parse() {\n \techo \"$1 -> $2\" >expect\n \ttest_expect_${4:-success} \"parse date ($1${3:+ TZ=$3})\" \"\n-- \n2.9.0.167.g9e4667c\n"},{"id":"289709","messageId":"20160620210901.GE3631@sigill.intra.peff.net","threadId":"42672","inReplyTo":"20160620200011.GC3631@sigill.intra.peff.net","subject":"[PATCH 0/3] fix local_tzoffset with far-in-future dates","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-20T21:09:01Z","receivedAt":"2016-06-20T21:16:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 20, 2016 at 04:00:12PM -0400, Jeff King wrote:\n\n> You _should_ be able to get the right answer by asking git for\n> --date=local, but it doesn't seem to work. Looks like it is because our\n> tm_to_time_t hits this code:\n> \n>   if (year < 0 || year > 129) /* algo only works for 1970-2099 */\n> \treturn -1;\n> \n> and the caller does not actually check the error. The resulting timezone\n> is the screwed-up -40643156, which is perhaps how it got into the commit\n> in the first place.\n\nSo here's a patch to fix that (along with some test infrastructure to\nsupport it). I still don't know how that screwed-up timestamp got _into_\na commit, so perhaps there is another bug lurking.  I couldn't convince\ngit to parse anything beyond 2100, and committing with\nGIT_AUTHOR_DATE='@5758122296 +0000' works just fine.\n\n  [1/3]: t0006: rename test-date's \"show\" to \"relative\"\n  [2/3]: t0006: test various date formats\n  [3/3]: local_tzoffset: detect errors from tm_to_time_t\n\n-Peff\n"},{"id":"289710","messageId":"20160620211029.GA31229@sigill.intra.peff.net","threadId":"42672","inReplyTo":"20160620210901.GE3631@sigill.intra.peff.net","subject":"[PATCH 1/3] t0006: rename test-date's \"show\" to \"relative\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-20T21:10:29Z","receivedAt":"2016-06-20T21:38:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The \"show\" tests are really only checking relative formats;\nwe should make that more clear.\n\nThis also frees up the \"show\" name to later check other\nformats. We could later fold \"relative\" into a more generic\n\"show\" command, but it's not worth it.  Relative times are a\nspecial case already because we have to munge the concept of\n\"now\" in our tests.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/helper/test-date.c |  8 ++++----\n t/t0006-date.sh      | 26 +++++++++++++-------------\n 2 files changed, 17 insertions(+), 17 deletions(-)\n\ndiff --git a/t/helper/test-date.c b/t/helper/test-date.c\nindex 63f3735..8ebcded 100644\n--- a/t/helper/test-date.c\n+++ b/t/helper/test-date.c\n@@ -1,11 +1,11 @@\n #include \"cache.h\"\n \n static const char *usage_msg = \"\\n\"\n-\"  test-date show [time_t]...\\n\"\n+\"  test-date relative [time_t]...\\n\"\n \"  test-date parse [date]...\\n\"\n \"  test-date approxidate [date]...\\n\";\n \n-static void show_dates(char **argv, struct timeval *now)\n+static void show_relative_dates(char **argv, struct timeval *now)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \n@@ -61,8 +61,8 @@ int main(int argc, char **argv)\n \targv++;\n \tif (!*argv)\n \t\tusage(usage_msg);\n-\tif (!strcmp(*argv, \"show\"))\n-\t\tshow_dates(argv+1, &now);\n+\tif (!strcmp(*argv, \"relative\"))\n+\t\tshow_relative_dates(argv+1, &now);\n \telse if (!strcmp(*argv, \"parse\"))\n \t\tparse_dates(argv+1, &now);\n \telse if (!strcmp(*argv, \"approxidate\"))\ndiff --git a/t/t0006-date.sh b/t/t0006-date.sh\nindex fac0986..fa05269 100755\n--- a/t/t0006-date.sh\n+++ b/t/t0006-date.sh\n@@ -6,26 +6,26 @@ test_description='test date parsing and printing'\n # arbitrary reference time: 2009-08-30 19:20:00\n TEST_DATE_NOW=1251660000; export TEST_DATE_NOW\n \n-check_show() {\n+check_relative() {\n \tt=$(($TEST_DATE_NOW - $1))\n \techo \"$t -> $2\" >expect\n \ttest_expect_${3:-success} \"relative date ($2)\" \"\n-\ttest-date show $t >actual &&\n+\ttest-date relative $t >actual &&\n \ttest_i18ncmp expect actual\n \t\"\n }\n \n-check_show 5 '5 seconds ago'\n-check_show 300 '5 minutes ago'\n-check_show 18000 '5 hours ago'\n-check_show 432000 '5 days ago'\n-check_show 1728000 '3 weeks ago'\n-check_show 13000000 '5 months ago'\n-check_show 37500000 '1 year, 2 months ago'\n-check_show 55188000 '1 year, 9 months ago'\n-check_show 630000000 '20 years ago'\n-check_show 31449600 '12 months ago'\n-check_show 62985600 '2 years ago'\n+check_relative 5 '5 seconds ago'\n+check_relative 300 '5 minutes ago'\n+check_relative 18000 '5 hours ago'\n+check_relative 432000 '5 days ago'\n+check_relative 1728000 '3 weeks ago'\n+check_relative 13000000 '5 months ago'\n+check_relative 37500000 '1 year, 2 months ago'\n+check_relative 55188000 '1 year, 9 months ago'\n+check_relative 630000000 '20 years ago'\n+check_relative 31449600 '12 months ago'\n+check_relative 62985600 '2 years ago'\n \n check_parse() {\n \techo \"$1 -> $2\" >expect\n-- \n2.9.0.167.g9e4667c\n\n"},{"id":"289712","messageId":"xmqqy45zse7o.fsf@gitster.mtv.corp.google.com","threadId":"42672","inReplyTo":"20160620210901.GE3631@sigill.intra.peff.net","subject":"Re: [PATCH 0/3] fix local_tzoffset with far-in-future dates","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-20T22:11:23Z","receivedAt":"2016-06-20T22:12:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I still don't know how that screwed-up timestamp got _into_\n> a commit, so perhaps there is another bug lurking.  I couldn't convince\n> git to parse anything beyond 2100, and committing with\n> GIT_AUTHOR_DATE='@5758122296 +0000' works just fine.\n\nInteresting.  The weirdest I could come up with was with\n\n    GIT_AUTHOR_DATE='@5758122296 -9999\n\nwhich gets turned into the same timestamp but with -10039 timezone\n(simply because 99 minutes is an hour and 39 minutes).\n\n>   [1/3]: t0006: rename test-date's \"show\" to \"relative\"\n>   [2/3]: t0006: test various date formats\n>   [3/3]: local_tzoffset: detect errors from tm_to_time_t\n\nThanks, will queue.\n"},{"id":"289714","messageId":"20160620222112.GB6431@sigill.intra.peff.net","threadId":"42672","inReplyTo":"xmqqy45zse7o.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 0/3] fix local_tzoffset with far-in-future dates","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-20T22:21:12Z","receivedAt":"2016-06-20T22:22:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 20, 2016 at 03:11:23PM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > I still don't know how that screwed-up timestamp got _into_\n> > a commit, so perhaps there is another bug lurking.  I couldn't convince\n> > git to parse anything beyond 2100, and committing with\n> > GIT_AUTHOR_DATE='@5758122296 +0000' works just fine.\n> \n> Interesting.  The weirdest I could come up with was with\n> \n>     GIT_AUTHOR_DATE='@5758122296 -9999\n> \n> which gets turned into the same timestamp but with -10039 timezone\n> (simply because 99 minutes is an hour and 39 minutes).\n\nYeah, as weird as that is, I think it's reasonable. We _could_ turn\nnonsense timezones into \"+0000\". That doesn't necessarily help the user\nmuch, but at least it's less bizarre than making a 46-year timezone\noffset.\n\nI also looked for other uses of tm_to_time_t without checking for an\nerror return. Most of them do check. The exception is datestamp(), but\nis calling it on the output of localtime(time()), which should generally\nbe sensible.\n\n-Peff\n"},{"id":"289727","messageId":"CAM+g_NtGWRCqaNz1DauZRReem0YPC6CaunHSwfhnB5LpvdGGcQ@mail.gmail.com","threadId":"42672","inReplyTo":"20160620222112.GB6431@sigill.intra.peff.net","subject":"Re: [PATCH 0/3] fix local_tzoffset with far-in-future dates","fromName":"Norbert Kiesel","fromEmail":"nkiesel@gmail.com","sentAt":"2016-06-21T06:37:50Z","receivedAt":"2016-06-21T06:38:54Z","isPatch":true,"sender":{"key":"nkiesel@gmail.com","avatar":null},"body":"There are more strange things happening with dates.  One example is\nthat `git commit --date=@4102444799` produces a commit with the\ncorrect author date \"Thu Dec 31 15:59:59 2099 -0800\" (for my local\ntimezone which is Americas/Los_Angeles), while `git commit\n--date=@4102444800` produces a commit with \"now\" as author date, as\ndoes any other larger number. `date --date=@4102444800` results in\n\"Thu Dec 31 16:00:00 PST 2099\". So seems 2100-01-01T00:00:00Z is a\nhard limit for git when using this format.\n\nOn Mon, Jun 20, 2016 at 3:21 PM, Jeff King <peff@peff.net> wrote:\n> On Mon, Jun 20, 2016 at 03:11:23PM -0700, Junio C Hamano wrote:\n>\n>> Jeff King <peff@peff.net> writes:\n>>\n>> > I still don't know how that screwed-up timestamp got _into_\n>> > a commit, so perhaps there is another bug lurking.  I couldn't convince\n>> > git to parse anything beyond 2100, and committing with\n>> > GIT_AUTHOR_DATE='@5758122296 +0000' works just fine.\n>>\n>> Interesting.  The weirdest I could come up with was with\n>>\n>>     GIT_AUTHOR_DATE='@5758122296 -9999\n>>\n>> which gets turned into the same timestamp but with -10039 timezone\n>> (simply because 99 minutes is an hour and 39 minutes).\n>\n> Yeah, as weird as that is, I think it's reasonable. We _could_ turn\n> nonsense timezones into \"+0000\". That doesn't necessarily help the user\n> much, but at least it's less bizarre than making a 46-year timezone\n> offset.\n>\n> I also looked for other uses of tm_to_time_t without checking for an\n> error return. Most of them do check. The exception is datestamp(), but\n> is calling it on the output of localtime(time()), which should generally\n> be sensible.\n>\n> -Peff\n"},{"id":"289765","messageId":"20160621121813.GA31030@sigill.intra.peff.net","threadId":"42672","inReplyTo":"CAM+g_NtGWRCqaNz1DauZRReem0YPC6CaunHSwfhnB5LpvdGGcQ@mail.gmail.com","subject":"Re: [PATCH 0/3] fix local_tzoffset with far-in-future dates","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-21T12:18:14Z","receivedAt":"2016-06-21T12:18:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 20, 2016 at 11:37:50PM -0700, Norbert Kiesel wrote:\n\n> There are more strange things happening with dates.  One example is\n> that `git commit --date=@4102444799` produces a commit with the\n> correct author date \"Thu Dec 31 15:59:59 2099 -0800\" (for my local\n> timezone which is Americas/Los_Angeles), while `git commit\n> --date=@4102444800` produces a commit with \"now\" as author date, as\n> does any other larger number. `date --date=@4102444800` results in\n> \"Thu Dec 31 16:00:00 PST 2099\". So seems 2100-01-01T00:00:00Z is a\n> hard limit for git when using this format.\n\nYes, I noticed that, too. I suspect it comes from the same source; the\ndate parser calls tm_to_time_t at some point which will refuse to handle\nthe date, and we fallback to something else. So certainly there is room\nfor improvement:\n\n  1. We could handle a wider range of dates in tm_to_time_t(). This is\n     essentially mktime(), but notice that mktime() was avoided for good\n     reasons long ago, so any proposal to just move to that would need\n     to figure out all those reasons and whether they are still valid.\n\n  2. We should perhaps be flagging an error here instead of falling back\n     to the current time. I suspect this is happening because --date\n     falls back to approxidate() when we fail to parse the date (so you\n     can say things like \"--date=last.friday\". Especially for cases with\n     \"@\", which indicate that no approximate parsing is really required.\n\n     Note that using GIT_AUTHOR_DATE _doesn't_ go through the date\n     parser, but expects a raw time_t. So that does work for these\n     far-future dates.\n\nI'm not planning on working on either of these in the near term, but I'd\nbe happy to review patches if somebody else wants to.\n\n-Peff\n"}]}