reflog expire: default expiry times swapped since 2.50

reflog expire: default expiry times swapped since 2.50

[r.norouzi]

What did you do before the bug happened? (Steps to reproduce your issue)

I made a reflog entry 60 days old on a branch that still contains the
commit, then i asked what "git reflog expire" would prune, with no
gc.reflogExpire or gc.reflogExpireUnreachable configured:

    git init -q -b main repro && cd repro
    t=$(date -d '60 days ago' +%s)
    GIT_COMMITTER_DATE="@$t +0000" GIT_AUTHOR_DATE="@$t +0000" \
            git commit -q --allow-empty -m old
    git commit -q --allow-empty -m new
    git reflog expire --dry-run --verbose main

What did you expect to happen? (Expected behavior)

The entry for "old" to be kept. The documentation of gc.reflogExpire says
it defaults to 90 days, and gc.reflogExpireUnreachable to 30 days.

What happened instead? (Actual behavior)

    prune commit (initial): old
    keep commit: new

With -c gc.reflogExpire=90.days.ago the entry is kept.

What's different between what you expected and what actually happened?

Reachable entries expire after 30 days instead of 90. Since the total
cut-off is checked first, every entry older than 30 days is now pruned,
and the unreachable cut-off never matters.

Anything else you want to add:

The defaults look swapped in 85658275702b (builtin/reflog: stop storing
default reflog expiry dates globaly), first released in 2.50.0. Before
it, builtin/reflog.c had

    default_reflog_expire_unreachable = now - 30 * 24 * 3600;
    default_reflog_expire = now - 90 * 24 * 3600;

and reflog.h now has

    .default_expire_total = now - 30 * 24 * 3600, \
    .default_expire_unreachable = now - 90 * 24 * 3600, \

master still has the reflog.h version. Seen with 2.55.0 (Git for
Windows 2.55.0.windows.5); the code in question is not platform
specific.

[PATCH] reflog: fix default expiry periods

[Pushkar Singh]

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.

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
+	)
+'
+
 test_expect_success 'corrupt and check' '
 
 	corrupt $F &&
-- 
2.56.0.rc1.320.g68acceee5b.dirty

Re: [PATCH] reflog: fix default expiry periods

[Junio C Hamano]

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.

Re: [PATCH] reflog: fix default expiry periods

[Jeff King]

On Tue, Sep 22, 2026 at 10:27:38AM -0700, Junio C Hamano wrote:

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

I had the same question. They were flipped by 8565827570
(builtin/reflog: stop storing default reflog expiry dates globally,
2025-04-08). That definitely should go in the commit message.

-Peff

Re: [PATCH] reflog: fix default expiry periods

[Patrick Steinhardt]

On Tue, Sep 22, 2026 at 02:02:51PM -0400, Jeff King wrote:
> On Tue, Sep 22, 2026 at 10:27:38AM -0700, Junio C Hamano wrote:
> 
> > 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.
> 
> I had the same question. They were flipped by 8565827570
> (builtin/reflog: stop storing default reflog expiry dates globally,
> 2025-04-08). That definitely should go in the commit message.

Oops, indeed. Thanks for finding this issue and fixing it!

Patrick

[PATCH v2] reflog: fix default expiry periods

[Pushkar Singh]

The default reflog expiry periods were swapped when they were moved to
REFLOG_EXPIRE_OPTIONS_INIT() by 85658275702b (builtin/reflog: stop storing
default reflog expiry dates globally).

This caused reachable entries to expire after 30 days instead of 90 days,
and unreachable entries after 90 days instead of 30 days.

Reported-by: r.norouzi <r.norouzi@proton.me>
Signed-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>
---
Changes in v2:
- Expand the regression test to cover both reachable and unreachable
  entries before and after their respective expiry periods.

 reflog.h          |  4 ++--
 t/t1410-reflog.sh | 45 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 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..c494aa5ef0 100755
--- a/t/t1410-reflog.sh
+++ b/t/t1410-reflog.sh
@@ -153,6 +153,51 @@ 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 uses the correct default expiry periods' '
+	test_when_finished "rm -rf reachable-keep reachable-expire unreachable" &&
+	git init reachable-keep &&
+	(
+		cd reachable-keep &&
+		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
+	) &&
+	git init reachable-expire &&
+	(
+		cd reachable-expire &&
+		timestamp=$(test-tool date timestamp "100.days.ago") &&
+		timestamp=${timestamp#* -> } &&
+		test_commit --no-tag --date "$timestamp +0000" old &&
+		git reflog expire --all &&
+		test_stdout_line_count = 0 git reflog refs/heads/main
+	) &&
+	git init unreachable &&
+	(
+		cd unreachable &&
+		test_commit --no-tag base &&
+		base=$(git rev-parse HEAD) &&
+		timestamp=$(test-tool date timestamp "20.days.ago") &&
+		timestamp=${timestamp#* -> } &&
+		test_commit --no-tag --date "$timestamp +0000" old-20 &&
+		old20=$(git rev-parse HEAD) &&
+		git update-ref refs/heads/main "$base" &&
+		timestamp=$(test-tool date timestamp "40.days.ago") &&
+		timestamp=${timestamp#* -> } &&
+		test_commit --no-tag --date "$timestamp +0000" old-40 &&
+		old40=$(git rev-parse HEAD) &&
+		git update-ref refs/heads/main "$base" &&
+		git rev-list --all --objects >reachable &&
+		test_grep ! "$old20" reachable &&
+		test_grep ! "$old40" reachable &&
+		git reflog expire --all &&
+		git reflog --format='%H' refs/heads/main >actual &&
+		test_grep "$old20" actual &&
+		test_grep ! "$old40" actual
+	)
+'
+
 test_expect_success 'corrupt and check' '
 
 	corrupt $F &&
-- 
2.53.0

Re: [PATCH v2] reflog: fix default expiry periods

[Junio C Hamano]

Pushkar Singh <pushkarkumarsingh1970@gmail.com> writes:

> The default reflog expiry periods were swapped when they were moved to
> REFLOG_EXPIRE_OPTIONS_INIT() by 85658275702b (builtin/reflog: stop storing
> default reflog expiry dates globally).
>
> This caused reachable entries to expire after 30 days instead of 90 days,
> and unreachable entries after 90 days instead of 30 days.
>
> Reported-by: r.norouzi <r.norouzi@proton.me>
> Signed-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>
> ---

The above reads very well.

>  #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, \
>  }

and the fix is very straight-forward.

> diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
> index 8f78cf4b01..c494aa5ef0 100755
> --- a/t/t1410-reflog.sh
> +++ b/t/t1410-reflog.sh
> @@ -153,6 +153,51 @@ 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 uses the correct default expiry periods' '
> +	test_when_finished "rm -rf reachable-keep reachable-expire unreachable" &&
> +	git init reachable-keep &&
> +	(
> +		cd reachable-keep &&
> +		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
> +	) &&
> +	git init reachable-expire &&
> +	(
> +		cd reachable-expire &&
> +		timestamp=$(test-tool date timestamp "100.days.ago") &&
> +		timestamp=${timestamp#* -> } &&
> +		test_commit --no-tag --date "$timestamp +0000" old &&
> +		git reflog expire --all &&
> +		test_stdout_line_count = 0 git reflog refs/heads/main
> +	) &&
> +	git init unreachable &&
> +	(
> +		cd unreachable &&
> +		test_commit --no-tag base &&
> +		base=$(git rev-parse HEAD) &&
> +		timestamp=$(test-tool date timestamp "20.days.ago") &&
> +		timestamp=${timestamp#* -> } &&
> +		test_commit --no-tag --date "$timestamp +0000" old-20 &&
> +		old20=$(git rev-parse HEAD) &&
> +		git update-ref refs/heads/main "$base" &&
> +		timestamp=$(test-tool date timestamp "40.days.ago") &&
> +		timestamp=${timestamp#* -> } &&
> +		test_commit --no-tag --date "$timestamp +0000" old-40 &&
> +		old40=$(git rev-parse HEAD) &&
> +		git update-ref refs/heads/main "$base" &&
> +		git rev-list --all --objects >reachable &&
> +		test_grep ! "$old20" reachable &&
> +		test_grep ! "$old40" reachable &&
> +		git reflog expire --all &&
> +		git reflog --format='%H' refs/heads/main >actual &&
> +		test_grep "$old20" actual &&
> +		test_grep ! "$old40" actual
> +	)
> +'

This one is curious in a few ways.

For reachable ones before and after the cut-off timestamp, we have
separate blocks to test them independently, but for unreachable
ones, we dedicatge only one block.  Is there a good reason for this
distinction?

As some people worry about repository set-up and tear-down cost, it
may please them more if you create a single test repository, prepare
four cases in it, and test them with a single "reflog expire --all".

On the other hand, it makes it easier to debug these tests if you
create one test repository for each of the four cases and test them
independently, but if we are going that route, we would rather want
to have one "test_expect_success" block for each of these four
cases.

This "one test_expect_success block that has three repositories, one
is used to test two cases and each of the other two is used to test
the remaining two cases separately" arrangement looks puzzling.

Re: [PATCH v2] reflog: fix default expiry periods

[Patrick Steinhardt]

On Wed, Sep 23, 2026 at 12:26:51PM -0700, Junio C Hamano wrote:
> Pushkar Singh <pushkarkumarsingh1970@gmail.com> writes:
> 
> > The default reflog expiry periods were swapped when they were moved to
> > REFLOG_EXPIRE_OPTIONS_INIT() by 85658275702b (builtin/reflog: stop storing
> > default reflog expiry dates globally).
> >
> > This caused reachable entries to expire after 30 days instead of 90 days,
> > and unreachable entries after 90 days instead of 30 days.
> >
> > Reported-by: r.norouzi <r.norouzi@proton.me>
> > Signed-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>
> > ---
> 
> The above reads very well.
> 
> >  #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, \
> >  }
> 
> and the fix is very straight-forward.

Is this something that we want to fast-track for Git 2.56?

Patrick

Re: [PATCH v2] reflog: fix default expiry periods

[Jeff King]

On Thu, Sep 24, 2026 at 04:12:04PM +0200, Patrick Steinhardt wrote:

> > >  #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, \
> > >  }
> > 
> > and the fix is very straight-forward.
> 
> Is this something that we want to fast-track for Git 2.56?

The breakage was in v2.50.0, so it is not a new regression. OTOH it
seems quite obvious and low-risk. I'd be OK either way.

-Peff

Re: [PATCH v2] reflog: fix default expiry periods

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Thu, Sep 24, 2026 at 04:12:04PM +0200, Patrick Steinhardt wrote:
>
>> > >  #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, \
>> > >  }
>> > 
>> > and the fix is very straight-forward.
>> 
>> Is this something that we want to fast-track for Git 2.56?
>
> The breakage was in v2.50.0, so it is not a new regression. OTOH it
> seems quite obvious and low-risk. I'd be OK either way.

Yeah, I didn't know the breakage was that old.  Perhaps not many
people are paying attention to reflog expiration?

Re: [PATCH v2] reflog: fix default expiry periods

[Jeff King]

On Thu, Sep 24, 2026 at 10:45:21AM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > On Thu, Sep 24, 2026 at 04:12:04PM +0200, Patrick Steinhardt wrote:
> >
> >> > >  #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, \
> >> > >  }
> >> > 
> >> > and the fix is very straight-forward.
> >> 
> >> Is this something that we want to fast-track for Git 2.56?
> >
> > The breakage was in v2.50.0, so it is not a new regression. OTOH it
> > seems quite obvious and low-risk. I'd be OK either way.
> 
> Yeah, I didn't know the breakage was that old.  Perhaps not many
> people are paying attention to reflog expiration?

Quite probably. The default expiration dates are somewhat arbitrary, and
the reflogs themselves are somewhat ephemeral. Probably people would
notice most on stashes, but those are also somewhat ephemeral.

-Peff

Re: [PATCH v2] reflog: fix default expiry periods

[Patrick Steinhardt]

On Thu, Sep 24, 2026 at 02:43:31PM -0400, Jeff King wrote:
> On Thu, Sep 24, 2026 at 10:45:21AM -0700, Junio C Hamano wrote:
> > Jeff King <peff@peff.net> writes:
> > > On Thu, Sep 24, 2026 at 04:12:04PM +0200, Patrick Steinhardt wrote:
> > >
> > >> > >  #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, \
> > >> > >  }
> > >> > 
> > >> > and the fix is very straight-forward.
> > >> 
> > >> Is this something that we want to fast-track for Git 2.56?
> > >
> > > The breakage was in v2.50.0, so it is not a new regression. OTOH it
> > > seems quite obvious and low-risk. I'd be OK either way.
> > 
> > Yeah, I didn't know the breakage was that old.  Perhaps not many
> > people are paying attention to reflog expiration?
> 
> Quite probably. The default expiration dates are somewhat arbitrary, and
> the reflogs themselves are somewhat ephemeral. Probably people would
> notice most on stashes, but those are also somewhat ephemeral.

Oh, I didn't realize that, either. In that case I agree it's not
necessary to fast-track this. Thanks!

Patrick

Re: [PATCH v2] reflog: fix default expiry periods

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> On Wed, Sep 23, 2026 at 12:26:51PM -0700, Junio C Hamano wrote:
>> Pushkar Singh <pushkarkumarsingh1970@gmail.com> writes:
>> 
>> > The default reflog expiry periods were swapped when they were moved to
>> > REFLOG_EXPIRE_OPTIONS_INIT() by 85658275702b (builtin/reflog: stop storing
>> > default reflog expiry dates globally).
>> >
>> > This caused reachable entries to expire after 30 days instead of 90 days,
>> > and unreachable entries after 90 days instead of 30 days.
>> >
>> > Reported-by: r.norouzi <r.norouzi@proton.me>
>> > Signed-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>
>> > ---
>> 
>> The above reads very well.
>> 
>> >  #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, \
>> >  }
>> 
>> and the fix is very straight-forward.
>
> Is this something that we want to fast-track for Git 2.56?

Probably.  The accompanied tests in the patch was not as nice as
what is quoted above, if I recall correctly, though.

[PATCH v3] reflog: fix default expiry periods

[Pushkar Singh]

The default reflog expiry periods were swapped when they were moved to
REFLOG_EXPIRE_OPTIONS_INIT() by 85658275702b (builtin/reflog: stop storing
default reflog expiry dates globally).

This caused reachable entries to expire after 30 days instead of 90 days,
and unreachable entries after 90 days instead of 30 days.

Reported-by: r.norouzi <r.norouzi@proton.me>
Signed-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>
---
Changes in v3:
- Split the regression test into four independent cases, covering
  reachable and unreachable entries before and after their expiry periods.

 reflog.h          |  4 +--
 t/t1410-reflog.sh | 66 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 68 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..93b5b49e1d 100755
--- a/t/t1410-reflog.sh
+++ b/t/t1410-reflog.sh
@@ -153,6 +153,72 @@ 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 reachable-keep" &&
+	git init reachable-keep &&
+	(
+		cd reachable-keep &&
+		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
+	)
+'
+
+test_expect_success 'reflog expire removes reachable entries after 90 days' '
+	test_when_finished "rm -rf reachable-expire" &&
+	git init reachable-expire &&
+	(
+		cd reachable-expire &&
+		timestamp=$(test-tool date timestamp "100.days.ago") &&
+		timestamp=${timestamp#* -> } &&
+		test_commit --no-tag --date "$timestamp +0000" old &&
+		git reflog expire --all &&
+		test_stdout_line_count = 0 git reflog refs/heads/main
+	)
+'
+
+test_expect_success 'reflog expire keeps unreachable entries for 30 days' '
+	test_when_finished "rm -rf unreachable-keep" &&
+	git init unreachable-keep &&
+	(
+		cd unreachable-keep &&
+		test_commit --no-tag base &&
+		base=$(git rev-parse HEAD) &&
+		timestamp=$(test-tool date timestamp "20.days.ago") &&
+		timestamp=${timestamp#* -> } &&
+		test_commit --no-tag --date "$timestamp +0000" old &&
+		old=$(git rev-parse HEAD) &&
+		git update-ref refs/heads/main "$base" &&
+		git rev-list --all --objects >reachable &&
+		test_grep ! "$old" reachable &&
+		git reflog expire --all &&
+		git reflog --format='%H' refs/heads/main >actual &&
+		test_grep "$old" actual
+	)
+'
+
+test_expect_success 'reflog expire removes unreachable entries after 30 days' '
+	test_when_finished "rm -rf unreachable-expire" &&
+	git init unreachable-expire &&
+	(
+		cd unreachable-expire &&
+		test_commit --no-tag base &&
+		base=$(git rev-parse HEAD) &&
+		timestamp=$(test-tool date timestamp "40.days.ago") &&
+		timestamp=${timestamp#* -> } &&
+		test_commit --no-tag --date "$timestamp +0000" old &&
+		old=$(git rev-parse HEAD) &&
+		git update-ref refs/heads/main "$base" &&
+		git rev-list --all --objects >reachable &&
+		test_grep ! "$old" reachable &&
+		git reflog expire --all &&
+		git reflog --format='%H' refs/heads/main >actual &&
+		test_grep ! "$old" actual
+	)
+'
+
 test_expect_success 'corrupt and check' '
 
 	corrupt $F &&
-- 
2.53.0

Re: [PATCH v3] reflog: fix default expiry periods

[Junio C Hamano]

Pushkar Singh <pushkarkumarsingh1970@gmail.com> writes:

> The default reflog expiry periods were swapped when they were moved to
> REFLOG_EXPIRE_OPTIONS_INIT() by 85658275702b (builtin/reflog: stop storing
> default reflog expiry dates globally).
>
> This caused reachable entries to expire after 30 days instead of 90 days,
> and unreachable entries after 90 days instead of 30 days.
>
> Reported-by: r.norouzi <r.norouzi@proton.me>
> Signed-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>
> ---
> Changes in v3:
> - Split the regression test into four independent cases, covering
>   reachable and unreachable entries before and after their expiry periods.
>
>  reflog.h          |  4 +--
>  t/t1410-reflog.sh | 66 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 68 insertions(+), 2 deletions(-)


This iteration has its tests much more nicely organized than the
previous round.  Looking very good.

After this patch graduates to 'master' and the dust settles, we
might want to clean-up the repetitive and verbose combinations of
turning a human-readable timespec into a machine-usable timestamp
into a single helper that is easier to use.  But that is outside the
scope of this change, I would say.

> 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..93b5b49e1d 100755
> --- a/t/t1410-reflog.sh
> +++ b/t/t1410-reflog.sh
> @@ -153,6 +153,72 @@ 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 reachable-keep" &&
> +	git init reachable-keep &&
> +	(
> +		cd reachable-keep &&
> +		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
> +	)
> +'
> +
> +test_expect_success 'reflog expire removes reachable entries after 90 days' '
> +	test_when_finished "rm -rf reachable-expire" &&
> +	git init reachable-expire &&
> +	(
> +		cd reachable-expire &&
> +		timestamp=$(test-tool date timestamp "100.days.ago") &&
> +		timestamp=${timestamp#* -> } &&
> +		test_commit --no-tag --date "$timestamp +0000" old &&
> +		git reflog expire --all &&
> +		test_stdout_line_count = 0 git reflog refs/heads/main
> +	)
> +'
> +
> +test_expect_success 'reflog expire keeps unreachable entries for 30 days' '
> +	test_when_finished "rm -rf unreachable-keep" &&
> +	git init unreachable-keep &&
> +	(
> +		cd unreachable-keep &&
> +		test_commit --no-tag base &&
> +		base=$(git rev-parse HEAD) &&
> +		timestamp=$(test-tool date timestamp "20.days.ago") &&
> +		timestamp=${timestamp#* -> } &&
> +		test_commit --no-tag --date "$timestamp +0000" old &&
> +		old=$(git rev-parse HEAD) &&
> +		git update-ref refs/heads/main "$base" &&
> +		git rev-list --all --objects >reachable &&
> +		test_grep ! "$old" reachable &&
> +		git reflog expire --all &&
> +		git reflog --format='%H' refs/heads/main >actual &&
> +		test_grep "$old" actual
> +	)
> +'
> +
> +test_expect_success 'reflog expire removes unreachable entries after 30 days' '
> +	test_when_finished "rm -rf unreachable-expire" &&
> +	git init unreachable-expire &&
> +	(
> +		cd unreachable-expire &&
> +		test_commit --no-tag base &&
> +		base=$(git rev-parse HEAD) &&
> +		timestamp=$(test-tool date timestamp "40.days.ago") &&
> +		timestamp=${timestamp#* -> } &&
> +		test_commit --no-tag --date "$timestamp +0000" old &&
> +		old=$(git rev-parse HEAD) &&
> +		git update-ref refs/heads/main "$base" &&
> +		git rev-list --all --objects >reachable &&
> +		test_grep ! "$old" reachable &&
> +		git reflog expire --all &&
> +		git reflog --format='%H' refs/heads/main >actual &&
> +		test_grep ! "$old" actual
> +	)
> +'
> +
>  test_expect_success 'corrupt and check' '
>  
>  	corrupt $F &&

Re: [PATCH v3] reflog: fix default expiry periods

[Patrick Steinhardt]

On Thu, Sep 24, 2026 at 05:58:44PM +0000, Pushkar Singh wrote:
> diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
> index 8f78cf4b01..93b5b49e1d 100755
> --- a/t/t1410-reflog.sh
> +++ b/t/t1410-reflog.sh
> @@ -153,6 +153,72 @@ 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 reachable-keep" &&
> +	git init reachable-keep &&
> +	(
> +		cd reachable-keep &&
> +		timestamp=$(test-tool date timestamp "60.days.ago") &&

Nit: I would've preferred to make this 89 days...

> +		timestamp=${timestamp#* -> } &&
> +		test_commit --no-tag --date "$timestamp +0000" old &&
> +		git reflog expire --all &&
> +		test_stdout_line_count = 1 git reflog refs/heads/main
> +	)
> +'
> +
> +test_expect_success 'reflog expire removes reachable entries after 90 days' '
> +	test_when_finished "rm -rf reachable-expire" &&
> +	git init reachable-expire &&
> +	(
> +		cd reachable-expire &&
> +		timestamp=$(test-tool date timestamp "100.days.ago") &&

... and this here exactly 90 days so that our test is a lot more narrow.
Same for the subsequent test, where we could've made it 29 and 30 days,
respectively. But I don't think that this necessitates a reroll.

Other than that I'm happy with this patch, thanks!

Patrick

Re: [PATCH v3] reflog: fix default expiry periods

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> On Thu, Sep 24, 2026 at 05:58:44PM +0000, Pushkar Singh wrote:
>> diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
>> index 8f78cf4b01..93b5b49e1d 100755
>> --- a/t/t1410-reflog.sh
>> +++ b/t/t1410-reflog.sh
>> @@ -153,6 +153,72 @@ 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 reachable-keep" &&
>> +	git init reachable-keep &&
>> +	(
>> +		cd reachable-keep &&
>> +		timestamp=$(test-tool date timestamp "60.days.ago") &&
>
> Nit: I would've preferred to make this 89 days...

Dates calculated as 89 days ago from the beginning of today, from
the end of today, and from this very minute can differ by almost 24
hours.  Because we are not interested in testing what semantics
approxidate() implements in test-tool date timestamp, but are
testing what expiry period reflog expire implements between 30 and
90 days, using numbers that are not too close to the edge spares us
from having to worry about boundary cases we do not care about.

So I wouldn't have preferred using 89 days there.

> Other than that I'm happy with this patch, thanks!
>
> Patrick

Thanks.

Re: [PATCH v3] reflog: fix default expiry periods

[Patrick Steinhardt]

On Mon, Sep 28, 2026 at 07:50:57AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > On Thu, Sep 24, 2026 at 05:58:44PM +0000, Pushkar Singh wrote:
> >> diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
> >> index 8f78cf4b01..93b5b49e1d 100755
> >> --- a/t/t1410-reflog.sh
> >> +++ b/t/t1410-reflog.sh
> >> @@ -153,6 +153,72 @@ 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 reachable-keep" &&
> >> +	git init reachable-keep &&
> >> +	(
> >> +		cd reachable-keep &&
> >> +		timestamp=$(test-tool date timestamp "60.days.ago") &&
> >
> > Nit: I would've preferred to make this 89 days...
> 
> Dates calculated as 89 days ago from the beginning of today, from
> the end of today, and from this very minute can differ by almost 24
> hours.  Because we are not interested in testing what semantics
> approxidate() implements in test-tool date timestamp, but are
> testing what expiry period reflog expire implements between 30 and
> 90 days, using numbers that are not too close to the edge spares us
> from having to worry about boundary cases we do not care about.
> 
> So I wouldn't have preferred using 89 days there.

Fair enough. I just find it a bit fishy to assert that we "[keep]
reachable entries for 90 days" by checking that we keep it for 60 days
but throw it away after 100 days. THat allows for a very wide range of
values that aren't 90 days.

So even if it shouldn't be 89 days, it could very well have been 88 days
without any risk for test flakiness.

Patrick

Re: [PATCH v3] reflog: fix default expiry periods

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> So even if it shouldn't be 89 days, it could very well have been 88 days
> without any risk for test flakiness.

That's fair.

Thanks.