All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Ronnie Sahlberg <sahlberg@google.com>,
	Michael Haggerty <mhagger@alum.mit.edu>
Subject: [PATCH 5/5] fast-import: stop using lock_ref_sha1
Date: Sat, 23 Aug 2014 01:33:22 -0400	[thread overview]
Message-ID: <20140823053322.GE18075@peff.net> (raw)
In-Reply-To: <20140823052334.GA17813@peff.net>

We can use lock_any_ref_for_update instead. Besides being
more flexible, the only difference between the two is that
lock_ref_sha1 does not allow "top-level" refs like
"refs/foo" to be updated. However, we know that we do not
have such a ref, because we explicitly add the "tags/"
prefix ourselves.

Note that we now must feed the whole name "refs/tags/X"
instead of just "tags/X" to the function. As a result, our
failure error message is uses the longer name. This is
probably a good thing, though.

As an interesting side note, if we forgot to switch this
input to the function, the tests do not currently catch it.
We import a tag "X" and then check that we can access it at
"tags/X". If we accidentally created "tags/X" at the
top-level $GIT_DIR instead of under "refs/", we would still
find it due to our ref lookup procedure!

We do not make such a mistake in this patch, of course, but
while we're thinking about it, let's make the fast-import
tests more robust by checking for fully qualified
refnames.

Signed-off-by: Jeff King <peff@peff.net>
---
As I mentioned, I'd be OK with dropping this one in favor of just
waiting for Ronnie's transaction patches to graduate.

 fast-import.c          | 4 ++--
 t/t9300-fast-import.sh | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index a1479e9..04a85a4 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1738,8 +1738,8 @@ static void dump_tags(void)
 
 	for (t = first_tag; t; t = t->next_tag) {
 		strbuf_reset(&ref_name);
-		strbuf_addf(&ref_name, "tags/%s", t->name);
-		lock = lock_ref_sha1(ref_name.buf, NULL);
+		strbuf_addf(&ref_name, "refs/tags/%s", t->name);
+		lock = lock_any_ref_for_update(ref_name.buf, NULL, 0, NULL);
 		if (!lock || write_ref_sha1(lock, t->sha1, msg) < 0)
 			failure |= error("Unable to update %s", ref_name.buf);
 	}
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 5fc9ef2..f4c6673 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -153,7 +153,7 @@ tag series-A
 An annotated tag without a tagger
 EOF
 test_expect_success 'A: verify tag/series-A' '
-	git cat-file tag tags/series-A >actual &&
+	git cat-file tag refs/tags/series-A >actual &&
 	test_cmp expect actual
 '
 
@@ -165,7 +165,7 @@ tag series-A-blob
 An annotated tag that annotates a blob.
 EOF
 test_expect_success 'A: verify tag/series-A-blob' '
-	git cat-file tag tags/series-A-blob >actual &&
+	git cat-file tag refs/tags/series-A-blob >actual &&
 	test_cmp expect actual
 '
 
@@ -232,8 +232,8 @@ EOF
 test_expect_success \
 	'A: tag blob by sha1' \
 	'git fast-import <input &&
-	git cat-file tag tags/series-A-blob-2 >actual &&
-	git cat-file tag tags/series-A-blob-3 >>actual &&
+	git cat-file tag refs/tags/series-A-blob-2 >actual &&
+	git cat-file tag refs/tags/series-A-blob-3 >>actual &&
 	test_cmp expect actual'
 
 test_tick
-- 
2.1.0.346.ga0367b9

  parent reply	other threads:[~2014-08-23  5:33 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-23  5:23 [PATCH 0/5] fix pack-refs pruning of refs/foo Jeff King
2014-08-23  5:26 ` [PATCH 1/5] git-prompt: do not look for refs/stash in $GIT_DIR Jeff King
2014-08-23  5:27 ` [PATCH 2/5] pack-refs: prune top-level refs like "refs/foo" Jeff King
2014-08-23  5:27 ` [PATCH 3/5] fast-import: clean up pack_data pointer in end_packfile Jeff King
2014-08-25 17:16   ` Ronnie Sahlberg
2014-08-25 18:55     ` Jeff King
2014-08-23  5:32 ` [PATCH 4/5] fast-import: fix buffer overflow in dump_tags Jeff King
2014-08-25 17:11   ` Ronnie Sahlberg
2014-08-23  5:33 ` Jeff King [this message]
2014-08-25 17:22   ` [PATCH 5/5] fast-import: stop using lock_ref_sha1 Ronnie Sahlberg
2014-08-23  7:29 ` [PATCH 0/5] fix pack-refs pruning of refs/foo Michael Haggerty
2014-08-23  7:46   ` Jeff King
2014-08-25 17:38 ` Ronnie Sahlberg
2014-08-25 18:56   ` Jeff King

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=20140823053322.GE18075@peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=mhagger@alum.mit.edu \
    --cc=sahlberg@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.