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

Previous thread: [EGIT PATCH 20/20] PackWriter test suite by Marek Zawirski on Sunday, June 15, 2008 - 2:45 pm. (1 message)

Next thread: gc and empty packed-refs by Allan Wind on Sunday, June 15, 2008 - 4:27 pm. (1 message)
From: Olivier Marin
Date: Sunday, June 15, 2008 - 4:20 pm

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 &&

--

From: Olivier Marin
Date: Tuesday, June 17, 2008 - 9:34 am

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 = ...
From: Johannes Schindelin
Date: Wednesday, June 18, 2008 - 8:06 am

Hi,


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

Ciao,
Dscho

--

Previous thread: [EGIT PATCH 20/20] PackWriter test suite by Marek Zawirski on Sunday, June 15, 2008 - 2:45 pm. (1 message)

Next thread: gc and empty packed-refs by Allan Wind on Sunday, June 15, 2008 - 4:27 pm. (1 message)