diff --git a/builtin/fetch.c b/builtin/fetch.c index 24645c4653..c7ff3480fb 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1643,7 +1643,8 @@ cleanup: 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 f15366bfce..c164374c26 100644 --- a/refs.c +++ b/refs.c @@ -1237,7 +1237,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 @@ -1246,6 +1246,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, @@ -3330,6 +3337,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 f29e486e33..bc5d8427a5 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 1b3bf26add..bc3347d18c 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -653,6 +653,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, @@ -683,16 +703,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; @@ -782,6 +803,24 @@ retry: goto retry; } else { unable_to_lock_message(ref_file.buf, myerr, err); + 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; } } @@ -837,6 +876,7 @@ retry: 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)) { @@ -844,14 +884,14 @@ retry: * 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'", @@ -873,8 +913,23 @@ retry: * 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)); } @@ -2616,9 +2671,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; @@ -2858,7 +2912,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/t1400-update-ref.sh b/t/t1400-update-ref.sh index b7415ec9d5..db7f5444da 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 83d1aadf9f..b7059cccaa 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -45,7 +45,25 @@ 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 + ) && + git clone --ref-format=reftable . case_sensitive_fd && + ( + 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 + ) ' test_expect_success "fetch test" ' @@ -1465,6 +1483,100 @@ 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_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_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_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