Bug with approxidate("never")?

Olivier Marin· Jun 15, 2008, 23:20 UTC · lore

Hi,

While I was playing in the stash area and with "reflog expire" I was surprised by the fact that "never" is not always never. For example, approxidate("never") returns 4294960096 for my timezone (CET-2), it loops with all negative timezones.

I fixed it by replacing date_never() with this one:

static void date_never(struct tm *tm, int *num)
{
        time_t n = 0;
        localtime_r(&n, tm);
}

but I not sure this is the right fix. Better idea?

If you want to try yourself, the following test case should trigger the problem.

diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
index 73f830d..b07d7bd 100755
--- a/t/t1410-reflog.sh
+++ b/t/t1410-reflog.sh
@@ -158,6 +158,17 @@ test_expect_success 'reflog expire' '
        check_fsck "dangling commit $K"
 '
 
+test_expect_success 'reflog expire --expire=never' '
+
+       TZ=CET-2 &&
+       git reflog expire --expire=never \
+               --stale-fix \
+               --all &&
+
+       loglen=$(wc -l <.git/logs/refs/heads/master) &&
+       test $loglen = 2
+'
+
 test_expect_success 'prune and fsck' '
 
        git prune &&

Olivier Marin· Jun 17, 2008, 16:34 UTC · re: Olivier Marin · lore

[PATCH] Fix approxidate("never") to always return 0

From: Olivier Marin <dkr@freesurf.fr>

Commit af66366a9feb0194ed04b1f538998021ece268a8 introduced the keyword "never" to be used with approxidate() but defined it with a fixed date without taking care of timezone. As a result approxidate() will return a timestamp in the future with a negative timezone.

With this patch, approxidate("never") always return 0 whatever your timezone is.

Also, print seconds since the Epoch in test-date.

Signed-off-by: Olivier Marin <dkr@freesurf.fr>
---

Without this patch:

	$ git --version
	git version 1.5.6.rc3.156.g1a01 (next)

	$ mkdir /tmp/repo1 && cd /tmp/repo1 && git init
	$ echo A > A && git add A && git commit -m A

	$ git config gc.reflogexpire never

	$ TZ=UTC git reflog expire --dry-run --verbose HEAD
	keep commit (initial): A

	$ TZ=UTC-2 git reflog expire --dry-run --verbose HEAD
	would prune commit (initial): A

This test does not trigger the problem with Junio's "Per-ref reflog expiry configuration" patch because it explicitly tests for "never" without relying on approxidate() but the bug still remains with --expire option and in date.c

This make me thinking about TZ=UTC forced into t/test-lib.sh. Should not test cases to be independant of the timezone?

 date.c      |    6 ++----
 test-date.c |    2 +-
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/date.c b/date.c
index a74ed86..1a4eb87 100644
--- a/date.c
+++ b/date.c
@@ -682,10 +682,8 @@ static void date_am(struct tm *tm, int *num)
 
 static void date_never(struct tm *tm, int *num)
 {
-	tm->tm_mon = tm->tm_wday = tm->tm_yday
-		= tm->tm_hour = tm->tm_min = tm->tm_sec = 0;
-	tm->tm_year = 70;
-	tm->tm_mday = 1;
+	time_t n = 0;
+	localtime_r(&n, tm);
 }
 
 static const struct special {
diff --git a/test-date.c b/test-date.c
index 62e8f23..b84e000 100644
--- a/test-date.c
+++ b/test-date.c
@@ -14,7 +14,7 @@ int main(int argc, char **argv)
 		printf("%s -> %s -> %s", argv[i], result, ctime(&t));
 
 		t = approxidate(argv[i]);
-		printf("%s -> %s\n", argv[i], ctime(&t));
+		printf("%s -> %lu -> %s\n", argv[i], t, ctime(&t));
 	}
 	return 0;
 }

-- 
1.5.6.rc3.204.gf01a.dirty

Johannes Schindelin· Jun 18, 2008, 15:06 UTC · re: Olivier Marin · lore

Re: [PATCH] Fix approxidate("never") to always return 0

Hi,

On Tue, 17 Jun 2008, Olivier Marin wrote:

Show 14 quoted lines

> diff --git a/date.c b/date.c
> index a74ed86..1a4eb87 100644
> --- a/date.c
> +++ b/date.c
> @@ -682,10 +682,8 @@ static void date_am(struct tm *tm, int *num)
>  
>  static void date_never(struct tm *tm, int *num)
>  {
> -	tm->tm_mon = tm->tm_wday = tm->tm_yday
> -		= tm->tm_hour = tm->tm_min = tm->tm_sec = 0;
> -	tm->tm_year = 70;
> -	tm->tm_mday = 1;
> +	time_t n = 0;
> +	localtime_r(&n, tm);

I would have reused local_tzoffset() and been explicit about the used date, but your version is shorter.

Ciao, Dscho