On Fri, Nov 07, 2025 at 05:15:32AM -0800, Karthik Nayak wrote:

Patrick Steinhardt [off-list ref] writes:

quoted

On Thu, Nov 06, 2025 at 09:39:25AM +0100, Karthik Nayak wrote:
The following test demonstrates this behaviour:

	test_expect_success "fetch single branch without explicit tag option" '
		git init source &&
		git -C source commit --allow-empty --message common &&
		git clone file://"$(pwd)"/source target &&
		(
			cd source &&
			git commit --allow-empty --message discard-me &&
			git tag discard-me &&
			git commit --amend --allow-empty --message fetch-me &&
			git tag fetch-me
		) &&

		# The "discard-me" tag does not point into the history that we are
		# about to fetch, so it should not have been created.
		git -C target fetch origin &&
		git -C target tag -l >actual &&
		echo "fetch-me" >expect &&

		# But with "--tags" we instruct git-fetch(1) to fetch all tags, so we
		# should now see it.
		git -C target fetch origin --tags &&

Here, we don't really backfill, but rather we request all tags from the
remote, hence we end up with the 'discard-me' tag. Not because of the
diverged history. I also confirmed this by adding a breakpoint into the
`backfill_tags()` function, while running this test.

Oh, exactly. But there's two fetches here: the first one only fetches
"fetch-me" because we don't pass "--tags". The second one was simply as
a demonstration that we would also fetch the other tag that doesn't
point into our fetched history with "--tags".

I notice though that the first fetch forgot to `test_cmp`.

quoted

		git -C target tag -l >actual &&
		cat >expect <<-\EOF &&
		discard-me
		fetch-me
		EOF
		test_cmp expect actual
	'

But I was able to slightly modify the test to get the required affect:

  test_expect_success "backfill tags when providing a refspec" '
  	git init source &&
  	git -C source commit --allow-empty --message common &&
  	git clone file://"$(pwd)"/source target &&
  	(
  	    cd source &&
  	    git commit --allow-empty --message history &&
  	    git tag history &&
  	    git commit --allow-empty --message fetch-me &&
  	    git tag fetch-me
  	) &&

  	# The "history" tag is backfilled eventhough we requested
  	# to only fetch the master
  	git -C target fetch origin master:branch &&
  	git -C target tag -l >actual &&
  	cat >expect <<-\EOF &&
  	fetch-me
  	history
  	EOF
  	test_cmp expect actual
  '

I will add this in. Thanks for the explanation, it really helped
consolidate my understanding here.

Yup, that should work, as well.

quoted

quoted

diff --git a/builtin/fetch.c b/builtin/fetch.c
index c7ff3480fb..d5aee5af10 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c

@@ -1686,6 +1686,42 @@ static void ref_transaction_rejection_handler(const char *refname,

[snip]

quoted

quoted

+	if (*transaction && !is_atomic) {
+		struct ref_rejection_data data = {
+			.conflict_msg_shown = 0,
+			.remote_name = remote_name,
+			.retcode = &retcode,
+		};
+
+		ref_transaction_for_each_rejected_update(*transaction,
+							 ref_transaction_rejection_handler,
+							 &data);
+
+		ref_transaction_free(*transaction);
+		*transaction = NULL;
+	}

Okay. Do we need to discern cases where this is called and we haven't
managed to even queue a single reference update?

I don't see a reason. This is anyways a post-commit action, if there are
no updates, there will be no rejections. So this will be a no-op.

I guess the question was rather whether we fear a negative consequence
by trying to commit an empty transaction. The commit doesn't know to
short-circuit empty transactions, so we'd still end up locking data even
though we eventually end up doing nothing.

Thanks!

Patrick