On Mon, Jul 04, 2022 at 02:50:57PM -0600, Derrick Stolee wrote:
> > + GIT_COMMITTER_DATE='1970-01-01T00:00:00 +0100'
>
> Because of the "+0100" in the time zone, this date
> becomes a negative value. The commit-graph does not
> store dates with more than 34 bits (and Git does
> not handle negative timestamps very well? Peff can
> clarify here).
I don't think we handle them at all. Certainly we'll refuse to parse
any, but here we get tricked by the timezone, and end up with the
underflow. Git writes 2^64 - 3600 into the commit object with this one.
The code in gm_time_t() tries to catch this (as well as overflow), but
this one gets handled by parse_date_basic(), which doesn't. Adding this
causes it barf:
diff --git a/date.c b/date.c
index 68a260c214..07a60fee35 100644
--- a/date.c
+++ b/date.c
@@ -893,8 +893,11 @@ int parse_date_basic(const char *date, timestamp_t *timestamp, int *offset)
}
}
- if (!tm_gmt)
+ if (!tm_gmt) {
+ if (*timestamp < *offset * 60)
+ return -1;
*timestamp -= *offset * 60;
+ }
return 0; /* success */
}
We should probably check for overflow there, as well, though I think in
practice it would get caught earlier. The timestamp variable comes from
tm_to_time_t(), which already has some limits.
Of course the right fix is for us to handle negative timestamps. I have
some patches working towards that, but...
> The commit-graph could certainly warn better here to
> say we do not have enough date bits to store this
> timestamp (the same would happen with a date beyond
> 2138 or something like that).
Right. By the time it gets to the commit-graph, it's a gigantic positive
time. But either way, yes, it should realize it's not representable and
omit the commit from the graph file.
That's something I haven't looked at for my negative timestamp patches
at all. I think it's OK for the commit-graph to simply punt on them, as
long as it doesn't die or produce wrong answers.
-Peff
