git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Michael Haggerty <mhagger@alum.mit.edu>
To: Junio C Hamano <gitster@pobox.com>, David Turner <novalis@novalis.org>
Cc: "Jeff King" <peff@peff.net>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Ramsay Jones" <ramsay@ramsayjones.plus.com>,
	"Torsten Bögershausen" <tboegi@web.de>,
	git@vger.kernel.org, "Michael Haggerty" <mhagger@alum.mit.edu>
Subject: [ADDENDUM v4] Yet more preparation for reference backends
Date: Mon, 13 Jun 2016 11:55:57 +0200	[thread overview]
Message-ID: <328d460b1730c0b192961234c06036514498c1b1.1465808670.git.mhagger@alum.mit.edu> (raw)
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

      reply	other threads:[~2016-06-13  9:56 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-06 16:13 [PATCH v2 00/33] Yet more preparation for reference backends Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 01/33] t1404: demonstrate a bug resolving references Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 02/33] commit_ref(): if there is an empty dir in the way, delete it Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 03/33] read_raw_ref(): don't get confused by an empty directory Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 04/33] safe_create_leading_directories(): improve docstring Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 05/33] remove_dir_recursively(): add docstring Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 06/33] refname_is_safe(): use skip_prefix() Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 07/33] refname_is_safe(): don't allow the empty string Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 08/33] refname_is_safe(): insist that the refname already be normalized Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 09/33] commit_ref_update(): write error message to *err, not stderr Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 10/33] rename_ref(): remove unneeded local variable Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 11/33] ref_transaction_commit(): remove local variable n Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 12/33] read_raw_ref(): rename flags argument to type Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 13/33] read_raw_ref(): clear *type at start of function Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 14/33] read_raw_ref(): rename symref argument to referent Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 15/33] read_raw_ref(): improve docstring Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 16/33] read_raw_ref(): move docstring to header file Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 17/33] lock_ref_sha1_basic(): remove unneeded local variable Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 18/33] refs: make error messages more consistent Michael Haggerty
2016-05-06 16:14 ` [PATCH v2 19/33] ref_transaction_create(): disallow recursive pruning Michael Haggerty
2016-05-06 16:14 ` [PATCH v2 20/33] ref_transaction_commit(): correctly report close_ref() failure Michael Haggerty
2016-05-06 16:14 ` [PATCH v2 21/33] delete_branches(): use resolve_refdup() Michael Haggerty
2016-05-06 16:14 ` [PATCH v2 22/33] refs: allow log-only updates Michael Haggerty
2016-05-06 16:14 ` [PATCH v2 23/33] refs: don't dereference on rename Michael Haggerty
2016-05-06 16:14 ` [PATCH v2 24/33] verify_refname_available(): adjust constness in declaration Michael Haggerty
2016-05-06 16:14 ` [PATCH v2 25/33] add_update(): initialize the whole ref_update Michael Haggerty
2016-05-06 16:14 ` [PATCH v2 26/33] lock_ref_for_update(): new function Michael Haggerty
2016-05-06 16:14 ` [PATCH v2 27/33] unlock_ref(): move definition higher in the file Michael Haggerty
2016-05-06 16:14 ` [PATCH v2 28/33] ref_transaction_update(): check refname_is_safe() at a minimum Michael Haggerty
2016-05-06 16:14 ` [PATCH v2 29/33] refs: resolve symbolic refs first Michael Haggerty
2016-05-12  7:45   ` Jeff King
2016-05-12  8:25     ` Jeff King
2016-05-13 12:33       ` Michael Haggerty
2016-05-13 12:35         ` [PATCH v3 " Michael Haggerty
2016-05-13 12:58           ` Jeff King
2016-05-13 12:51         ` [PATCH v2 " Jeff King
2016-05-14  9:02         ` Torsten Bögershausen
2016-05-06 16:14 ` [PATCH v2 30/33] lock_ref_for_update(): don't re-read non-symbolic references Michael Haggerty
2016-05-06 16:14 ` [PATCH v2 31/33] lock_ref_for_update(): don't resolve symrefs Michael Haggerty
2016-05-06 16:14 ` [PATCH v2 32/33] commit_ref_update(): remove the flags parameter Michael Haggerty
2016-05-06 16:14 ` [PATCH v2 33/33] lock_ref_sha1_basic(): only handle REF_NODEREF mode Michael Haggerty
2016-05-09 20:12 ` [PATCH v2 00/33] Yet more preparation for reference backends David Turner
2016-05-09 21:05   ` Junio C Hamano
2016-05-09 21:50     ` Michael Haggerty
2016-05-09 22:04       ` Junio C Hamano
2016-05-12  7:55         ` Jeff King
2016-05-12 16:10           ` Junio C Hamano
2016-05-10 21:32 ` Junio C Hamano
2016-06-10 12:50 ` Michael Haggerty
2016-06-10 15:43   ` Junio C Hamano
2016-06-13  9:55     ` Michael Haggerty [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=328d460b1730c0b192961234c06036514498c1b1.1465808670.git.mhagger@alum.mit.edu \
    --to=mhagger@alum.mit.edu \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=novalis@novalis.org \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    --cc=ramsay@ramsayjones.plus.com \
    --cc=tboegi@web.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).