From: Karthik Nayak Date: Wed, 17 Sep 2025 15:25:10 GMT Subject: [PATCH v4 0/4] refs/files: fix issues with git-fetch on case-insensitive FS Message-ID: <20250917-587-git-fetch-1-fails-fetches-on-case-insensitive-repositories-v4-0-da3c74a08ed0@gmail.com> In-Reply-To: <20250902-587-git-fetch-1-fails-fetches-on-case-insensitive-repositories-v1-0-35e69bbb507d@gmail.com> Hello! With Git 2.51 we moved 'git-fetch(1)' and 'git-receive-pack(1)' to use batched updates while doing reference updates. This provided a nice perf boost since both commands will now use a single transaction for reference updates. This removes the overhead of using individual transaction per reference update and also avoids unnecessary auto-compaction between reference updates in the reftable backend. However, in the files-backend it does introduce a few bugs around conflicts. The reported bug was around case-insensitive filesystems [1], but we also fix some adjacent issues: 1. When fetching references such as: - refs/heads/foo - refs/heads/Foo Earlier we would simply overwrite the first reference with the second and continue. Since Git 2.51 we simply abort stating a conflict. This is resolved in the first commit by explicitly categorizing the error as non-GENERIC. This allows batched updates to reject the particular update, while updating the rest. 2. When fetching references and a lock for a particular reference already exits. We treat this is a GENERIC error, which fails the entire update. By categorizing this error as non-GENERIC, we can reject this specific update and update the other references. 3. When fetching references such as with F/D conflict: - refs/heads/foo - refs/heads/Foo/bar Earlier we would apply the first, while the second would fail due to conflict. Since Git 2.51, the lock files for both would be created, but the 'commit' phase would abruptly end leaving the lock files. The second commit fixes this by ensuring that on case-insensitive filesystems we lowercase the refnames for availability check to ensure F/D are caught and reported to the user. 4. When fetching references with D/F conflict: - refs/heads/Foo/bar - refs/heads/foo The creation of the second reference's lock in `lock_raw_ref()` catches the D/F conflict, but we mark this as a GENERIC error. By categorizing this as non-GENERIC, we can allow the updates to continue while rejecting this specific error. This also applies to D/F conflicts in case-sensitive systems where there exists a lock in the repo 'refs/heads/foo/bar.lock' causing a conflict while fetching 'refs/heads/foo'. - Karthik [1]: https://lore.kernel.org/all/YQXPR01MB3046197EF39296549EE6DD669A33A@YQXPR01MB3046.CANPRD01.PROD.OUTLOOK.COM/ Signed-off-by: Karthik Nayak --- Changes in v4: - Fixes to typos in commit messages. - Use curly braces for if..else clause with inlined comments. - Link to v3: https://lore.kernel.org/r/20250913-587-git-fetch-1-fails-fetches-on-case-insensitive-repositories-v3-0-195569740b57@gmail.com Changes in v3: - Rename duplicate_reference_case_cmp() to transaction_has_case_conflicting_update() and add comments. - Improve commit messages. - Add an additional test in the 4th commit to showcase D/F conflicts in case-sensistive file systems. - Link to v2: https://lore.kernel.org/r/20250908-587-git-fetch-1-fails-fetches-on-case-insensitive-repositories-v2-0-b2eb2459befb@gmail.com Changes in v2: - This version fixes two more issues: - Fetching while locks already exist in the repository - D/F conflicts while fetching - Add a specific error to the first case, so we can nicely show a relevant error. Also check explicitly that the issue is due to case-insensitive filesystems. - Cleanup the commit messages. - Use `string_list_append_nodup()` with `strbuf_detach`, reducing the number of allocations. --- builtin/fetch.c | 21 ++++++++-- refs.c | 11 ++++- refs.h | 2 + refs/files-backend.c | 78 ++++++++++++++++++++++++++++------ t/t1400-update-ref.sh | 53 +++++++++++++++++++++++ t/t5510-fetch.sh | 114 +++++++++++++++++++++++++++++++++++++++++++++++++- 6 files changed, 262 insertions(+), 17 deletions(-) Karthik Nayak (4): refs/files: catch conflicts on case-insensitive file-systems refs/files: use correct error type when lock exists refs/files: handle F/D conflicts in case-insensitive FS refs/files: handle D/F conflicts during locking Range-diff versus v3: 1: ec89ebee70 ! 1: 203a32f814 refs/files: catch conflicts on case-insensitive file-systems @@ Metadata ## Commit message ## refs/files: catch conflicts on case-insensitive file-systems - During the 'prepare' phase of reference transaction in the files + During the 'prepare' phase of a reference transaction in the files backend, we create the lock files for references to be created. When using batched updates on case-insensitive filesystems, the entire batched updates would be aborted if there are conflicting names such as: @@ Commit message This should be an okay compromise since with the files backend, there is no scenario possible where we would retain all colliding references. - Let's also be more pro-active and notify users on case-insensitive + Let's also be more proactive and notify users on case-insensitive filesystems about such problems by providing a brief about the issue while also recommending using the reftable backend, which doesn't have the same issue. 2: e231d44125 ! 2: f06c1a89b9 refs/files: use correct error type when lock exists @@ Commit message reference exists, then `lock_raw_ref()` throws: - REF_TRANSACTION_ERROR_CASE_CONFLICT: when there is a conflict - because transaction contains conflicting references while being on a - case-insensitive filesystem. + because the transaction contains conflicting references while being + on a case-insensitive filesystem. - REF_TRANSACTION_ERROR_GENERIC: for all other errors. @@ refs/files-backend.c: static enum ref_transaction_error lock_raw_ref(struct file - ret = REF_TRANSACTION_ERROR_CASE_CONFLICT; + if (myerr == EEXIST) { + if (ignore_case && -+ transaction_has_case_conflicting_update(transaction, update)) ++ transaction_has_case_conflicting_update(transaction, update)) { + /* + * In case-insensitive filesystems, ensure that conflicts within a + * given transaction are handled. Pre-existing refs on a + * case-insensitive system will be overridden without any issue. + */ + ret = REF_TRANSACTION_ERROR_CASE_CONFLICT; -+ else ++ } else { + /* + * Pre-existing case-conflicting reference locks should also be + * specially categorized to avoid failing all batched updates. + */ + ret = REF_TRANSACTION_ERROR_CREATE_EXISTS; ++ } + } + goto error_return; 3: 0100f5a48d ! 3: e1b3f8106c refs/files: handle F/D conflicts in case-insensitive FS @@ Commit message 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 + To do this, simply use a `struct strbuf` to convert the refname to + lowercase 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. 4: 64def2c0fb = 4: 5c4886a1fb refs/files: handle D/F conflicts during locking base-commit: c44beea485f0f2feaf460e2ac87fdd5608d63cf0 change-id: 20250822-587-git-fetch-1-fails-fetches-on-case-insensitive-repositories-0a187c7ac41e Thanks - Karthik