From: Karthik Nayak Date: Wed, 17 Sep 2025 15:30:18 GMT Subject: Re: [PATCH v3 1/4] refs/files: catch conflicts on case-insensitive file-systems Message-ID: In-Reply-To: Karthik Nayak writes: >>> + >>> /* >>> * Lock refname, without following symrefs, and set *lock_p to point >>> * at a newly-allocated lock object. Fill in lock->old_oid, referent, >>> @@ -677,16 +697,17 @@ static void unlock_ref(struct ref_lock *lock) >>> * - Generate informative error messages in the case of failure >>> */ >>> static enum ref_transaction_error lock_raw_ref(struct files_ref_store *refs, >>> - struct ref_update *update, >>> + struct ref_transaction *transaction, >>> size_t update_idx, >>> int mustexist, >>> struct string_list *refnames_to_check, >>> - const struct string_list *extras, >>> struct ref_lock **lock_p, >>> struct strbuf *referent, >>> struct strbuf *err) >>> { >>> enum ref_transaction_error ret = REF_TRANSACTION_ERROR_GENERIC; >>> + struct ref_update *update = transaction->updates[update_idx]; >>> + const struct string_list *extras = &transaction->refnames; >>> const char *refname = update->refname; >>> unsigned int *type = &update->type; >>> struct ref_lock *lock; >>> @@ -776,6 +797,9 @@ static enum ref_transaction_error lock_raw_ref(struct files_ref_store *refs, >>> goto retry; >>> } else { >>> unable_to_lock_message(ref_file.buf, myerr, err); >> >> huh, so if if we have a lockfile error due to a case-insensitve >> filesystem, does this mean we print the error message from >> `unable_to_lock_message()` and the new message? >> >> If so, I wonder if we would be better off skipping the former since it >> could be a bit misleading. >> > > I would say both are necessary. The errors added here are more technical > and really talk about why we faced an issue. The error in > 'builtin/fetch.c' is more about guidance to how to overcome that issue. > > Also this error is client agnostic, so we'd add the error here for users > of both regular transactions and batched updates. The error in > 'builtin/fetch.c' is very specific to users of 'git-fetch(1)'. So I > think both hold value. > My response was a bit confusing here. To answer your question directly. No we do not print both messages. The error message is appended to the `strbuf err` as you see. This 'strbuf' is provided by the callee, specifically in 'git-fetch(1)', we only print this error message if the ref transaction failed. Since 'git-fetch(1)' uses batched updates, the transaction will only fail when we encounter a GENERIC error type. Otherwise, the transaction succeeds with/without rejected updates. After which 'git-fetch(1)' will print out specific messages for rejected updates. For example, with the test added in this patch: $ ~/code/git/build/bin-wrappers/git fetch -f origin "refs/heads/*:refs/heads/*" remote: Enumerating objects: 3, done. remote: Counting objects: 100% (3/3), done. remote: Total 3 (delta 0), reused 0 (delta 0), pack-reused 0 (from 0) Unpacking objects: 100% (3/3), 713 bytes | 713.00 KiB/s, done. From ../base * [new branch] bRanch1 -> bRanch1 * [new branch] branch1 -> branch1 * [new branch] master -> master * [new branch] bRanch1 -> origin/bRanch1 * [new branch] branch1 -> origin/branch1 * [new branch] master -> origin/master error: You're on a case-insensitive filesystem, and the remote you are trying to fetch from has references that only differ in casing. It is impossible to store such references with the 'files' backend. You can either accept this as-is, in which case you won't be able to store all remote references on disk. Or you can alternatively migrate your repository to use the 'reftable' backend with the following command: git refs migrate --ref-format=reftable Please keep in mind that not all implementations of Git support this new format yet. So if you use tools other than Git to access this repository it may not be an option to migrate to reftables. error: fetching ref refs/remotes/origin/branch1 failed: reference conflict due to case-insensitive filesystem Hope that clarifies it!