Re: [PATCH 0/6] Use time_t

[Junio C Hamano]

Jeff King  writes:

> On Tue, Feb 28, 2017 at 02:27:22PM -0800, Junio C Hamano wrote:
>
>> Jeff King  writes:
>> 
>> > ... We can certainly stick with it for now (it's awkward if you
>> > really do have an entry on Jan 1 1970, but other than that it's an OK
>> > marker). I agree that the most negatively value is probably a saner
>> > choice, but we can switch to it after the dust settles.
>> 
>> I was trying to suggest that we should strive to switch to the most
>> negative or whatever the most implausible value in the new range
>> (and leave it as a possible bug to be fixed if we missed a place
>> that still used "0 is impossible") while doing the ulong to time_t
>> (or timestamp_t that is i64).  
>> 
>> "safer in the short term" wasn't meant to be "let's not spend time
>> to do quality work".  As long as we are switching, we should follow
>> it through.
>
> Sure, I'd be much happier to see it done now. I just didn't want to pile
> on the requirements to the point that step 1 doesn't get done.

Ah, that was what you meant.

I was assuming that we are switching to a longer _signed_ type.  It
felt silly to tell users "you can use timestamps before the epoch
now with this change, but you cannot express time exactly at the
epoch".

I am perfectly OK with switching to a longer _unsigned_ type with
the "0 is impossible" [*1*] intact, aka "safer in the short term",
if we want to it make our first step.  That may be a smaller step,
but still a step in the right direction.


[Footnote]

*1* It could be "-1 is impossible", I didn't actually check.
Funnily enough, ISO C99 uses (time_t)(-1) to signal an error
when returning value from mktime() and time().

Re: [PATCH 0/6] Use time_t

[René Scharfe]

Am 28.02.2017 um 21:54 schrieb Johannes Schindelin:

Hi Junio,

On Tue, 28 Feb 2017, Junio C Hamano wrote:


René Scharfe  writes:


Am 28.02.2017 um 15:28 schrieb Jeff King:


It looks from the discussion like the sanest path forward is our own
signed-64bit timestamp_t. That's unfortunate compared to using the
standard time_t, but hopefully it would reduce the number of knobs
(like TIME_T_IS_INT64) in the long run.


Glibc will get a way to enable 64-bit time_t on 32-bit platforms
eventually (https://sourceware.org/glibc/wiki/Y2038ProofnessDesign).
Can platforms that won't provide a 64-bit time_t by 2038 be actually
used at that point?  How would we get time information on them?  How
would a custom timestamp_t help us?


That's a sensible "wait, let's step back a bit".  I take it that you are
saying "time_t is just fine", and I am inclined to agree.

Right now, they may be able to have future timestamps ranging to
year 2100 and switching to time_t would limit their ability to
express future time to 2038 but they would be able to express
timestamp in the past to cover most of 20th century.  Given that
these 32-bit time_t software platforms will die off before year 2038
(either by underlying hardware getting obsolete, or software updated
to handle 64-bit time_t), the (temporary) loss of 2038-2100 range
would not be too big a deal to warrant additional complexity.


You seem to assume that time_t is required to be signed. But from my
understanding that is only guaranteed by POSIX, not by ISO C.

We may very well buy ourselves a ton of trouble if we decide to switch to
`time_t` rather than to `int64_t`.


True, and time_t doesn't even have to be an integer type.  But which 
platforms capable of running git use something else than int32_t or int64_t?


René

Re: [PATCH 0/6] Use time_t

[René Scharfe]

Am 01.03.2017 um 00:10 schrieb Johannes Schindelin:

Hi René,

On Tue, 28 Feb 2017, René Scharfe wrote:


Am 28.02.2017 um 21:54 schrieb Johannes Schindelin:


On Tue, 28 Feb 2017, Junio C Hamano wrote:


René Scharfe  writes:


Am 28.02.2017 um 15:28 schrieb Jeff King:


It looks from the discussion like the sanest path forward is our
own signed-64bit timestamp_t. That's unfortunate compared to
using the standard time_t, but hopefully it would reduce the
number of knobs (like TIME_T_IS_INT64) in the long run.


Glibc will get a way to enable 64-bit time_t on 32-bit platforms
eventually
(https://sourceware.org/glibc/wiki/Y2038ProofnessDesign).  Can
platforms that won't provide a 64-bit time_t by 2038 be actually
used at that point?  How would we get time information on them?
How would a custom timestamp_t help us?


That's a sensible "wait, let's step back a bit".  I take it that you
are saying "time_t is just fine", and I am inclined to agree.

Right now, they may be able to have future timestamps ranging to
year 2100 and switching to time_t would limit their ability to
express future time to 2038 but they would be able to express
timestamp in the past to cover most of 20th century.  Given that
these 32-bit time_t software platforms will die off before year 2038
(either by underlying hardware getting obsolete, or software updated
to handle 64-bit time_t), the (temporary) loss of 2038-2100 range
would not be too big a deal to warrant additional complexity.


You seem to assume that time_t is required to be signed. But from my
understanding that is only guaranteed by POSIX, not by ISO C.

We may very well buy ourselves a ton of trouble if we decide to switch
to `time_t` rather than to `int64_t`.


True, and time_t doesn't even have to be an integer type.  But which
platforms capable of running git use something else than int32_t or
int64_t?


That kind of thinking is dangerous. We don't know what platforms are
running Git, and we have a very clear example how we got it very wrong
recently, when we broke building with musl by requiring REG_STARTEND
support [*1*].


In general that's true, and if nobody can add to the list (glibc: 
int32_t on 32-bit platforms, int64_t on 64-bit platforms for now; 
NetBSD, OpenBSD, Windows int64_t) then we shouldn't make assumptions 
about time_t that go beyond the C standard -- at least not without 
verifying them, e.g. with asserts or tests.  I'd especially be 
interested in hearing about platforms that use a floating point type.


Systems lacking REG_STARTEND can use compat/regex/ -- that's doesn't 
sound too bad.  (I didn't follow the original discussion too closely, 
and don't mean to open it up again.)



So why gamble? If we switch to uint64_t, it would definitely provide the
smoothest upgrade path. It is what the code assumed implicitly when we
broke 32-bit in v2.9.1.


We can assume that time_t exists everywhere and is used by functions 
like gettimeofday(2) and localtime(3).  Why invent our own layer on top? 
 What would it be able to do that time_t alone can't?  I understand a 
need to handle times before 1970 for historical documents, but what good 
would it do to have the ability to handle dates beyond 2038, today?


Platforms that don't use the next two decades to provide a time_t that 
can store the current time by then will have bigger problems than a 
crippled git.



If anybody really wants to support negative timestamps, it should be done
on top of my work. My current patch series does not even start to try to
address the ramifications of negative timestamps (see e.g. the use of
strtoull() for parsing). It is quite unreasonable to ask for such a
fundamental design change when it could very easily be done incrementally
instead, when needed, by someone who needs it.


I'm confused.  Your patch series converts to time_t.  Why not use it? 
That would prepare ourselves for the bright future when we'll have a 
64-bit time_t in every libc. :)


René

Re: [PATCH 0/6] Use time_t

[Junio C Hamano]

Jeff King  writes:

> ... We can certainly stick with it for now (it's awkward if you
> really do have an entry on Jan 1 1970, but other than that it's an OK
> marker). I agree that the most negatively value is probably a saner
> choice, but we can switch to it after the dust settles.

I was trying to suggest that we should strive to switch to the most
negative or whatever the most implausible value in the new range
(and leave it as a possible bug to be fixed if we missed a place
that still used "0 is impossible") while doing the ulong to time_t
(or timestamp_t that is i64).  

"safer in the short term" wasn't meant to be "let's not spend time
to do quality work".  As long as we are switching, we should follow
it through.

>> But we need to cross the bridge to signed timestamp sometime, and I
>> do not see any reason why that somtime should not be now.
>
> Yep.
>
> -Peff

Re: [PATCH 0/6] Use time_t

[Johannes Schindelin]

Hi René,

On Tue, 28 Feb 2017, René Scharfe wrote:

> Am 28.02.2017 um 21:54 schrieb Johannes Schindelin:
> >
> > On Tue, 28 Feb 2017, Junio C Hamano wrote:
> >
> > > René Scharfe  writes:
> > >
> > > > Am 28.02.2017 um 15:28 schrieb Jeff King:
> > > >
> > > > > It looks from the discussion like the sanest path forward is our
> > > > > own signed-64bit timestamp_t. That's unfortunate compared to
> > > > > using the standard time_t, but hopefully it would reduce the
> > > > > number of knobs (like TIME_T_IS_INT64) in the long run.
> > > >
> > > > Glibc will get a way to enable 64-bit time_t on 32-bit platforms
> > > > eventually
> > > > (https://sourceware.org/glibc/wiki/Y2038ProofnessDesign).  Can
> > > > platforms that won't provide a 64-bit time_t by 2038 be actually
> > > > used at that point?  How would we get time information on them?
> > > > How would a custom timestamp_t help us?
> > >
> > > That's a sensible "wait, let's step back a bit".  I take it that you
> > > are saying "time_t is just fine", and I am inclined to agree.
> > >
> > > Right now, they may be able to have future timestamps ranging to
> > > year 2100 and switching to time_t would limit their ability to
> > > express future time to 2038 but they would be able to express
> > > timestamp in the past to cover most of 20th century.  Given that
> > > these 32-bit time_t software platforms will die off before year 2038
> > > (either by underlying hardware getting obsolete, or software updated
> > > to handle 64-bit time_t), the (temporary) loss of 2038-2100 range
> > > would not be too big a deal to warrant additional complexity.
> >
> > You seem to assume that time_t is required to be signed. But from my
> > understanding that is only guaranteed by POSIX, not by ISO C.
> >
> > We may very well buy ourselves a ton of trouble if we decide to switch
> > to `time_t` rather than to `int64_t`.
> 
> True, and time_t doesn't even have to be an integer type.  But which
> platforms capable of running git use something else than int32_t or
> int64_t?

That kind of thinking is dangerous. We don't know what platforms are
running Git, and we have a very clear example how we got it very wrong
recently, when we broke building with musl by requiring REG_STARTEND
support [*1*].

So why gamble? If we switch to uint64_t, it would definitely provide the
smoothest upgrade path. It is what the code assumed implicitly when we
broke 32-bit in v2.9.1.

If anybody really wants to support negative timestamps, it should be done
on top of my work. My current patch series does not even start to try to
address the ramifications of negative timestamps (see e.g. the use of
strtoull() for parsing). It is quite unreasonable to ask for such a
fundamental design change when it could very easily be done incrementally
instead, when needed, by someone who needs it.

My work would pave the way for that effort, of course. But this is really
as far as I can go with this patch series, given that I have bigger fish
to fry than to support negative timestamps.

Ciao,
Dscho

Footnote *1*: I still deeply regret deviating from my v1 that did *not*
require REG_STARTEND, but would have kept things working for platforms
without REG_STARTEND by simulating it.

But our thinking was: who would want to run Git in an environment so
ridiculously old that it does not have that clearly useful REG_STARTEND
support? Our answer was "nobody". And it was incorrect.

Re: [PATCH 0/6] Use time_t

[Jeff King]

On Tue, Feb 28, 2017 at 02:27:22PM -0800, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > ... We can certainly stick with it for now (it's awkward if you
> > really do have an entry on Jan 1 1970, but other than that it's an OK
> > marker). I agree that the most negatively value is probably a saner
> > choice, but we can switch to it after the dust settles.
> 
> I was trying to suggest that we should strive to switch to the most
> negative or whatever the most implausible value in the new range
> (and leave it as a possible bug to be fixed if we missed a place
> that still used "0 is impossible") while doing the ulong to time_t
> (or timestamp_t that is i64).  
> 
> "safer in the short term" wasn't meant to be "let's not spend time
> to do quality work".  As long as we are switching, we should follow
> it through.

Sure, I'd be much happier to see it done now. I just didn't want to pile
on the requirements to the point that step 1 doesn't get done. But I
haven't even looked at the code changes needed for time_t. I suspect
Dscho has a better feel for it at this point.

-Peff

Re: [PATCH 0/6] Use time_t

[Jeff King]

On Tue, Feb 28, 2017 at 09:54:58PM +0100, Johannes Schindelin wrote:

> > Right now, they may be able to have future timestamps ranging to
> > year 2100 and switching to time_t would limit their ability to
> > express future time to 2038 but they would be able to express
> > timestamp in the past to cover most of 20th century.  Given that
> > these 32-bit time_t software platforms will die off before year 2038
> > (either by underlying hardware getting obsolete, or software updated
> > to handle 64-bit time_t), the (temporary) loss of 2038-2100 range
> > would not be too big a deal to warrant additional complexity.
> 
> You seem to assume that time_t is required to be signed. But from my
> understanding that is only guaranteed by POSIX, not by ISO C.

I wonder how common that is in practice, and whether it is worth
treating it as a quality-of-implementation issue. IOW, to say "your
platform time_t doesn't handle negative times, so you get Jan 1 1970 for
any dates before then. Complain to your platform vendor".

I'm not sure how much complexity it would add to the code.  Either way,
when we parse an ascii-decimal timestamp from an object, we need to do
bounds checking. Whether that bound is at "0" or "LONG_MIN", I don't
think that it changes much.

Meanwhile, if we were to have a negative timestamp_t but the system
time_t is unsigned, we have to do a bounds-check any time we use a
system function like gmtime(), or risk funny wrap-around bugs.

-Peff

Re: [PATCH 0/6] Use time_t

[Johannes Schindelin]

Hi Junio,

On Tue, 28 Feb 2017, Junio C Hamano wrote:

> René Scharfe  writes:
> 
> > Am 28.02.2017 um 15:28 schrieb Jeff King:
> >
> >> It looks from the discussion like the sanest path forward is our own
> >> signed-64bit timestamp_t. That's unfortunate compared to using the
> >> standard time_t, but hopefully it would reduce the number of knobs
> >> (like TIME_T_IS_INT64) in the long run.
> >
> > Glibc will get a way to enable 64-bit time_t on 32-bit platforms
> > eventually (https://sourceware.org/glibc/wiki/Y2038ProofnessDesign).
> > Can platforms that won't provide a 64-bit time_t by 2038 be actually
> > used at that point?  How would we get time information on them?  How
> > would a custom timestamp_t help us?
> 
> That's a sensible "wait, let's step back a bit".  I take it that you are
> saying "time_t is just fine", and I am inclined to agree.
> 
> Right now, they may be able to have future timestamps ranging to
> year 2100 and switching to time_t would limit their ability to
> express future time to 2038 but they would be able to express
> timestamp in the past to cover most of 20th century.  Given that
> these 32-bit time_t software platforms will die off before year 2038
> (either by underlying hardware getting obsolete, or software updated
> to handle 64-bit time_t), the (temporary) loss of 2038-2100 range
> would not be too big a deal to warrant additional complexity.

You seem to assume that time_t is required to be signed. But from my
understanding that is only guaranteed by POSIX, not by ISO C.

We may very well buy ourselves a ton of trouble if we decide to switch to
`time_t` rather than to `int64_t`.

Ciao,
Johannes

Re: [PATCH 0/6] Use time_t

[Jeff King]

On Tue, Feb 28, 2017 at 10:55:49AM -0800, Junio C Hamano wrote:

> > Glibc will get a way to enable 64-bit time_t on 32-bit platforms
> > eventually
> > (https://sourceware.org/glibc/wiki/Y2038ProofnessDesign). Can
> > platforms that won't provide a 64-bit time_t by 2038 be actually used
> > at that point?  How would we get time information on them?  How would
> > a custom timestamp_t help us?
> 
> That's a sensible "wait, let's step back a bit".  I take it that you
> are saying "time_t is just fine", and I am inclined to agree.
> 
> Right now, they may be able to have future timestamps ranging to
> year 2100 and switching to time_t would limit their ability to
> express future time to 2038 but they would be able to express
> timestamp in the past to cover most of 20th century.  Given that
> these 32-bit time_t software platforms will die off before year 2038
> (either by underlying hardware getting obsolete, or software updated
> to handle 64-bit time_t), the (temporary) loss of 2038-2100 range
> would not be too big a deal to warrant additional complexity.

For what it's worth, I'm on board with just using time_t if it reduces
the overall complexity. I agree that the "loss" of far-future timestamp
handling is unlikely to matter between now and 2038, and those systems
will have to figure out their time_t problems by then. By actually using
time_t we get to piggy-back on their solution.

-Peff

Re: [PATCH 0/6] Use time_t

[Jeff King]

On Tue, Feb 28, 2017 at 09:26:23AM -0800, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > I do not just agree, but I think the move to a signed timestamp is a big
> > improvement. Git's object format is happy to represent times before
> > 1970, but the code is not. I know this has been a pain for people who
> > import ancient histories into Git.
> >
> > It looks from the discussion like the sanest path forward is our own
> > signed-64bit timestamp_t. That's unfortunate compared to using the
> > standard time_t, but hopefully it would reduce the number of knobs (like
> > TIME_T_IS_INT64) in the long run.
> 
> Keeping it unsigned is safer in the short-term.  There are some
> places that uses 0 as "impossible time" (e.g. somebody tried to
> parse a string as time and returns a failure) and these places need
> to be found and be replaced with probably the most negative value
> that timestamp_t cn represent.  Another possible special value we
> may use is for "expiring everything" but I think we tend to just use
> the timestamp of the present time for that purpose and not UONG_MAX,
> so we should be OK there.

Yeah. I think I was the one who invented the "0 is impossible"
convention. We can certainly stick with it for now (it's awkward if you
really do have an entry on Jan 1 1970, but other than that it's an OK
marker). I agree that the most negatively value is probably a saner
choice, but we can switch to it after the dust settles.

> But we need to cross the bridge to signed timestamp sometime, and I
> do not see any reason why that somtime should not be now.

Yep.

-Peff

Re: [PATCH 0/6] Use time_t

[Junio C Hamano]

René Scharfe  writes:

> Am 28.02.2017 um 15:28 schrieb Jeff King:
>
>> It looks from the discussion like the sanest path forward is our own
>> signed-64bit timestamp_t. That's unfortunate compared to using the
>> standard time_t, but hopefully it would reduce the number of knobs (like
>> TIME_T_IS_INT64) in the long run.
>
> Glibc will get a way to enable 64-bit time_t on 32-bit platforms
> eventually
> (https://sourceware.org/glibc/wiki/Y2038ProofnessDesign). Can
> platforms that won't provide a 64-bit time_t by 2038 be actually used
> at that point?  How would we get time information on them?  How would
> a custom timestamp_t help us?

That's a sensible "wait, let's step back a bit".  I take it that you
are saying "time_t is just fine", and I am inclined to agree.

Right now, they may be able to have future timestamps ranging to
year 2100 and switching to time_t would limit their ability to
express future time to 2038 but they would be able to express
timestamp in the past to cover most of 20th century.  Given that
these 32-bit time_t software platforms will die off before year 2038
(either by underlying hardware getting obsolete, or software updated
to handle 64-bit time_t), the (temporary) loss of 2038-2100 range
would not be too big a deal to warrant additional complexity.

> Regarding the need for knobs: We could let the compiler chose between
> strtoll() and strtol() based on the size of time_t, in an inline
> function.  The maximum value can be calculated using its size as
> well. And we could use PRIdMAX and cast to intmax_t for printing.

Thanks.

Re: [PATCH 0/6] Use time_t

[Junio C Hamano]

Jeff King  writes:

> I do not just agree, but I think the move to a signed timestamp is a big
> improvement. Git's object format is happy to represent times before
> 1970, but the code is not. I know this has been a pain for people who
> import ancient histories into Git.
>
> It looks from the discussion like the sanest path forward is our own
> signed-64bit timestamp_t. That's unfortunate compared to using the
> standard time_t, but hopefully it would reduce the number of knobs (like
> TIME_T_IS_INT64) in the long run.

Keeping it unsigned is safer in the short-term.  There are some
places that uses 0 as "impossible time" (e.g. somebody tried to
parse a string as time and returns a failure) and these places need
to be found and be replaced with probably the most negative value
that timestamp_t cn represent.  Another possible special value we
may use is for "expiring everything" but I think we tend to just use
the timestamp of the present time for that purpose and not UONG_MAX,
so we should be OK there.

But we need to cross the bridge to signed timestamp sometime, and I
do not see any reason why that somtime should not be now.

Re: [PATCH 0/6] Use time_t

[René Scharfe]

Am 28.02.2017 um 15:28 schrieb Jeff King:

On Mon, Feb 27, 2017 at 10:30:20PM +0100, Johannes Schindelin wrote:


One notable fallout of this patch series is that on 64-bit Linux (and
other platforms where `unsigned long` is 64-bit), we now limit the range
of dates to LONG_MAX (i.e. the *signed* maximum value). This needs to be
done as `time_t` can be signed (and indeed is at least on my Ubuntu
setup).

Obviously, I think that we can live with that, and I hope that all
interested parties agree.


I do not just agree, but I think the move to a signed timestamp is a big
improvement. Git's object format is happy to represent times before
1970, but the code is not. I know this has been a pain for people who
import ancient histories into Git.

It looks from the discussion like the sanest path forward is our own
signed-64bit timestamp_t. That's unfortunate compared to using the
standard time_t, but hopefully it would reduce the number of knobs (like
TIME_T_IS_INT64) in the long run.


Glibc will get a way to enable 64-bit time_t on 32-bit platforms 
eventually (https://sourceware.org/glibc/wiki/Y2038ProofnessDesign). 
Can platforms that won't provide a 64-bit time_t by 2038 be actually 
used at that point?  How would we get time information on them?  How 
would a custom timestamp_t help us?


Regarding the need for knobs: We could let the compiler chose between 
strtoll() and strtol() based on the size of time_t, in an inline 
function.  The maximum value can be calculated using its size as well. 
And we could use PRIdMAX and cast to intmax_t for printing.


René

Re: [PATCH 0/6] Use time_t

[Johannes Schindelin]

Hi Peff,

On Tue, 28 Feb 2017, Jeff King wrote:

> On Mon, Feb 27, 2017 at 10:30:20PM +0100, Johannes Schindelin wrote:
> 
> > One notable fallout of this patch series is that on 64-bit Linux (and
> > other platforms where `unsigned long` is 64-bit), we now limit the
> > range of dates to LONG_MAX (i.e. the *signed* maximum value). This
> > needs to be done as `time_t` can be signed (and indeed is at least on
> > my Ubuntu setup).
> > 
> > Obviously, I think that we can live with that, and I hope that all
> > interested parties agree.
> 
> I do not just agree, but I think the move to a signed timestamp is a big
> improvement. Git's object format is happy to represent times before
> 1970, but the code is not. I know this has been a pain for people who
> import ancient histories into Git.
> 
> It looks from the discussion like the sanest path forward is our own
> signed-64bit timestamp_t. That's unfortunate compared to using the
> standard time_t, but hopefully it would reduce the number of knobs (like
> TIME_T_IS_INT64) in the long run.

Boy am I happy that I did not go ahead and changed the code to use
uint64_t yet...

I'll let the dust settle a bit and then make further changes and send out
v2.

Ciao,
Dscho

Re: [PATCH 0/6] Use time_t

[Jeff King]

On Mon, Feb 27, 2017 at 10:30:20PM +0100, Johannes Schindelin wrote:

> One notable fallout of this patch series is that on 64-bit Linux (and
> other platforms where `unsigned long` is 64-bit), we now limit the range
> of dates to LONG_MAX (i.e. the *signed* maximum value). This needs to be
> done as `time_t` can be signed (and indeed is at least on my Ubuntu
> setup).
> 
> Obviously, I think that we can live with that, and I hope that all
> interested parties agree.

I do not just agree, but I think the move to a signed timestamp is a big
improvement. Git's object format is happy to represent times before
1970, but the code is not. I know this has been a pain for people who
import ancient histories into Git.

It looks from the discussion like the sanest path forward is our own
signed-64bit timestamp_t. That's unfortunate compared to using the
standard time_t, but hopefully it would reduce the number of knobs (like
TIME_T_IS_INT64) in the long run.

-Peff

Re: [PATCH 0/6] Use time_t

[Johannes Schindelin]

Hi Junio,

On Mon, 27 Feb 2017, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > One notable fallout of this patch series is that on 64-bit Linux (and
> > other platforms where `unsigned long` is 64-bit), we now limit the
> > range of dates to LONG_MAX (i.e. the *signed* maximum value). This
> > needs to be done as `time_t` can be signed (and indeed is at least on
> > my Ubuntu setup).
> >
> > Obviously, I think that we can live with that, and I hope that all
> > interested parties agree.
> 
> s/ulong/time_t/ is definintely a good change, and it will take us to a
> place we would want to be in in some future.  

Actually. I have to take back the part where I hoped that all interested
parties would agree. The problem is 32-bit Linux:

$ cat >a1.c <<-\EOF
#include 
#include 
#include 

int main(int argc, char **argv)
{
printf("sizeof(long): %d, sizeof(time_t): %d, ulong_max: %lu\n",
   (int)sizeof(long), (int)sizeof(time_t), ULONG_MAX);
return 0;
}
EOF

$ gcc -m32 -Wall -o a1 a1.c

$ ./a1
sizeof(long): 4, sizeof(time_t): 4, ulong_max: 4294967295

So. Not only is `long` a 32-bit on 32-bit Linux, but so is `time_t`. And
with that, switching from `ULONG_MAX` as the maximal time we can represent
in Git to `LONG_MAX` is kind of a serious problem.

> As long as there remains no platform we care about whose time_t and long
> are still 32-bit signed integer, there will be a fallout to them with
> this change.

Sorry, I do not understand the verb "remains" in conjunction with "no
platform"...

Do you mean to say that currently no platform we care about has 32-bit
signed time_t/long?

If so, I just demonstrated this to be unfortunately incorrect.

> It appears that we use uint64_t in many places in our code.  So
> while philosophically time_t is the right type, uint64_t might be
> practically a safer alternative type to use at the endgame patch in
> this series.

Yes, I think you are right. We should use uint64_t instead of time_t, but
*semantically* we should not even use uint64_t. We should introduce our
own data type instead of repeating the mistake to use a data type that
does not convey its role to the reader.

Currently, I am favoring timestamp_t.

> I haven't seen it yet, but presumably the last one 6/6 is the endgame?

Maybe it took a while to get sent out, but it made it into public inbox:
http://public-inbox.org/git/75efe76cbb0636741a7c3aec9e21459bc1dc3cbe.1488231002.git.johannes.schinde...@gmx.de/

Ciao,
Johannes

Re: [PATCH 0/6] Use time_t

[Junio C Hamano]

Johannes Schindelin  writes:

> One notable fallout of this patch series is that on 64-bit Linux (and
> other platforms where `unsigned long` is 64-bit), we now limit the range
> of dates to LONG_MAX (i.e. the *signed* maximum value). This needs to be
> done as `time_t` can be signed (and indeed is at least on my Ubuntu
> setup).
>
> Obviously, I think that we can live with that, and I hope that all
> interested parties agree.

s/ulong/time_t/ is definintely a good change, and it will take us to
a place we would want to be in in some future.  

As long as there remains no platform we care about whose time_t and
long are still 32-bit signed integer, there will be a fallout to
them with this change.  Probably it is of a larger impact than
losing the upper half of a 64-bit timestamp range on larger boxes.
Hopefully those platforms have died out (or at least we don't mind
breaking them)?

It appears that we use uint64_t in many places in our code.  So
while philosophically time_t is the right type, uint64_t might be
practically a safer alternative type to use at the endgame patch in
this series.  I haven't seen it yet, but presumably the last one 6/6
is the endgame?

[PATCH 0/6] Use time_t

[Johannes Schindelin]

Git v2.9.2 was released in a hurry to accomodate for platforms like
Windows, where the `unsigned long` data type is 32-bit even for 64-bit
setups.

The quick fix was to simply disable all the testing with "absurd" future
dates.

However, we can do much better than that, as `time_t` exists, and at
least on 64-bit Windows it is 64-bit. Meaning: we *can* support these
absurd future dates on those platforms.

So let's do this.

One notable fallout of this patch series is that on 64-bit Linux (and
other platforms where `unsigned long` is 64-bit), we now limit the range
of dates to LONG_MAX (i.e. the *signed* maximum value). This needs to be
done as `time_t` can be signed (and indeed is at least on my Ubuntu
setup).

Obviously, I think that we can live with that, and I hope that all
interested parties agree.


Johannes Schindelin (6):
  t0006 & t5000: prepare for 64-bit time_t
  Specify explicitly where we parse timestamps
  Introduce a new "printf format" for timestamps
  Prepare for timestamps to use 64-bit signed types
  ref-filter: avoid using `unsigned long` for catch-all data type
  Use time_t where appropriate

 Documentation/technical/api-parse-options.txt |  8 +--
 Makefile  |  4 ++
 archive-tar.c |  5 +-
 builtin/am.c  |  4 +-
 builtin/blame.c   | 14 ++---
 builtin/fsck.c|  6 +-
 builtin/gc.c  |  2 +-
 builtin/log.c |  4 +-
 builtin/merge-base.c  |  2 +-
 builtin/name-rev.c|  6 +-
 builtin/pack-objects.c|  4 +-
 builtin/prune.c   |  4 +-
 builtin/receive-pack.c| 10 +--
 builtin/reflog.c  | 24 +++
 builtin/show-branch.c |  4 +-
 builtin/worktree.c|  4 +-
 bundle.c  |  4 +-
 cache.h   | 14 ++---
 commit.c  | 16 ++---
 commit.h  |  2 +-
 config.mak.uname  |  2 +
 credential-cache--daemon.c| 12 ++--
 date.c| 90 +--
 fetch-pack.c  |  8 +--
 fsck.c|  2 +-
 git-compat-util.h | 10 +++
 http-backend.c|  4 +-
 parse-options-cb.c|  4 +-
 pretty.c  |  4 +-
 reachable.c   | 10 ++-
 reachable.h   |  4 +-
 ref-filter.c  | 22 +++
 reflog-walk.c |  8 +--
 refs.c| 14 ++---
 refs.h|  8 +--
 refs/files-backend.c  |  4 +-
 revision.c|  6 +-
 revision.h|  4 +-
 sha1_name.c   |  6 +-
 t/helper/test-date.c  | 11 ++--
 t/helper/test-parse-options.c |  4 +-
 t/t0006-date.sh   |  4 +-
 t/t5000-tar-tree.sh   |  6 +-
 t/test-lib.sh |  2 +
 tag.c |  4 +-
 tag.h |  2 +-
 upload-pack.c |  8 +--
 vcs-svn/fast_export.c |  8 +--
 vcs-svn/fast_export.h |  4 +-
 vcs-svn/svndump.c |  2 +-
 wt-status.c   |  2 +-
 51 files changed, 221 insertions(+), 199 deletions(-)


base-commit: e7e07d5a4fcc2a203d9873968ad3e6bd4d7419d7
Published-As: https://github.com/dscho/git/releases/tag/time_t-may-be-int64-v1
Fetch-It-Via: git fetch https://github.com/dscho/git time_t-may-be-int64-v1

-- 
2.11.1.windows.1.379.g44ae0bc