git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] push: fix --force-if-includes when remote-tracking ref has no reflog
@ 2026-09-03  1:05 Aleksei Sviridkin
  2026-09-03 16:16 ` Junio C Hamano
                   ` (2 more replies)
  0 siblings, 3 replies; 25+ messages in thread
From: Aleksei Sviridkin @ 2026-09-03  1:05 UTC (permalink / raw)
  To: git; +Cc: Srinidhi Kaushik, René Scharfe, Junio C Hamano,
	Aleksei Sviridkin

Since 99a1f9ae10 (push: add reflog check for "--force-if-includes",
2020-10-03), is_reachable_in_reflog() stops walking the reflog of the
local branch at entries older than the newest reflog entry of the
remote-tracking ref. That timestamp is read by a callback of
refs_for_each_reflog_ent_reverse() into a variable that is never
initialized, so when the remote-tracking ref has no reflog the walk
is cut off at whatever happens to be on the stack.

With the files backend a remote-tracking ref created by "git clone"
has no reflog and does not get one until it moves. On my machine the
leftover value exceeds any real timestamp: the walk stops at the very
first entry, never reaches the "Created from" entry that "checkout
--track" wrote, and the push is rejected with "remote ref updated
since checkout" although nothing on the remote has changed.

Initialize the timestamp to zero, so that a remote-tracking ref
without reflog makes the walk cover the whole reflog of the local
branch, as documented.

Signed-off-by: Aleksei Sviridkin <f@lex.la>
Assisted-by: LLM
---
The new test fails without the fix on my machine (macOS, arm64). As
the value read is uninitialized, other platforms may pass it by luck.

 remote.c            |  2 +-
 t/t5533-push-cas.sh | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/remote.c b/remote.c
index 00723b3..6d30169 100644
--- a/remote.c
+++ b/remote.c
@@ -2751,7 +2751,7 @@ static int check_and_collect_until(const char *refname UNUSED,
  */
 static int is_reachable_in_reflog(const char *local, const struct ref *remote)
 {
-	timestamp_t date;
+	timestamp_t date = 0;
 	struct commit *commit;
 	struct commit **chunk;
 	struct check_and_collect_until_cb_data cb;
diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh
index cba26a8..77f46f3 100755
--- a/t/t5533-push-cas.sh
+++ b/t/t5533-push-cas.sh
@@ -396,4 +396,22 @@ test_expect_success '"--force-if-includes" should allow deletes' '
 	)
 '
 
+test_expect_success '"--force-if-includes" should allow forced update when remote-tracking ref has no reflog' '
+	rm -fr dst src &&
+	git init --bare dst &&
+	git push dst main main:branch &&
+	git clone --no-local dst src &&
+	test_when_finished "rm -fr dst src" &&
+	(
+		cd src &&
+		# a clone leaves the remote-tracking refs without reflog
+		# entries with the files backend, but not with reftable
+		git reflog expire --all --expire=all &&
+		git switch -c branch --track origin/branch &&
+		git reset --hard HEAD^ &&
+		test_commit D &&
+		git push --force-if-includes --force-with-lease="branch"
+	)
+'
+
 test_done

base-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH] push: fix --force-if-includes when remote-tracking ref has no reflog
  2026-09-03  1:05 [PATCH] push: fix --force-if-includes when remote-tracking ref has no reflog Aleksei Sviridkin
@ 2026-09-03 16:16 ` Junio C Hamano
  2026-09-03 20:00   ` Aleksei Sviridkin
  2026-09-04 12:44 ` [PATCH v2] " Aleksei Sviridkin
  2026-09-05 17:13 ` [PATCH v3] " Aleksei Sviridkin
  2 siblings, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2026-09-03 16:16 UTC (permalink / raw)
  To: Aleksei Sviridkin; +Cc: git, Srinidhi Kaushik, René Scharfe

Aleksei Sviridkin <f@lex.la> writes:

> Since 99a1f9ae10 (push: add reflog check for "--force-if-includes",
> 2020-10-03), is_reachable_in_reflog() stops walking the reflog of the
> local branch at entries older than the newest reflog entry of the
> remote-tracking ref. That timestamp is read by a callback of
> refs_for_each_reflog_ent_reverse() into a variable that is never
> initialized, so when the remote-tracking ref has no reflog the walk
> is cut off at whatever happens to be on the stack.
>
> With the files backend a remote-tracking ref created by "git clone"
> has no reflog and does not get one until it moves. On my machine the
> leftover value exceeds any real timestamp: the walk stops at the very
> first entry, never reaches the "Created from" entry that "checkout
> --track" wrote, and the push is rejected with "remote ref updated
> since checkout" although nothing on the remote has changed.
>
> Initialize the timestamp to zero, so that a remote-tracking ref
> without reflog makes the walk cover the whole reflog of the local
> branch, as documented.
>
> Signed-off-by: Aleksei Sviridkin <f@lex.la>
> Assisted-by: LLM

The last line adds no useful information, though.  Besides, you are
fully responsible for whatever LLM emitted and contributed into this
patch, so your sign-off must be the last line in the trailers.

> ---
> The new test fails without the fix on my machine (macOS, arm64). As
> the value read is uninitialized, other platforms may pass it by luck.

The code change looks good.

It is a bit surprising to see the fallout from a change 6 years ago
to be addressed now, and makes me wonder what else changed recently.
Certainly year 2026 is not the first year in which macOS on arm64
started becoming widely used, or you are not the only user of Git on
that platform.

>  remote.c            |  2 +-
>  t/t5533-push-cas.sh | 18 ++++++++++++++++++
>  2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/remote.c b/remote.c
> index 00723b3..6d30169 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -2751,7 +2751,7 @@ static int check_and_collect_until(const char *refname UNUSED,
>   */
>  static int is_reachable_in_reflog(const char *local, const struct ref *remote)
>  {
> -	timestamp_t date;
> +	timestamp_t date = 0;
>  	struct commit *commit;
>  	struct commit **chunk;
>  	struct check_and_collect_until_cb_data cb;
> diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh
> index cba26a8..77f46f3 100755
> --- a/t/t5533-push-cas.sh
> +++ b/t/t5533-push-cas.sh
> @@ -396,4 +396,22 @@ test_expect_success '"--force-if-includes" should allow deletes' '
>  	)
>  '
>  
> +test_expect_success '"--force-if-includes" should allow forced update when remote-tracking ref has no reflog' '
> +	rm -fr dst src &&
> +	git init --bare dst &&
> +	git push dst main main:branch &&
> +	git clone --no-local dst src &&
> +	test_when_finished "rm -fr dst src" &&

You'd want to move "test_when_finished" immediately before "git init
--bare dst", no?  That way, you can clean things up after any or the
"init", "push", "clone" fails (as well as the main part of the test
that is done in the subdirectory).

> +	(
> +		cd src &&
> +		# a clone leaves the remote-tracking refs without reflog
> +		# entries with the files backend, but not with reftable
> +		git reflog expire --all --expire=all &&
> +		git switch -c branch --track origin/branch &&
> +		git reset --hard HEAD^ &&
> +		test_commit D &&
> +		git push --force-if-includes --force-with-lease="branch"
> +	)
> +'
> +
>  test_done
>
> base-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] push: fix --force-if-includes when remote-tracking ref has no reflog
  2026-09-03 16:16 ` Junio C Hamano
@ 2026-09-03 20:00   ` Aleksei Sviridkin
  2026-09-03 20:11     ` Junio C Hamano
                       ` (2 more replies)
  0 siblings, 3 replies; 25+ messages in thread
From: Aleksei Sviridkin @ 2026-09-03 20:00 UTC (permalink / raw)
  To: git; +Cc: Aleksei Sviridkin, Junio C Hamano

Junio C Hamano <gitster@pobox.com> writes:
> your sign-off must be the last line in the trailers.

I took Assisted-by from the kernel, which asks for it. I could not find
anything either way in git's guidelines, so I followed the kernel. I will
put the sign-off last in v2, and drop Assisted-by if you would rather not
have it.

> It is a bit surprising to see the fallout from a change 6 years ago
> to be addressed now, and makes me wonder what else changed recently.

The date is read from uninitialized stack, so it only bites when the
leftover happens to be larger than a real timestamp. That is build- and
layout-dependent, not really macOS/arm64 specific. I hit a layout where
it triggers every time, so the test is reliable here, but on another
build it can pass by luck, as you saw.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] push: fix --force-if-includes when remote-tracking ref has no reflog
  2026-09-03 20:00   ` Aleksei Sviridkin
@ 2026-09-03 20:11     ` Junio C Hamano
  2026-09-03 21:45       ` Aleksei Sviridkin
  2026-09-04  1:03     ` Kristoffer Haugsbakk
  2026-09-04 16:48     ` Junio C Hamano
  2 siblings, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2026-09-03 20:11 UTC (permalink / raw)
  To: Aleksei Sviridkin; +Cc: git

Aleksei Sviridkin <f@lex.la> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>> your sign-off must be the last line in the trailers.
>
> I took Assisted-by from the kernel, which asks for it. I could not find
> anything either way in git's guidelines, so I followed the kernel. I will
> put the sign-off last in v2, and drop Assisted-by if you would rather not
> have it.

I didn't mean to say that Assisted-by was useless.  What I meant was
that "LLM" on that trailer has no information contennts.  Which LLM?

We have the following in Documentation/SubmittingPatches by the way.

[[ai]]
=== Use of Artificial Intelligence (AI)

The Developer's Certificate of Origin requires contributors to certify
that they know the origin of their contributions to the project and
that they have the right to submit it under the project's license.
It's not yet clear that this can be legally satisfied when submitting
significant amount of content that has been generated by AI tools.

Another issue with AI generated content is that AIs still often
hallucinate or just produce bad code, commit messages, documentation
or output, even when you point out their mistakes.

To avoid these issues, we will reject anything that looks AI
generated, that sounds overly formal or bloated, that looks like AI
slop, that looks good on the surface but makes no sense, or that
senders don’t understand or cannot explain.

We strongly recommend using AI tools carefully and responsibly.

Contributors would often benefit more from AI by using it to guide and
help them step by step towards producing a solution by themselves
rather than by asking for a full solution that they would then mostly
copy-paste. They can also use AI to help with debugging, or with
checking for obvious mistakes, things that can be improved, things
that don’t match our style, guidelines or our feedback, before sending
it to us.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] push: fix --force-if-includes when remote-tracking ref has no reflog
  2026-09-03 20:11     ` Junio C Hamano
@ 2026-09-03 21:45       ` Aleksei Sviridkin
  0 siblings, 0 replies; 25+ messages in thread
From: Aleksei Sviridkin @ 2026-09-03 21:45 UTC (permalink / raw)
  To: git; +Cc: Aleksei Sviridkin, Junio C Hamano

Junio C Hamano <gitster@pobox.com> writes:
> I didn't mean to say that Assisted-by was useless.  What I meant was
> that "LLM" on that trailer has no information contennts.  Which LLM?

On "which LLM": I took the trailer from the kernel, and they dropped the
model name on purpose. Christian Brauner's 816d9992d9ed
(coding-assistants: simplify attribution) turned
Assisted-by: AGENT_NAME:MODEL_VERSION into a bare Assisted-by: LLM,
because naming the model "provides free advertising to proprietary
software companies while adding little or no useful information". It
fits my case anyway: this went through a router mixing models from three
vendors, so there is no single model to name, and git's guidelines don't
ask for one either. In case you are curious: k3, sol-5.6, fable 5.1,
opus 5 and sonnet 5. If you would still rather I drop the trailer, I
will.

I read the AI section. These are small fixes, I went through the whole
change myself and can explain any part of it. I'll keep what I send
lean.

v2 with the sign-off last and the test_when_finished fix goes out once
24 hours have passed since v1.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] push: fix --force-if-includes when remote-tracking ref has no reflog
  2026-09-03 20:00   ` Aleksei Sviridkin
  2026-09-03 20:11     ` Junio C Hamano
@ 2026-09-04  1:03     ` Kristoffer Haugsbakk
  2026-09-04 16:48     ` Junio C Hamano
  2 siblings, 0 replies; 25+ messages in thread
From: Kristoffer Haugsbakk @ 2026-09-04  1:03 UTC (permalink / raw)
  To: Aleksei Sviridkin, git; +Cc: Junio C Hamano

On Thu, Sep 3, 2026, at 22:00, Aleksei Sviridkin wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>> your sign-off must be the last line in the trailers.
>
> I took Assisted-by from the kernel, which asks for it. I could not find
> anything either way in git's guidelines, so I followed the kernel. 

If there is no mention of it in the guidelines
the fallback guideline becomes the one for
an operating system kernel? ’,:|

Maybe you saw the few ones that have landed
in the last months in this project. But those don't
follow the recent Linux workflow change to just
“LLM”.

> I will
> put the sign-off last in v2, and drop Assisted-by if you would rather not
> have it.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2] push: fix --force-if-includes when remote-tracking ref has no reflog
  2026-09-03  1:05 [PATCH] push: fix --force-if-includes when remote-tracking ref has no reflog Aleksei Sviridkin
  2026-09-03 16:16 ` Junio C Hamano
@ 2026-09-04 12:44 ` Aleksei Sviridkin
  2026-09-04 15:42   ` Junio C Hamano
  2026-09-05 17:13 ` [PATCH v3] " Aleksei Sviridkin
  2 siblings, 1 reply; 25+ messages in thread
From: Aleksei Sviridkin @ 2026-09-04 12:44 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Aleksei Sviridkin

Since 99a1f9ae10 (push: add reflog check for "--force-if-includes",
2020-10-03), is_reachable_in_reflog() stops walking the reflog of the
local branch at entries older than the newest reflog entry of the
remote-tracking ref. That timestamp is read by a callback of
refs_for_each_reflog_ent_reverse() into a variable that is never
initialized, so when the remote-tracking ref has no reflog the walk
is cut off at whatever happens to be on the stack.

With the files backend a remote-tracking ref created by "git clone"
has no reflog and does not get one until it moves. On my machine the
leftover value exceeds any real timestamp: the walk stops at the very
first entry, never reaches the "Created from" entry that "checkout
--track" wrote, and the push is rejected with "remote ref updated
since checkout" although nothing on the remote has changed.

Initialize the timestamp to zero, so that a remote-tracking ref
without reflog makes the walk cover the whole reflog of the local
branch, as documented.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
---
Changes since v1:
  - sign-off is now the last trailer
  - test_when_finished moved ahead of the setup so a failed init, push
    or clone still cleans up

 remote.c            |  2 +-
 t/t5533-push-cas.sh | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/remote.c b/remote.c
index 00723b385e..6d301698ca 100644
--- a/remote.c
+++ b/remote.c
@@ -2751,7 +2751,7 @@ static int check_and_collect_until(const char *refname UNUSED,
  */
 static int is_reachable_in_reflog(const char *local, const struct ref *remote)
 {
-	timestamp_t date;
+	timestamp_t date = 0;
 	struct commit *commit;
 	struct commit **chunk;
 	struct check_and_collect_until_cb_data cb;
diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh
index cba26a872d..bb8878c593 100755
--- a/t/t5533-push-cas.sh
+++ b/t/t5533-push-cas.sh
@@ -396,4 +396,22 @@ test_expect_success '"--force-if-includes" should allow deletes' '
 	)
 '
 
+test_expect_success '"--force-if-includes" should allow forced update when remote-tracking ref has no reflog' '
+	rm -fr dst src &&
+	test_when_finished "rm -fr dst src" &&
+	git init --bare dst &&
+	git push dst main main:branch &&
+	git clone --no-local dst src &&
+	(
+		cd src &&
+		# a clone leaves the remote-tracking refs without reflog
+		# entries with the files backend, but not with reftable
+		git reflog expire --all --expire=all &&
+		git switch -c branch --track origin/branch &&
+		git reset --hard HEAD^ &&
+		test_commit D &&
+		git push --force-if-includes --force-with-lease="branch"
+	)
+'
+
 test_done

base-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH v2] push: fix --force-if-includes when remote-tracking ref has no reflog
  2026-09-04 12:44 ` [PATCH v2] " Aleksei Sviridkin
@ 2026-09-04 15:42   ` Junio C Hamano
  2026-09-06  0:45     ` Junio C Hamano
  0 siblings, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2026-09-04 15:42 UTC (permalink / raw)
  To: Aleksei Sviridkin; +Cc: git

Aleksei Sviridkin <f@lex.la> writes:

>  static int is_reachable_in_reflog(const char *local, const struct ref *remote)
>  {
> -	timestamp_t date;
> +	timestamp_t date = 0;
>  	struct commit *commit;
>  	struct commit **chunk;
>  	struct check_and_collect_until_cb_data cb;

This gives a known value to the "date" variable, solving the issue
of using an uninitialized variable.  But how do we know if "0" a
reasonable fall-back value?  Why is it better than "now" or perhaps
"2 weeks ago"?

We pretend that the latest entry of the remote-tracking ref was from
year 1970.  And then that timestamp is used as a cut-off time for
check_and_collect_until().  What's the ramification of that?

> Since 99a1f9ae10 (push: add reflog check for "--force-if-includes",
> 2020-10-03), is_reachable_in_reflog() stops walking the reflog of the
> local branch at entries older than the newest reflog entry of the
> remote-tracking ref. That timestamp is read by a callback of
> refs_for_each_reflog_ent_reverse() into a variable that is never
> initialized, so when the remote-tracking ref has no reflog the walk
> is cut off at whatever happens to be on the stack.

This is almost good as-is.  I'd end the above with "... has no reflog,
the variable that holds the timestamp stays uninitialized".

> With the files backend a remote-tracking ref created by "git clone"
> has no reflog and does not get one until it moves. On my machine the
> leftover value exceeds any real timestamp: the walk stops at the very
> first entry, never reaches the "Created from" entry that "checkout
> --track" wrote, and the push is rejected with "remote ref updated
> since checkout" although nothing on the remote has changed.

That describes what happens (eh, rather, what does not happen) when
that uninitialized timestamp is more recent than the current time.

It does not explain why it is sensible to set it to year 1970, which
would force everything to be inspected.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] push: fix --force-if-includes when remote-tracking ref has no reflog
  2026-09-03 20:00   ` Aleksei Sviridkin
  2026-09-03 20:11     ` Junio C Hamano
  2026-09-04  1:03     ` Kristoffer Haugsbakk
@ 2026-09-04 16:48     ` Junio C Hamano
  2026-09-05 17:13       ` Aleksei Sviridkin
  2 siblings, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2026-09-04 16:48 UTC (permalink / raw)
  To: Aleksei Sviridkin; +Cc: git

Aleksei Sviridkin <f@lex.la> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>> your sign-off must be the last line in the trailers.
>
> I took Assisted-by from the kernel, which asks for it. I could not find
> anything either way in git's guidelines, so I followed the kernel. I will
> put the sign-off last in v2, and drop Assisted-by if you would rather not
> have it.

We are not the kernel ;-)

Quite honestly, I would rather not have patches filled with AI slop
that is often walls of text filled with "eh, that may not be wrong
per-se, but is it relevant?" descriptions and we can never tell what
was used as the original material to copy from.  I prefer patches
with human-readable explanations and known origin.

Thanks.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3] push: fix --force-if-includes when remote-tracking ref has no reflog
  2026-09-03  1:05 [PATCH] push: fix --force-if-includes when remote-tracking ref has no reflog Aleksei Sviridkin
  2026-09-03 16:16 ` Junio C Hamano
  2026-09-04 12:44 ` [PATCH v2] " Aleksei Sviridkin
@ 2026-09-05 17:13 ` Aleksei Sviridkin
  2026-09-29  1:10   ` Tyler Cipriani
  2026-09-29  9:13   ` [PATCH v4] " Aleksei Sviridkin
  2 siblings, 2 replies; 25+ messages in thread
From: Aleksei Sviridkin @ 2026-09-05 17:13 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Aleksei Sviridkin

Since 99a1f9ae10 (push: add reflog check for "--force-if-includes",
2020-10-03), is_reachable_in_reflog() stops walking the reflog of the
local branch at entries older than the newest reflog entry of the
remote-tracking ref. That timestamp is read by a callback of
refs_for_each_reflog_ent_reverse(), so when the remote-tracking ref
has no reflog, the variable that holds the timestamp stays
uninitialized.

With the files backend a remote-tracking ref created by "git clone"
has no reflog and does not get one until it moves. On my machine the
leftover value exceeds any real timestamp: the walk stops at the very
first entry, never reaches the "Created from" entry that "checkout
--track" wrote, and the push is rejected with "remote ref updated
since checkout" although nothing on the remote has changed.

The cut-off is an optimization that rests on an assumption: an entry
older than the moment the remote-tracking ref last moved is not
expected to be the one being looked for. Without a reflog there is
no such moment, hence no cut-off to apply. Initialize the timestamp
to zero to say exactly that: timestamp_t is unsigned, so no entry
compares older than zero and the comparison never fires. Using
"now", or any fixed age, would instead cut the walk off at the first
entry older than that bound, which is how the failure happens in
the first place. The price is paid only when no matching entry is
found: the walk then reaches the oldest entry and falls back to the
merge-base check over what it collected, where the cut-off would
have stopped it earlier.

Signed-off-by: Aleksei Sviridkin <f@lex.la>
---
Changes since v2:
  - reworded the first paragraph as you suggested
  - explain why zero is the fallback rather than "now" or a fixed age
  - dropped the Assisted-by trailer

 remote.c            |  2 +-
 t/t5533-push-cas.sh | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/remote.c b/remote.c
index 00723b385e..6d301698ca 100644
--- a/remote.c
+++ b/remote.c
@@ -2751,7 +2751,7 @@ static int check_and_collect_until(const char *refname UNUSED,
  */
 static int is_reachable_in_reflog(const char *local, const struct ref *remote)
 {
-	timestamp_t date;
+	timestamp_t date = 0;
 	struct commit *commit;
 	struct commit **chunk;
 	struct check_and_collect_until_cb_data cb;
diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh
index cba26a872d..bb8878c593 100755
--- a/t/t5533-push-cas.sh
+++ b/t/t5533-push-cas.sh
@@ -396,4 +396,22 @@ test_expect_success '"--force-if-includes" should allow deletes' '
 	)
 '
 
+test_expect_success '"--force-if-includes" should allow forced update when remote-tracking ref has no reflog' '
+	rm -fr dst src &&
+	test_when_finished "rm -fr dst src" &&
+	git init --bare dst &&
+	git push dst main main:branch &&
+	git clone --no-local dst src &&
+	(
+		cd src &&
+		# a clone leaves the remote-tracking refs without reflog
+		# entries with the files backend, but not with reftable
+		git reflog expire --all --expire=all &&
+		git switch -c branch --track origin/branch &&
+		git reset --hard HEAD^ &&
+		test_commit D &&
+		git push --force-if-includes --force-with-lease="branch"
+	)
+'
+
 test_done

base-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH] push: fix --force-if-includes when remote-tracking ref has no reflog
  2026-09-04 16:48     ` Junio C Hamano
@ 2026-09-05 17:13       ` Aleksei Sviridkin
  2026-09-06  9:39         ` Kristoffer Haugsbakk
  0 siblings, 1 reply; 25+ messages in thread
From: Aleksei Sviridkin @ 2026-09-05 17:13 UTC (permalink / raw)
  To: git; +Cc: Aleksei Sviridkin, Junio C Hamano

Junio C Hamano <gitster@pobox.com> writes:
> I prefer patches with human-readable explanations and known origin.

Dropped the trailer. You will not see it again.

On origin: this comes out of git's own tree, not an outside corpus, and
that is why every claim in the message names a file or a commit you can
check. The change here is one line, timestamp_t date; becoming
timestamp_t date = 0;. I read the whole thing and can explain any line
of it, and the sign-off is what carries that.

v3 uses your wording for the first paragraph and explains why zero is
the right fallback rather than "now" or a fixed age.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2] push: fix --force-if-includes when remote-tracking ref has no reflog
  2026-09-04 15:42   ` Junio C Hamano
@ 2026-09-06  0:45     ` Junio C Hamano
  2026-09-06 16:50       ` Aleksei Sviridkin
  0 siblings, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2026-09-06  0:45 UTC (permalink / raw)
  To: Aleksei Sviridkin; +Cc: git

Junio C Hamano <gitster@pobox.com> writes:

> Aleksei Sviridkin <f@lex.la> writes:
>
>>  static int is_reachable_in_reflog(const char *local, const struct ref *remote)
>>  {
>> -	timestamp_t date;
>> +	timestamp_t date = 0;
>>  	struct commit *commit;
>>  	struct commit **chunk;
>>  	struct check_and_collect_until_cb_data cb;
>
> This gives a known value to the "date" variable, solving the issue
> of using an uninitialized variable.  But how do we know if "0" a
> reasonable fall-back value?  Why is it better than "now" or perhaps
> "2 weeks ago"?

Thinking about it a bit more, let's imagine that we had reflog
enabled and did not have to suffer from this "uninitialized
variable" problem.  Even if the reflog for the remote-tracking
branch were enabled long ago and had plenty of entries, it wouldn't
have any entry older than 90 days, or the value gc.reflogExpire is
set.  Which suggests to me that gc.reflogExpire or 90 days ago would
be a lot more reasonable than year 1970 to use as a fallback cutoff
date.

Thanks.


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] push: fix --force-if-includes when remote-tracking ref has no reflog
  2026-09-05 17:13       ` Aleksei Sviridkin
@ 2026-09-06  9:39         ` Kristoffer Haugsbakk
  2026-09-06 17:14           ` Junio C Hamano
  0 siblings, 1 reply; 25+ messages in thread
From: Kristoffer Haugsbakk @ 2026-09-06  9:39 UTC (permalink / raw)
  To: Aleksei Sviridkin, git; +Cc: Junio C Hamano, Thomas Bachem

On Sat, Sep 5, 2026, at 19:13, Aleksei Sviridkin wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>> I prefer patches with human-readable explanations and known origin.

The following are just drive by comments on this point.

On Sat, Sep 5, 2026, at 19:13, Aleksei Sviridkin wrote:
> Dropped the trailer. You will not see it again.
>
> On origin: this comes out of git's own tree, not an outside corpus, and

Cool that it is only trained on Git’s corpus.

> that is why every claim in the message names a file or a commit you can
> check. The change here is one line, timestamp_t date; becoming
> timestamp_t date = 0;. I read the whole thing and can explain any line
> of it, [...]

If one can explain any and all of it, then having an AI write it is not
necessary. Referring to the “human-readable explanation” point.

> and the sign-off is what carries that.

Weird LLM-like phrases like this one seems to be used a lot on these
`Assisted-by` submissions (by you and Thomas Bachem recently).
Inanimate nouns gain subject-agency in sentences instead of saying
something plainly like, I have signed off on this and I mean it. And
this is on patch discussions, not in the commit messages (which are
already marked as being potentially AI written).

I feel like I should take two courses in linguistics in order to
articulate this uncanny valley feeling.

By commit message volume, I would have expected the commit messages (if
they are LLM-assisted) to read more like Jeff King log messages given
the corpus training.

>[snip]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2] push: fix --force-if-includes when remote-tracking ref has no reflog
  2026-09-06  0:45     ` Junio C Hamano
@ 2026-09-06 16:50       ` Aleksei Sviridkin
  2026-09-08  3:47         ` Junio C Hamano
  0 siblings, 1 reply; 25+ messages in thread
From: Aleksei Sviridkin @ 2026-09-06 16:50 UTC (permalink / raw)
  To: git; +Cc: Aleksei Sviridkin, Junio C Hamano

Junio C Hamano <gitster@pobox.com> writes:
> Which suggests to me that gc.reflogExpire or 90 days ago would be a
> lot more reasonable than year 1970 to use as a fallback cutoff date.

Entries older than 90 days do survive. The reflog expires when gc or
"git reflog expire" runs, not on its own, so I could build a branch
whose matching reflog entry is 200 days old and still sitting there.

A/B on one scenario with only the fallback differing: with now minus 90
days the push is rejected, with zero it goes through as a forced update.
The branch was created at the remote tip 200 days ago, that entry being
the matching one, rewound below the tip 150 days ago, one recent commit
on top, so the tip is not an ancestor of anything newer.

It takes two crossings of the bound to bite, which is why my first two
attempts to reproduce it failed. The match is tested before the
cut-off, so the entry sitting at the bound is still inspected, and the
merge-base fallback still covers the case where the tip is reachable
from something collected. You need a non-matching entry past the bound
and the tip unreachable from what was collected.

v3 went out a few hours before your mail; its third paragraph argues
zero over "now" or a fixed age. Your call.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] push: fix --force-if-includes when remote-tracking ref has no reflog
  2026-09-06  9:39         ` Kristoffer Haugsbakk
@ 2026-09-06 17:14           ` Junio C Hamano
  2026-09-07  4:54             ` Thomas Bachem
  0 siblings, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2026-09-06 17:14 UTC (permalink / raw)
  To: Kristoffer Haugsbakk; +Cc: Aleksei Sviridkin, git, Thomas Bachem

"Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:

> By commit message volume, I would have expected the commit messages (if
> they are LLM-assisted) to read more like Jeff King log messages given
> the corpus training.

;-)

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] push: fix --force-if-includes when remote-tracking ref has no reflog
  2026-09-06 17:14           ` Junio C Hamano
@ 2026-09-07  4:54             ` Thomas Bachem
  2026-09-07  6:23               ` Weijie Yuan
  0 siblings, 1 reply; 25+ messages in thread
From: Thomas Bachem @ 2026-09-07  4:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Kristoffer Haugsbakk, Aleksei Sviridkin, git

On 06/09/2026 19:14, Junio C Hamano wrote:
> "Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:
>
>> By commit message volume, I would have expected the commit messages (if
>> they are LLM-assisted) to read more like Jeff King log messages given
>> the corpus training.
>
> ;-)
I can ask for that from the next reroll on, if it helps ;-)

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] push: fix --force-if-includes when remote-tracking ref has no reflog
  2026-09-07  4:54             ` Thomas Bachem
@ 2026-09-07  6:23               ` Weijie Yuan
  0 siblings, 0 replies; 25+ messages in thread
From: Weijie Yuan @ 2026-09-07  6:23 UTC (permalink / raw)
  To: Thomas Bachem
  Cc: Junio C Hamano, Kristoffer Haugsbakk, Aleksei Sviridkin, git

On Mon, Sep 07, 2026 at 06:54:28AM +0200, Thomas Bachem wrote:
> On 06/09/2026 19:14, Junio C Hamano wrote:
> > "Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:
> >
> >> By commit message volume, I would have expected the commit messages (if
> >> they are LLM-assisted) to read more like Jeff King log messages given
> >> the corpus training.
> >
> > ;-)
> I can ask for that from the next reroll on, if it helps ;-)

I've already had an LLM do this for a while in my toy projects,
feeding it a few representative patch series from Peff.

Thanks, Peff! ;-)

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2] push: fix --force-if-includes when remote-tracking ref has no reflog
  2026-09-06 16:50       ` Aleksei Sviridkin
@ 2026-09-08  3:47         ` Junio C Hamano
  2026-09-09  6:56           ` Aleksei Sviridkin
  0 siblings, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2026-09-08  3:47 UTC (permalink / raw)
  To: Aleksei Sviridkin; +Cc: git

Aleksei Sviridkin <f@lex.la> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>> Which suggests to me that gc.reflogExpire or 90 days ago would be a
>> lot more reasonable than year 1970 to use as a fallback cutoff date.
>
> Entries older than 90 days do survive. The reflog expires when gc or
> "git reflog expire" runs, not on its own, so I could build a branch
> whose matching reflog entry is 200 days old and still sitting there.

It is only true for those who conciously disable the gc, isn't it?

It all depends on how hard it is to recover from such a failure, and
it may not even matter in practice what value we set, as it will
become a non-issue once they pull from there or push into there even
once.

But in the context of discussing what the fallback default ought to
be, I somehow sounds more like a poor excuse rather than a sensible
argument.  Doesn't it force a behaviour that would happen only to
those people who deliberately choose to ignore cutoff and who are
willing to spend cycles to go back to the beginning of history, to
all users, including those who do not make such customization, no?


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2] push: fix --force-if-includes when remote-tracking ref has no reflog
  2026-09-08  3:47         ` Junio C Hamano
@ 2026-09-09  6:56           ` Aleksei Sviridkin
  2026-09-10  0:57             ` Junio C Hamano
  0 siblings, 1 reply; 25+ messages in thread
From: Aleksei Sviridkin @ 2026-09-09  6:56 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Aleksei Sviridkin

Junio C Hamano <gitster@pobox.com> writes:
> It is only true for those who conciously disable the gc, isn't it?

No. The fallback is not about expiry, it is reached when the
remote-tracking ref has no reflog, and a plain clone leaves it that
way: after "git clone --no-local" on the files backend, "git reflog
exists refs/remotes/origin/main" returns 1, with core.logAllRefUpdates
at its default and gc untouched. The same source cloned with
--ref-format=reftable gets one entry.

Nothing expires on a calendar either. Entries go when "git reflog
expire" runs, and "git gc --auto" decides by loose object count
(gc.auto, 6700), so a quiet repository expires nothing.

> Doesn't it force a behaviour that would happen only to those people
> who deliberately choose to ignore cutoff and who are willing to spend
> cycles to go back to the beginning of history, to all users,
> including those who do not make such customization, no?

Measured that. One repository, 20000 entries in the local branch's
reflog, remote-tracking ref without a reflog, both values reject the
push so only the work differs. Median of 7 runs:

  entries spread over 200 days      zero 0.325s   90 days 0.069s
  after "git reflog expire --all"   zero 0.070s   90 days 0.069s
  20000 entries inside 90 days      zero 0.319s   90 days 0.321s

The expire run left 5 of the 20000, since all but five are 100 to 200
days old here. So the cost lands only on a repository that still holds
entries older than 90 days, which is the one where gc has not run.

In that repository, when the matching entry is one of the old ones, the
same walk is what decides: zero accepts in 0.322s, 90 days rejects in
0.086s.

With no match it is 0.26s of extra work for the same answer.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2] push: fix --force-if-includes when remote-tracking ref has no reflog
  2026-09-09  6:56           ` Aleksei Sviridkin
@ 2026-09-10  0:57             ` Junio C Hamano
  2026-09-10  8:31               ` Aleksei Sviridkin
  2026-09-25 20:53               ` Tyler Cipriani
  0 siblings, 2 replies; 25+ messages in thread
From: Junio C Hamano @ 2026-09-10  0:57 UTC (permalink / raw)
  To: Aleksei Sviridkin; +Cc: git

Aleksei Sviridkin <f@lex.la> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>> It is only true for those who conciously disable the gc, isn't it?
>
> No. The fallback is not about expiry, it is reached when the
> remote-tracking ref has no reflog, and a plain clone leaves it that
> way: after "git clone --no-local" on the files backend, "git reflog
> exists refs/remotes/origin/main" returns 1, with core.logAllRefUpdates
> at its default and gc untouched. The same source cloned with
> --ref-format=reftable gets one entry.
>
> Nothing expires on a calendar either. Entries go when "git reflog
> expire" runs, and "git gc --auto" decides by loose object count
> (gc.auto, 6700), so a quiet repository expires nothing.

Sorry but I am confused.  Your sample below is with 20000 local
reflog worth of activities, which is hardly a "quiet repository".
Besides, we are talking about "push" so optimizing for a quiet
repository does not sound like a useful mentail exercise to do.

>> Doesn't it force a behaviour that would happen only to those people
>> who deliberately choose to ignore cutoff and who are willing to spend
>> cycles to go back to the beginning of history, to all users,
>> including those who do not make such customization, no?
>
> Measured that. One repository, 20000 entries in the local branch's
> reflog, remote-tracking ref without a reflog, both values reject the
> push so only the work differs. Median of 7 runs:
>
>   entries spread over 200 days      zero 0.325s   90 days 0.069s
>   after "git reflog expire --all"   zero 0.070s   90 days 0.069s
>   20000 entries inside 90 days      zero 0.319s   90 days 0.321s
>
> The expire run left 5 of the 20000, since all but five are 100 to 200
> days old here. So the cost lands only on a repository that still holds
> entries older than 90 days, which is the one where gc has not run.
>
> In that repository, when the matching entry is one of the old ones, the
> same walk is what decides: zero accepts in 0.322s, 90 days rejects in
> 0.086s.
>
> With no match it is 0.26s of extra work for the same answer.

Thanks.

Doesn't that mean it is more logical to use the default gc
expiration timeout than year 1970 and in any cases using the usual
gc expiration would not waste more time than using 1970, right?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2] push: fix --force-if-includes when remote-tracking ref has no reflog
  2026-09-10  0:57             ` Junio C Hamano
@ 2026-09-10  8:31               ` Aleksei Sviridkin
  2026-09-25 20:53               ` Tyler Cipriani
  1 sibling, 0 replies; 25+ messages in thread
From: Aleksei Sviridkin @ 2026-09-10  8:31 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Aleksei Sviridkin

Junio C Hamano <gitster@pobox.com> writes:
> Sorry but I am confused.  Your sample below is with 20000 local
> reflog worth of activities, which is hardly a "quiet repository".

Two things got joined there. The 20000 entries are the worst case
for measuring the walk's cost. The repositories that keep entries
older than 90 days are ordinary ones where "git gc --auto" never
crossed 6700 loose objects, and that needs no configuration.

> Doesn't that mean it is more logical to use the default gc
> expiration timeout than year 1970 and in any cases using the usual
> gc expiration would not waste more time than using 1970, right?

On time, yes. The cutoff never takes longer than zero. But it saves
time only by ending the search early, and ending the search early is
what rejects a valid push. Same repository, matching entry 200 days
old: the cutoff rejects in 0.086s, zero accepts in 0.322s. Where the
cutoff cannot change the verdict, both take the same time: 0.070s vs
0.069s after expiry, 0.319s vs 0.321s with everything inside 90 days.

The cutoff is faster than zero only where it gives the wrong answer.
If that trade is acceptable, gc.reflogExpire is a one-line change,
and the commit message should then say the fallback can still reject
a correct push when the matching entry is older than the cutoff.
Your call.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2] push: fix --force-if-includes when remote-tracking ref has no reflog
  2026-09-10  0:57             ` Junio C Hamano
  2026-09-10  8:31               ` Aleksei Sviridkin
@ 2026-09-25 20:53               ` Tyler Cipriani
  2026-09-25 21:58                 ` Junio C Hamano
  1 sibling, 1 reply; 25+ messages in thread
From: Tyler Cipriani @ 2026-09-25 20:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Aleksei Sviridkin, git

On 26-09-09 17:57:01, Junio C Hamano wrote:

<snip>

>Doesn't that mean it is more logical to use the default gc
>expiration timeout than year 1970 and in any cases using the usual
>gc expiration would not waste more time than using 1970, right?

I like date=0 (i.e., 1970).

Tested locally, in _most_ cases both give the right answer. But
date=<cutoff> can give the wrong answer in a subset of cases, and date=0
can give a slower answer in a subset of cases.

I think the wrong answer is worse, and I think the case where
date=<cutoff> provides a wrong answer is common for me (with default gc
settings and lots of old git clones).

The important bits of is_reachable_in_reflog:

- date: initial: either gc.reflogExpire (default: 90 days) or 0. Later:
   maybe set by a walk of remote reflog.
- remote: e.g., remotes/origin/<x>
- local: e.g., refs/heads/<x>
- remote->old_oid: advertised oid for remote ref

We need to find remote->old_oid in the local reflog. We can't build on a
commit we've never fetched, so we set date to the last time remote's
reflog moved to bound our walk of local.

But remote's reflog can expire or be empty, so it needs an initial
value.

With date=0 (and remote gone: older than 90 days + gc, removed, or fresh
clone), we walk local until we find the remote->old_oid or we run out of
reflog to walk. But local is also subject to gc, so by default that's 90
days without having to bound anything. Since both reflogs are gc'd, the
time difference should be minimal.

With date=<cutoff> is only faster where we have no remote reflog, we
don't have remote->old_oid in our local, and our local reflog has
entries older than the typical gc cutoff; viz. I'm rebuilding history
without the remote tip and: (a) expired my remote reflog manually (b)
have my remote reflog gc configured differently than my local or (c) I
have gc turned off.

But in one case, date=0 gives the right answer and the cutoff date gives
the wrong answer:

     git clone ...            # 1. files backend, no remote reflog
     git reset --hard HEAD^   # 2. start a rewrite
     ...                      # 3. do nothing for gc.reflogExpire amount
     ...                      #    of time.
     ...                      #    Remote never moves/we never fetch.
     git commit ...           # 4. Finish rewrite and push
     git push --force-if-includes --force-with-lease origin main

Push fails with date=gc.reflogExpire (wrong). Push succeeds with date=0
(right).

And nothing about gc config need be tweaked from the defaults for this
to happen---git gc can even happen (provided it runs between the initial
clone and the reset, since the reflogUnreachable prune is 30 days by
default). But in small repos, gc may not have been triggered at all.

So date=0 is always correct and should have equivalent in runtime in
most cases. And it neatly side-steps what cut off should we use?
gc.reflogExpire vs.  gc.<remote>.reflogExpire vs.
gc.<local>.reflogExpire vs. flat 90 days vs. do we respect
gc.reflogExpire=never.

Thanks.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2] push: fix --force-if-includes when remote-tracking ref has no reflog
  2026-09-25 20:53               ` Tyler Cipriani
@ 2026-09-25 21:58                 ` Junio C Hamano
  0 siblings, 0 replies; 25+ messages in thread
From: Junio C Hamano @ 2026-09-25 21:58 UTC (permalink / raw)
  To: Tyler Cipriani; +Cc: Aleksei Sviridkin, git

Tyler Cipriani <tyler@tylercipriani.com> writes:

> So date=0 is always correct and should have equivalent in runtime in
> most cases. And it neatly side-steps what cut off should we use?
> gc.reflogExpire vs.  gc.<remote>.reflogExpire vs.
> gc.<local>.reflogExpire vs. flat 90 days vs. do we respect
> gc.reflogExpire=never.

With a reflog that has never been expired, using all the available
information will always work with better information than with
cutoff, so that is not surprising at all ;-).


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3] push: fix --force-if-includes when remote-tracking ref has no reflog
  2026-09-05 17:13 ` [PATCH v3] " Aleksei Sviridkin
@ 2026-09-29  1:10   ` Tyler Cipriani
  2026-09-29  9:13   ` [PATCH v4] " Aleksei Sviridkin
  1 sibling, 0 replies; 25+ messages in thread
From: Tyler Cipriani @ 2026-09-29  1:10 UTC (permalink / raw)
  To: Aleksei Sviridkin; +Cc: git, Junio C Hamano

On 26-09-05 20:13:30, Aleksei Sviridkin wrote:

Code looks right to me with the date=0 fallback.

push.useForceIfIncludes is meant to tighten --force-with-lease's checks.
Getting a wrong answer with advice telling me to pull would push me (pun
intended) to drop the config and ditch the feature. For
--force-if-includes, being wrong here is worse than being slow here.

Re: being slow. I tried to recreate some numbers from this thread with
my own test case using linux.git and 2k reflog entries. In the cases I
tried:

- With a commit-graph (which gc should write), walking + merge-base
   checks on 2k entries took 15ms. So a few microseconds per reflog entry
   roughly jibes with numbers from this thread.
- Without a commit-graph, each batched call to
   repo_in_merge_bases_many() walks the commit history from scratch:
   rejection took two minutes for 2k entries.

But my tests were artificial worst-case scenarios. And today, on my
build, "date" already happens to be a low number. For folks like me,
setting date=0 is a non-change and I've been unable to find any
complaints of slowness on the mailing list (or by searching the web).

For folks where date happens to be a high number: this gets the feature
working correctly. Bonus: doubling batch size after each call to
repo_in_merge_bases_many took my 2min down to 10s, locally; a viable
speed up if needed (but separate from this change).

>Since 99a1f9ae10 (push: add reflog check for "--force-if-includes",
>2020-10-03), is_reachable_in_reflog() stops walking the reflog of the
>local branch at entries older than the newest reflog entry of the
>remote-tracking ref. That timestamp is read by a callback of
>refs_for_each_reflog_ent_reverse(), so when the remote-tracking ref
>has no reflog, the variable that holds the timestamp stays
>uninitialized.
>
>With the files backend a remote-tracking ref created by "git clone"
>has no reflog and does not get one until it moves. On my machine the
>leftover value exceeds any real timestamp: the walk stops at the very
>first entry, never reaches the "Created from" entry that "checkout
>--track" wrote, and the push is rejected with "remote ref updated
>since checkout" although nothing on the remote has changed.
>
>The cut-off is an optimization that rests on an assumption: an entry
>older than the moment the remote-tracking ref last moved is not
>expected to be the one being looked for. Without a reflog there is
>no such moment, hence no cut-off to apply. Initialize the timestamp
>to zero to say exactly that: timestamp_t is unsigned, so no entry
>compares older than zero and the comparison never fires. Using
>"now", or any fixed age, would instead cut the walk off at the first
>entry older than that bound, which is how the failure happens in
>the first place. The price is paid only when no matching entry is
>found: the walk then reaches the oldest entry and falls back to the
>merge-base check over what it collected, where the cut-off would
>have stopped it earlier.

The last paragraph of this log message is hard to read for me; I think
people could come away from reading it with the wrong information.

Nits:

- The final paragraph of the log message starts with "The cut-off", but
   it's the first time you've used "cut-off." What cut-off?
- "an entry older than [...] is not expected to be the one being looked
   for" - passive voice, stacked verb phrases ("is not expected/to be"),
   and a subject separated from its verb by 9 words made this hard to
   follow. And it leaves questions: Why is <who or what> not looking at
   <what> entry?
- "Without a reflog" - which reflog? remote-tracking or local?
- Unclear referents:
     - "exactly that"
     - "that bound"

Problems (with more nits :)):

- "there is no such moment"
   - Readability: referring back to "moment" that came 23 words before
     this "moment" made me re-read this a few times.
   - Inaccuracy: there may have been a moment when the remote-tracking
     ref last moved, but there is no reliable record of it because there
     is no remote-tracking reflog. That is, someone may have removed the
     reflog, or the reflog could have been GC'd (neither case is
     mentioned in your message).
- Most importantly, since the way I parse it is technically incorrect:
   "the walk then reaches the oldest entry and falls back to the
   merge-base check...where the cut-off would have stopped it earlier." -
   Stopped what earlier? I read this sentence split on "where" (i.e., Y
   does this, whereas X does that).

   Read that way, the final sentence reads as:

   Walk without a cut-off:

   (a) "reaches the oldest entry"
   (b) "falls back to the merge-base check"

   vs.

   Walk with a cut-off: stops earlier and therefore does neither.

   But a walk with a cut-off falls back to a merge-base check, too. The
   difference is that without a cut-off you reach the oldest entry and
   therefore pass more local reflog entries to the merge-base check;
   i.e., potentially more calls to repo_in_merge_bases_many()

>
>Signed-off-by: Aleksei Sviridkin <f@lex.la>
>---
>Changes since v2:
>  - reworded the first paragraph as you suggested
>  - explain why zero is the fallback rather than "now" or a fixed age
>  - dropped the Assisted-by trailer
>
> remote.c            |  2 +-
> t/t5533-push-cas.sh | 18 ++++++++++++++++++
> 2 files changed, 19 insertions(+), 1 deletion(-)
>
>diff --git a/remote.c b/remote.c
>index 00723b385e..6d301698ca 100644
>--- a/remote.c
>+++ b/remote.c
>@@ -2751,7 +2751,7 @@ static int check_and_collect_until(const char *refname UNUSED,
>  */
> static int is_reachable_in_reflog(const char *local, const struct ref *remote)
> {
>-	timestamp_t date;
>+	timestamp_t date = 0;
> 	struct commit *commit;
> 	struct commit **chunk;
> 	struct check_and_collect_until_cb_data cb;
>diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh
>index cba26a872d..bb8878c593 100755
>--- a/t/t5533-push-cas.sh
>+++ b/t/t5533-push-cas.sh
>@@ -396,4 +396,22 @@ test_expect_success '"--force-if-includes" should allow deletes' '
> 	)
> '
>
>+test_expect_success '"--force-if-includes" should allow forced update when remote-tracking ref has no reflog' '
>+	rm -fr dst src &&
>+	test_when_finished "rm -fr dst src" &&
>+	git init --bare dst &&
>+	git push dst main main:branch &&
>+	git clone --no-local dst src &&
>+	(
>+		cd src &&
>+		# a clone leaves the remote-tracking refs without reflog
>+		# entries with the files backend, but not with reftable
>+		git reflog expire --all --expire=all &&
>+		git switch -c branch --track origin/branch &&
>+		git reset --hard HEAD^ &&
>+		test_commit D &&
>+		git push --force-if-includes --force-with-lease="branch"
>+	)
>+'
>+
> test_done

Tested: passes with the fix.

Without the fix it also passes on my machine. gdb says that the value of
date is 2 for me (Linux x86_64, gcc (Debian 14.2.0-19) 14.2.0, on
Trixie). To get the test to fail reliably, had to build with:

     make CFLAGS_APPEND=-ftrivial-auto-var-init=pattern

So, CI probably would miss date becoming uninitialized again. It also
fails with a date set to a timestamp 90 days ago due, since test dates
are 2005.

Minor nit: surrounding tests in t/t5533-push-cas.sh use
setup_src_dup_dst, which would simplify the test setup.

I'd be happy to give a Reviewed-by once the log message is clearer.

Thanks.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v4] push: fix --force-if-includes when remote-tracking ref has no reflog
  2026-09-05 17:13 ` [PATCH v3] " Aleksei Sviridkin
  2026-09-29  1:10   ` Tyler Cipriani
@ 2026-09-29  9:13   ` Aleksei Sviridkin
  1 sibling, 0 replies; 25+ messages in thread
From: Aleksei Sviridkin @ 2026-09-29  9:13 UTC (permalink / raw)
  To: git; +Cc: Aleksei Sviridkin, Junio C Hamano, Tyler Cipriani

Since 99a1f9ae10 (push: add reflog check for "--force-if-includes",
2020-10-03), is_reachable_in_reflog() looks for the remote tip in the
local branch's reflog and stops at entries older than the newest entry
of the remote-tracking ref's reflog. That timestamp comes from a
callback of refs_for_each_reflog_ent_reverse(), which never runs when
the remote-tracking ref has no reflog, so the variable stays
uninitialized.

With the files backend a remote-tracking ref that "git clone" created
has no reflog until it moves. On my machine the leftover value exceeded
any real timestamp, so the walk stopped at the first entry and the push
was rejected with "remote ref updated since checkout" though nothing on
the remote had changed.

That stopping point assumes an entry older than the last recorded move
of the remote-tracking ref cannot be the one we want. The record itself
can be missing: never written, deleted, or expired by gc. Initialize
the timestamp to zero for a missing record. timestamp_t is unsigned, so
nothing compares older and the walk stops only at the remote tip or at
the end of the local reflog. "Now" brings the bug straight back. A
fixed age narrows it: the push is rejected when the remote tip is
recorded only past the first entry older than that age and nothing
collected reaches it.

When the remote tip is not in the local reflog at all, a stopped walk
and a full one fall back to the same merge-base check, and the full one
hands it more entries.

Signed-off-by: Aleksei Sviridkin <f@lex.la>
---
Changes since v3:

- log message rewritten. Two things in it were wrong, not just
  unclear: it read as if a walk that stops at the cut-off skips the
  merge-base check, and it said there is "no such moment" when what is
  missing is the record of it.
- test uses setup_src_dup_dst and expires only the remote-tracking
  reflog.

t5533 passes 24/24 with the fix on files and on reftable, and the new
test fails on both without it. That failure is only reliable when built
with

	make CFLAGS_APPEND=-ftrivial-auto-var-init=pattern

otherwise the stack may hold a small number, as on your machine. So CI
would not catch this going uninitialized again.

One detail the expire hides: on files it leaves no reflog at all, on
reftable an empty one. The callback does not run either way.

The batch size growth looks worth its own patch. Not touched here.

 remote.c            |  2 +-
 t/t5533-push-cas.sh | 16 ++++++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/remote.c b/remote.c
index 00723b385e..6d301698ca 100644
--- a/remote.c
+++ b/remote.c
@@ -2751,7 +2751,7 @@ static int check_and_collect_until(const char *refname UNUSED,
  */
 static int is_reachable_in_reflog(const char *local, const struct ref *remote)
 {
-	timestamp_t date;
+	timestamp_t date = 0;
 	struct commit *commit;
 	struct commit **chunk;
 	struct check_and_collect_until_cb_data cb;
diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh
index cba26a872d..c9aaeec8d1 100755
--- a/t/t5533-push-cas.sh
+++ b/t/t5533-push-cas.sh
@@ -396,4 +396,20 @@ test_expect_success '"--force-if-includes" should allow deletes' '
 	)
 '
 
+test_expect_success '"--force-if-includes" should allow forced update when remote-tracking ref has no reflog' '
+	setup_src_dup_dst &&
+	test_when_finished "rm -fr dst src dup" &&
+	(
+		cd src &&
+		git switch branch &&
+		git pull --rebase origin branch &&
+		# the bug needs a remote-tracking ref with no reflog, and
+		# the fetch above wrote one
+		git reflog expire --expire=all refs/remotes/origin/branch &&
+		git reset --hard HEAD^ &&
+		test_commit I &&
+		git push --force-if-includes --force-with-lease="branch"
+	)
+'
+
 test_done
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2026-09-29  9:13 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-03  1:05 [PATCH] push: fix --force-if-includes when remote-tracking ref has no reflog Aleksei Sviridkin
2026-09-03 16:16 ` Junio C Hamano
2026-09-03 20:00   ` Aleksei Sviridkin
2026-09-03 20:11     ` Junio C Hamano
2026-09-03 21:45       ` Aleksei Sviridkin
2026-09-04  1:03     ` Kristoffer Haugsbakk
2026-09-04 16:48     ` Junio C Hamano
2026-09-05 17:13       ` Aleksei Sviridkin
2026-09-06  9:39         ` Kristoffer Haugsbakk
2026-09-06 17:14           ` Junio C Hamano
2026-09-07  4:54             ` Thomas Bachem
2026-09-07  6:23               ` Weijie Yuan
2026-09-04 12:44 ` [PATCH v2] " Aleksei Sviridkin
2026-09-04 15:42   ` Junio C Hamano
2026-09-06  0:45     ` Junio C Hamano
2026-09-06 16:50       ` Aleksei Sviridkin
2026-09-08  3:47         ` Junio C Hamano
2026-09-09  6:56           ` Aleksei Sviridkin
2026-09-10  0:57             ` Junio C Hamano
2026-09-10  8:31               ` Aleksei Sviridkin
2026-09-25 20:53               ` Tyler Cipriani
2026-09-25 21:58                 ` Junio C Hamano
2026-09-05 17:13 ` [PATCH v3] " Aleksei Sviridkin
2026-09-29  1:10   ` Tyler Cipriani
2026-09-29  9:13   ` [PATCH v4] " Aleksei Sviridkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).