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

[PATCH v2 11/12] initial_ref_transaction_commit(): check for ref D/F conflicts

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Jun 13, 2015, 14:42 UTC
Message-ID
<95cc41220378be062b4f8406f2e1adf6da529abc.1434206062.git.mhagger@alum.mit.edu>
In-Reply-To
<cover.1434206062.git.mhagger@alum.mit.edu>

In initial_ref_transaction_commit(), check for D/F conflicts (i.e., the type of conflict that exists between "refs/foo" and "refs/foo/bar") among the references being created and between the references being created and any hypothetical existing references.

Ideally, there shouldn't *be* any existing references when this function is called. But, at least in the case of the "testgit" remote helper, "clone" can be called after the remote-tracking "HEAD" and "master" branches have already been created. So let's just do the full-blown check.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)
diff --git a/refs.c b/refs.c
index dfe9090..340d67f 100644
--- a/refs.c
+++ b/refs.c
@@ -4062,9 +4062,19 @@ cleanup:
 	return ret;
 }
 
+static int ref_present(const char *refname,
+		       const struct object_id *oid, int flags, void *cb_data)
+{
+	struct string_list *affected_refnames = cb_data;
+
+	return string_list_has_string(affected_refnames, refname);
+}
+
 int initial_ref_transaction_commit(struct ref_transaction *transaction,
 				   struct strbuf *err)
 {
+	struct ref_dir *loose_refs = get_loose_refs(&ref_cache);
+	struct ref_dir *packed_refs = get_packed_refs(&ref_cache);
 	int ret = 0, i;
 	int n = transaction->nr;
 	struct ref_update **updates = transaction->updates;
@@ -4084,12 +4094,36 @@ int initial_ref_transaction_commit(struct ref_transaction *transaction,
 		goto cleanup;
 	}
 
+	/*
+	 * It's really undefined to call this function in an active
+	 * repository or when there are existing references: we are
+	 * only locking and changing packed-refs, so (1) any
+	 * simultaneous processes might try to change a reference at
+	 * the same time we do, and (2) any existing loose versions of
+	 * the references that we are setting would have precedence
+	 * over our values. But some remote helpers create the remote
+	 * "HEAD" and "master" branches before calling this function,
+	 * so here we really only check that none of the references
+	 * that we are creating already exists.
+	 */
+	if (for_each_rawref(ref_present, &affected_refnames))
+		die("BUG: initial ref transaction called with existing refs");
+
 	for (i = 0; i < n; i++) {
 		struct ref_update *update = updates[i];
 
 		if ((update->flags & REF_HAVE_OLD) &&
 		    !is_null_sha1(update->old_sha1))
 			die("BUG: initial ref transaction with old_sha1 set");
+		if (verify_refname_available(update->refname,
+					     &affected_refnames, NULL,
+					     loose_refs, err) ||
+		    verify_refname_available(update->refname,
+					     &affected_refnames, NULL,
+					     packed_refs, err)) {
+			ret = TRANSACTION_NAME_CONFLICT;
+			goto cleanup;
+		}
 	}
 
 	if (lock_packed_refs(0)) {

-- 
2.1.4
Previous: Michael HaggertyNext: Michael Haggerty
Message 23 of 30 in “Improve "refs" module encapsulation”
  1. 00/12 Improve "refs" module encapsulationMichael Haggerty, Jun 13, 2015
  2. 01/12 delete_ref(): move declaration to refs.hMichael Haggerty, Jun 13, 2015
  3. 02/12 remove_branches(): remove temporaryMichael Haggerty, Jun 13, 2015
  4. 03/12 delete_ref(): handle special case more explicitlyMichael Haggerty, Jun 13, 2015
  5. Junio C HamanoJun 15, 2015
  6. 04/12 delete_refs(): new function for the refs APIMichael Haggerty, Jun 13, 2015
  7. 05/12 delete_refs(): improve error messageMichael Haggerty, Jun 13, 2015
  8. Junio C HamanoJun 15, 2015
  9. Michael HaggertyJun 19, 2015
  10. 06/12 prune_remote(): use delete_refs()Michael Haggerty, Jun 13, 2015
  11. Junio C HamanoJun 15, 2015
  12. Jeff KingJun 15, 2015
  13. Junio C HamanoJun 15, 2015
  14. 07/12 repack_without_refs(): make function privateMichael Haggerty, Jun 13, 2015
  15. 08/12 initial_ref_transaction_commit(): function for initial ref creationMichael Haggerty, Jun 13, 2015
  16. Junio C HamanoJun 15, 2015
  17. Junio C HamanoJun 15, 2015
  18. Jeff KingJun 15, 2015
  19. Junio C HamanoJun 15, 2015
  20. Michael HaggertyJun 20, 2015
  21. 09/12 refs: remove some functions from the module's public interfaceMichael Haggerty, Jun 13, 2015
  22. 10/12 initial_ref_transaction_commit(): check for duplicate refsMichael Haggerty, Jun 13, 2015
  23. 11/12 initial_ref_transaction_commit(): check for ref D/F conflictsMichael Haggerty, Jun 13, 2015
  24. 12/12 refs: move the remaining ref module declarations to refs.hMichael Haggerty, Jun 13, 2015
  25. Junio C HamanoJun 15, 2015
  26. Jeff KingJun 15, 2015
  27. Junio C HamanoJun 15, 2015
  28. Michael HaggertyJun 20, 2015
  29. Junio C HamanoJun 20, 2015
  30. Stefan BellerJun 15, 2015

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.