threads / patch / 20765

patchRound-down years in "years+months" relative date view

Subject: [PATCH] Round-down years in "years+months" relative date view

## tl;dr

47 messages between Aug 27, 2009 and Sep 1, 2009. Diffs are folded; open one to read it.

replies: 46people: 6as markdown or json

David Reiss· Aug 27, 2009, 23:39 UTC · lore

Previously, a commit from 1 year and 7 months ago would display as "2 years, 7 months ago".

Signed-off-by: David Reiss <dreiss@facebook.com>
---

Here's my test script. Let me know if you'd rather have it as part of the test suite.

#!/bin/sh set -e REPO="git-relative-dates-test" rm -rf "$REPO" mkdir "$REPO" cd "$REPO" git init NOW=`date +%s` env GIT_AUTHOR_DATE=`expr $NOW - \( 365 + 220 \) \* 24 \* 60 \* 60` git commit --allow-empty -m old-commit git log --date=relative

 date.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Show changes to date.c +1 −1
diff --git a/date.c b/date.c
index 1de1845..e848d96 100644
--- a/date.c
+++ b/date.c
@@ -137,7 +137,7 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
 		}
 		/* Give years and months for 5 years or so */
 		if (diff < 1825) {
-			unsigned long years = (diff + 183) / 365;
+			unsigned long years = diff / 365;
 			unsigned long months = (diff % 365 + 15) / 30;
 			int n;
 			n = snprintf(timebuf, sizeof(timebuf), "%lu year%s",
-- 
1.6.0.4
Jeff King· Aug 28, 2009, 06:05 UTC · re: David Reiss · lore

Re: [PATCH] Round-down years in "years+months" relative date view

On Thu, Aug 27, 2009 at 04:39:38PM -0700, David Reiss wrote:
> Previously, a commit from 1 year and 7 months ago would display as
> "2 years, 7 months ago".
Wow, embarrassing.
Acked-by: Jeff King <peff@peff.net>
> Here's my test script.  Let me know if you'd rather have it as part
> of the test suite.

I couldn't find any tests related to relative date processing, so it would be really nice to have some. But I'm not sure of the best way to do it without dealing with race conditions. Annoyingly, show_date calls gettimeofday at a pretty low level, so there isn't a way of instrumenting it short of LD_PRELOAD trickery (which is probably not very portable).

But maybe a patch like this is worth doing, which would allow us to test in a repeatable fashion:

---
Show changes to date.c +29 −2
diff --git a/date.c b/date.c
index e848d96..db2f831 100644
--- a/date.c
+++ b/date.c
@@ -86,6 +86,33 @@ static int local_tzoffset(unsigned long time)
 	return offset * eastwest;
 }
 
+static int current_time(struct timeval *now)
+{
+	static struct timeval fake_time;
+	static int use_fake_time = -1;
+
+	if (use_fake_time == -1) {
+		const char *x = getenv("GIT_FAKE_TIME");
+		if (x) {
+			char buf[50];
+			if (parse_date(x, buf, sizeof(buf)) <= 0)
+				die("unable to parse GIT_FAKE_TIME");
+			fake_time.tv_sec = strtoul(buf, NULL, 10);
+			fake_time.tv_usec = 0;
+			use_fake_time = 1;
+		}
+		else
+			use_fake_time = 0;
+	}
+
+	if (use_fake_time == 1) {
+		memcpy(now, &fake_time, sizeof(*now));
+		return 0;
+	}
+
+	return gettimeofday(now, NULL);
+}
+
 const char *show_date(unsigned long time, int tz, enum date_mode mode)
 {
 	struct tm *tm;
@@ -99,7 +126,7 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
 	if (mode == DATE_RELATIVE) {
 		unsigned long diff;
 		struct timeval now;
-		gettimeofday(&now, NULL);
+		current_time(&now);
 		if (now.tv_sec < time)
 			return "in the future";
 		diff = now.tv_sec - time;
@@ -929,7 +956,7 @@ unsigned long approxidate(const char *date)
 	if (parse_date(date, buffer, sizeof(buffer)) > 0)
 		return strtoul(buffer, NULL, 10);
 
-	gettimeofday(&tv, NULL);
+	current_time(&tv);
 	time_sec = tv.tv_sec;
 	localtime_r(&time_sec, &tm);
 	now = tm;
Alex Riesen· Aug 28, 2009, 07:58 UTC · re: Jeff King · lore

Re: [PATCH] Round-down years in "years+months" relative date view

On Fri, Aug 28, 2009 at 08:05, Jeff King<peff@peff.net> wrote:
Show 18 quoted lines
> On Thu, Aug 27, 2009 at 04:39:38PM -0700, David Reiss wrote:
>
>> Previously, a commit from 1 year and 7 months ago would display as
>> "2 years, 7 months ago".
>
> Wow, embarrassing.
>
> Acked-by: Jeff King <peff@peff.net>
>
>> Here's my test script.  Let me know if you'd rather have it as part
>> of the test suite.
>
> I couldn't find any tests related to relative date processing, so it
> would be really nice to have some. But I'm not sure of the best way to
> do it without dealing with race conditions. Annoyingly, show_date calls
> gettimeofday at a pretty low level, so there isn't a way of
> instrumenting it short of LD_PRELOAD trickery (which is probably not
> very portable).

Maybe better prepare the _test_ so that it uses current time and time arithmetics then put yet another cludge in operational code? Especially when we already have a greate number of GIT_ environment variables, documented nowhere, with effects not immediately obvious:

  $ git grep -n '"GIT_'| perl -ne '/"(GIT_\w+)/ && print "$1\n"' |
sort |uniq | wc -l
  49
  $ git grep -n '"GIT_'|grep ^Documentation
  $
GIT_FLUSH? GIT_SEND_EMAIL_NOTTY?! GIT_CHERRY_PICK_HELP?!!
Jeff King· Aug 28, 2009, 15:02 UTC · re: Alex Riesen · lore

Re: [PATCH] Round-down years in "years+months" relative date view

On Fri, Aug 28, 2009 at 09:58:27AM +0200, Alex Riesen wrote:
Show 11 quoted lines
> > I couldn't find any tests related to relative date processing, so it
> > would be really nice to have some. But I'm not sure of the best way to
> > do it without dealing with race conditions. Annoyingly, show_date calls
> > gettimeofday at a pretty low level, so there isn't a way of
> > instrumenting it short of LD_PRELOAD trickery (which is probably not
> > very portable).
> 
> Maybe better prepare the _test_ so that it uses current time and time
> arithmetics then put yet another cludge in operational code? Especially
> when we already have a greate number of GIT_ environment variables,
> documented nowhere, with effects not immediately obvious:

But that's the point: you can't do that without a race condition. Your test gets a sense of the current time, then runs git, which checks the current time again. How many seconds elapsed between the two checks?

I guess it is good enough for testing large time spans, but I was hoping for a comprehensive time test.

-Peff
Alex Riesen· Aug 28, 2009, 17:00 UTC · re: Jeff King · lore

Re: [PATCH] Round-down years in "years+months" relative date view

On Fri, Aug 28, 2009 at 17:02, Jeff King<peff@peff.net> wrote:
> But that's the point: you can't do that without a race condition. Your
> test gets a sense of the current time, then runs git, which checks the
> current time again. How many seconds elapsed between the two checks?
How _many_ do you need?
Jeff King· Aug 28, 2009, 17:15 UTC · re: Alex Riesen · lore

Re: [PATCH] Round-down years in "years+months" relative date view

On Fri, Aug 28, 2009 at 07:00:59PM +0200, Alex Riesen wrote:
Show 6 quoted lines
> On Fri, Aug 28, 2009 at 17:02, Jeff King<peff@peff.net> wrote:
> > But that's the point: you can't do that without a race condition. Your
> > test gets a sense of the current time, then runs git, which checks the
> > current time again. How many seconds elapsed between the two checks?
> 
> How _many_ do you need?

I don't understand what you're trying to say. My point is that if you are checking results to a one-second precision, you need to know whether zero seconds elapsed, or one second, or two seconds, or whatever to get a consistent result.

-Peff
Alex Riesen· Aug 28, 2009, 18:21 UTC · re: Jeff King · lore

Re: [PATCH] Round-down years in "years+months" relative date view

On Fri, Aug 28, 2009 at 19:15, Jeff King<peff@peff.net> wrote:
Show 13 quoted lines
> On Fri, Aug 28, 2009 at 07:00:59PM +0200, Alex Riesen wrote:
>
>> On Fri, Aug 28, 2009 at 17:02, Jeff King<peff@peff.net> wrote:
>> > But that's the point: you can't do that without a race condition. Your
>> > test gets a sense of the current time, then runs git, which checks the
>> > current time again. How many seconds elapsed between the two checks?
>>
>> How _many_ do you need?
>
> I don't understand what you're trying to say. My point is that if you
> are checking results to a one-second precision, you need to know whether
> zero seconds elapsed, or one second, or two seconds, or whatever to get
> a consistent result.

Taking this particular case as an example, can't we just set the time of the _commit_ back in time? We can. And we don't need to know the difference precisely, it can be something like /[0-9]+ ago/, can't it? Ok, it is possible, that something goes terribly wrong and the test suite freezes for an extended period of time, so the pattern above does not apply anymore. In this case, wont you prefer the test suite to break? Ok, maybe not, if the freeze was an Ctrl-Z pressed at unlucky moment. Which involves an operator online and looking, and action and reaction will be both visible.

So, yes, it is not absolutely safe, but this approach has no side effects on the working code. And very low probability of something go wrong.

A Large Angry SCM· Aug 28, 2009, 22:01 UTC · re: Jeff King · lore

Re: [PATCH] Round-down years in "years+months" relative date view

Jeff King wrote:
Show 12 quoted lines
> On Fri, Aug 28, 2009 at 07:00:59PM +0200, Alex Riesen wrote:
> 
>> On Fri, Aug 28, 2009 at 17:02, Jeff King<peff@peff.net> wrote:
>>> But that's the point: you can't do that without a race condition. Your
>>> test gets a sense of the current time, then runs git, which checks the
>>> current time again. How many seconds elapsed between the two checks?
>> How _many_ do you need?
> 
> I don't understand what you're trying to say. My point is that if you
> are checking results to a one-second precision, you need to know whether
> zero seconds elapsed, or one second, or two seconds, or whatever to get
> a consistent result.

To no-one in particular, Gitzilla mumbles "To do this right(tm) would probably require LD_PRELOAD magic with a environment variable invocation.

Nicolas Pitre· Aug 28, 2009, 17:28 UTC · re: Jeff King · lore

Re: [PATCH] Round-down years in "years+months" relative date view

On Fri, 28 Aug 2009, Jeff King wrote:
Show 20 quoted lines
> On Fri, Aug 28, 2009 at 09:58:27AM +0200, Alex Riesen wrote:
> 
> > > I couldn't find any tests related to relative date processing, so it
> > > would be really nice to have some. But I'm not sure of the best way to
> > > do it without dealing with race conditions. Annoyingly, show_date calls
> > > gettimeofday at a pretty low level, so there isn't a way of
> > > instrumenting it short of LD_PRELOAD trickery (which is probably not
> > > very portable).
> > 
> > Maybe better prepare the _test_ so that it uses current time and time
> > arithmetics then put yet another cludge in operational code? Especially
> > when we already have a greate number of GIT_ environment variables,
> > documented nowhere, with effects not immediately obvious:
> 
> But that's the point: you can't do that without a race condition. Your
> test gets a sense of the current time, then runs git, which checks the
> current time again. How many seconds elapsed between the two checks?
> 
> I guess it is good enough for testing large time spans, but I was hoping
> for a comprehensive time test.

I agree with your concern. This is why I created the --index-version switch to pack-objects.

However I was hoping for a current time trickery solution that could live in test-date.c instead of interfering with the main code in such a way.

Did a quick test to override the library version:
Show changes to test-date.c +5 −0
diff --git a/test-date.c b/test-date.c
index 62e8f23..0bcd0c9 100644
--- a/test-date.c
+++ b/test-date.c
@@ -1,5 +1,10 @@
 #include "cache.h"
 
+int gettimeofday(struct timeval *tv, struct timezone *tz)
+{
+	return 0;
+}
+
 int main(int argc, char **argv)
 {
 	int i;

Result:

$ ./test-date now
now -> bad -> Wed Dec 31 19:00:00 1969
now -> Tue Jan 22 10:48:24 10199

So this seems to work.  ;-)


Nicolas
Jeff King· Aug 28, 2009, 18:01 UTC · re: Nicolas Pitre · lore

Re: [PATCH] Round-down years in "years+months" relative date view

On Fri, Aug 28, 2009 at 01:28:34PM -0400, Nicolas Pitre wrote:
Show 5 quoted lines
> However I was hoping for a current time trickery solution that could 
> live in test-date.c instead of interfering with the main code in such a 
> way.
> 
> Did a quick test to override the library version:

Thanks, that is a much better solution. And I don't know offhand of any portability problems in overriding the library at link time.

-Peff
Alex Riesen· Aug 28, 2009, 18:27 UTC · re: Jeff King · lore

Re: [PATCH] Round-down years in "years+months" relative date view

On Fri, Aug 28, 2009 at 20:01, Jeff King<peff@peff.net> wrote:
Show 11 quoted lines
> On Fri, Aug 28, 2009 at 01:28:34PM -0400, Nicolas Pitre wrote:
>
>> However I was hoping for a current time trickery solution that could
>> live in test-date.c instead of interfering with the main code in such a
>> way.
>>
>> Did a quick test to override the library version:
>
> Thanks, that is a much better solution. And I don't know offhand of any
> portability problems in overriding the library at link time.
>
Microsoft's compiler and libraries? MacOSX?
Jeff King· Aug 28, 2009, 18:39 UTC · re: Alex Riesen · lore

Re: [PATCH] Round-down years in "years+months" relative date view

On Fri, Aug 28, 2009 at 08:27:06PM +0200, Alex Riesen wrote:
> > Thanks, that is a much better solution. And I don't know offhand of any
> > portability problems in overriding the library at link time.
> 
> Microsoft's compiler and libraries? MacOSX?

Are you saying you know those to be platforms with problems, or are you asking whether those platforms will have problems?

-Peff
Alex Riesen· Aug 28, 2009, 18:42 UTC · re: Jeff King · lore

Re: [PATCH] Round-down years in "years+months" relative date view

On Fri, Aug 28, 2009 at 20:39, Jeff King<peff@peff.net> wrote:
Show 9 quoted lines
> On Fri, Aug 28, 2009 at 08:27:06PM +0200, Alex Riesen wrote:
>
>> > Thanks, that is a much better solution. And I don't know offhand of any
>> > portability problems in overriding the library at link time.
>>
>> Microsoft's compiler and libraries? MacOSX?
>
> Are you saying you know those to be platforms with problems, or are you
> asking whether those platforms will have problems?
Both: MS never had weak/vague linkage, but I don't know about MacOSX.
I suspect them to have the same deficiency, but I'd be glad to be wrong.
Alex Riesen· Aug 28, 2009, 18:49 UTC · re: Alex Riesen · lore

Re: [PATCH] Round-down years in "years+months" relative date view

On Fri, Aug 28, 2009 at 20:42, Alex Riesen<raa.lkml@gmail.com> wrote:
Show 6 quoted lines
> On Fri, Aug 28, 2009 at 20:39, Jeff King<peff@peff.net> wrote:
>> On Fri, Aug 28, 2009 at 08:27:06PM +0200, Alex Riesen wrote:
>>
>>> > Thanks, that is a much better solution. And I don't know offhand of any
>>> > portability problems in overriding the library at link time.
>>>

Hm, how about supplying show_date and approxidate with current time? Like this:

/* exported */ const char *show_date_rel(unsigned long time, int tz, struct timeval *now) { ... the DATE_RELATIVE path of show_date }

const char *show_date(unsigned long time, int tz, enum date_mode mode)
{
  struct timeval now;
  if (mode == DATE_RELATIVE) {
    gettimeofday(&now, NULL);
    return show_date_rel(time, tz, &now);
  }
  ... other paths of old show_date
}

Still affects the performance, but much less, and no side effects. And you can test show_date_rel path in test-date.c

Nicolas Pitre· Aug 28, 2009, 19:00 UTC · re: Alex Riesen · lore

Re: [PATCH] Round-down years in "years+months" relative date view

On Fri, 28 Aug 2009, Alex Riesen wrote:
Show 12 quoted lines
> On Fri, Aug 28, 2009 at 20:39, Jeff King<peff@peff.net> wrote:
> > On Fri, Aug 28, 2009 at 08:27:06PM +0200, Alex Riesen wrote:
> >
> >> > Thanks, that is a much better solution. And I don't know offhand of any
> >> > portability problems in overriding the library at link time.
> >>
> >> Microsoft's compiler and libraries? MacOSX?
> >
> > Are you saying you know those to be platforms with problems, or are you
> > asking whether those platforms will have problems?
> 
> Both: MS never had weak/vague linkage, but I don't know about MacOSX.

This is not about weak or vague linkage. This is plain basic linker feature where no library object needs to be linked if there is no symbol to resolve.

> I suspect them to have the same deficiency, but I'd be glad to be wrong.
Are you able to test it?
Nicolas
Alex Riesen· Aug 28, 2009, 19:08 UTC · re: Nicolas Pitre · lore

Re: [PATCH] Round-down years in "years+months" relative date view

On Fri, Aug 28, 2009 at 21:00, Nicolas Pitre<nico@cam.org> wrote:
Show 10 quoted lines
>> >> Microsoft's compiler and libraries? MacOSX?
>> >
>> > Are you saying you know those to be platforms with problems, or are you
>> > asking whether those platforms will have problems?
>>
>> Both: MS never had weak/vague linkage, but I don't know about MacOSX.
>
> This is not about weak or vague linkage.  This is plain basic linker
> feature where no library object needs to be linked if there is no symbol
> to resolve.

Maybe I missed something, but wasn't the idea to overwrite gettimeofday with a public gettimeofday, defined in one of the object files? And shouldn't a linker complain regarding duplicated symbols, unless the other (library) symbol is defined as a weak symbol, allowing overriding it with another symbol of stronger linkage?

>> I suspect them to have the same deficiency, but I'd be glad to be wrong.
>
> Are you able to test it?
>
Only the MS, but it is not interesting.
Nicolas Pitre· Aug 28, 2009, 19:27 UTC · re: Alex Riesen · lore

Re: [PATCH] Round-down years in "years+months" relative date view

On Fri, 28 Aug 2009, Alex Riesen wrote:
Show 14 quoted lines
> On Fri, Aug 28, 2009 at 21:00, Nicolas Pitre<nico@cam.org> wrote:
> >> >> Microsoft's compiler and libraries? MacOSX?
> >> >
> >> > Are you saying you know those to be platforms with problems, or are you
> >> > asking whether those platforms will have problems?
> >>
> >> Both: MS never had weak/vague linkage, but I don't know about MacOSX.
> >
> > This is not about weak or vague linkage.  This is plain basic linker
> > feature where no library object needs to be linked if there is no symbol
> > to resolve.
> 
> Maybe I missed something, but wasn't the idea to overwrite gettimeofday
> with a public gettimeofday, defined in one of the object files?
Yes, in test-date.o.
> And shouldn't a linker complain regarding duplicated symbols, unless
> the other (library) symbol is defined as a weak symbol, allowing
> overriding it with another symbol of stronger linkage?

Normally a linker would search for new objects to link only when there are still symbols to resolve. If the library is well architected (mind you I don't know if that is the case on Windows or OS X) you should find many small object files in a library, so to have only related functions together in a single object for only the needed code to be linked in the final binary. Hence the printf symbol should be in a separate object file than gettimeofday, etc.

Only if the library's object file containing gettimeofday also contains another symbol pulled by the linker will you see a duplicated symbol error. But this is still a possibility. So your proposal is probably cleaner.

Nicolas
Alex Riesen· Aug 28, 2009, 19:49 UTC · re: Nicolas Pitre · lore

Re: [PATCH] Round-down years in "years+months" relative date view

Nicolas Pitre, Fri, Aug 28, 2009 21:27:53 +0200:
Show 17 quoted lines
> On Fri, 28 Aug 2009, Alex Riesen wrote:
> > And shouldn't a linker complain regarding duplicated symbols, unless
> > the other (library) symbol is defined as a weak symbol, allowing
> > overriding it with another symbol of stronger linkage?
> 
> Normally a linker would search for new objects to link only when there 
> are still symbols to resolve.  If the library is well architected (mind 
> you I don't know if that is the case on Windows or OS X) you should find 
> many small object files in a library, so to have only related functions 
> together in a single object for only the needed code to be linked in the 
> final binary. Hence the printf symbol should be in a separate object 
> file than gettimeofday, etc.
> 
> Only if the library's object file containing gettimeofday also contains 
> another symbol pulled by the linker will you see a duplicated symbol 
> error.  But this is still a possibility.  So your proposal is probably 
> cleaner.
Is it so for dynamic linking as well? Like in libc.so?
Nicolas Pitre· Aug 28, 2009, 20:01 UTC · re: Alex Riesen · lore

Re: [PATCH] Round-down years in "years+months" relative date view

On Fri, 28 Aug 2009, Alex Riesen wrote:
Show 20 quoted lines
> Nicolas Pitre, Fri, Aug 28, 2009 21:27:53 +0200:
> > On Fri, 28 Aug 2009, Alex Riesen wrote:
> > > And shouldn't a linker complain regarding duplicated symbols, unless
> > > the other (library) symbol is defined as a weak symbol, allowing
> > > overriding it with another symbol of stronger linkage?
> > 
> > Normally a linker would search for new objects to link only when there 
> > are still symbols to resolve.  If the library is well architected (mind 
> > you I don't know if that is the case on Windows or OS X) you should find 
> > many small object files in a library, so to have only related functions 
> > together in a single object for only the needed code to be linked in the 
> > final binary. Hence the printf symbol should be in a separate object 
> > file than gettimeofday, etc.
> > 
> > Only if the library's object file containing gettimeofday also contains 
> > another symbol pulled by the linker will you see a duplicated symbol 
> > error.  But this is still a possibility.  So your proposal is probably 
> > cleaner.
> 
> Is it so for dynamic linking as well? Like in libc.so?
Yes.  The linker still links against stubs in that case.
Nicolas
Alex Riesen· Aug 28, 2009, 19:03 UTC · re: Nicolas Pitre · lore

Re: [PATCH] Round-down years in "years+months" relative date view

>From b51bc56816490c71cd37f52be73a06cef6b9bf14 Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa.lkml@gmail.com>
Date: Fri, 28 Aug 2009 20:59:59 +0200
Subject: [PATCH] Add date formatting functions with current time explicitely formatted

It should allow safe testing of this part of the code. --- Nicolas Pitre, Fri, Aug 28, 2009 19:28:34 +0200:

Show 25 quoted lines
> On Fri, 28 Aug 2009, Jeff King wrote:
> > On Fri, Aug 28, 2009 at 09:58:27AM +0200, Alex Riesen wrote:
> > 
> > > > I couldn't find any tests related to relative date processing, so it
> > > > would be really nice to have some. But I'm not sure of the best way to
> > > > do it without dealing with race conditions. Annoyingly, show_date calls
> > > > gettimeofday at a pretty low level, so there isn't a way of
> > > > instrumenting it short of LD_PRELOAD trickery (which is probably not
> > > > very portable).
> > > 
> > > Maybe better prepare the _test_ so that it uses current time and time
> > > arithmetics then put yet another cludge in operational code? Especially
> > > when we already have a greate number of GIT_ environment variables,
> > > documented nowhere, with effects not immediately obvious:
> > 
> > But that's the point: you can't do that without a race condition. Your
> > test gets a sense of the current time, then runs git, which checks the
> > current time again. How many seconds elapsed between the two checks?
> > 
> > I guess it is good enough for testing large time spans, but I was hoping
> > for a comprehensive time test.
> 
> I agree with your concern.  This is why I created the --index-version 
> switch to pack-objects.
> 
This is what I mean with "supplying current time":
 cache.h |    2 +
 date.c  |  130 ++++++++++++++++++++++++++++++++++----------------------------
 2 files changed, 73 insertions(+), 59 deletions(-)
Show changes to 2 files +73 −59

cache.h, date.c

diff --git a/cache.h b/cache.h
index dd7f71e..3fb0166 100644
--- a/cache.h
+++ b/cache.h
@@ -731,9 +731,11 @@ enum date_mode {
 };
 
 const char *show_date(unsigned long time, int timezone, enum date_mode mode);
+const char *show_date_relative(unsigned long time, int tz, const struct timeval *now);
 int parse_date(const char *date, char *buf, int bufsize);
 void datestamp(char *buf, int bufsize);
 unsigned long approxidate(const char *);
+unsigned long approxidate_relative(const char *date, const struct timeval *now);
 enum date_mode parse_date_format(const char *format);
 
 #define IDENT_WARN_ON_NO_NAME  1
diff --git a/date.c b/date.c
index 409a17d..08b4b49 100644
--- a/date.c
+++ b/date.c
@@ -84,6 +84,67 @@ static int local_tzoffset(unsigned long time)
 	return offset * eastwest;
 }
 
+const char *show_date_relative(unsigned long time, int tz, const struct timeval *now)
+{
+	static char timebuf[100 /* TODO: can be optimized */];
+	unsigned long diff;
+	if (now->tv_sec < time)
+		return "in the future";
+	diff = now->tv_sec - time;
+	if (diff < 90) {
+		snprintf(timebuf, sizeof(timebuf), "%lu seconds ago", diff);
+		return timebuf;
+	}
+	/* Turn it into minutes */
+	diff = (diff + 30) / 60;
+	if (diff < 90) {
+		snprintf(timebuf, sizeof(timebuf), "%lu minutes ago", diff);
+		return timebuf;
+	}
+	/* Turn it into hours */
+	diff = (diff + 30) / 60;
+	if (diff < 36) {
+		snprintf(timebuf, sizeof(timebuf), "%lu hours ago", diff);
+		return timebuf;
+	}
+	/* We deal with number of days from here on */
+	diff = (diff + 12) / 24;
+	if (diff < 14) {
+		snprintf(timebuf, sizeof(timebuf), "%lu days ago", diff);
+		return timebuf;
+	}
+	/* Say weeks for the past 10 weeks or so */
+	if (diff < 70) {
+		snprintf(timebuf, sizeof(timebuf), "%lu weeks ago", (diff + 3) / 7);
+		return timebuf;
+	}
+	/* Say months for the past 12 months or so */
+	if (diff < 360) {
+		snprintf(timebuf, sizeof(timebuf), "%lu months ago", (diff + 15) / 30);
+		return timebuf;
+	}
+	/* Give years and months for 5 years or so */
+	if (diff < 1825) {
+		unsigned long years = (diff + 183) / 365;
+		unsigned long months = (diff % 365 + 15) / 30;
+		int n;
+		n = snprintf(timebuf, sizeof(timebuf), "%lu year%s",
+			     years, (years > 1 ? "s" : ""));
+		if (months)
+			snprintf(timebuf + n, sizeof(timebuf) - n,
+				 ", %lu month%s ago",
+				 months, (months > 1 ? "s" : ""));
+		else
+			snprintf(timebuf + n, sizeof(timebuf) - n,
+				 " ago");
+		return timebuf;
+	}
+	/* Otherwise, just years. Centuries is probably overkill. */
+	snprintf(timebuf, sizeof(timebuf), "%lu years ago", (diff + 183) / 365);
+	return timebuf;
+
+}
+
 const char *show_date(unsigned long time, int tz, enum date_mode mode)
 {
 	struct tm *tm;
@@ -95,63 +156,9 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
 	}
 
 	if (mode == DATE_RELATIVE) {
-		unsigned long diff;
 		struct timeval now;
 		gettimeofday(&now, NULL);
-		if (now.tv_sec < time)
-			return "in the future";
-		diff = now.tv_sec - time;
-		if (diff < 90) {
-			snprintf(timebuf, sizeof(timebuf), "%lu seconds ago", diff);
-			return timebuf;
-		}
-		/* Turn it into minutes */
-		diff = (diff + 30) / 60;
-		if (diff < 90) {
-			snprintf(timebuf, sizeof(timebuf), "%lu minutes ago", diff);
-			return timebuf;
-		}
-		/* Turn it into hours */
-		diff = (diff + 30) / 60;
-		if (diff < 36) {
-			snprintf(timebuf, sizeof(timebuf), "%lu hours ago", diff);
-			return timebuf;
-		}
-		/* We deal with number of days from here on */
-		diff = (diff + 12) / 24;
-		if (diff < 14) {
-			snprintf(timebuf, sizeof(timebuf), "%lu days ago", diff);
-			return timebuf;
-		}
-		/* Say weeks for the past 10 weeks or so */
-		if (diff < 70) {
-			snprintf(timebuf, sizeof(timebuf), "%lu weeks ago", (diff + 3) / 7);
-			return timebuf;
-		}
-		/* Say months for the past 12 months or so */
-		if (diff < 360) {
-			snprintf(timebuf, sizeof(timebuf), "%lu months ago", (diff + 15) / 30);
-			return timebuf;
-		}
-		/* Give years and months for 5 years or so */
-		if (diff < 1825) {
-			unsigned long years = (diff + 183) / 365;
-			unsigned long months = (diff % 365 + 15) / 30;
-			int n;
-			n = snprintf(timebuf, sizeof(timebuf), "%lu year%s",
-					years, (years > 1 ? "s" : ""));
-			if (months)
-				snprintf(timebuf + n, sizeof(timebuf) - n,
-					", %lu month%s ago",
-					months, (months > 1 ? "s" : ""));
-			else
-				snprintf(timebuf + n, sizeof(timebuf) - n,
-					" ago");
-			return timebuf;
-		}
-		/* Otherwise, just years. Centuries is probably overkill. */
-		snprintf(timebuf, sizeof(timebuf), "%lu years ago", (diff + 183) / 365);
-		return timebuf;
+		return show_date_relative(time, tz, &now);
 	}
 
 	if (mode == DATE_LOCAL)
@@ -866,19 +873,17 @@ static const char *approxidate_digit(const char *date, struct tm *tm, int *num)
 	return end;
 }
 
-unsigned long approxidate(const char *date)
+unsigned long approxidate_relative(const char *date, const struct timeval *tv)
 {
 	int number = 0;
 	struct tm tm, now;
-	struct timeval tv;
 	time_t time_sec;
 	char buffer[50];
 
 	if (parse_date(date, buffer, sizeof(buffer)) > 0)
 		return strtoul(buffer, NULL, 10);
 
-	gettimeofday(&tv, NULL);
-	time_sec = tv.tv_sec;
+	time_sec = tv->tv_sec;
 	localtime_r(&time_sec, &tm);
 	now = tm;
 	for (;;) {
@@ -899,3 +904,10 @@ unsigned long approxidate(const char *date)
 		tm.tm_year--;
 	return mktime(&tm);
 }
+
+unsigned long approxidate(const char *date)
+{
+	struct timeval tv;
+	gettimeofday(&tv, NULL);
+	return approxidate_relative(date, &tv);
+}
-- 
1.6.4.1.261.gf9874
Jeff King· Aug 28, 2009, 19:15 UTC · re: Alex Riesen · lore

Re: [PATCH] Round-down years in "years+months" relative date view

On Fri, Aug 28, 2009 at 09:03:19PM +0200, Alex Riesen wrote:
Show 6 quoted lines
> +unsigned long approxidate(const char *date)
> +{
> +	struct timeval tv;
> +	gettimeofday(&tv, NULL);
> +	return approxidate_relative(date, &tv);
> +}

This now always calls gettimeofday, whereas the original approxidate only did if parse_date failed.

I think you could also make this patch much smaller by just wrapping the whole function and using a '0' sentinel for "you need to fill in the time." Like:

---
Show changes to date.c +10 −3
diff --git a/date.c b/date.c
index 409a17d..b084d19 100644
--- a/date.c
+++ b/date.c
@@ -86,6 +86,14 @@ static int local_tzoffset(unsigned long time)
 
 const char *show_date(unsigned long time, int tz, enum date_mode mode)
 {
+	struct timeval now;
+	now.tv_sec = 0;
+	show_date_at_time(time, tz, mode, &now);
+}
+
+const char *show_date_at_time(unsigned long time, int tz, enum date_mode mode,
+		struct timeval now)
+{
 	struct tm *tm;
 	static char timebuf[200];
 
@@ -96,8 +104,8 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
 
 	if (mode == DATE_RELATIVE) {
 		unsigned long diff;
-		struct timeval now;
-		gettimeofday(&now, NULL);
+		if (!now.tv_sec)
+			gettimeofday(&now, NULL);
 		if (now.tv_sec < time)
 			return "in the future";
 		diff = now.tv_sec - time;

On the other hand, refactoring the relative date code into its own
function is probably a good thing in the long run.

-Peff
Alex Riesen· Aug 28, 2009, 19:20 UTC · re: Jeff King · lore

Re: [PATCH] Round-down years in "years+months" relative date view

On Fri, Aug 28, 2009 at 21:15, Jeff King<peff@peff.net> wrote:
Show 11 quoted lines
> On Fri, Aug 28, 2009 at 09:03:19PM +0200, Alex Riesen wrote:
>
>> +unsigned long approxidate(const char *date)
>> +{
>> +     struct timeval tv;
>> +     gettimeofday(&tv, NULL);
>> +     return approxidate_relative(date, &tv);
>> +}
>
> This now always calls gettimeofday, whereas the original approxidate
> only did if parse_date failed.
Oh, bugger...
> On the other hand, refactoring the relative date code into its own
> function is probably a good thing in the long run.
Exactly.
Alex Riesen· Aug 28, 2009, 19:33 UTC · re: Alex Riesen · lore

Re: [PATCH] Round-down years in "years+months" relative date view

From fe67532bdf095dc9ebc0c7dd67be384e807a197c Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa.lkml@gmail.com>
Date: Fri, 28 Aug 2009 20:59:59 +0200
Subject: [PATCH] Add date formatting functions with current time explicitely formatted

It should allow safe testing of this part of the code. --- Alex Riesen, Fri, Aug 28, 2009 21:20:50 +0200:

Show 15 quoted lines
> On Fri, Aug 28, 2009 at 21:15, Jeff King<peff@peff.net> wrote:
> > On Fri, Aug 28, 2009 at 09:03:19PM +0200, Alex Riesen wrote:
> >
> >> +unsigned long approxidate(const char *date)
> >> +{
> >> +     struct timeval tv;
> >> +     gettimeofday(&tv, NULL);
> >> +     return approxidate_relative(date, &tv);
> >> +}
> >
> > This now always calls gettimeofday, whereas the original approxidate
> > only did if parse_date failed.
> 
> Oh, bugger...
> 
Rather like this, I mean :)
 cache.h |    2 +
 date.c  |  150 ++++++++++++++++++++++++++++++++++++--------------------------
 2 files changed, 89 insertions(+), 63 deletions(-)
Show changes to 2 files +89 −63

cache.h, date.c

diff --git a/cache.h b/cache.h
index dd7f71e..3fb0166 100644
--- a/cache.h
+++ b/cache.h
@@ -731,9 +731,11 @@ enum date_mode {
 };
 
 const char *show_date(unsigned long time, int timezone, enum date_mode mode);
+const char *show_date_relative(unsigned long time, int tz, const struct timeval *now);
 int parse_date(const char *date, char *buf, int bufsize);
 void datestamp(char *buf, int bufsize);
 unsigned long approxidate(const char *);
+unsigned long approxidate_relative(const char *date, const struct timeval *now);
 enum date_mode parse_date_format(const char *format);
 
 #define IDENT_WARN_ON_NO_NAME  1
diff --git a/date.c b/date.c
index 409a17d..171e68f 100644
--- a/date.c
+++ b/date.c
@@ -84,6 +84,67 @@ static int local_tzoffset(unsigned long time)
 	return offset * eastwest;
 }
 
+const char *show_date_relative(unsigned long time, int tz, const struct timeval *now)
+{
+	static char timebuf[100 /* TODO: can be optimized */];
+	unsigned long diff;
+	if (now->tv_sec < time)
+		return "in the future";
+	diff = now->tv_sec - time;
+	if (diff < 90) {
+		snprintf(timebuf, sizeof(timebuf), "%lu seconds ago", diff);
+		return timebuf;
+	}
+	/* Turn it into minutes */
+	diff = (diff + 30) / 60;
+	if (diff < 90) {
+		snprintf(timebuf, sizeof(timebuf), "%lu minutes ago", diff);
+		return timebuf;
+	}
+	/* Turn it into hours */
+	diff = (diff + 30) / 60;
+	if (diff < 36) {
+		snprintf(timebuf, sizeof(timebuf), "%lu hours ago", diff);
+		return timebuf;
+	}
+	/* We deal with number of days from here on */
+	diff = (diff + 12) / 24;
+	if (diff < 14) {
+		snprintf(timebuf, sizeof(timebuf), "%lu days ago", diff);
+		return timebuf;
+	}
+	/* Say weeks for the past 10 weeks or so */
+	if (diff < 70) {
+		snprintf(timebuf, sizeof(timebuf), "%lu weeks ago", (diff + 3) / 7);
+		return timebuf;
+	}
+	/* Say months for the past 12 months or so */
+	if (diff < 360) {
+		snprintf(timebuf, sizeof(timebuf), "%lu months ago", (diff + 15) / 30);
+		return timebuf;
+	}
+	/* Give years and months for 5 years or so */
+	if (diff < 1825) {
+		unsigned long years = (diff + 183) / 365;
+		unsigned long months = (diff % 365 + 15) / 30;
+		int n;
+		n = snprintf(timebuf, sizeof(timebuf), "%lu year%s",
+			     years, (years > 1 ? "s" : ""));
+		if (months)
+			snprintf(timebuf + n, sizeof(timebuf) - n,
+				 ", %lu month%s ago",
+				 months, (months > 1 ? "s" : ""));
+		else
+			snprintf(timebuf + n, sizeof(timebuf) - n,
+				 " ago");
+		return timebuf;
+	}
+	/* Otherwise, just years. Centuries is probably overkill. */
+	snprintf(timebuf, sizeof(timebuf), "%lu years ago", (diff + 183) / 365);
+	return timebuf;
+
+}
+
 const char *show_date(unsigned long time, int tz, enum date_mode mode)
 {
 	struct tm *tm;
@@ -95,63 +156,9 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
 	}
 
 	if (mode == DATE_RELATIVE) {
-		unsigned long diff;
 		struct timeval now;
 		gettimeofday(&now, NULL);
-		if (now.tv_sec < time)
-			return "in the future";
-		diff = now.tv_sec - time;
-		if (diff < 90) {
-			snprintf(timebuf, sizeof(timebuf), "%lu seconds ago", diff);
-			return timebuf;
-		}
-		/* Turn it into minutes */
-		diff = (diff + 30) / 60;
-		if (diff < 90) {
-			snprintf(timebuf, sizeof(timebuf), "%lu minutes ago", diff);
-			return timebuf;
-		}
-		/* Turn it into hours */
-		diff = (diff + 30) / 60;
-		if (diff < 36) {
-			snprintf(timebuf, sizeof(timebuf), "%lu hours ago", diff);
-			return timebuf;
-		}
-		/* We deal with number of days from here on */
-		diff = (diff + 12) / 24;
-		if (diff < 14) {
-			snprintf(timebuf, sizeof(timebuf), "%lu days ago", diff);
-			return timebuf;
-		}
-		/* Say weeks for the past 10 weeks or so */
-		if (diff < 70) {
-			snprintf(timebuf, sizeof(timebuf), "%lu weeks ago", (diff + 3) / 7);
-			return timebuf;
-		}
-		/* Say months for the past 12 months or so */
-		if (diff < 360) {
-			snprintf(timebuf, sizeof(timebuf), "%lu months ago", (diff + 15) / 30);
-			return timebuf;
-		}
-		/* Give years and months for 5 years or so */
-		if (diff < 1825) {
-			unsigned long years = (diff + 183) / 365;
-			unsigned long months = (diff % 365 + 15) / 30;
-			int n;
-			n = snprintf(timebuf, sizeof(timebuf), "%lu year%s",
-					years, (years > 1 ? "s" : ""));
-			if (months)
-				snprintf(timebuf + n, sizeof(timebuf) - n,
-					", %lu month%s ago",
-					months, (months > 1 ? "s" : ""));
-			else
-				snprintf(timebuf + n, sizeof(timebuf) - n,
-					" ago");
-			return timebuf;
-		}
-		/* Otherwise, just years. Centuries is probably overkill. */
-		snprintf(timebuf, sizeof(timebuf), "%lu years ago", (diff + 183) / 365);
-		return timebuf;
+		return show_date_relative(time, tz, &now);
 	}
 
 	if (mode == DATE_LOCAL)
@@ -866,19 +873,13 @@ static const char *approxidate_digit(const char *date, struct tm *tm, int *num)
 	return end;
 }
 
-unsigned long approxidate(const char *date)
+static unsigned long approximation(const char *date, const struct timeval *tv)
 {
 	int number = 0;
 	struct tm tm, now;
-	struct timeval tv;
 	time_t time_sec;
-	char buffer[50];
 
-	if (parse_date(date, buffer, sizeof(buffer)) > 0)
-		return strtoul(buffer, NULL, 10);
-
-	gettimeofday(&tv, NULL);
-	time_sec = tv.tv_sec;
+	time_sec = tv->tv_sec;
 	localtime_r(&time_sec, &tm);
 	now = tm;
 	for (;;) {
@@ -899,3 +900,26 @@ unsigned long approxidate(const char *date)
 		tm.tm_year--;
 	return mktime(&tm);
 }
+
+unsigned long approxidate_relative(const char *date, const struct timeval *tv)
+{
+	char buffer[50];
+
+	if (parse_date(date, buffer, sizeof(buffer)) > 0)
+		return strtoul(buffer, NULL, 10);
+
+	return approximation(date, tv);
+}
+
+unsigned long approxidate(const char *date)
+{
+	struct timeval tv;
+	char buffer[50];
+
+	if (parse_date(date, buffer, sizeof(buffer)) > 0)
+		return strtoul(buffer, NULL, 10);
+
+	gettimeofday(&tv, NULL);
+	return approximation(date, &tv);
+}
+
-- 
1.6.4.1.261.gf9874
Alex Riesen· Aug 28, 2009, 20:52 UTC · re: Alex Riesen · lore

[PATCH] Allow testing of _relative family of time formatting and parsing functions

To complement the testability of approxidate. --- Alex Riesen, Fri, Aug 28, 2009 21:33:02 +0200:

> 
> It should allow safe testing of this part of the code.
And this should really allow testing of it:
    $ ./test-date '10.days.ago'
    10.days.ago -> bad -> Thu Jan  1 01:00:00 1970
    10.days.ago -> Tue Aug 18 22:50:20 2009
    relative: 10.days.ago -> Fri Dec 22 12:00:00 1989
    relative: 10 days ago, out of Fri Dec 22 12:00:00 1989
    $

According to Wikipedia, absolutely nothing of note happened at the day 10 January, 1990.

 test-date.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)
Show changes to test-date.c +17 −0
diff --git a/test-date.c b/test-date.c
index 62e8f23..dcc7973 100644
--- a/test-date.c
+++ b/test-date.c
@@ -4,6 +4,17 @@ int main(int argc, char **argv)
 {
 	int i;
 
+	struct tm tm;
+	struct timeval when = {0, 0};
+	tm.tm_sec  = 0;
+	tm.tm_min  = 0;
+	tm.tm_hour = 12;
+	tm.tm_mday = 1;
+	tm.tm_mon  = 0  /* January */;
+	tm.tm_year = 90 /* 1990 */ ;
+	tm.tm_isdst = -1;
+	when.tv_sec = mktime(&tm);
+
 	for (i = 1; i < argc; i++) {
 		char result[100];
 		time_t t;
@@ -15,6 +26,12 @@ int main(int argc, char **argv)
 
 		t = approxidate(argv[i]);
 		printf("%s -> %s\n", argv[i], ctime(&t));
+
+		t = approxidate_relative(argv[i], &when);
+		printf("relative: %s -> %s\n", argv[i], ctime(&t));
+
+		printf("relative: %s, out of %s\n",
+		       show_date_relative(t, 0, &when), ctime(&t));
 	}
 	return 0;
 }
-- 
1.6.4.1.263.g468a
Alex Riesen· Aug 28, 2009, 20:54 UTC · re: Alex Riesen · lore

Re: [PATCH] Allow testing of _relative family of time formatting and parsing functions

Alex Riesen, Fri, Aug 28, 2009 22:52:32 +0200:
> According to Wikipedia, absolutely nothing of note happened
> at the day 10 January, 1990.
1 Jan. 1990, not 10.
Junio C Hamano· Aug 29, 2009, 21:46 UTC · re: Alex Riesen · lore

Re: [PATCH] Allow testing of _relative family of time formatting and parsing functions

Alex Riesen <raa.lkml@gmail.com> writes:
Show 17 quoted lines
> To complement the testability of approxidate.
> ---
> Alex Riesen, Fri, Aug 28, 2009 21:33:02 +0200:
>> 
>> It should allow safe testing of this part of the code.
>
> And this should really allow testing of it:
>
>     $ ./test-date '10.days.ago'
>     10.days.ago -> bad -> Thu Jan  1 01:00:00 1970
>     10.days.ago -> Tue Aug 18 22:50:20 2009
>
>     relative: 10.days.ago -> Fri Dec 22 12:00:00 1989
>
>     relative: 10 days ago, out of Fri Dec 22 12:00:00 1989
>
>     $

What are these blank lines for? Is this intended as a serious submission for inclusion? I am having a hrad time to guess, especially you did not sign this off, nor Cc'ed me.

Alex Riesen· Aug 30, 2009, 07:25 UTC · re: Junio C Hamano · lore

Re: [PATCH] Allow testing of _relative family of time formatting and parsing functions

On Sat, Aug 29, 2009 at 23:46, Junio C Hamano<gitster@pobox.com> wrote:
Show 20 quoted lines
> Alex Riesen <raa.lkml@gmail.com> writes:
>> To complement the testability of approxidate.
>> ---
>> Alex Riesen, Fri, Aug 28, 2009 21:33:02 +0200:
>>>
>>> It should allow safe testing of this part of the code.
>>
>> And this should really allow testing of it:
>>
>>     $ ./test-date '10.days.ago'
>>     10.days.ago -> bad -> Thu Jan  1 01:00:00 1970
>>     10.days.ago -> Tue Aug 18 22:50:20 2009
>>
>>     relative: 10.days.ago -> Fri Dec 22 12:00:00 1989
>>
>>     relative: 10 days ago, out of Fri Dec 22 12:00:00 1989
>>
>>     $
>
> What are these blank lines for?
ctime(3) artifact (it adds a \n in the output buffer), which I missed.
> Is this intended as a serious submission for inclusion?
Not yet. AFAICS, test-date is never used in our test suite.
> I am having a hrad time to guess, especially you did not
> sign this off, nor Cc'ed me.

Right, that's because I'm not sure myself. Frankly, I'm not convinced we have to test every single thing. In my experience, the bigger a test suite, the less are people inclined to use it (including setting up automatic test runs).

Jeff, Nicolas? Is this test enough? Are there any other code paths you want to include in the test?

And sorry for having you missed in Cc:, that wasn't intended.
Jeff King· Aug 30, 2009, 07:51 UTC · re: Alex Riesen · lore

Re: [PATCH] Allow testing of _relative family of time formatting and parsing functions

On Sun, Aug 30, 2009 at 09:25:11AM +0200, Alex Riesen wrote:
> > Is this intended as a serious submission for inclusion?
> 
> Not yet. AFAICS, test-date is never used in our test suite.

No, it isn't, but I think the point of this is to change that. So it is useless without an extra patch to the test suite. I'll try to put something together.

Show 7 quoted lines
> Right, that's because I'm not sure myself. Frankly, I'm not
> convinced we have to test every single thing. In my experience,
> the bigger a test suite, the less are people inclined to use it
> (including setting up automatic test runs).
> 
> Jeff, Nicolas? Is this test enough? Are there any other code
> paths you want to include in the test?

I think this is a useful addition to the test suite. The bug David fixed was obvious, but it sat for a year because of poor test coverage. Linus fixed several approxidate bugs recently. The approxidate code is notoriously temperamental, so it is a good thing to be checking for regressions.

And I don't think our test suite is nearly big enough to start worrying about getting people not to use it. Without CVS and SVN tests, I can run it on 3-year-old hardware in less than a minute. Either you bother to run it or not, but I doubt that adding one new test script is going to break the bank.

-Peff
Alex Riesen· Aug 30, 2009, 08:10 UTC · re: Jeff King · lore

Re: [PATCH] Allow testing of _relative family of time formatting and parsing functions

On Sun, Aug 30, 2009 at 09:51, Jeff King<peff@peff.net> wrote:
Show 9 quoted lines
> On Sun, Aug 30, 2009 at 09:25:11AM +0200, Alex Riesen wrote:
>
>> > Is this intended as a serious submission for inclusion?
>>
>> Not yet. AFAICS, test-date is never used in our test suite.
>
> No, it isn't, but I think the point of this is to change that.
> So it is useless without an extra patch to the test suite. I'll try to
> put something together.
Thanks :)
Alex Riesen· Aug 30, 2009, 09:13 UTC · re: Alex Riesen · lore

[PATCH 1/2] Add date formatting and parsing functions relative to a given time

The main purpose is to allow predictable testing of the code.
Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---

Have show_date_relative supplied the output buffer. As it is a new interface, it can as well be a little bit more generic than its sole caller. test-date.c is updated and shall follow in a moment.

And, after a while thinking, I am convinced that Jeff has a point and used a more "internal" name for approxidate's recent "bottom half".

 cache.h |    5 ++
 date.c  |  152 +++++++++++++++++++++++++++++++++++++--------------------------
 2 files changed, 94 insertions(+), 63 deletions(-)
Show changes to 2 files +94 −63

cache.h, date.c

diff --git a/cache.h b/cache.h
index dd7f71e..1586f33 100644
--- a/cache.h
+++ b/cache.h
@@ -731,9 +731,14 @@ enum date_mode {
 };
 
 const char *show_date(unsigned long time, int timezone, enum date_mode mode);
+const char *show_date_relative(unsigned long time, int tz,
+			       const struct timeval *now,
+			       char *timebuf,
+			       size_t timebuf_size);
 int parse_date(const char *date, char *buf, int bufsize);
 void datestamp(char *buf, int bufsize);
 unsigned long approxidate(const char *);
+unsigned long approxidate_relative(const char *date, const struct timeval *now);
 enum date_mode parse_date_format(const char *format);
 
 #define IDENT_WARN_ON_NO_NAME  1
diff --git a/date.c b/date.c
index f011692..0b0f7a7 100644
--- a/date.c
+++ b/date.c
@@ -84,6 +84,68 @@ static int local_tzoffset(unsigned long time)
 	return offset * eastwest;
 }
 
+const char *show_date_relative(unsigned long time, int tz,
+			       const struct timeval *now,
+			       char *timebuf,
+			       size_t timebuf_size)
+{
+	unsigned long diff;
+	if (now->tv_sec < time)
+		return "in the future";
+	diff = now->tv_sec - time;
+	if (diff < 90) {
+		snprintf(timebuf, timebuf_size, "%lu seconds ago", diff);
+		return timebuf;
+	}
+	/* Turn it into minutes */
+	diff = (diff + 30) / 60;
+	if (diff < 90) {
+		snprintf(timebuf, timebuf_size, "%lu minutes ago", diff);
+		return timebuf;
+	}
+	/* Turn it into hours */
+	diff = (diff + 30) / 60;
+	if (diff < 36) {
+		snprintf(timebuf, timebuf_size, "%lu hours ago", diff);
+		return timebuf;
+	}
+	/* We deal with number of days from here on */
+	diff = (diff + 12) / 24;
+	if (diff < 14) {
+		snprintf(timebuf, timebuf_size, "%lu days ago", diff);
+		return timebuf;
+	}
+	/* Say weeks for the past 10 weeks or so */
+	if (diff < 70) {
+		snprintf(timebuf, timebuf_size, "%lu weeks ago", (diff + 3) / 7);
+		return timebuf;
+	}
+	/* Say months for the past 12 months or so */
+	if (diff < 360) {
+		snprintf(timebuf, timebuf_size, "%lu months ago", (diff + 15) / 30);
+		return timebuf;
+	}
+	/* Give years and months for 5 years or so */
+	if (diff < 1825) {
+		unsigned long years = diff / 365;
+		unsigned long months = (diff % 365 + 15) / 30;
+		int n;
+		n = snprintf(timebuf, timebuf_size, "%lu year%s",
+			     years, (years > 1 ? "s" : ""));
+		if (months)
+			snprintf(timebuf + n, timebuf_size - n,
+				 ", %lu month%s ago",
+				 months, (months > 1 ? "s" : ""));
+		else
+			snprintf(timebuf + n, timebuf_size - n,
+				 " ago");
+		return timebuf;
+	}
+	/* Otherwise, just years. Centuries is probably overkill. */
+	snprintf(timebuf, timebuf_size, "%lu years ago", (diff + 183) / 365);
+	return timebuf;
+}
+
 const char *show_date(unsigned long time, int tz, enum date_mode mode)
 {
 	struct tm *tm;
@@ -95,63 +157,10 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
 	}
 
 	if (mode == DATE_RELATIVE) {
-		unsigned long diff;
 		struct timeval now;
 		gettimeofday(&now, NULL);
-		if (now.tv_sec < time)
-			return "in the future";
-		diff = now.tv_sec - time;
-		if (diff < 90) {
-			snprintf(timebuf, sizeof(timebuf), "%lu seconds ago", diff);
-			return timebuf;
-		}
-		/* Turn it into minutes */
-		diff = (diff + 30) / 60;
-		if (diff < 90) {
-			snprintf(timebuf, sizeof(timebuf), "%lu minutes ago", diff);
-			return timebuf;
-		}
-		/* Turn it into hours */
-		diff = (diff + 30) / 60;
-		if (diff < 36) {
-			snprintf(timebuf, sizeof(timebuf), "%lu hours ago", diff);
-			return timebuf;
-		}
-		/* We deal with number of days from here on */
-		diff = (diff + 12) / 24;
-		if (diff < 14) {
-			snprintf(timebuf, sizeof(timebuf), "%lu days ago", diff);
-			return timebuf;
-		}
-		/* Say weeks for the past 10 weeks or so */
-		if (diff < 70) {
-			snprintf(timebuf, sizeof(timebuf), "%lu weeks ago", (diff + 3) / 7);
-			return timebuf;
-		}
-		/* Say months for the past 12 months or so */
-		if (diff < 360) {
-			snprintf(timebuf, sizeof(timebuf), "%lu months ago", (diff + 15) / 30);
-			return timebuf;
-		}
-		/* Give years and months for 5 years or so */
-		if (diff < 1825) {
-			unsigned long years = diff / 365;
-			unsigned long months = (diff % 365 + 15) / 30;
-			int n;
-			n = snprintf(timebuf, sizeof(timebuf), "%lu year%s",
-					years, (years > 1 ? "s" : ""));
-			if (months)
-				snprintf(timebuf + n, sizeof(timebuf) - n,
-					", %lu month%s ago",
-					months, (months > 1 ? "s" : ""));
-			else
-				snprintf(timebuf + n, sizeof(timebuf) - n,
-					" ago");
-			return timebuf;
-		}
-		/* Otherwise, just years. Centuries is probably overkill. */
-		snprintf(timebuf, sizeof(timebuf), "%lu years ago", (diff + 183) / 365);
-		return timebuf;
+		return show_date_relative(time, tz, &now,
+					  timebuf, sizeof(timebuf));
 	}
 
 	if (mode == DATE_LOCAL)
@@ -866,19 +875,13 @@ static const char *approxidate_digit(const char *date, struct tm *tm, int *num)
 	return end;
 }
 
-unsigned long approxidate(const char *date)
+static unsigned long approxidate_str(const char *date, const struct timeval *tv)
 {
 	int number = 0;
 	struct tm tm, now;
-	struct timeval tv;
 	time_t time_sec;
-	char buffer[50];
 
-	if (parse_date(date, buffer, sizeof(buffer)) > 0)
-		return strtoul(buffer, NULL, 10);
-
-	gettimeofday(&tv, NULL);
-	time_sec = tv.tv_sec;
+	time_sec = tv->tv_sec;
 	localtime_r(&time_sec, &tm);
 	now = tm;
 	for (;;) {
@@ -899,3 +902,26 @@ unsigned long approxidate(const char *date)
 		tm.tm_year--;
 	return mktime(&tm);
 }
+
+unsigned long approxidate_relative(const char *date, const struct timeval *tv)
+{
+	char buffer[50];
+
+	if (parse_date(date, buffer, sizeof(buffer)) > 0)
+		return strtoul(buffer, NULL, 10);
+
+	return approxidate_str(date, tv);
+}
+
+unsigned long approxidate(const char *date)
+{
+	struct timeval tv;
+	char buffer[50];
+
+	if (parse_date(date, buffer, sizeof(buffer)) > 0)
+		return strtoul(buffer, NULL, 10);
+
+	gettimeofday(&tv, NULL);
+	return approxidate_str(date, &tv);
+}
+
-- 
1.6.4.1.294.g16262
Alex Riesen· Aug 30, 2009, 09:15 UTC · re: Alex Riesen · lore

[PATCH 2/2] Allow testing of _relative family of time formatting and parsing functions

To complement the testability of approxidate.
Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
Alex Riesen, Sun, Aug 30, 2009 11:13:46 +0200:
> test-date.c is updated and shall follow in a moment.
here it goes.
 test-date.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)
Show changes to test-date.c +20 −0
diff --git a/test-date.c b/test-date.c
index 62e8f23..4cb146d 100644
--- a/test-date.c
+++ b/test-date.c
@@ -4,6 +4,19 @@ int main(int argc, char **argv)
 {
 	int i;
 
+	/* see date.c, function show_date_relative */
+	char timebuf[(sizeof(long) * 5 / 2 + sizeof(" minutes ago,")) * 2];
+	struct tm tm;
+	struct timeval when = {0, 0};
+	tm.tm_sec  = 0;
+	tm.tm_min  = 0;
+	tm.tm_hour = 12;
+	tm.tm_mday = 1;
+	tm.tm_mon  = 0  /* January */;
+	tm.tm_year = 90 /* 1990 */ ;
+	tm.tm_isdst = -1;
+	when.tv_sec = mktime(&tm);
+
 	for (i = 1; i < argc; i++) {
 		char result[100];
 		time_t t;
@@ -15,6 +28,13 @@ int main(int argc, char **argv)
 
 		t = approxidate(argv[i]);
 		printf("%s -> %s\n", argv[i], ctime(&t));
+
+		t = approxidate_relative(argv[i], &when);
+		printf("relative: %s -> %s", argv[i], ctime(&t));
+
+		printf("relative: %s, out of %s",
+		       show_date_relative(t, 0, &when, timebuf,sizeof(timebuf)),
+		       ctime(&t));
 	}
 	return 0;
 }
-- 
1.6.4.1.294.g16262
Jeff King· Aug 30, 2009, 09:15 UTC · re: Alex Riesen · lore

Re: [PATCH 1/2] Add date formatting and parsing functions relative to a given time

On Sun, Aug 30, 2009 at 11:13:46AM +0200, Alex Riesen wrote:
> Have show_date_relative supplied the output buffer. As it is a new
> interface, it can as well be a little bit more generic than its sole
> caller. test-date.c is updated and shall follow in a moment.

FYI, I am munging test-date to match the test script I am writing, so don't bother with that patch.

-Peff
Jeff King· Aug 30, 2009, 09:36 UTC · re: Jeff King · lore

Re: [PATCH 1/2] Add date formatting and parsing functions relative to a given time

On Sun, Aug 30, 2009 at 05:15:57AM -0400, Jeff King wrote:
> FYI, I am munging test-date to match the test script I am writing, so
> don't bother with that patch.

Here is what my patch is looking like. Please give any comments, and then I will resubmit in a form that will be simpler for Junio, which should be a series with:

  - your patch to refactor date.c
  - this patch (this version uses the original interface to
    show_relative; I will tweak to match the new patch you just sent)
  - another patch to go on top of lt/approxidate to test recent fixes
    from Linus
---
Show changes to 2 files +122 −8

t/t0006-date.sh, test-date.c

diff --git a/t/t0006-date.sh b/t/t0006-date.sh
new file mode 100755
index 0000000..4beb44b
--- /dev/null
+++ b/t/t0006-date.sh
@@ -0,0 +1,71 @@
+#!/bin/sh
+
+test_description='test date parsing and printing'
+. ./test-lib.sh
+
+# arbitrary reference time: 2009-08-30 19:20:00
+TEST_DATE_NOW=1251660000; export TEST_DATE_NOW
+
+check_show() {
+	t=$(($TEST_DATE_NOW - $1))
+	echo "$t -> $2" >expect
+	test_expect_success "relative date ($2)" "
+	test-date show $t >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_show 5 '5 seconds ago'
+check_show 300 '5 minutes ago'
+check_show 18000 '5 hours ago'
+check_show 432000 '5 days ago'
+check_show 1728000 '3 weeks ago'
+check_show 13000000 '5 months ago'
+check_show 37500000 '1 year, 2 months ago'
+check_show 55188000 '1 year, 9 months ago'
+check_show 630000000 '20 years ago'
+
+check_parse() {
+	echo "$1 -> $2" >expect
+	test_expect_success "parse date ($1)" "
+	test-date parse '$1' >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_parse 2008 bad
+check_parse 2008-02 bad
+check_parse 2008-02-14 '2008-02-14 00:00:00 +0000'
+check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000'
+
+check_approxidate() {
+	echo "$1 -> $2 +0000" >expect
+	test_expect_success "parse approxidate ($1)" "
+	test-date approxidate '$1' >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_approxidate now '2009-08-30 19:20:00'
+check_approxidate '5 seconds ago' '2009-08-30 19:19:55'
+check_approxidate 5.seconds.ago '2009-08-30 19:19:55'
+check_approxidate 10.minutes.ago '2009-08-30 19:10:00'
+check_approxidate yesterday '2009-08-29 19:20:00'
+check_approxidate 3.days.ago '2009-08-27 19:20:00'
+check_approxidate 3.weeks.ago '2009-08-09 19:20:00'
+check_approxidate 3.months.ago '2009-05-30 19:20:00'
+check_approxidate 2.years.3.months.ago '2007-05-30 19:20:00'
+
+check_approxidate '6am yesterday' '2009-08-29 06:00:00'
+check_approxidate '6pm yesterday' '2009-08-29 18:00:00'
+check_approxidate '3:00' '2009-08-30 03:00:00'
+check_approxidate '15:00' '2009-08-30 15:00:00'
+check_approxidate 'noon today' '2009-08-30 12:00:00'
+check_approxidate 'noon yesterday' '2009-08-29 12:00:00'
+
+check_approxidate 'last tuesday' '2009-08-25 19:20:00'
+check_approxidate 'July 5th' '2009-07-05 19:20:00'
+check_approxidate '06/05/2009' '2009-06-05 00:00:00'
+check_approxidate '06.05.2009' '2009-05-06 00:00:00'
+
+test_done
diff --git a/test-date.c b/test-date.c
index 62e8f23..8d263a3 100644
--- a/test-date.c
+++ b/test-date.c
@@ -1,20 +1,63 @@
 #include "cache.h"
 
-int main(int argc, char **argv)
+static const char *usage_msg = "\n"
+"  test-date show [time_t]...\n"
+"  test-date parse [date]...\n"
+"  test-date approxidate [date]...\n";
+
+static void show_dates(char **argv, struct timeval *now)
 {
-	int i;
+	for (; *argv; argv++) {
+		time_t t = atoi(*argv);
+		printf("%s -> %s\n", *argv, show_date_relative(t, 0, now));
+	}
+}
 
-	for (i = 1; i < argc; i++) {
+static void parse_dates(char **argv, struct timeval *now)
+{
+	for (; *argv; argv++) {
 		char result[100];
 		time_t t;
 
-		memcpy(result, "bad", 4);
-		parse_date(argv[i], result, sizeof(result));
+		parse_date(*argv, result, sizeof(result));
 		t = strtoul(result, NULL, 0);
-		printf("%s -> %s -> %s", argv[i], result, ctime(&t));
+		printf("%s -> %s\n", *argv,
+			t ? show_date(t, 0, DATE_ISO8601) : "bad");
+	}
+}
+
+static void parse_approxidate(char **argv, struct timeval *now)
+{
+	for (; *argv; argv++) {
+		time_t t;
+		t = approxidate_relative(*argv, now);
+		printf("%s -> %s\n", *argv, show_date(t, 0, DATE_ISO8601));
+	}
+}
 
-		t = approxidate(argv[i]);
-		printf("%s -> %s\n", argv[i], ctime(&t));
+int main(int argc, char **argv)
+{
+	struct timeval now;
+	const char *x;
+
+	x = getenv("TEST_DATE_NOW");
+	if (x) {
+		now.tv_sec = atoi(x);
+		now.tv_usec = 0;
 	}
+	else
+		gettimeofday(&now, NULL);
+
+	argv++;
+	if (!*argv)
+		usage(usage_msg);
+	if (!strcmp(*argv, "show"))
+		show_dates(argv+1, &now);
+	else if (!strcmp(*argv, "parse"))
+		parse_dates(argv+1, &now);
+	else if (!strcmp(*argv, "approxidate"))
+		parse_approxidate(argv+1, &now);
+	else
+		usage(usage_msg);
 	return 0;
 }
Alex Riesen· Aug 30, 2009, 09:56 UTC · re: Jeff King · lore

Re: [PATCH 1/2] Add date formatting and parsing functions relative to a given time

On Sun, Aug 30, 2009 at 11:36, Jeff King<peff@peff.net> wrote:
Show 12 quoted lines
> On Sun, Aug 30, 2009 at 05:15:57AM -0400, Jeff King wrote:
>
>> FYI, I am munging test-date to match the test script I am writing, so
>> don't bother with that patch.
>
> Here is what my patch is looking like. Please give any comments, and
> then I will resubmit in a form that will be simpler for Junio, which
> should be a series with:
>
>  - your patch to refactor date.c
>  - this patch (this version uses the original interface to
>    show_relative; I will tweak to match the new patch you just sent)
Yes, I think this is the only comment I can make.
> +# arbitrary reference time: 2009-08-30 19:20:00

The world changed since 1980 :) There is already three things happened at the day (http://en.wikipedia.org/wiki/August_2009), and it is not evening yet (well, here in Europe)

Show 9 quoted lines
> +check_show 5 '5 seconds ago'
> +check_show 300 '5 minutes ago'
> +check_show 18000 '5 hours ago'
> +check_show 432000 '5 days ago'
> +check_show 1728000 '3 weeks ago'
> +check_show 13000000 '5 months ago'
> +check_show 37500000 '1 year, 2 months ago'
> +check_show 55188000 '1 year, 9 months ago'
> +check_show 630000000 '20 years ago'

check_show 630000000 '20.years.ago'? (Arbitrary, non-whitespace delimiters, which was an advertised feature, to make shell's life easier)

Jeff King· Aug 30, 2009, 10:08 UTC · re: Alex Riesen · lore

Re: [PATCH 1/2] Add date formatting and parsing functions relative to a given time

On Sun, Aug 30, 2009 at 11:56:37AM +0200, Alex Riesen wrote:
Show 13 quoted lines
> > +check_show 5 '5 seconds ago'
> > +check_show 300 '5 minutes ago'
> > +check_show 18000 '5 hours ago'
> > +check_show 432000 '5 days ago'
> > +check_show 1728000 '3 weeks ago'
> > +check_show 13000000 '5 months ago'
> > +check_show 37500000 '1 year, 2 months ago'
> > +check_show 55188000 '1 year, 9 months ago'
> > +check_show 630000000 '20 years ago'
> 
> check_show 630000000 '20.years.ago'?
> (Arbitrary, non-whitespace delimiters, which was an
> advertised feature, to make shell's life easier)

This part is about checking what show_date produces (the first number is an offset from now in seconds, and the second is what we expect), so it always has spaces.

See the check_approxidate section further down for an example of parsing what you are talking about.

-Peff
Alex Riesen· Aug 30, 2009, 11:17 UTC · re: Jeff King · lore

Re: [PATCH 1/2] Add date formatting and parsing functions relative to a given time

On Sun, Aug 30, 2009 at 12:08, Jeff King<peff@peff.net> wrote:
Show 12 quoted lines
> On Sun, Aug 30, 2009 at 11:56:37AM +0200, Alex Riesen wrote:
>>
>> check_show 630000000 '20.years.ago'?
>> (Arbitrary, non-whitespace delimiters, which was an
>> advertised feature, to make shell's life easier)
>
> This part is about checking what show_date produces (the first number is
> an offset from now in seconds, and the second is what we expect), so it
> always has spaces.
>
> See the check_approxidate section further down for an example of parsing
> what you are talking about.
Ah, I see
Jeff King· Aug 30, 2009, 21:43 UTC · re: Jeff King · lore

[PATCH 1/3] Add date formatting and parsing functions relative to a given time

From: Alex Riesen <raa.lkml@gmail.com>
The main purpose is to allow predictable testing of the code.
Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
---
 cache.h |    5 ++
 date.c  |  152 +++++++++++++++++++++++++++++++++++++--------------------------
 2 files changed, 94 insertions(+), 63 deletions(-)
Show changes to 2 files +94 −63

cache.h, date.c

diff --git a/cache.h b/cache.h
index 808daba..5fad24c 100644
--- a/cache.h
+++ b/cache.h
@@ -731,9 +731,14 @@ enum date_mode {
 };
 
 const char *show_date(unsigned long time, int timezone, enum date_mode mode);
+const char *show_date_relative(unsigned long time, int tz,
+			       const struct timeval *now,
+			       char *timebuf,
+			       size_t timebuf_size);
 int parse_date(const char *date, char *buf, int bufsize);
 void datestamp(char *buf, int bufsize);
 unsigned long approxidate(const char *);
+unsigned long approxidate_relative(const char *date, const struct timeval *now);
 enum date_mode parse_date_format(const char *format);
 
 #define IDENT_WARN_ON_NO_NAME  1
diff --git a/date.c b/date.c
index f011692..0b0f7a7 100644
--- a/date.c
+++ b/date.c
@@ -84,6 +84,68 @@ static int local_tzoffset(unsigned long time)
 	return offset * eastwest;
 }
 
+const char *show_date_relative(unsigned long time, int tz,
+			       const struct timeval *now,
+			       char *timebuf,
+			       size_t timebuf_size)
+{
+	unsigned long diff;
+	if (now->tv_sec < time)
+		return "in the future";
+	diff = now->tv_sec - time;
+	if (diff < 90) {
+		snprintf(timebuf, timebuf_size, "%lu seconds ago", diff);
+		return timebuf;
+	}
+	/* Turn it into minutes */
+	diff = (diff + 30) / 60;
+	if (diff < 90) {
+		snprintf(timebuf, timebuf_size, "%lu minutes ago", diff);
+		return timebuf;
+	}
+	/* Turn it into hours */
+	diff = (diff + 30) / 60;
+	if (diff < 36) {
+		snprintf(timebuf, timebuf_size, "%lu hours ago", diff);
+		return timebuf;
+	}
+	/* We deal with number of days from here on */
+	diff = (diff + 12) / 24;
+	if (diff < 14) {
+		snprintf(timebuf, timebuf_size, "%lu days ago", diff);
+		return timebuf;
+	}
+	/* Say weeks for the past 10 weeks or so */
+	if (diff < 70) {
+		snprintf(timebuf, timebuf_size, "%lu weeks ago", (diff + 3) / 7);
+		return timebuf;
+	}
+	/* Say months for the past 12 months or so */
+	if (diff < 360) {
+		snprintf(timebuf, timebuf_size, "%lu months ago", (diff + 15) / 30);
+		return timebuf;
+	}
+	/* Give years and months for 5 years or so */
+	if (diff < 1825) {
+		unsigned long years = diff / 365;
+		unsigned long months = (diff % 365 + 15) / 30;
+		int n;
+		n = snprintf(timebuf, timebuf_size, "%lu year%s",
+			     years, (years > 1 ? "s" : ""));
+		if (months)
+			snprintf(timebuf + n, timebuf_size - n,
+				 ", %lu month%s ago",
+				 months, (months > 1 ? "s" : ""));
+		else
+			snprintf(timebuf + n, timebuf_size - n,
+				 " ago");
+		return timebuf;
+	}
+	/* Otherwise, just years. Centuries is probably overkill. */
+	snprintf(timebuf, timebuf_size, "%lu years ago", (diff + 183) / 365);
+	return timebuf;
+}
+
 const char *show_date(unsigned long time, int tz, enum date_mode mode)
 {
 	struct tm *tm;
@@ -95,63 +157,10 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
 	}
 
 	if (mode == DATE_RELATIVE) {
-		unsigned long diff;
 		struct timeval now;
 		gettimeofday(&now, NULL);
-		if (now.tv_sec < time)
-			return "in the future";
-		diff = now.tv_sec - time;
-		if (diff < 90) {
-			snprintf(timebuf, sizeof(timebuf), "%lu seconds ago", diff);
-			return timebuf;
-		}
-		/* Turn it into minutes */
-		diff = (diff + 30) / 60;
-		if (diff < 90) {
-			snprintf(timebuf, sizeof(timebuf), "%lu minutes ago", diff);
-			return timebuf;
-		}
-		/* Turn it into hours */
-		diff = (diff + 30) / 60;
-		if (diff < 36) {
-			snprintf(timebuf, sizeof(timebuf), "%lu hours ago", diff);
-			return timebuf;
-		}
-		/* We deal with number of days from here on */
-		diff = (diff + 12) / 24;
-		if (diff < 14) {
-			snprintf(timebuf, sizeof(timebuf), "%lu days ago", diff);
-			return timebuf;
-		}
-		/* Say weeks for the past 10 weeks or so */
-		if (diff < 70) {
-			snprintf(timebuf, sizeof(timebuf), "%lu weeks ago", (diff + 3) / 7);
-			return timebuf;
-		}
-		/* Say months for the past 12 months or so */
-		if (diff < 360) {
-			snprintf(timebuf, sizeof(timebuf), "%lu months ago", (diff + 15) / 30);
-			return timebuf;
-		}
-		/* Give years and months for 5 years or so */
-		if (diff < 1825) {
-			unsigned long years = diff / 365;
-			unsigned long months = (diff % 365 + 15) / 30;
-			int n;
-			n = snprintf(timebuf, sizeof(timebuf), "%lu year%s",
-					years, (years > 1 ? "s" : ""));
-			if (months)
-				snprintf(timebuf + n, sizeof(timebuf) - n,
-					", %lu month%s ago",
-					months, (months > 1 ? "s" : ""));
-			else
-				snprintf(timebuf + n, sizeof(timebuf) - n,
-					" ago");
-			return timebuf;
-		}
-		/* Otherwise, just years. Centuries is probably overkill. */
-		snprintf(timebuf, sizeof(timebuf), "%lu years ago", (diff + 183) / 365);
-		return timebuf;
+		return show_date_relative(time, tz, &now,
+					  timebuf, sizeof(timebuf));
 	}
 
 	if (mode == DATE_LOCAL)
@@ -866,19 +875,13 @@ static const char *approxidate_digit(const char *date, struct tm *tm, int *num)
 	return end;
 }
 
-unsigned long approxidate(const char *date)
+static unsigned long approxidate_str(const char *date, const struct timeval *tv)
 {
 	int number = 0;
 	struct tm tm, now;
-	struct timeval tv;
 	time_t time_sec;
-	char buffer[50];
 
-	if (parse_date(date, buffer, sizeof(buffer)) > 0)
-		return strtoul(buffer, NULL, 10);
-
-	gettimeofday(&tv, NULL);
-	time_sec = tv.tv_sec;
+	time_sec = tv->tv_sec;
 	localtime_r(&time_sec, &tm);
 	now = tm;
 	for (;;) {
@@ -899,3 +902,26 @@ unsigned long approxidate(const char *date)
 		tm.tm_year--;
 	return mktime(&tm);
 }
+
+unsigned long approxidate_relative(const char *date, const struct timeval *tv)
+{
+	char buffer[50];
+
+	if (parse_date(date, buffer, sizeof(buffer)) > 0)
+		return strtoul(buffer, NULL, 10);
+
+	return approxidate_str(date, tv);
+}
+
+unsigned long approxidate(const char *date)
+{
+	struct timeval tv;
+	char buffer[50];
+
+	if (parse_date(date, buffer, sizeof(buffer)) > 0)
+		return strtoul(buffer, NULL, 10);
+
+	gettimeofday(&tv, NULL);
+	return approxidate_str(date, &tv);
+}
+
-- 
1.6.4.2.375.g73938
Jeff King· Aug 30, 2009, 21:51 UTC · re: Jeff King · lore

Re: [PATCH 1/3] Add date formatting and parsing functions relative to a given time

On Sun, Aug 30, 2009 at 05:43:09PM -0400, Jeff King wrote:
Show 7 quoted lines
> From: Alex Riesen <raa.lkml@gmail.com>
> 
> The main purpose is to allow predictable testing of the code.
> 
> Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
> Signed-off-by: Jeff King <peff@peff.net>
> ---

Bleh. I just started working on a 4/3 that would test Linus' recent approxidate changes, but then I realized that this massive date.c refactoring conflicts with his changes.

I think the most sane thing is to rebase the whole series on top of lt/approxidate. Let me see what I can do.

-Peff
Jeff King· Aug 31, 2009, 02:22 UTC · re: Jeff King · lore

Re: [PATCH 1/3] Add date formatting and parsing functions relative to a given time

On Sun, Aug 30, 2009 at 05:51:27PM -0400, Jeff King wrote:
> I think the most sane thing is to rebase the whole series on top of
> lt/approxidate. Let me see what I can do.

And here it is. It was a little more complex than a simple rebase because lt/approxidate actually introduced new bugs. :) Hopefully this will be the last re-roll required.

The new series applies on top of lt/approxidate, and contains:
  [1/4]: Add date formatting and parsing functions relative to a given time
  [2/4]: refactor test-date interface
  [3/4]: tests: add date printing and parsing tests
  [4/4]: fix approxidate parsing of relative months and years
-Peff
Jeff King· Aug 31, 2009, 02:26 UTC · re: Jeff King · lore

[PATCH v2 1/4] Add date formatting and parsing functions relative to a given time

From: Alex Riesen <raa.lkml@gmail.com>
The main purpose is to allow predictable testing of the code.
Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
---
Same as previous 1/3, but rebased onto lt/approxidate topic. The merge
ended up as quite a mess because of textual differences, so I had to
fix up a fair bit by hand; Alex, please confirm that I didn't screw
anything up too badly right before putting your name at the top. ;)
 cache.h |    5 ++
 date.c  |  150 ++++++++++++++++++++++++++++++++++++--------------------------
 2 files changed, 92 insertions(+), 63 deletions(-)
Show changes to 2 files +92 −63

cache.h, date.c

diff --git a/cache.h b/cache.h
index 808daba..5fad24c 100644
--- a/cache.h
+++ b/cache.h
@@ -731,9 +731,14 @@ enum date_mode {
 };
 
 const char *show_date(unsigned long time, int timezone, enum date_mode mode);
+const char *show_date_relative(unsigned long time, int tz,
+			       const struct timeval *now,
+			       char *timebuf,
+			       size_t timebuf_size);
 int parse_date(const char *date, char *buf, int bufsize);
 void datestamp(char *buf, int bufsize);
 unsigned long approxidate(const char *);
+unsigned long approxidate_relative(const char *date, const struct timeval *now);
 enum date_mode parse_date_format(const char *format);
 
 #define IDENT_WARN_ON_NO_NAME  1
diff --git a/date.c b/date.c
index e848d96..8e57e5e 100644
--- a/date.c
+++ b/date.c
@@ -86,6 +86,67 @@ static int local_tzoffset(unsigned long time)
 	return offset * eastwest;
 }
 
+const char *show_date_relative(unsigned long time, int tz,
+			       const struct timeval *now,
+			       char *timebuf,
+			       size_t timebuf_size)
+{
+	unsigned long diff;
+	if (now->tv_sec < time)
+		return "in the future";
+	diff = now->tv_sec - time;
+	if (diff < 90) {
+		snprintf(timebuf, timebuf_size, "%lu seconds ago", diff);
+		return timebuf;
+	}
+	/* Turn it into minutes */
+	diff = (diff + 30) / 60;
+	if (diff < 90) {
+		snprintf(timebuf, timebuf_size, "%lu minutes ago", diff);
+		return timebuf;
+	}
+	/* Turn it into hours */
+	diff = (diff + 30) / 60;
+	if (diff < 36) {
+		snprintf(timebuf, timebuf_size, "%lu hours ago", diff);
+		return timebuf;
+	}
+	/* We deal with number of days from here on */
+	diff = (diff + 12) / 24;
+	if (diff < 14) {
+		snprintf(timebuf, timebuf_size, "%lu days ago", diff);
+		return timebuf;
+	}
+	/* Say weeks for the past 10 weeks or so */
+	if (diff < 70) {
+		snprintf(timebuf, timebuf_size, "%lu weeks ago", (diff + 3) / 7);
+		return timebuf;
+	}
+	/* Say months for the past 12 months or so */
+	if (diff < 360) {
+		snprintf(timebuf, timebuf_size, "%lu months ago", (diff + 15) / 30);
+		return timebuf;
+	}
+	/* Give years and months for 5 years or so */
+	if (diff < 1825) {
+		unsigned long years = diff / 365;
+		unsigned long months = (diff % 365 + 15) / 30;
+		int n;
+		n = snprintf(timebuf, timebuf_size, "%lu year%s",
+				years, (years > 1 ? "s" : ""));
+		if (months)
+			snprintf(timebuf + n, timebuf_size - n,
+					", %lu month%s ago",
+					months, (months > 1 ? "s" : ""));
+		else
+			snprintf(timebuf + n, timebuf_size - n, " ago");
+		return timebuf;
+	}
+	/* Otherwise, just years. Centuries is probably overkill. */
+	snprintf(timebuf, timebuf_size, "%lu years ago", (diff + 183) / 365);
+	return timebuf;
+}
+
 const char *show_date(unsigned long time, int tz, enum date_mode mode)
 {
 	struct tm *tm;
@@ -97,63 +158,10 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
 	}
 
 	if (mode == DATE_RELATIVE) {
-		unsigned long diff;
 		struct timeval now;
 		gettimeofday(&now, NULL);
-		if (now.tv_sec < time)
-			return "in the future";
-		diff = now.tv_sec - time;
-		if (diff < 90) {
-			snprintf(timebuf, sizeof(timebuf), "%lu seconds ago", diff);
-			return timebuf;
-		}
-		/* Turn it into minutes */
-		diff = (diff + 30) / 60;
-		if (diff < 90) {
-			snprintf(timebuf, sizeof(timebuf), "%lu minutes ago", diff);
-			return timebuf;
-		}
-		/* Turn it into hours */
-		diff = (diff + 30) / 60;
-		if (diff < 36) {
-			snprintf(timebuf, sizeof(timebuf), "%lu hours ago", diff);
-			return timebuf;
-		}
-		/* We deal with number of days from here on */
-		diff = (diff + 12) / 24;
-		if (diff < 14) {
-			snprintf(timebuf, sizeof(timebuf), "%lu days ago", diff);
-			return timebuf;
-		}
-		/* Say weeks for the past 10 weeks or so */
-		if (diff < 70) {
-			snprintf(timebuf, sizeof(timebuf), "%lu weeks ago", (diff + 3) / 7);
-			return timebuf;
-		}
-		/* Say months for the past 12 months or so */
-		if (diff < 360) {
-			snprintf(timebuf, sizeof(timebuf), "%lu months ago", (diff + 15) / 30);
-			return timebuf;
-		}
-		/* Give years and months for 5 years or so */
-		if (diff < 1825) {
-			unsigned long years = diff / 365;
-			unsigned long months = (diff % 365 + 15) / 30;
-			int n;
-			n = snprintf(timebuf, sizeof(timebuf), "%lu year%s",
-					years, (years > 1 ? "s" : ""));
-			if (months)
-				snprintf(timebuf + n, sizeof(timebuf) - n,
-					", %lu month%s ago",
-					months, (months > 1 ? "s" : ""));
-			else
-				snprintf(timebuf + n, sizeof(timebuf) - n,
-					" ago");
-			return timebuf;
-		}
-		/* Otherwise, just years. Centuries is probably overkill. */
-		snprintf(timebuf, sizeof(timebuf), "%lu years ago", (diff + 183) / 365);
-		return timebuf;
+		return show_date_relative(time, tz, &now,
+					  timebuf, sizeof(timebuf));
 	}
 
 	if (mode == DATE_LOCAL)
@@ -918,19 +926,13 @@ static void pending_number(struct tm *tm, int *num)
 	}
 }
 
-unsigned long approxidate(const char *date)
+static unsigned long approxidate_str(const char *date, const struct timeval *tv)
 {
 	int number = 0;
 	struct tm tm, now;
-	struct timeval tv;
 	time_t time_sec;
-	char buffer[50];
 
-	if (parse_date(date, buffer, sizeof(buffer)) > 0)
-		return strtoul(buffer, NULL, 10);
-
-	gettimeofday(&tv, NULL);
-	time_sec = tv.tv_sec;
+	time_sec = tv->tv_sec;
 	localtime_r(&time_sec, &tm);
 	now = tm;
 
@@ -954,3 +956,25 @@ unsigned long approxidate(const char *date)
 	pending_number(&tm, &number);
 	return update_tm(&tm, &now, 0);
 }
+
+unsigned long approxidate_relative(const char *date, const struct timeval *tv)
+{
+	char buffer[50];
+
+	if (parse_date(date, buffer, sizeof(buffer)) > 0)
+		return strtoul(buffer, NULL, 0);
+
+	return approxidate_str(date, tv);
+}
+
+unsigned long approxidate(const char *date)
+{
+	struct timeval tv;
+	char buffer[50];
+
+	if (parse_date(date, buffer, sizeof(buffer)) > 0)
+		return strtoul(buffer, NULL, 0);
+
+	gettimeofday(&tv, NULL);
+	return approxidate_str(date, &tv);
+}
-- 
1.6.4.2.373.g5881fd
Alex Riesen· Aug 31, 2009, 06:08 UTC · re: Jeff King · lore

Re: [PATCH v2 1/4] Add date formatting and parsing functions relative to a given time

On Mon, Aug 31, 2009 at 04:26, Jeff King<peff@peff.net> wrote:
Show 11 quoted lines
> From: Alex Riesen <raa.lkml@gmail.com>
>
> The main purpose is to allow predictable testing of the code.
>
> Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> Same as previous 1/3, but rebased onto lt/approxidate topic. The merge
> ended up as quite a mess because of textual differences, so I had to
> fix up a fair bit by hand; Alex, please confirm that I didn't screw
> anything up too badly right before putting your name at the top. ;)
Looks good.
Jeff King· Aug 31, 2009, 02:26 UTC · re: Jeff King · lore

[PATCH v2 2/4] refactor test-date interface

The test-date program goes back to the early days of git, where it was presumably used to do manual sanity checks on changes to the date code. However, it is not actually used by the test suite to do any sort of automatic of systematic tests.

This patch refactors the interface to the program to try to make it more suitable for use by the test suite. There should be no fallouts to changing the interface since it is not actually installed and is not internally called by any other programs.

The changes are:
  - add a "mode" parameter so the caller can specify which
    operation to test
  - add a mode to test relative date output from show_date
  - allow faking a fixed time via the TEST_DATE_NOW
    environment variable, which allows consistent automated
    testing
  - drop the use of ctime for showing dates in favor of our
    internal iso8601 printing routines. The ctime output is
    somewhat redundant (because of the day-of-week) which
    makes writing test cases more annoying.
Signed-off-by: Jeff King <peff@peff.net>
---
Same as earlier 2/3.
 test-date.c |   86 +++++++++++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 66 insertions(+), 20 deletions(-)
 rewrite test-date.c (63%)
Show changes to test-date.c +66 −20
diff --git a/test-date.c b/test-date.c
dissimilarity index 63%
index 62e8f23..5b0a220 100644
--- a/test-date.c
+++ b/test-date.c
@@ -1,20 +1,66 @@
-#include "cache.h"
-
-int main(int argc, char **argv)
-{
-	int i;
-
-	for (i = 1; i < argc; i++) {
-		char result[100];
-		time_t t;
-
-		memcpy(result, "bad", 4);
-		parse_date(argv[i], result, sizeof(result));
-		t = strtoul(result, NULL, 0);
-		printf("%s -> %s -> %s", argv[i], result, ctime(&t));
-
-		t = approxidate(argv[i]);
-		printf("%s -> %s\n", argv[i], ctime(&t));
-	}
-	return 0;
-}
+#include "cache.h"
+
+static const char *usage_msg = "\n"
+"  test-date show [time_t]...\n"
+"  test-date parse [date]...\n"
+"  test-date approxidate [date]...\n";
+
+static void show_dates(char **argv, struct timeval *now)
+{
+	char buf[128];
+
+	for (; *argv; argv++) {
+		time_t t = atoi(*argv);
+		show_date_relative(t, 0, now, buf, sizeof(buf));
+		printf("%s -> %s\n", *argv, buf);
+	}
+}
+
+static void parse_dates(char **argv, struct timeval *now)
+{
+	for (; *argv; argv++) {
+		char result[100];
+		time_t t;
+
+		parse_date(*argv, result, sizeof(result));
+		t = strtoul(result, NULL, 0);
+		printf("%s -> %s\n", *argv,
+			t ? show_date(t, 0, DATE_ISO8601) : "bad");
+	}
+}
+
+static void parse_approxidate(char **argv, struct timeval *now)
+{
+	for (; *argv; argv++) {
+		time_t t;
+		t = approxidate_relative(*argv, now);
+		printf("%s -> %s\n", *argv, show_date(t, 0, DATE_ISO8601));
+	}
+}
+
+int main(int argc, char **argv)
+{
+	struct timeval now;
+	const char *x;
+
+	x = getenv("TEST_DATE_NOW");
+	if (x) {
+		now.tv_sec = atoi(x);
+		now.tv_usec = 0;
+	}
+	else
+		gettimeofday(&now, NULL);
+
+	argv++;
+	if (!*argv)
+		usage(usage_msg);
+	if (!strcmp(*argv, "show"))
+		show_dates(argv+1, &now);
+	else if (!strcmp(*argv, "parse"))
+		parse_dates(argv+1, &now);
+	else if (!strcmp(*argv, "approxidate"))
+		parse_approxidate(argv+1, &now);
+	else
+		usage(usage_msg);
+	return 0;
+}
-- 
1.6.4.2.373.g5881fd
Jeff King· Aug 31, 2009, 02:30 UTC · re: Jeff King · lore

[PATCH v2 3/4] tests: add date printing and parsing tests

Until now, there was no coverage of relative date printing or approxidate parsing routines (mainly because we had no way of faking the "now" time for relative date calculations, which made consistent testing impossible).

This new script tries to exercise the basic features of show_date and approxidate. Most of the tests are just "this obvious thing works" to prevent future regressions, with a few exceptions:

  - We confirm the fix in 607a9e8 that relative year/month
    dates in the latter half of a year round correctly.
  - We confirm that the improvements in b5373e9 and 1bddb25
    work.
  - A few tests are marked to expect failure, which are
    regressions recently introduced by the two commits
    above.
Signed-off-by: Jeff King <peff@peff.net>
---
Similar to earlier 3/3, but improvements and regressions from
lt/approxidate included.

Linus, when you posted the approxidate fixes earlier, you mentioned you might have some other corner cases. I think you were just referring to the stuff you improved in the followup patch, but if you know of more broken-ness, we should probably include it here.

 t/t0006-date.sh |   75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 75 insertions(+), 0 deletions(-)
 create mode 100755 t/t0006-date.sh
Show changes to t/t0006-date.sh +75 −0
diff --git a/t/t0006-date.sh b/t/t0006-date.sh
new file mode 100755
index 0000000..02cd565
--- /dev/null
+++ b/t/t0006-date.sh
@@ -0,0 +1,75 @@
+#!/bin/sh
+
+test_description='test date parsing and printing'
+. ./test-lib.sh
+
+# arbitrary reference time: 2009-08-30 19:20:00
+TEST_DATE_NOW=1251660000; export TEST_DATE_NOW
+
+check_show() {
+	t=$(($TEST_DATE_NOW - $1))
+	echo "$t -> $2" >expect
+	test_expect_${3:-success} "relative date ($2)" "
+	test-date show $t >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_show 5 '5 seconds ago'
+check_show 300 '5 minutes ago'
+check_show 18000 '5 hours ago'
+check_show 432000 '5 days ago'
+check_show 1728000 '3 weeks ago'
+check_show 13000000 '5 months ago'
+check_show 37500000 '1 year, 2 months ago'
+check_show 55188000 '1 year, 9 months ago'
+check_show 630000000 '20 years ago'
+
+check_parse() {
+	echo "$1 -> $2" >expect
+	test_expect_${3:-success} "parse date ($1)" "
+	test-date parse '$1' >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_parse 2008 bad
+check_parse 2008-02 bad
+check_parse 2008-02-14 bad
+check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000'
+
+check_approxidate() {
+	echo "$1 -> $2 +0000" >expect
+	test_expect_${3:-success} "parse approxidate ($1)" "
+	test-date approxidate '$1' >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_approxidate now '2009-08-30 19:20:00'
+check_approxidate '5 seconds ago' '2009-08-30 19:19:55'
+check_approxidate 5.seconds.ago '2009-08-30 19:19:55'
+check_approxidate 10.minutes.ago '2009-08-30 19:10:00'
+check_approxidate yesterday '2009-08-29 19:20:00'
+check_approxidate 3.days.ago '2009-08-27 19:20:00'
+check_approxidate 3.weeks.ago '2009-08-09 19:20:00'
+check_approxidate 3.months.ago '2009-05-30 19:20:00' failure
+check_approxidate 2.years.3.months.ago '2007-05-30 19:20:00' failure
+
+check_approxidate '6am yesterday' '2009-08-29 06:00:00'
+check_approxidate '6pm yesterday' '2009-08-29 18:00:00'
+check_approxidate '3:00' '2009-08-30 03:00:00'
+check_approxidate '15:00' '2009-08-30 15:00:00'
+check_approxidate 'noon today' '2009-08-30 12:00:00'
+check_approxidate 'noon yesterday' '2009-08-29 12:00:00'
+
+check_approxidate 'last tuesday' '2009-08-25 19:20:00'
+check_approxidate 'July 5th' '2009-07-05 19:20:00'
+check_approxidate '06/05/2009' '2009-06-05 19:20:00'
+check_approxidate '06.05.2009' '2009-05-06 19:20:00'
+
+check_approxidate 'Jun 6, 5AM' '2009-06-06 05:00:00'
+check_approxidate '5AM Jun 6' '2009-06-06 05:00:00'
+check_approxidate '6AM, June 7, 2009' '2009-06-07 06:00:00'
+
+test_done
-- 
1.6.4.2.373.g5881fd
Jeff King· Sep 1, 2009, 03:03 UTC · re: Jeff King · lore

Re: [PATCH v2 3/4] tests: add date printing and parsing tests

On Sun, Aug 30, 2009 at 10:30:15PM -0400, Jeff King wrote:
>   - We confirm that the improvements in b5373e9 and 1bddb25
>     work.

Ugh. I just realized (when explaining how awesome git resurrect was in another mail) that I managed to bungle these commit hashes (and the one mentioned in the following patch).

What happened is that I was building on the topic branch and lazily did a "rebase -i origin" to fix up my patches. I left the first two patches untouched, of course, but they still ended up with new committer information.

As my patches are merged to 'next' already, I think it is too late to fixup the commit message. But for posterity, the correct referenced commits are 9029055 and 36e4986.

Caveat rebaser.
-Peff
Jeff King· Aug 31, 2009, 02:31 UTC · re: Jeff King · lore

[PATCH v2 4/4] fix approxidate parsing of relative months and years

These were broken by b5373e9. The problem is that the code marks the month and year with "-1" for "we don't know it yet", but the month and year code paths were not adjusted to fill in the current time before doing their calculations (whereas other units follow a different code path and are fine).

Signed-off-by: Jeff King <peff@peff.net>
---
This one is new from the last series, as it fixes bugs only found in
lt/approxidate.
 date.c          |    5 ++++-
 t/t0006-date.sh |    4 ++--
 2 files changed, 6 insertions(+), 3 deletions(-)
Show changes to 2 files +6 −3

date.c, t/t0006-date.sh

diff --git a/date.c b/date.c
index 8e57e5e..e9ee4aa 100644
--- a/date.c
+++ b/date.c
@@ -857,7 +857,9 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm
 	}
 
 	if (match_string(date, "months") >= 5) {
-		int n = tm->tm_mon - *num;
+		int n;
+		update_tm(tm, now, 0); /* fill in date fields if needed */
+		n = tm->tm_mon - *num;
 		*num = 0;
 		while (n < 0) {
 			n += 12;
@@ -868,6 +870,7 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm
 	}
 
 	if (match_string(date, "years") >= 4) {
+		update_tm(tm, now, 0); /* fill in date fields if needed */
 		tm->tm_year -= *num;
 		*num = 0;
 		return end;
diff --git a/t/t0006-date.sh b/t/t0006-date.sh
index 02cd565..a4d8fa8 100755
--- a/t/t0006-date.sh
+++ b/t/t0006-date.sh
@@ -53,8 +53,8 @@ check_approxidate 10.minutes.ago '2009-08-30 19:10:00'
 check_approxidate yesterday '2009-08-29 19:20:00'
 check_approxidate 3.days.ago '2009-08-27 19:20:00'
 check_approxidate 3.weeks.ago '2009-08-09 19:20:00'
-check_approxidate 3.months.ago '2009-05-30 19:20:00' failure
-check_approxidate 2.years.3.months.ago '2007-05-30 19:20:00' failure
+check_approxidate 3.months.ago '2009-05-30 19:20:00'
+check_approxidate 2.years.3.months.ago '2007-05-30 19:20:00'
 
 check_approxidate '6am yesterday' '2009-08-29 06:00:00'
 check_approxidate '6pm yesterday' '2009-08-29 18:00:00'
-- 
1.6.4.2.373.g5881fd
Jeff King· Aug 30, 2009, 21:46 UTC · re: Jeff King · lore

[PATCH 2/3] refactor test-date interface

The test-date program goes back to the early days of git, where it was presumably used to do manual sanity checks on changes to the date code. However, it is not actually used by the test suite to do any sort of automatic of systematic tests.

This patch refactors the interface to the program to try to make it more suitable for use by the test suite. There should be no fallouts to changing the interface since it is not actually installed and is not internally called by any other programs.

The changes are:
  - add a "mode" parameter so the caller can specify which
    operation to test
  - add a mode to test relative date output from show_date
  - allow faking a fixed time via the TEST_DATE_NOW
    environment variable, which allows consistent automated
    testing
  - drop the use of ctime for showing dates in favor of our
    internal iso8601 printing routines. The ctime output is
    somewhat redundant (because of the day-of-week) which
    makes writing test cases more annoying.
Signed-off-by: Jeff King <peff@peff.net>
---
I mulled over replacing ctime for a bit, as we are testing git's date
code with other parts of git's date code. But it really is more
convenient for writing test cases to use iso8601, since you don't have
to calculate the day-of-week (and I also think it is a bit more
readable). And our iso8601 code is dead simple, so I am not too worried
about a bug in it hiding a bug elsewhere.
 test-date.c |   86 +++++++++++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 66 insertions(+), 20 deletions(-)
 rewrite test-date.c (63%)
Show changes to test-date.c +66 −20
diff --git a/test-date.c b/test-date.c
dissimilarity index 63%
index 62e8f23..5b0a220 100644
--- a/test-date.c
+++ b/test-date.c
@@ -1,20 +1,66 @@
-#include "cache.h"
-
-int main(int argc, char **argv)
-{
-	int i;
-
-	for (i = 1; i < argc; i++) {
-		char result[100];
-		time_t t;
-
-		memcpy(result, "bad", 4);
-		parse_date(argv[i], result, sizeof(result));
-		t = strtoul(result, NULL, 0);
-		printf("%s -> %s -> %s", argv[i], result, ctime(&t));
-
-		t = approxidate(argv[i]);
-		printf("%s -> %s\n", argv[i], ctime(&t));
-	}
-	return 0;
-}
+#include "cache.h"
+
+static const char *usage_msg = "\n"
+"  test-date show [time_t]...\n"
+"  test-date parse [date]...\n"
+"  test-date approxidate [date]...\n";
+
+static void show_dates(char **argv, struct timeval *now)
+{
+	char buf[128];
+
+	for (; *argv; argv++) {
+		time_t t = atoi(*argv);
+		show_date_relative(t, 0, now, buf, sizeof(buf));
+		printf("%s -> %s\n", *argv, buf);
+	}
+}
+
+static void parse_dates(char **argv, struct timeval *now)
+{
+	for (; *argv; argv++) {
+		char result[100];
+		time_t t;
+
+		parse_date(*argv, result, sizeof(result));
+		t = strtoul(result, NULL, 0);
+		printf("%s -> %s\n", *argv,
+			t ? show_date(t, 0, DATE_ISO8601) : "bad");
+	}
+}
+
+static void parse_approxidate(char **argv, struct timeval *now)
+{
+	for (; *argv; argv++) {
+		time_t t;
+		t = approxidate_relative(*argv, now);
+		printf("%s -> %s\n", *argv, show_date(t, 0, DATE_ISO8601));
+	}
+}
+
+int main(int argc, char **argv)
+{
+	struct timeval now;
+	const char *x;
+
+	x = getenv("TEST_DATE_NOW");
+	if (x) {
+		now.tv_sec = atoi(x);
+		now.tv_usec = 0;
+	}
+	else
+		gettimeofday(&now, NULL);
+
+	argv++;
+	if (!*argv)
+		usage(usage_msg);
+	if (!strcmp(*argv, "show"))
+		show_dates(argv+1, &now);
+	else if (!strcmp(*argv, "parse"))
+		parse_dates(argv+1, &now);
+	else if (!strcmp(*argv, "approxidate"))
+		parse_approxidate(argv+1, &now);
+	else
+		usage(usage_msg);
+	return 0;
+}
-- 
1.6.4.2.375.g73938
Jeff King· Aug 30, 2009, 21:47 UTC · re: Jeff King · lore

[PATCH 3/3] tests: add date printing and parsing tests

Until now, there was no coverage of relative date printing or approxidate parsing routines (mainly because we had no way of faking the "now" time for relative date calculations, which made consistent testing impossible).

This new script tries to exercise the basic features of show_date and approxidate. The only specific problem case tested is showing relative year/month dates in the latter half of a year, as fixed by 607a9e8.

Signed-off-by: Jeff King <peff@peff.net>
---
Like I said, this is really just to exercise the basic code paths.
But now that the infrastructure is there, we can add any corner cases
or verify new features or bug fixes as they come up. Patches welcome. :)
 t/t0006-date.sh |   71 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 71 insertions(+), 0 deletions(-)
 create mode 100755 t/t0006-date.sh
Show changes to t/t0006-date.sh +71 −0
diff --git a/t/t0006-date.sh b/t/t0006-date.sh
new file mode 100755
index 0000000..4beb44b
--- /dev/null
+++ b/t/t0006-date.sh
@@ -0,0 +1,71 @@
+#!/bin/sh
+
+test_description='test date parsing and printing'
+. ./test-lib.sh
+
+# arbitrary reference time: 2009-08-30 19:20:00
+TEST_DATE_NOW=1251660000; export TEST_DATE_NOW
+
+check_show() {
+	t=$(($TEST_DATE_NOW - $1))
+	echo "$t -> $2" >expect
+	test_expect_success "relative date ($2)" "
+	test-date show $t >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_show 5 '5 seconds ago'
+check_show 300 '5 minutes ago'
+check_show 18000 '5 hours ago'
+check_show 432000 '5 days ago'
+check_show 1728000 '3 weeks ago'
+check_show 13000000 '5 months ago'
+check_show 37500000 '1 year, 2 months ago'
+check_show 55188000 '1 year, 9 months ago'
+check_show 630000000 '20 years ago'
+
+check_parse() {
+	echo "$1 -> $2" >expect
+	test_expect_success "parse date ($1)" "
+	test-date parse '$1' >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_parse 2008 bad
+check_parse 2008-02 bad
+check_parse 2008-02-14 '2008-02-14 00:00:00 +0000'
+check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000'
+
+check_approxidate() {
+	echo "$1 -> $2 +0000" >expect
+	test_expect_success "parse approxidate ($1)" "
+	test-date approxidate '$1' >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_approxidate now '2009-08-30 19:20:00'
+check_approxidate '5 seconds ago' '2009-08-30 19:19:55'
+check_approxidate 5.seconds.ago '2009-08-30 19:19:55'
+check_approxidate 10.minutes.ago '2009-08-30 19:10:00'
+check_approxidate yesterday '2009-08-29 19:20:00'
+check_approxidate 3.days.ago '2009-08-27 19:20:00'
+check_approxidate 3.weeks.ago '2009-08-09 19:20:00'
+check_approxidate 3.months.ago '2009-05-30 19:20:00'
+check_approxidate 2.years.3.months.ago '2007-05-30 19:20:00'
+
+check_approxidate '6am yesterday' '2009-08-29 06:00:00'
+check_approxidate '6pm yesterday' '2009-08-29 18:00:00'
+check_approxidate '3:00' '2009-08-30 03:00:00'
+check_approxidate '15:00' '2009-08-30 15:00:00'
+check_approxidate 'noon today' '2009-08-30 12:00:00'
+check_approxidate 'noon yesterday' '2009-08-29 12:00:00'
+
+check_approxidate 'last tuesday' '2009-08-25 19:20:00'
+check_approxidate 'July 5th' '2009-07-05 19:20:00'
+check_approxidate '06/05/2009' '2009-06-05 00:00:00'
+check_approxidate '06.05.2009' '2009-05-06 00:00:00'
+
+test_done
-- 
1.6.4.2.375.g73938

← back to recent threads