From 4b67acb3c24526ebb237e57f34d4729a3af37fc5 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Fri, 22 Aug 2025 10:39:03 +0200 Subject: [PATCH 1/5] refs/files: fix issues with git-fetch on case-insensitive FS 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 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. --- b4-submit-tracking --- { "series": { "revision": 3, "change-id": "20250822-587-git-fetch-1-fails-fetches-on-case-insensitive-repositories-0a187c7ac41e", "prefixes": [], "history": { "v1": [ "20250902-587-git-fetch-1-fails-fetches-on-case-insensitive-repositories-v1-0-35e69bbb507d@gmail.com" ], "v2": [ "20250908-587-git-fetch-1-fails-fetches-on-case-insensitive-repositories-v2-0-b2eb2459befb@gmail.com" ] } } } -- GitLab From d452e9ea34e30d6835520dfcf85bdb8ed7ff94f9 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Fri, 22 Aug 2025 11:00:41 +0200 Subject: [PATCH 2/5] refs/files: catch conflicts on case-insensitive file-systems During the 'prepare' phase of 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: refs/heads/Foo refs/heads/foo This affects all commands which were migrated to use batched updates in Git 2.51, including 'git-fetch(1)' and 'git-receive-pack(1)'. Before that, reference updates would be applied serially with one transaction used per update. When users fetched multiple references on case-insensitive systems, subsequent references would simply overwrite any earlier references. So when fetching: refs/heads/foo: 5f34ec0bfeac225b1c854340257a65b106f70ea6 refs/heads/Foo: ec3053b0977e83d9b67fc32c4527a117953994f3 refs/heads/sample: 2eefd1150e06d8fca1ddfa684dec016f36bf4e56 The user would simply end up with: refs/heads/foo: ec3053b0977e83d9b67fc32c4527a117953994f3 refs/heads/sample: 2eefd1150e06d8fca1ddfa684dec016f36bf4e56 This is buggy behavior since the user is never informed about the overrides performed and missing references. Nevertheless, the user is left with a working repository with a subset of the references. Since Git 2.51, in such situations fetches would simply fail without updating any references. Which is also buggy behavior and worse off since the user is left without any references. The error is triggered in `lock_raw_ref()` where the files backend attempts to create a lock file. When a lock file already exists the function returns a 'REF_TRANSACTION_ERROR_GENERIC'. When this happens, the entire batched updates, not individual operation, is aborted as if it were in a transaction. Change this to return 'REF_TRANSACTION_ERROR_CASE_CONFLICT' instead to aid the batched update mechanism to simply reject such errors. The change only affects batched updates since batched updates will reject individual updates with non-generic errors. So specifically this would only affect: 1. git fetch 2. git receive-pack 3. git update-ref --batch-updates This bubbles the error type up to `files_transaction_prepare()` which tries to lock each reference update. So if the locking fails, we check if the rejection type can be ignored, which is done by calling `ref_transaction_maybe_set_rejected()`. As the error type is now 'REF_TRANSACTION_ERROR_CASE_CONFLICT', the specific reference update would simply be rejected, while other updates in the transaction would continue to be applied. This allows partial application of references in case-insensitive filesystems when fetching colliding references. While the earlier implementation allowed the last reference to be applied overriding the initial references, this change would allow the first reference to be applied while rejecting consequent collisions. 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 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. Reported-by: Joe Drew Helped-by: Patrick Steinhardt Signed-off-by: Karthik Nayak --- builtin/fetch.c | 21 ++++++++++++++--- refs.c | 2 ++ refs.h | 2 ++ refs/files-backend.c | 33 +++++++++++++++++++++++---- t/t1400-update-ref.sh | 53 +++++++++++++++++++++++++++++++++++++++++++ t/t5510-fetch.sh | 22 +++++++++++++++++- 6 files changed, 124 insertions(+), 9 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 24645c46533..c7ff3480fb1 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1643,7 +1643,8 @@ static int set_head(const struct ref *remote_refs, struct remote *remote) struct ref_rejection_data { int *retcode; - int conflict_msg_shown; + bool conflict_msg_shown; + bool case_sensitive_msg_shown; const char *remote_name; }; @@ -1657,11 +1658,25 @@ static void ref_transaction_rejection_handler(const char *refname, { struct ref_rejection_data *data = cb_data; - if (err == REF_TRANSACTION_ERROR_NAME_CONFLICT && !data->conflict_msg_shown) { + if (err == REF_TRANSACTION_ERROR_CASE_CONFLICT && ignore_case && + !data->case_sensitive_msg_shown) { + error(_("You're on a case-insensitive filesystem, and the remote you are\n" + "trying to fetch from has references that only differ in casing. It\n" + "is impossible to store such references with the 'files' backend. You\n" + "can either accept this as-is, in which case you won't be able to\n" + "store all remote references on disk. Or you can alternatively\n" + "migrate your repository to use the 'reftable' backend with the\n" + "following command:\n\n git refs migrate --ref-format=reftable\n\n" + "Please keep in mind that not all implementations of Git support this\n" + "new format yet. So if you use tools other than Git to access this\n" + "repository it may not be an option to migrate to reftables.\n")); + data->case_sensitive_msg_shown = true; + } else if (err == REF_TRANSACTION_ERROR_NAME_CONFLICT && + !data->conflict_msg_shown) { error(_("some local refs could not be updated; try running\n" " 'git remote prune %s' to remove any old, conflicting " "branches"), data->remote_name); - data->conflict_msg_shown = 1; + data->conflict_msg_shown = true; } else { const char *reason = ref_transaction_error_msg(err); diff --git a/refs.c b/refs.c index bfdbe718b70..4c1c339ed9b 100644 --- a/refs.c +++ b/refs.c @@ -3321,6 +3321,8 @@ const char *ref_transaction_error_msg(enum ref_transaction_error err) return "invalid new value provided"; case REF_TRANSACTION_ERROR_EXPECTED_SYMREF: return "expected symref but found regular ref"; + case REF_TRANSACTION_ERROR_CASE_CONFLICT: + return "reference conflict due to case-insensitive filesystem"; default: return "unknown failure"; } diff --git a/refs.h b/refs.h index eedbb599c52..41915086b36 100644 --- a/refs.h +++ b/refs.h @@ -31,6 +31,8 @@ enum ref_transaction_error { REF_TRANSACTION_ERROR_INVALID_NEW_VALUE = -6, /* Expected ref to be symref, but is a regular ref */ REF_TRANSACTION_ERROR_EXPECTED_SYMREF = -7, + /* Cannot create ref due to case-insensitive filesystem */ + REF_TRANSACTION_ERROR_CASE_CONFLICT = -8, }; /* diff --git a/refs/files-backend.c b/refs/files-backend.c index 088b52c740b..01df32904be 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -647,6 +647,26 @@ static void unlock_ref(struct ref_lock *lock) } } +/* + * Check if the transaction has another update with a case-insensitive refname + * match. + * + * If the update is part of the transaction, we only check up to that index. + * Further updates are expected to call this function to match previous indices. + */ +static bool transaction_has_case_conflicting_update(struct ref_transaction *transaction, + struct ref_update *update) +{ + for (size_t i = 0; i < transaction->nr; i++) { + if (transaction->updates[i] == update) + break; + + if (!strcasecmp(transaction->updates[i]->refname, update->refname)) + return true; + } + return false; +} + /* * 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); + if (myerr == EEXIST && ignore_case && + transaction_has_case_conflicting_update(transaction, update)) + ret = REF_TRANSACTION_ERROR_CASE_CONFLICT; goto error_return; } } @@ -2583,9 +2607,8 @@ static enum ref_transaction_error lock_ref_for_update(struct files_ref_store *re if (lock) { lock->count++; } else { - ret = lock_raw_ref(refs, update, update_idx, mustexist, - refnames_to_check, &transaction->refnames, - &lock, &referent, err); + ret = lock_raw_ref(refs, transaction, update_idx, mustexist, + refnames_to_check, &lock, &referent, err); if (ret) { char *reason; diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh index 96648a6e5df..08d5df2af72 100755 --- a/t/t1400-update-ref.sh +++ b/t/t1400-update-ref.sh @@ -2294,6 +2294,59 @@ do ) ' + test_expect_success CASE_INSENSITIVE_FS,REFFILES "stdin $type batch-updates existing reference" ' + git init repo && + test_when_finished "rm -fr repo" && + ( + cd repo && + test_commit one && + old_head=$(git rev-parse HEAD) && + test_commit two && + head=$(git rev-parse HEAD) && + + { + format_command $type "create refs/heads/foo" "$head" && + format_command $type "create refs/heads/ref" "$old_head" && + format_command $type "create refs/heads/Foo" "$old_head" + } >stdin && + git update-ref $type --stdin --batch-updates stdout && + + echo $head >expect && + git rev-parse refs/heads/foo >actual && + echo $old_head >expect && + git rev-parse refs/heads/ref >actual && + test_cmp expect actual && + test_grep -q "reference conflict due to case-insensitive filesystem" stdout + ) + ' + + test_expect_success CASE_INSENSITIVE_FS "stdin $type batch-updates existing reference" ' + git init --ref-format=reftable repo && + test_when_finished "rm -fr repo" && + ( + cd repo && + test_commit one && + old_head=$(git rev-parse HEAD) && + test_commit two && + head=$(git rev-parse HEAD) && + + { + format_command $type "create refs/heads/foo" "$head" && + format_command $type "create refs/heads/ref" "$old_head" && + format_command $type "create refs/heads/Foo" "$old_head" + } >stdin && + git update-ref $type --stdin --batch-updates stdout && + + echo $head >expect && + git rev-parse refs/heads/foo >actual && + echo $old_head >expect && + git rev-parse refs/heads/ref >actual && + test_cmp expect actual && + git rev-parse refs/heads/Foo >actual && + test_cmp expect actual + ) + ' + test_expect_success "stdin $type batch-updates delete incorrect symbolic ref" ' git init repo && test_when_finished "rm -fr repo" && diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index ebc696546bc..57f60da81bd 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -47,7 +47,13 @@ test_expect_success "clone and setup child repos" ' git config set branch.main.merge refs/heads/one ) && git clone . bundle && - git clone . seven + git clone . seven && + git clone --ref-format=reftable . case_sensitive && + ( + cd case_sensitive && + git branch branch1 && + git branch bRanch1 + ) ' test_expect_success "fetch test" ' @@ -1526,6 +1532,20 @@ test_expect_success SYMLINKS 'clone does not get confused by a D/F conflict' ' test_path_is_missing whoops ' +test_expect_success CASE_INSENSITIVE_FS,REFFILES 'existing references in a case insensitive filesystem' ' + test_when_finished rm -rf case_insensitive && + ( + git init --bare case_insensitive && + cd case_insensitive && + git remote add origin -- ../case_sensitive && + test_must_fail git fetch -f origin "refs/heads/*:refs/heads/*" 2>err && + test_grep "You${SQ}re on a case-insensitive filesystem" err && + git rev-parse refs/heads/main >expect && + git rev-parse refs/heads/branch1 >actual && + test_cmp expect actual + ) +' + . "$TEST_DIRECTORY"/lib-httpd.sh start_httpd -- GitLab From 4c59492c5fb566fcc3d6a8684cf9a44abb8d7cab Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Sat, 6 Sep 2025 22:06:59 +0200 Subject: [PATCH 3/5] refs/files: use correct error type when lock exists When fetching references into a repository, if a lock for a particular 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. - REF_TRANSACTION_ERROR_GENERIC: for all other errors. The latter causes the entire set of batched updates to fail, even in case sensitive filessystems. Instead, return a 'REF_TRANSACTION_ERROR_CREATE_EXISTS' error. This allows batched updates to reject the individual update which conflicts with the existing file, while updating the rest of the references. Signed-off-by: Karthik Nayak --- refs/files-backend.c | 20 +++++++++++++++++--- t/t5510-fetch.sh | 26 ++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 01df32904be..69e50a16dbb 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -797,9 +797,23 @@ 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); - if (myerr == EEXIST && ignore_case && - transaction_has_case_conflicting_update(transaction, update)) - ret = REF_TRANSACTION_ERROR_CASE_CONFLICT; + if (myerr == EEXIST) { + if (ignore_case && + 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 + /* + * 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; } } diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index 57f60da81bd..6f8db0ace4c 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -1546,6 +1546,32 @@ test_expect_success CASE_INSENSITIVE_FS,REFFILES 'existing references in a case ) ' +test_expect_success REFFILES 'existing reference lock in repo' ' + test_when_finished rm -rf base repo && + ( + git init --ref-format=reftable base && + cd base && + echo >file update && + git add . && + git commit -m "updated" && + git branch -M main && + + git update-ref refs/heads/foo @ && + git update-ref refs/heads/branch @ && + cd .. && + + git init --ref-format=files --bare repo && + cd repo && + git remote add origin ../base && + touch refs/heads/foo.lock && + test_must_fail git fetch -f origin "refs/heads/*:refs/heads/*" 2>err && + test_grep "error: fetching ref refs/heads/foo failed: reference already exists" err && + git rev-parse refs/heads/main >expect && + git rev-parse refs/heads/branch >actual && + test_cmp expect actual + ) +' + . "$TEST_DIRECTORY"/lib-httpd.sh start_httpd -- GitLab From 8d8904dd4a457ef6ca7fcf3628fdf621094e11ae Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Fri, 29 Aug 2025 16:34:28 +0200 Subject: [PATCH 4/5] refs/files: handle F/D conflicts in case-insensitive FS 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 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. 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 69e50a16dbb..817b56f4ce5 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)); + } else { + item = string_list_append(refnames_to_check, refname); + } + item->util = xmalloc(sizeof(update_idx)); memcpy(item->util, &update_idx, sizeof(update_idx)); } @@ -2831,7 +2846,7 @@ static int files_transaction_prepare(struct ref_store *ref_store, "ref_transaction_prepare"); size_t i; int ret = 0; - struct string_list refnames_to_check = STRING_LIST_INIT_NODUP; + struct string_list refnames_to_check = STRING_LIST_INIT_DUP; char *head_ref = NULL; int head_type; struct files_transaction_backend_data *backend_data; diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index 6f8db0ace4c..08dbea6503c 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 ) ' @@ -1572,6 +1578,20 @@ test_expect_success REFFILES 'existing reference lock in repo' ' ) ' +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 + ) +' + . "$TEST_DIRECTORY"/lib-httpd.sh start_httpd -- GitLab From 84e36c73e3a49335f8bdd1395a073710196e9a5b Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Mon, 8 Sep 2025 08:44:32 +0200 Subject: [PATCH 5/5] refs/files: handle D/F conflicts during locking The previous commit added the necessary validation and checks for F/D conflicts in the files backend when working on case insensitive systems. There is still a possibility for D/F conflicts. This is a different from the F/D since for F/D conflicts, there would not be a conflict during the lock creation phase: refs/heads/foo.lock refs/heads/foo/bar.lock However there would be a conflict when the locks are committed, since we cannot have 'refs/heads/foo/bar' and 'refs/heads/foo'. These kinds of conflicts are checked and resolved in `refs_verify_refnames_available()`, so the previous commit ensured that for case-insensitive filesystems, we would lowercase the inputs to that function. For D/F conflicts, there is a conflict during the lock creation phase itself: refs/heads/foo/bar.lock refs/heads/foo.lock As in `lock_raw_ref()` after creating the lock, we also check for D/F conflicts. This can occur in case-insensitive filesystems when trying to fetch case-conflicted references like: refs/heads/Foo/new refs/heads/foo D/F conflicts can also occur in case-sensitive filesystems, when the repository already contains a directory with a lock file 'refs/heads/foo/bar.lock' and trying to fetch 'refs/heads/foo'. This doesn't concern directories containing garbage files as those are handled on a higher level. To fix this, simply categorize the error as a name conflict. Also remove this reference from the list of valid refnames for availability checks. By categorizing the error and removing it from the list of valid references, batched updates now knows to reject such reference updates and apply the other reference updates. Fix a small typo in `ref_transaction_maybe_set_rejected()` while here. Helped-by: Junio C Hamano Signed-off-by: Karthik Nayak --- refs.c | 9 ++++++++- refs/files-backend.c | 11 ++++++----- t/t5510-fetch.sh | 46 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 6 deletions(-) diff --git a/refs.c b/refs.c index 4c1c339ed9b..e7109ea5fea 100644 --- a/refs.c +++ b/refs.c @@ -1223,7 +1223,7 @@ int ref_transaction_maybe_set_rejected(struct ref_transaction *transaction, return 0; if (!transaction->rejections) - BUG("transaction not inititalized with failure support"); + BUG("transaction not initialized with failure support"); /* * Don't accept generic errors, since these errors are not user @@ -1232,6 +1232,13 @@ int ref_transaction_maybe_set_rejected(struct ref_transaction *transaction, if (err == REF_TRANSACTION_ERROR_GENERIC) return 0; + /* + * Rejected refnames shouldn't be considered in the availability + * checks, so remove them from the list. + */ + string_list_remove(&transaction->refnames, + transaction->updates[update_idx]->refname, 0); + transaction->updates[update_idx]->rejection_err = err; ALLOC_GROW(transaction->rejections->update_indices, transaction->rejections->nr + 1, diff --git a/refs/files-backend.c b/refs/files-backend.c index 817b56f4ce5..fec7713ea65 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -869,6 +869,7 @@ static enum ref_transaction_error lock_raw_ref(struct files_ref_store *refs, goto error_return; } else if (remove_dir_recursively(&ref_file, REMOVE_DIR_EMPTY_ONLY)) { + ret = REF_TRANSACTION_ERROR_NAME_CONFLICT; if (refs_verify_refname_available( &refs->base, refname, extras, NULL, 0, err)) { @@ -876,14 +877,14 @@ static enum ref_transaction_error lock_raw_ref(struct files_ref_store *refs, * The error message set by * verify_refname_available() is OK. */ - ret = REF_TRANSACTION_ERROR_NAME_CONFLICT; goto error_return; } else { /* - * We can't delete the directory, - * but we also don't know of any - * references that it should - * contain. + * Directory conflicts can occur if there + * is an existing lock file in the directory + * or if the filesystem is case-insensitive + * and the directory contains a valid reference + * but conflicts with the update. */ strbuf_addf(err, "there is a non-empty directory '%s' " "blocking reference '%s'", diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index 08dbea6503c..6b2739db264 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -59,6 +59,12 @@ test_expect_success "clone and setup child repos" ' cd case_sensitive_fd && git branch foo/bar && git branch Foo + ) && + git clone --ref-format=reftable . case_sensitive_df && + ( + cd case_sensitive_df && + git branch Foo/bar && + git branch foo ) ' @@ -1592,6 +1598,46 @@ test_expect_success CASE_INSENSITIVE_FS,REFFILES 'F/D conflict on case insensiti ) ' +test_expect_success CASE_INSENSITIVE_FS,REFFILES 'D/F 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_df && + 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 + ) +' + +test_expect_success REFFILES 'D/F conflict on case sensitive filesystem with lock' ' + ( + git init --ref-format=reftable base && + cd base && + echo >file update && + git add . && + git commit -m "updated" && + git branch -M main && + + git update-ref refs/heads/foo @ && + git update-ref refs/heads/branch @ && + cd .. && + + git init --ref-format=files --bare repo && + cd repo && + git remote add origin ../base && + mkdir refs/heads/foo && + touch refs/heads/foo/random.lock && + test_must_fail git fetch origin "refs/heads/*:refs/heads/*" 2>err && + test_grep "some local refs could not be updated; try running" err && + git rev-parse refs/heads/main >expect && + git rev-parse refs/heads/branch >actual && + test_cmp expect actual + ) +' + . "$TEST_DIRECTORY"/lib-httpd.sh start_httpd -- GitLab