git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Michael Haggerty <mhagger@alum.mit.edu>
To: Junio C Hamano <gitster@pobox.com>
Cc: Stefan Beller <sbeller@google.com>, Jeff King <peff@peff.net>,
	git@vger.kernel.org, Michael Haggerty <mhagger@alum.mit.edu>
Subject: [PATCH 09/13] initial_ref_transaction_commit(): function for initial ref creation
Date: Mon,  8 Jun 2015 13:45:43 +0200	[thread overview]
Message-ID: <7d0f71d1d15f6811658f32cad5a9e74dbd94e8c6.1433763494.git.mhagger@alum.mit.edu> (raw)
In-Reply-To: <cover.1433763494.git.mhagger@alum.mit.edu>

"git clone" uses shortcuts when creating the initial set of
references:

* It writes them directly to packed-refs.

* It doesn't lock the individual references (though it does lock the
  packed-refs file).

* It doesn't check for refname conflicts between two new references or
  between one new reference and any hypothetical old ones.

* It doesn't create reflog entries for the reference creations.

This functionality was implemented in builtin/clone.c. But really that
file shouldn't have such intimate knowledge of how references are
stored. So provide a new function in the refs API,
initial_ref_transaction_commit(), which can be used for initial
reference creation. The new function is based on the ref_transaction
interface.

This means that we can make some other functions private to the refs
module. That will be done in a followup commit.

It would seem to make sense to add a test here that there are no
existing references, because that is how the function *should* be
used. But in fact, the "testgit" remote helper appears to call it
*after* having set up refs/remotes/<name>/HEAD and
refs/remotes/<name>/master, so we can't be so strict. For now, the
function trusts its caller to only call it when it makes sense. Future
commits will add some more limited sanity checks.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 builtin/clone.c | 19 +++++++++++++++----
 refs.c          | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 refs.h          | 14 ++++++++++++++
 3 files changed, 76 insertions(+), 4 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index b878252..bd2a50a 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -500,16 +500,27 @@ static void write_remote_refs(const struct ref *local_refs)
 {
 	const struct ref *r;
 
-	lock_packed_refs(LOCK_DIE_ON_ERROR);
+	struct ref_transaction *t;
+	struct strbuf err = STRBUF_INIT;
+
+	t = ref_transaction_begin(&err);
+	if (!t)
+		die(err.buf);
 
 	for (r = local_refs; r; r = r->next) {
 		if (!r->peer_ref)
 			continue;
-		add_packed_ref(r->peer_ref->name, r->old_sha1);
+		if (ref_transaction_create(t, r->peer_ref->name, r->old_sha1,
+					   0, NULL, &err))
+			die(err.buf);
+	}
+
+	if (initial_ref_transaction_commit(t, &err)) {
+		die(err.buf);
 	}
 
-	if (commit_packed_refs())
-		die_errno("unable to overwrite old ref-pack file");
+	strbuf_release(&err);
+	ref_transaction_free(t);
 }
 
 static void write_followtags(const struct ref *refs, const char *msg)
diff --git a/refs.c b/refs.c
index dc17984..ec4b8a0 100644
--- a/refs.c
+++ b/refs.c
@@ -4041,6 +4041,53 @@ cleanup:
 	return ret;
 }
 
+int initial_ref_transaction_commit(struct ref_transaction *transaction,
+				   struct strbuf *err)
+{
+	int ret = 0, i;
+	int n = transaction->nr;
+	struct ref_update **updates = transaction->updates;
+
+	assert(err);
+
+	if (transaction->state != REF_TRANSACTION_OPEN)
+		die("BUG: commit called for transaction that is not open");
+
+	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 (lock_packed_refs(0)) {
+		strbuf_addf(err, "unable to lock packed-refs file: %s",
+			    strerror(errno));
+		ret = TRANSACTION_GENERIC_ERROR;
+		goto cleanup;
+	}
+
+	for (i = 0; i < n; i++) {
+		struct ref_update *update = updates[i];
+
+		if ((update->flags & REF_HAVE_NEW) &&
+		    !is_null_sha1(update->new_sha1))
+			add_packed_ref(update->refname, update->new_sha1);
+	}
+
+	if (commit_packed_refs()) {
+		strbuf_addf(err, "unable to commit packed-refs file: %s",
+			    strerror(errno));
+		ret = TRANSACTION_GENERIC_ERROR;
+		goto cleanup;
+	}
+
+cleanup:
+	transaction->state = REF_TRANSACTION_CLOSED;
+	return ret;
+}
+
 char *shorten_unambiguous_ref(const char *refname, int strict)
 {
 	int i;
diff --git a/refs.h b/refs.h
index 3420c98..fa600b5 100644
--- a/refs.h
+++ b/refs.h
@@ -365,6 +365,20 @@ int ref_transaction_commit(struct ref_transaction *transaction,
 			   struct strbuf *err);
 
 /*
+ * Like ref_transaction_commit(), but optimized for creating
+ * references when originally initializing a repository (e.g., by "git
+ * clone"). It writes the new references directly to packed-refs
+ * without locking the individual references.
+ *
+ * It is a bug to call this function when there might be other
+ * processes accessing the repository or if there are existing
+ * references that might conflict with the ones being created. All
+ * old_sha1 values must either be absent or NULL_SHA1.
+ */
+int initial_ref_transaction_commit(struct ref_transaction *transaction,
+				   struct strbuf *err);
+
+/*
  * Free an existing transaction and all associated data.
  */
 void ref_transaction_free(struct ref_transaction *transaction);
-- 
2.1.4

  parent reply	other threads:[~2015-06-08 11:46 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-08 11:45 [PATCH 00/13] Improve "refs" module encapsulation Michael Haggerty
2015-06-08 11:45 ` [PATCH 01/13] delete_ref(): move declaration to refs.h Michael Haggerty
2015-06-08 16:43   ` Stefan Beller
2015-06-09 10:10     ` Michael Haggerty
2015-06-09 16:42       ` Stefan Beller
2015-06-13 14:30         ` Michael Haggerty
2015-06-08 11:45 ` [PATCH 02/13] remove_branches(): remove temporary Michael Haggerty
2015-06-08 16:45   ` Stefan Beller
2015-06-08 11:45 ` [PATCH 03/13] delete_ref(): handle special case more explicitly Michael Haggerty
2015-06-08 16:48   ` Stefan Beller
2015-06-09 10:17     ` Michael Haggerty
2015-06-08 11:45 ` [PATCH 04/13] delete_refs(): new function for the refs API Michael Haggerty
2015-06-08 11:45 ` [PATCH 05/13] delete_refs(): improve error message Michael Haggerty
2015-06-09 18:47   ` Junio C Hamano
2015-06-08 11:45 ` [PATCH 06/13] delete_refs(): convert error message to lower case Michael Haggerty
2015-06-08 16:51   ` Stefan Beller
2015-06-09 10:23     ` Michael Haggerty
2015-06-08 11:45 ` [PATCH 07/13] prune_remote(): use delete_refs() Michael Haggerty
2015-06-08 16:57   ` Stefan Beller
2015-06-08 17:12     ` Jeff King
2015-06-09 10:50       ` Michael Haggerty
2015-06-09 11:53         ` Jeff King
2015-06-08 11:45 ` [PATCH 08/13] repack_without_refs(): make function private Michael Haggerty
2015-06-08 11:45 ` Michael Haggerty [this message]
2015-06-08 11:45 ` [PATCH 10/13] refs: remove some functions from the module's public interface Michael Haggerty
2015-06-08 11:45 ` [PATCH 11/13] initial_ref_transaction_commit(): check for duplicate refs Michael Haggerty
2015-06-08 11:45 ` [PATCH 12/13] initial_ref_transaction_commit(): check for ref D/F conflicts Michael Haggerty
2015-06-08 11:45 ` [PATCH 13/13] refs: move the remaining ref module declarations to refs.h Michael Haggerty
2015-06-08 17:03 ` [PATCH 00/13] Improve "refs" module encapsulation Stefan Beller
2015-06-09 18:47 ` Junio C Hamano

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

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

  git send-email \
    --in-reply-to=7d0f71d1d15f6811658f32cad5a9e74dbd94e8c6.1433763494.git.mhagger@alum.mit.edu \
    --to=mhagger@alum.mit.edu \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=sbeller@google.com \
    /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.
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 NNTP newsgroup(s).