From: Karthik Nayak Date: Wed, 17 Sep 2025 07:42:37 GMT Subject: Re: [PATCH v3 3/4] refs/files: handle F/D conflicts in case-insensitive FS Message-ID: In-Reply-To: Justin Tobler writes: > On 25/09/13 10:54PM, Karthik Nayak wrote: >> 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' >> >> 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 file system, 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 > > s/a lower case/lowercase/ > Will change. Thanks. >> 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. > > So IIUC, this also isn't related to the batched updates change and is > just an existing issue caused by case-insensitive filesystems and F/D > conflicts. With this change, we now properly detect F/D conflicts in > these situations and thus are able to cleanup lockfiles that would > previously be left behind. > Yup. That's correct! >> Reported-by: Junio C Hamano >> Signed-off-by: Karthik Nayak >> --- >> refs/files-backend.c | 19 +++++++++++++++++-- >> t/t5510-fetch.sh | 20 ++++++++++++++++++++ >> 2 files changed, 37 insertions(+), 2 deletions(-) >> >> diff --git a/refs/files-backend.c b/refs/files-backend.c >> index 69e50a16db..817b56f4ce 100644 >> --- a/refs/files-backend.c >> +++ b/refs/files-backend.c >> @@ -905,8 +905,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_nodup(refnames_to_check, >> + strbuf_detach(&lower, NULL)); > > For case-insensitive file-systems, we instead append a lowercased > version of the reference name which gets used to check for F/D > conflicts. Makes sense. > >> + } else { >> + item = string_list_append(refnames_to_check, refname); >> + } >> + > > -Justin