Thread (43 messages) 43 messages, 5 authors, 2025-09-17

Re: [PATCH 2/2] refs/files: handle F/D conflicts in case-insensitive FS

From: Patrick Steinhardt <hidden>
Date: 2025-09-03 07:40:48

On Tue, Sep 02, 2025 at 10:34:26AM +0200, Karthik Nayak wrote:
Similar to the previous commit, when using the files-backend on
case-insensitive filesystems, there is possibility of hitting F/D
conflicts when creating references within a single transaction, such as:

  - 'refs/heads/foo'
  - 'refs/heads/Foo/bar'
Great, I wanted to ask about this scenario.
Ideally such conflicts are caught in `refs_verify_refnames_available()`
which is responsible for checking F/D conflicts within a given
transaction. This utility function is shared across the reference
backends. As such, it doesn't consider the issues of using a
case-insensitive, which only affects the files-backend.

While one solution would be to make the function aware of such issues.
This feels like leaking implementation details of file-backend specific
issues into the utility function. So opt for the more simpler option, of
lowercasing all references sent to this function when on a
case-insensitive filesystem and operating on the files-backend.

To do this, simply use a `struct strbuf` to convert the refname to a
lower case and append it to the list of refnames to be checked. Since we
use a `struct strbuf` and the memory is cleared right after, make sure
that the string list duplicates all provided string.

Without this change, the user would simply be left with a repository
with '.lock' files which were created in the 'prepare' phase of the
transaction, as the 'commit' phase would simply abort and not do the
necessary cleanup.
Oh, that's a clever hack. Does this also work for the case where we have
preexisting refs already that differ only in casing? I guess it should
given that the lookups we perform should yield those refs regardless of
their casing.

In any case, if we don't already have such a test it would be great to
also verify that this works as expected.
quoted hunk ↗ jump to hunk
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 9f58ea4858..466cdfe121 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -869,8 +869,23 @@ static enum ref_transaction_error lock_raw_ref(struct files_ref_store *refs,
 		 * If the ref did not exist and we are creating it, we have to
 		 * make sure there is no existing packed ref that conflicts
 		 * with refname. This check is deferred so that we can batch it.
+		 *
+		 * For case-insensitive filesystems, we should also check for F/D
+		 * conflicts between 'foo' and 'Foo/bar'. So let's lowercase
+		 * the refname.
 		 */
-		item = string_list_append(refnames_to_check, refname);
+		if (ignore_case) {
+			struct strbuf lower = STRBUF_INIT;
+
+			strbuf_addstr(&lower, refname);
+			strbuf_tolower(&lower);
+
+			item = string_list_append(refnames_to_check, lower.buf);
+			strbuf_release(&lower);
Can we use `string_list_append_nodup()` together with `strbuf_detach()`
here to avoid one memory allocation?
quoted hunk ↗ jump to hunk
+		} else {
+			item = string_list_append(refnames_to_check, refname);
+		}
+
 		item->util = xmalloc(sizeof(update_idx));
 		memcpy(item->util, &update_idx, sizeof(update_idx));
 	}
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 57f60da81b..84dc68e5f3 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -53,6 +53,12 @@ test_expect_success "clone and setup child repos" '
 		cd case_sensitive &&
 		git branch branch1 &&
 		git branch bRanch1
+	) &&
+	git clone --ref-format=reftable . case_sensitive_fd &&
+	(
+		cd case_sensitive_fd &&
+		git branch foo/bar &&
+		git branch Foo
 	)
 '
 
Nice idea to use the reftable format here.
quoted hunk ↗ jump to hunk
@@ -1546,6 +1552,20 @@ test_expect_success CASE_INSENSITIVE_FS,REFFILES 'existing references in a case
 	)
 '
 
+test_expect_success CASE_INSENSITIVE_FS,REFFILES 'F/D conflict on case insensitive filesystem' '
+	test_when_finished rm -rf case_insensitive &&
+	(
+		git init --bare case_insensitive &&
+		cd case_insensitive &&
+		git remote add origin -- ../case_sensitive_fd &&
+		test_must_fail git fetch -f origin "refs/heads/*:refs/heads/*" 2>err &&
+		test_grep "failed: refname conflict" err &&
+		git rev-parse refs/heads/main >expect &&
+		git rev-parse refs/heads/foo/bar >actual &&
+		test_cmp expect actual
+	)
+'
Okay, so we again only end up with one of these references, which is the
best we can do.

atrick
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help