Re: [PATCH] reflog: fix default expiry periods

[Junio C Hamano <gitster@pobox.com>]

Pushkar Singh <pushkarkumarsingh1970@gmail.com> writes:

> The default expiry periods for reflog entries are reversed in
> REFLOG_EXPIRE_OPTIONS_INIT(). This causes reachable entries to
> expire after 30 days instead of the documented 90 days.
>
> Restore the intended 90-day expiry for reachable entries and
> 30-day expiry for unreachable entries.
>
> Add a regression test for a reachable reflog entry that is
> 60 days old.

Does this breakage date back to the beginning of the reflog expire
feature, or was it working perfectly fine but was broken sometime
ago?  Do we know where the breakge happened, and why?  Those are a
lot more relevant things to describe in the first paragraph.  Once
the problem is described clearly enough in a simple case like this,
the solution to the problem is so obvious that latter two paragraphs
do not need to be said.

> Reported-by: r.norouzi <r.norouzi@proton.me>
> Signed-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>
> ---
>  reflog.h          |  4 ++--
>  t/t1410-reflog.sh | 13 +++++++++++++
>  2 files changed, 15 insertions(+), 2 deletions(-)

> diff --git a/reflog.h b/reflog.h
> index b996712c00..3bdd1ca8a5 100644
> --- a/reflog.h
> +++ b/reflog.h
> @@ -23,8 +23,8 @@ struct reflog_expire_options {
>  	int recno;
>  };
>  #define REFLOG_EXPIRE_OPTIONS_INIT(now) { \
> -	.default_expire_total = now - 30 * 24 * 3600, \
> -	.default_expire_unreachable = now - 90 * 24 * 3600, \
> +	.default_expire_total = now - 90 * 24 * 3600, \
> +	.default_expire_unreachable = now - 30 * 24 * 3600, \
>  }
>  
>  /*
> diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
> index 8f78cf4b01..1239c078ef 100755
> --- a/t/t1410-reflog.sh
> +++ b/t/t1410-reflog.sh
> @@ -153,6 +153,19 @@ test_expect_success 'reflog expire should not barf on an annotated tag' '
>  	test_grep ! "error: [Oo]bject .* not a commit" err
>  '
>  
> +test_expect_success 'reflog expire keeps reachable entries for 90 days' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		timestamp=$(test-tool date timestamp "60.days.ago") &&
> +		timestamp=${timestamp#* -> } &&
> +		test_commit --no-tag --date "$timestamp +0000" old &&
> +		git reflog expire --all &&
> +		test_stdout_line_count = 1 git reflog refs/heads/main
> +	)
> +'

Do you also want to see what happens to entries that point at
unreachable objects that is say 20 days and 40 days old,
respectively, plus an entry that points at a reachable object that
is 100 days old?  "Keeping" is "not expiring" and driven by the same
logic, so it is better to test both sides of the same coin, isn't it?

>  test_expect_success 'corrupt and check' '
>  
>  	corrupt $F &&

Thanks.