git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v6 7/8] refs: support rejection in batch updates during F/D checks

From
Karthik Nayak <karthik.188@gmail.com>
Date
Apr 8, 2025, 08:51 UTC
Message-ID
<20250408085120.614893-8-karthik.188@gmail.com>
In-Reply-To
<20250408085120.614893-1-karthik.188@gmail.com>

The `refs_verify_refnames_available()` is used to batch check refnames for F/D conflicts. While this is the more performant alternative than its individual version, it does not provide rejection capabilities on a single update level. For batched updates, this would mean a rejection of the entire transaction whenever one reference has a F/D conflict.

Modify the function to call `ref_transaction_maybe_set_rejected()` to check if a single update can be rejected. Since this function is only internally used within 'refs/' and we want to pass in a `struct ref_transaction *` as a variable. We also move and mark `refs_verify_refnames_available()` to 'refs-internal.h' to be an internal function.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 refs.c                  | 37 ++++++++++++++++++++++++++++++++++---
 refs.h                  | 12 ------------
 refs/files-backend.c    | 27 ++++++++++++++++++---------
 refs/refs-internal.h    | 16 ++++++++++++++++
 refs/reftable-backend.c | 11 ++++++++---
 5 files changed, 76 insertions(+), 27 deletions(-)
diff --git a/refs.c b/refs.c
index b34ec198f5..41d6247e70 100644
--- a/refs.c
+++ b/refs.c
@@ -2540,6 +2540,7 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs
 					  const struct string_list *refnames,
 					  const struct string_list *extras,
 					  const struct string_list *skip,
+					  struct ref_transaction *transaction,
 					  unsigned int initial_transaction,
 					  struct strbuf *err)
 {
@@ -2547,6 +2548,7 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs
 	struct strbuf referent = STRBUF_INIT;
 	struct string_list_item *item;
 	struct ref_iterator *iter = NULL;
+	struct strset conflicting_dirnames;
 	struct strset dirnames;
 	int ret = REF_TRANSACTION_ERROR_NAME_CONFLICT;
 
@@ -2557,9 +2559,11 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs
 
 	assert(err);
 
+	strset_init(&conflicting_dirnames);
 	strset_init(&dirnames);
 
 	for_each_string_list_item(item, refnames) {
+		const size_t *update_idx = (size_t *)item->util;
 		const char *refname = item->string;
 		const char *extra_refname;
 		struct object_id oid;
@@ -2597,14 +2601,30 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs
 				continue;
 
 			if (!initial_transaction &&
-			    !refs_read_raw_ref(refs, dirname.buf, &oid, &referent,
-					       &type, &ignore_errno)) {
+			    (strset_contains(&conflicting_dirnames, dirname.buf) ||
+			     !refs_read_raw_ref(refs, dirname.buf, &oid, &referent,
+						       &type, &ignore_errno))) {
+				if (transaction && ref_transaction_maybe_set_rejected(
+					    transaction, *update_idx,
+					    REF_TRANSACTION_ERROR_NAME_CONFLICT)) {
+					strset_remove(&dirnames, dirname.buf);
+					strset_add(&conflicting_dirnames, dirname.buf);
+					continue;
+				}
+
 				strbuf_addf(err, _("'%s' exists; cannot create '%s'"),
 					    dirname.buf, refname);
 				goto cleanup;
 			}
 
 			if (extras && string_list_has_string(extras, dirname.buf)) {
+				if (transaction && ref_transaction_maybe_set_rejected(
+					    transaction, *update_idx,
+					    REF_TRANSACTION_ERROR_NAME_CONFLICT)) {
+					strset_remove(&dirnames, dirname.buf);
+					continue;
+				}
+
 				strbuf_addf(err, _("cannot process '%s' and '%s' at the same time"),
 					    refname, dirname.buf);
 				goto cleanup;
@@ -2637,6 +2657,11 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs
 				    string_list_has_string(skip, iter->refname))
 					continue;
 
+				if (transaction && ref_transaction_maybe_set_rejected(
+					    transaction, *update_idx,
+					    REF_TRANSACTION_ERROR_NAME_CONFLICT))
+					continue;
+
 				strbuf_addf(err, _("'%s' exists; cannot create '%s'"),
 					    iter->refname, refname);
 				goto cleanup;
@@ -2648,6 +2673,11 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs
 
 		extra_refname = find_descendant_ref(dirname.buf, extras, skip);
 		if (extra_refname) {
+			if (transaction && ref_transaction_maybe_set_rejected(
+				    transaction, *update_idx,
+				    REF_TRANSACTION_ERROR_NAME_CONFLICT))
+				continue;
+
 			strbuf_addf(err, _("cannot process '%s' and '%s' at the same time"),
 				    refname, extra_refname);
 			goto cleanup;
@@ -2659,6 +2689,7 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs
 cleanup:
 	strbuf_release(&referent);
 	strbuf_release(&dirname);
+	strset_clear(&conflicting_dirnames);
 	strset_clear(&dirnames);
 	ref_iterator_free(iter);
 	return ret;
@@ -2679,7 +2710,7 @@ enum ref_transaction_error refs_verify_refname_available(
 	};
 
 	return refs_verify_refnames_available(refs, &refnames, extras, skip,
-					      initial_transaction, err);
+					      NULL, initial_transaction, err);
 }
 
 struct do_for_each_reflog_help {
diff --git a/refs.h b/refs.h
index c48c800478..46a6008e07 100644
--- a/refs.h
+++ b/refs.h
@@ -141,18 +141,6 @@ enum ref_transaction_error refs_verify_refname_available(struct ref_store *refs,
 						 unsigned int initial_transaction,
 						 struct strbuf *err);
 
-/*
- * Same as `refs_verify_refname_available()`, but checking for a list of
- * refnames instead of only a single item. This is more efficient in the case
- * where one needs to check multiple refnames.
- */
-enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs,
-					  const struct string_list *refnames,
-					  const struct string_list *extras,
-					  const struct string_list *skip,
-					  unsigned int initial_transaction,
-					  struct strbuf *err);
-
 int refs_ref_exists(struct ref_store *refs, const char *refname);
 
 int should_autocreate_reflog(enum log_refs_config log_all_ref_updates,
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 256c69b942..b96a511977 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -677,16 +677,18 @@ 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,
-					       const char *refname,
+					       struct ref_update *update,
+					       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,
-					       unsigned int *type,
 					       struct strbuf *err)
 {
 	enum ref_transaction_error ret = REF_TRANSACTION_ERROR_GENERIC;
+	const char *refname = update->refname;
+	unsigned int *type = &update->type;
 	struct ref_lock *lock;
 	struct strbuf ref_file = STRBUF_INIT;
 	int attempts_remaining = 3;
@@ -785,6 +787,8 @@ static enum ref_transaction_error lock_raw_ref(struct files_ref_store *refs,
 
 	if (files_read_raw_ref(&refs->base, refname, &lock->old_oid, referent,
 			       type, &failure_errno)) {
+		struct string_list_item *item;
+
 		if (failure_errno == ENOENT) {
 			if (mustexist) {
 				/* Garden variety missing reference. */
@@ -864,7 +868,9 @@ static enum ref_transaction_error lock_raw_ref(struct files_ref_store *refs,
 		 * make sure there is no existing packed ref that conflicts
 		 * with refname. This check is deferred so that we can batch it.
 		 */
-		string_list_append(refnames_to_check, refname);
+		item = string_list_append(refnames_to_check, refname);
+		item->util = xmalloc(sizeof(update_idx));
+		memcpy(item->util, &update_idx, sizeof(update_idx));
 	}
 
 	ret = 0;
@@ -2547,6 +2553,7 @@ struct files_transaction_backend_data {
  */
 static enum ref_transaction_error lock_ref_for_update(struct files_ref_store *refs,
 						      struct ref_update *update,
+						      size_t update_idx,
 						      struct ref_transaction *transaction,
 						      const char *head_ref,
 						      struct string_list *refnames_to_check,
@@ -2575,9 +2582,9 @@ 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->refname, mustexist,
+		ret = lock_raw_ref(refs, update, update_idx, mustexist,
 				   refnames_to_check, &transaction->refnames,
-				   &lock, &referent, &update->type, err);
+				   &lock, &referent, err);
 		if (ret) {
 			char *reason;
 
@@ -2849,7 +2856,7 @@ static int files_transaction_prepare(struct ref_store *ref_store,
 	for (i = 0; i < transaction->nr; i++) {
 		struct ref_update *update = transaction->updates[i];
 
-		ret = lock_ref_for_update(refs, update, transaction,
+		ret = lock_ref_for_update(refs, update, i, transaction,
 					  head_ref, &refnames_to_check,
 					  err);
 		if (ret) {
@@ -2905,7 +2912,8 @@ static int files_transaction_prepare(struct ref_store *ref_store,
 	 * So instead, we accept the race for now.
 	 */
 	if (refs_verify_refnames_available(refs->packed_ref_store, &refnames_to_check,
-					   &transaction->refnames, NULL, 0, err)) {
+					   &transaction->refnames, NULL, transaction,
+					   0, err)) {
 		ret = REF_TRANSACTION_ERROR_NAME_CONFLICT;
 		goto cleanup;
 	}
@@ -2951,7 +2959,7 @@ static int files_transaction_prepare(struct ref_store *ref_store,
 
 cleanup:
 	free(head_ref);
-	string_list_clear(&refnames_to_check, 0);
+	string_list_clear(&refnames_to_check, 1);
 
 	if (ret)
 		files_transaction_cleanup(refs, transaction);
@@ -3097,7 +3105,8 @@ static int files_transaction_finish_initial(struct files_ref_store *refs,
 	}
 
 	if (refs_verify_refnames_available(&refs->base, &refnames_to_check,
-					   &affected_refnames, NULL, 1, err)) {
+					   &affected_refnames, NULL, transaction,
+					   1, err)) {
 		packed_refs_unlock(refs->packed_ref_store);
 		ret = REF_TRANSACTION_ERROR_NAME_CONFLICT;
 		goto cleanup;
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 73a5379b73..f868870851 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -806,4 +806,20 @@ enum ref_transaction_error ref_update_check_old_target(const char *referent,
  */
 int ref_update_expects_existing_old_ref(struct ref_update *update);
 
+/*
+ * Same as `refs_verify_refname_available()`, but checking for a list of
+ * refnames instead of only a single item. This is more efficient in the case
+ * where one needs to check multiple refnames.
+ *
+ * If using batched updates, then individual updates are marked rejected,
+ * reference backends are then in charge of not committing those updates.
+ */
+enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs,
+					  const struct string_list *refnames,
+					  const struct string_list *extras,
+					  const struct string_list *skip,
+					  struct ref_transaction *transaction,
+					  unsigned int initial_transaction,
+					  struct strbuf *err);
+
 #endif /* REFS_REFS_INTERNAL_H */
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 5db4a108b9..4c3817f4ec 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -1074,6 +1074,7 @@ static enum ref_transaction_error prepare_single_update(struct reftable_ref_stor
 							struct ref_transaction *transaction,
 							struct reftable_backend *be,
 							struct ref_update *u,
+							size_t update_idx,
 							struct string_list *refnames_to_check,
 							unsigned int head_type,
 							struct strbuf *head_referent,
@@ -1149,6 +1150,7 @@ static enum ref_transaction_error prepare_single_update(struct reftable_ref_stor
 	if (ret < 0)
 		return REF_TRANSACTION_ERROR_GENERIC;
 	if (ret > 0 && !ref_update_expects_existing_old_ref(u)) {
+		struct string_list_item *item;
 		/*
 		 * The reference does not exist, and we either have no
 		 * old object ID or expect the reference to not exist.
@@ -1158,7 +1160,9 @@ static enum ref_transaction_error prepare_single_update(struct reftable_ref_stor
 		 * can output a proper error message instead of failing
 		 * at a later point.
 		 */
-		string_list_append(refnames_to_check, u->refname);
+		item = string_list_append(refnames_to_check, u->refname);
+		item->util = xmalloc(sizeof(update_idx));
+		memcpy(item->util, &update_idx, sizeof(update_idx));
 
 		/*
 		 * There is no need to write the reference deletion
@@ -1368,7 +1372,7 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,
 
 	for (i = 0; i < transaction->nr; i++) {
 		ret = prepare_single_update(refs, tx_data, transaction, be,
-					    transaction->updates[i],
+					    transaction->updates[i], i,
 					    &refnames_to_check, head_type,
 					    &head_referent, &referent, err);
 		if (ret) {
@@ -1384,6 +1388,7 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,
 
 	ret = refs_verify_refnames_available(ref_store, &refnames_to_check,
 					     &transaction->refnames, NULL,
+					     transaction,
 					     transaction->flags & REF_TRANSACTION_FLAG_INITIAL,
 					     err);
 	if (ret < 0)
@@ -1402,7 +1407,7 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,
 	}
 	strbuf_release(&referent);
 	strbuf_release(&head_referent);
-	string_list_clear(&refnames_to_check, 0);
+	string_list_clear(&refnames_to_check, 1);
 
 	return ret;
 }

-- 
2.48.1
Previous: Karthik NayakNext: Karthik Nayak
Message 44 of 49 in “refs: introduce support for partial reference transactions”
  1. 0/6 refs: introduce support for partial reference transactionsKarthik Nayak, Feb 7, 2025
  2. 1/6 refs/files: remove duplicate check in `split_symref_update()`Karthik Nayak, Feb 7, 2025
  3. Patrick SteinhardtFeb 7, 2025
  4. Karthik NayakFeb 11, 2025
  5. 2/6 refs: move duplicate refname update check to generic layerKarthik Nayak, Feb 7, 2025
  6. Patrick SteinhardtFeb 7, 2025
  7. Karthik NayakFeb 11, 2025
  8. 3/6 refs/files: remove duplicate duplicates checkKarthik Nayak, Feb 7, 2025
  9. Patrick SteinhardtFeb 7, 2025
  10. 4/6 refs/reftable: extract code from the transaction preparationKarthik Nayak, Feb 7, 2025
  11. 5/6 refs: implement partial reference transaction supportKarthik Nayak, Feb 7, 2025
  12. Patrick SteinhardtFeb 7, 2025
  13. Karthik NayakFeb 21, 2025
  14. 6/6 update-ref: add --allow-partial flag for stdin modeKarthik Nayak, Feb 7, 2025
  15. Patrick SteinhardtFeb 7, 2025
  16. Karthik NayakFeb 21, 2025
  17. Phillip WoodFeb 11, 2025
  18. Phillip WoodFeb 11, 2025
  19. Karthik NayakFeb 12, 2025
  20. Karthik NayakFeb 12, 2025
  21. Phillip WoodFeb 19, 2025
  22. Patrick SteinhardtFeb 19, 2025
  23. Karthik NayakFeb 21, 2025
  24. 0/8 refs: introduce support for batched reference updatesKarthik Nayak, Mar 27, 2025
  25. 1/8 refs/files: remove redundant check in split_symref_update()Karthik Nayak, Mar 27, 2025
  26. 2/8 refs: move duplicate refname update check to generic layerKarthik Nayak, Mar 27, 2025
  27. 3/8 refs/files: remove duplicate duplicates checkKarthik Nayak, Mar 27, 2025
  28. 4/8 refs/reftable: extract code from the transaction preparationKarthik Nayak, Mar 27, 2025
  29. 5/8 refs: introduce enum-based transaction error typesKarthik Nayak, Mar 27, 2025
  30. 6/8 refs: implement batch reference update supportKarthik Nayak, Mar 27, 2025
  31. 7/8 refs: support rejection in batch updates during F/D checksKarthik Nayak, Mar 27, 2025
  32. 8/8 update-ref: add --batch-updates flag for stdin modeKarthik Nayak, Mar 27, 2025
  33. Jean-Noël AVILAMar 28, 2025
  34. Junio C HamanoMar 29, 2025
  35. Karthik NayakMar 29, 2025
  36. Patrick SteinhardtMar 28, 2025
  37. 0/8 refs: introduce support for batched reference updatesKarthik Nayak, Apr 8, 2025
  38. 1/8 refs/files: remove redundant check in split_symref_update()Karthik Nayak, Apr 8, 2025
  39. 2/8 refs: move duplicate refname update check to generic layerKarthik Nayak, Apr 8, 2025
  40. 3/8 refs/files: remove duplicate duplicates checkKarthik Nayak, Apr 8, 2025
  41. 4/8 refs/reftable: extract code from the transaction preparationKarthik Nayak, Apr 8, 2025
  42. 5/8 refs: introduce enum-based transaction error typesKarthik Nayak, Apr 8, 2025
  43. 6/8 refs: implement batch reference update supportKarthik Nayak, Apr 8, 2025
  44. 7/8 refs: support rejection in batch updates during F/D checksKarthik Nayak, Apr 8, 2025
  45. 8/8 update-ref: add --batch-updates flag for stdin modeKarthik Nayak, Apr 8, 2025
  46. Junio C HamanoApr 8, 2025
  47. Karthik NayakApr 8, 2025
  48. Junio C HamanoApr 8, 2025
  49. Karthik NayakApr 10, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.