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

[ADDENDUM v4] Yet more preparation for reference backends

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Jun 13, 2016, 09:55 UTC
Message-ID
<328d460b1730c0b192961234c06036514498c1b1.1465808670.git.mhagger@alum.mit.edu>
In-Reply-To
<xmqq8tydgic2.fsf@gitster.mtv.corp.google.com>

Below is the delta needed to fixed the bug in the mh/split-under-lock patch series that I mentioned in an earlier email [1], plus a little tweak to make the docstring for lock_ref_for_update() clearer.

I actually fixed the bug in preparatory commit
    ref_transaction_commit(): remove local variable n
, whose subject was accordingly changed to
    ref_transaction_commit(): remove local variables n and updates

[2]. See branch "split-under-lock" in my GitHub fork [3] for the full revised patch series (including the changes to a later patch that are necessary when it is rebased onto the fix). In the same repo I've also rebased dependent branches update-ref-errors, ref-iterators, and ref-store on top of this one; all of those rebases were trivial.

Here, for the convenience of reviewers, I present the delta between the old and new end states of the split-under-lock patch series. (It didn't seem justified to re-send the whole 33-patch series for this logically small change.)

Michael

[1] http://article.gmane.org/gmane.comp.version-control.git/297002 [2] https://github.com/gitster/git/commit/efe472813d60befd72d6e2797934c90b22a26c93 [3] https://github.com/mhagger/git

---
 refs/files-backend.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 1230dfb..bbf96ad 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3393,7 +3393,8 @@ static const char *original_update_refname(struct ref_update *update)
  * - Lock the reference referred to by update.
  * - Read the reference under lock.
  * - Check that its old SHA-1 value (if specified) is correct, and in
- *   any case record it for later use in the reflog.
+ *   any case record it in update->lock->old_oid for later use when
+ *   writing the reflog.
  * - If it is a symref update without REF_NODEREF, split it up into a
  *   REF_LOG_ONLY update of the symref and add a separate update for
  *   the referent to transaction.
@@ -3556,7 +3557,6 @@ int ref_transaction_commit(struct ref_transaction *transaction,
 			   struct strbuf *err)
 {
 	int ret = 0, i;
-	struct ref_update **updates = transaction->updates;
 	struct string_list refs_to_delete = STRING_LIST_INIT_NODUP;
 	struct string_list_item *ref_to_delete;
 	struct string_list affected_refnames = STRING_LIST_INIT_NODUP;
@@ -3582,7 +3582,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,
 	 * same refname as any existing ones.)
 	 */
 	for (i = 0; i < transaction->nr; i++) {
-		struct ref_update *update = updates[i];
+		struct ref_update *update = transaction->updates[i];
 		struct string_list_item *item =
 			string_list_append(&affected_refnames, update->refname);
 
@@ -3632,7 +3632,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,
 	 * open at a time to avoid running out of file descriptors.
 	 */
 	for (i = 0; i < transaction->nr; i++) {
-		struct ref_update *update = updates[i];
+		struct ref_update *update = transaction->updates[i];
 
 		ret = lock_ref_for_update(update, transaction, head_ref,
 					  &affected_refnames, err);
@@ -3642,7 +3642,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,
 
 	/* Perform updates first so live commits remain referenced */
 	for (i = 0; i < transaction->nr; i++) {
-		struct ref_update *update = updates[i];
+		struct ref_update *update = transaction->updates[i];
 		struct ref_lock *lock = update->lock;
 
 		if (update->flags & REF_NEEDS_COMMIT ||
@@ -3674,7 +3674,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,
 	}
 	/* Perform deletes now that updates are safely completed */
 	for (i = 0; i < transaction->nr; i++) {
-		struct ref_update *update = updates[i];
+		struct ref_update *update = transaction->updates[i];
 
 		if (update->flags & REF_DELETING &&
 		    !(update->flags & REF_LOG_ONLY)) {
@@ -3701,8 +3701,8 @@ cleanup:
 	transaction->state = REF_TRANSACTION_CLOSED;
 
 	for (i = 0; i < transaction->nr; i++)
-		if (updates[i]->lock)
-			unlock_ref(updates[i]->lock);
+		if (transaction->updates[i]->lock)
+			unlock_ref(transaction->updates[i]->lock);
 	string_list_clear(&refs_to_delete, 0);
 	free(head_ref);
 	string_list_clear(&affected_refnames, 0);
@@ -3722,7 +3722,6 @@ int initial_ref_transaction_commit(struct ref_transaction *transaction,
 				   struct strbuf *err)
 {
 	int ret = 0, i;
-	struct ref_update **updates = transaction->updates;
 	struct string_list affected_refnames = STRING_LIST_INIT_NODUP;
 
 	assert(err);
@@ -3732,7 +3731,8 @@ int initial_ref_transaction_commit(struct ref_transaction *transaction,
 
 	/* Fail if a refname appears more than once in the transaction: */
 	for (i = 0; i < transaction->nr; i++)
-		string_list_append(&affected_refnames, updates[i]->refname);
+		string_list_append(&affected_refnames,
+				   transaction->updates[i]->refname);
 	string_list_sort(&affected_refnames);
 	if (ref_update_reject_duplicates(&affected_refnames, err)) {
 		ret = TRANSACTION_GENERIC_ERROR;
@@ -3755,7 +3755,7 @@ int initial_ref_transaction_commit(struct ref_transaction *transaction,
 		die("BUG: initial ref transaction called with existing refs");
 
 	for (i = 0; i < transaction->nr; i++) {
-		struct ref_update *update = updates[i];
+		struct ref_update *update = transaction->updates[i];
 
 		if ((update->flags & REF_HAVE_OLD) &&
 		    !is_null_sha1(update->old_sha1))
@@ -3776,7 +3776,7 @@ int initial_ref_transaction_commit(struct ref_transaction *transaction,
 	}
 
 	for (i = 0; i < transaction->nr; i++) {
-		struct ref_update *update = updates[i];
+		struct ref_update *update = transaction->updates[i];
 
 		if ((update->flags & REF_HAVE_NEW) &&
 		    !is_null_sha1(update->new_sha1))

-- 
2.8.1
Previous: Junio C Hamano
Message 51 of 51 in “Yet more preparation for reference backends”
  1. 00/33 Yet more preparation for reference backendsMichael Haggerty, May 6, 2016
  2. 01/33 t1404: demonstrate a bug resolving referencesMichael Haggerty, May 6, 2016
  3. 02/33 commit_ref(): if there is an empty dir in the way, delete itMichael Haggerty, May 6, 2016
  4. 03/33 read_raw_ref(): don't get confused by an empty directoryMichael Haggerty, May 6, 2016
  5. 04/33 safe_create_leading_directories(): improve docstringMichael Haggerty, May 6, 2016
  6. 05/33 remove_dir_recursively(): add docstringMichael Haggerty, May 6, 2016
  7. 06/33 refname_is_safe(): use skip_prefix()Michael Haggerty, May 6, 2016
  8. 07/33 refname_is_safe(): don't allow the empty stringMichael Haggerty, May 6, 2016
  9. 08/33 refname_is_safe(): insist that the refname already be normalizedMichael Haggerty, May 6, 2016
  10. 09/33 commit_ref_update(): write error message to *err, not stderrMichael Haggerty, May 6, 2016
  11. 10/33 rename_ref(): remove unneeded local variableMichael Haggerty, May 6, 2016
  12. 11/33 ref_transaction_commit(): remove local variable nMichael Haggerty, May 6, 2016
  13. 12/33 read_raw_ref(): rename flags argument to typeMichael Haggerty, May 6, 2016
  14. 13/33 read_raw_ref(): clear *type at start of functionMichael Haggerty, May 6, 2016
  15. 14/33 read_raw_ref(): rename symref argument to referentMichael Haggerty, May 6, 2016
  16. 15/33 read_raw_ref(): improve docstringMichael Haggerty, May 6, 2016
  17. 16/33 read_raw_ref(): move docstring to header fileMichael Haggerty, May 6, 2016
  18. 17/33 lock_ref_sha1_basic(): remove unneeded local variableMichael Haggerty, May 6, 2016
  19. 18/33 refs: make error messages more consistentMichael Haggerty, May 6, 2016
  20. 19/33 ref_transaction_create(): disallow recursive pruningMichael Haggerty, May 6, 2016
  21. 20/33 ref_transaction_commit(): correctly report close_ref() failureMichael Haggerty, May 6, 2016
  22. 21/33 delete_branches(): use resolve_refdup()Michael Haggerty, May 6, 2016
  23. 22/33 refs: allow log-only updatesMichael Haggerty, May 6, 2016
  24. 23/33 refs: don't dereference on renameMichael Haggerty, May 6, 2016
  25. 24/33 verify_refname_available(): adjust constness in declarationMichael Haggerty, May 6, 2016
  26. 25/33 add_update(): initialize the whole ref_updateMichael Haggerty, May 6, 2016
  27. 26/33 lock_ref_for_update(): new functionMichael Haggerty, May 6, 2016
  28. 27/33 unlock_ref(): move definition higher in the fileMichael Haggerty, May 6, 2016
  29. 28/33 ref_transaction_update(): check refname_is_safe() at a minimumMichael Haggerty, May 6, 2016
  30. 29/33 refs: resolve symbolic refs firstMichael Haggerty, May 6, 2016
  31. Jeff KingMay 12, 2016
  32. Jeff KingMay 12, 2016
  33. Michael HaggertyMay 13, 2016
  34. 29/33 refs: resolve symbolic refs firstMichael Haggerty, May 13, 2016
  35. Jeff KingMay 13, 2016
  36. Jeff KingMay 13, 2016
  37. Torsten BögershausenMay 14, 2016
  38. 30/33 lock_ref_for_update(): don't re-read non-symbolic referencesMichael Haggerty, May 6, 2016
  39. 31/33 lock_ref_for_update(): don't resolve symrefsMichael Haggerty, May 6, 2016
  40. 32/33 commit_ref_update(): remove the flags parameterMichael Haggerty, May 6, 2016
  41. 33/33 lock_ref_sha1_basic(): only handle REF_NODEREF modeMichael Haggerty, May 6, 2016
  42. David TurnerMay 9, 2016
  43. Junio C HamanoMay 9, 2016
  44. Michael HaggertyMay 9, 2016
  45. Junio C HamanoMay 9, 2016
  46. Jeff KingMay 12, 2016
  47. Junio C HamanoMay 12, 2016
  48. Junio C HamanoMay 10, 2016
  49. Michael HaggertyJun 10, 2016
  50. Junio C HamanoJun 10, 2016
  51. [ADDENDUM v4] Yet more preparation for reference backendsMichael Haggerty, Jun 13, 2016

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.