5 ms·

I wish they had not gone with uint32_t for storing mtimes, since they now have to deal with the 2038 problem, sometime in the future. I am surprised they didn'

I wish they had not gone with uint32_t for storing mtimes, since they now have to deal with the 2038 problem, sometime in the future. I am surprised they didn't directly use time_t, so that they wouldn't have to deal with this (since some platforms have already gone to 64 bit time_t)

kevingadd 4y ago
Wouldn't that mean if a platform changed time_t formats it would invalidate all their stored files?
- deleted 4y ago
  [deleted]
cesarb 4y ago
> I wish they had not gone with uint32_t for storing mtimes, since they now have to deal with the 2038 problem, sometime in the future. Since uint32_t is unsigned, wouldn't it be the Y2106 problem instead? > I am surprised they didn't directly use time_t, so that they wouldn't have to deal with this (since some platforms have already gone to 64 bit time_t) You mentioned the problem yourself without noticing: some platforms have gone to 64-bit time_t, but others haven't. This is a file format, which can be shared by multiple platforms, so it cannot use types which change size depending on the platform.
- 0x0 4y ago
  But you could always a fit a uint32_t mtime in a uint64_t file format field...? Wouldn't it be better to future-proof the file format?
  - cesarb 4y ago
    That would double the disk space use of that particular file, for something which won't be necessary for more than 80 years (the mtime of a git object or pack file will always be in the past, unless the computer clock is wrong); and when that time arrives, a new file format can be defined (and the old format kept as read-only, with new data being written to the new file).
grogers 4y ago
Well if they use unsigned 32 bit they at least extended it to Y2106 :-) But for this use case it's not really an issue though. FTA it sounded like they always write the mtime as now. It's unlikely they wouldn't GC the repo in 68 years to make wraparound an issue.
est31 4y ago
For on-disk formats, time_t would probably not be a good choice, but indeed, they have a time_t to uint32_t conversion going on, that is not even saturating, just cutting bits off: https://github.com/git/git/blob/e188ec3a735ae52a0d0d3c22f9df6b29fa613b1e/builtin/pack-objects.c#L3497-L3503 https://github.com/git/git/blob/e188ec3a735ae52a0d0d3c22f9df... https://github.com/git/git/blob/e188ec3a735ae52a0d0d3c22f9df6b29fa613b1e/builtin/pack-objects.c#L3423-L3425 https://github.com/git/git/blob/e188ec3a735ae52a0d0d3c22f9df...

kevingadd 4y ago

Wouldn't that mean if a platform changed time_t formats it would invalidate all their stored files?

deleted 4y ago
[deleted]

cesarb 4y ago

> I wish they had not gone with uint32_t for storing mtimes, since they now have to deal with the 2038 problem, sometime in the future. Since uint32_t is unsigned, wouldn't it be the Y2106 problem instead? > I am surprised they didn't directly use time_t, so that they wouldn't have to deal with this (since some platforms have already gone to 64 bit time_t) You mentioned the problem yourself without noticing: some platforms have gone to 64-bit time_t, but others haven't. This is a file format, which can be shared by multiple platforms, so it cannot use types which change size depending on the platform.

0x0 4y ago
But you could always a fit a uint32_t mtime in a uint64_t file format field...? Wouldn't it be better to future-proof the file format?
- cesarb 4y ago
  That would double the disk space use of that particular file, for something which won't be necessary for more than 80 years (the mtime of a git object or pack file will always be in the past, unless the computer clock is wrong); and when that time arrives, a new file format can be defined (and the old format kept as read-only, with new data being written to the new file).

grogers 4y ago

Well if they use unsigned 32 bit they at least extended it to Y2106 :-) But for this use case it's not really an issue though. FTA it sounded like they always write the mtime as now. It's unlikely they wouldn't GC the repo in 68 years to make wraparound an issue.

est31 4y ago

For on-disk formats, time_t would probably not be a good choice, but indeed, they have a time_t to uint32_t conversion going on, that is not even saturating, just cutting bits off: https://github.com/git/git/blob/e188ec3a735ae52a0d0d3c22f9df6b29fa613b1e/builtin/pack-objects.c#L3497-L3503 https://github.com/git/git/blob/e188ec3a735ae52a0d0d3c22f9df... https://github.com/git/git/blob/e188ec3a735ae52a0d0d3c22f9df6b29fa613b1e/builtin/pack-objects.c#L3423-L3425 https://github.com/git/git/blob/e188ec3a735ae52a0d0d3c22f9df...