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

[PATCH v2 02/11] refs/reftable: perform explicit D/F check when writing symrefs

From
Patrick Steinhardt <ps@pks.im>
Date
Apr 4, 2024, 05:48 UTC
Message-ID
<61901719066a2d0b70b99b275552030326fd375a.1712209149.git.ps@pks.im>
In-Reply-To
<cover.1712209149.git.ps@pks.im>

We already perform explicit D/F checks in all reftable callbacks which write refs, except when writing symrefs. For one this leads to an error message which isn't perfectly actionable because we only tell the user that there was a D/F conflict, but not which refs conflicted with each other. But second, once all ref updating callbacks explicitly check for D/F conflicts, we can disable the D/F checks in the reftable library itself and thus avoid some duplicated efforts.

Refactor the code that writes symref tables to explicitly call into `refs_verify_refname_available()` when writing symrefs.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 refs/reftable-backend.c    | 20 +++++++++++++++++---
 t/t0610-reftable-basics.sh |  2 +-
 2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 0358da14db..8a54b0d8b2 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -1217,6 +1217,7 @@ static int reftable_be_pack_refs(struct ref_store *ref_store,
 struct write_create_symref_arg {
 	struct reftable_ref_store *refs;
 	struct reftable_stack *stack;
+	struct strbuf *err;
 	const char *refname;
 	const char *target;
 	const char *logmsg;
@@ -1239,6 +1240,11 @@ static int write_create_symref_table(struct reftable_writer *writer, void *cb_da
 
 	reftable_writer_set_limits(writer, ts, ts);
 
+	ret = refs_verify_refname_available(&create->refs->base, create->refname,
+					    NULL, NULL, create->err);
+	if (ret < 0)
+		return ret;
+
 	ret = reftable_writer_add_ref(writer, &ref);
 	if (ret)
 		return ret;
@@ -1280,12 +1286,14 @@ static int reftable_be_create_symref(struct ref_store *ref_store,
 	struct reftable_ref_store *refs =
 		reftable_be_downcast(ref_store, REF_STORE_WRITE, "create_symref");
 	struct reftable_stack *stack = stack_for(refs, refname, &refname);
+	struct strbuf err = STRBUF_INIT;
 	struct write_create_symref_arg arg = {
 		.refs = refs,
 		.stack = stack,
 		.refname = refname,
 		.target = target,
 		.logmsg = logmsg,
+		.err = &err,
 	};
 	int ret;
 
@@ -1301,9 +1309,15 @@ static int reftable_be_create_symref(struct ref_store *ref_store,
 
 done:
 	assert(ret != REFTABLE_API_ERROR);
-	if (ret)
-		error("unable to write symref for %s: %s", refname,
-		      reftable_error_str(ret));
+	if (ret) {
+		if (err.len)
+			error("%s", err.buf);
+		else
+			error("unable to write symref for %s: %s", refname,
+			      reftable_error_str(ret));
+	}
+
+	strbuf_release(&err);
 	return ret;
 }
 
diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
index 055231a707..12b0004781 100755
--- a/t/t0610-reftable-basics.sh
+++ b/t/t0610-reftable-basics.sh
@@ -255,7 +255,7 @@ test_expect_success 'ref transaction: creating symbolic ref fails with F/D confl
 	git init repo &&
 	test_commit -C repo A &&
 	cat >expect <<-EOF &&
-	error: unable to write symref for refs/heads: file/directory conflict
+	error: ${SQ}refs/heads/main${SQ} exists; cannot create ${SQ}refs/heads${SQ}
 	EOF
 	test_must_fail git -C repo symbolic-ref refs/heads refs/heads/foo 2>err &&
 	test_cmp expect err
-- 
2.44.GIT
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 18 of 49 in “reftable: optimize write performance”
  1. 0/9 reftable: optimize write performancePatrick Steinhardt, Apr 2, 2024
  2. 1/9 refs/reftable: fix D/F conflict error message on ref copyPatrick Steinhardt, Apr 2, 2024
  3. Junio C HamanoApr 3, 2024
  4. 2/9 refs/reftable: perform explicit D/F check when writing symrefsPatrick Steinhardt, Apr 2, 2024
  5. 3/9 refs/reftable: skip duplicate name checksPatrick Steinhardt, Apr 2, 2024
  6. 4/9 refs/reftable: don't recompute committer identPatrick Steinhardt, Apr 2, 2024
  7. Junio C HamanoApr 3, 2024
  8. Patrick SteinhardtApr 4, 2024
  9. 5/9 reftable/writer: refactorings for `writer_add_record()`Patrick Steinhardt, Apr 2, 2024
  10. 6/9 reftable/writer: refactorings for `writer_flush_nonempty_block()`Patrick Steinhardt, Apr 2, 2024
  11. 7/9 reftable/block: reuse zstream when writing log blocksPatrick Steinhardt, Apr 2, 2024
  12. Junio C HamanoApr 3, 2024
  13. Patrick SteinhardtApr 4, 2024
  14. 8/9 reftable/block: reuse compressed arrayPatrick Steinhardt, Apr 2, 2024
  15. 9/9 reftable/writer: reset `last_key` instead of releasing itPatrick Steinhardt, Apr 2, 2024
  16. 00/11 reftable: optimize write performancePatrick Steinhardt, Apr 4, 2024
  17. 01/11 refs/reftable: fix D/F conflict error message on ref copyPatrick Steinhardt, Apr 4, 2024
  18. 02/11 refs/reftable: perform explicit D/F check when writing symrefsPatrick Steinhardt, Apr 4, 2024
  19. 03/11 refs/reftable: skip duplicate name checksPatrick Steinhardt, Apr 4, 2024
  20. 04/11 reftable: remove name checksPatrick Steinhardt, Apr 4, 2024
  21. 05/11 refs/reftable: don't recompute committer identPatrick Steinhardt, Apr 4, 2024
  22. 06/11 reftable/writer: refactorings for `writer_add_record()`Patrick Steinhardt, Apr 4, 2024
  23. Han-Wen NienhuysApr 4, 2024
  24. Patrick SteinhardtApr 4, 2024
  25. 07/11 reftable/writer: refactorings for `writer_flush_nonempty_block()`Patrick Steinhardt, Apr 4, 2024
  26. 08/11 reftable/writer: unify releasing memoryPatrick Steinhardt, Apr 4, 2024
  27. Han-Wen NienhuysApr 4, 2024
  28. Patrick SteinhardtApr 4, 2024
  29. Han-Wen NienhuysApr 4, 2024
  30. Patrick SteinhardtApr 4, 2024
  31. 09/11 reftable/writer: reset `last_key` instead of releasing itPatrick Steinhardt, Apr 4, 2024
  32. 10/11 reftable/block: reuse zstream when writing log blocksPatrick Steinhardt, Apr 4, 2024
  33. 11/11 reftable/block: reuse compressed arrayPatrick Steinhardt, Apr 4, 2024
  34. Han-Wen NienhuysApr 4, 2024
  35. Patrick SteinhardtApr 4, 2024
  36. 00/11 reftable: optimize write performancePatrick Steinhardt, Apr 8, 2024
  37. 01/11 refs/reftable: fix D/F conflict error message on ref copyPatrick Steinhardt, Apr 8, 2024
  38. 02/11 refs/reftable: perform explicit D/F check when writing symrefsPatrick Steinhardt, Apr 8, 2024
  39. 03/11 refs/reftable: skip duplicate name checksPatrick Steinhardt, Apr 8, 2024
  40. 04/11 reftable: remove name checksPatrick Steinhardt, Apr 8, 2024
  41. 05/11 refs/reftable: don't recompute committer identPatrick Steinhardt, Apr 8, 2024
  42. 06/11 reftable/writer: refactorings for `writer_add_record()`Patrick Steinhardt, Apr 8, 2024
  43. 07/11 reftable/writer: refactorings for `writer_flush_nonempty_block()`Patrick Steinhardt, Apr 8, 2024
  44. 08/11 reftable/writer: unify releasing memoryPatrick Steinhardt, Apr 8, 2024
  45. 09/11 reftable/writer: reset `last_key` instead of releasing itPatrick Steinhardt, Apr 8, 2024
  46. 10/11 reftable/block: reuse zstream when writing log blocksPatrick Steinhardt, Apr 8, 2024
  47. 11/11 reftable/block: reuse compressed arrayPatrick Steinhardt, Apr 8, 2024
  48. Junio C HamanoApr 9, 2024
  49. Patrick SteinhardtApr 9, 2024

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.