git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
	Michael Haggerty <mhagger@alum.mit.edu>
Subject: [PATCH 1/2] pack-refs: write peeled entry for non-tags
Date: Sat, 16 Mar 2013 05:01:03 -0400	[thread overview]
Message-ID: <20130316090103.GA26855@sigill.intra.peff.net> (raw)
In-Reply-To: <20130316090018.GA26708@sigill.intra.peff.net>

When we pack an annotated tag ref, we write not only the
sha1 of the tag object along with the ref, but also the sha1
obtained by peeling the tag. This lets readers of the
pack-refs file know the peeled value without having to
actually load the object, speeding up upload-pack's ref
advertisement.

The writer marks a packed-refs file with peeled refs using
the "peeled" trait at the top of the file. When the reader
sees this trait, it knows that each ref is either followed
by its peeled value, or it is not an annotated tag.

However, there is a mismatch between the assumptions of the
reader and writer. The writer will only peel refs under
refs/tags, but the reader does not know this; it will assume
a ref without a peeled value must not be a tag object. Thus
an annotated tag object placed outside of the refs/tags
hierarchy will not have its peeled value printed by
upload-pack.

The simplest way to fix this is to start writing peel values
for all refs. This matches what the reader expects for both
new and old versions of git.

Signed-off-by: Jeff King <peff@peff.net>
---
 pack-refs.c         | 16 ++++++++--------
 t/t3211-peel-ref.sh | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 8 deletions(-)
 create mode 100755 t/t3211-peel-ref.sh

diff --git a/pack-refs.c b/pack-refs.c
index f09a054..10832d7 100644
--- a/pack-refs.c
+++ b/pack-refs.c
@@ -27,6 +27,7 @@ static int handle_one_ref(const char *path, const unsigned char *sha1,
 			  int flags, void *cb_data)
 {
 	struct pack_refs_cb_data *cb = cb_data;
+	struct object *o;
 	int is_tag_ref;
 
 	/* Do not pack the symbolic refs */
@@ -39,14 +40,13 @@ static int handle_one_ref(const char *path, const unsigned char *sha1,
 		return 0;
 
 	fprintf(cb->refs_file, "%s %s\n", sha1_to_hex(sha1), path);
-	if (is_tag_ref) {
-		struct object *o = parse_object(sha1);
-		if (o->type == OBJ_TAG) {
-			o = deref_tag(o, path, 0);
-			if (o)
-				fprintf(cb->refs_file, "^%s\n",
-					sha1_to_hex(o->sha1));
-		}
+
+	o = parse_object(sha1);
+	if (o->type == OBJ_TAG) {
+		o = deref_tag(o, path, 0);
+		if (o)
+			fprintf(cb->refs_file, "^%s\n",
+				sha1_to_hex(o->sha1));
 	}
 
 	if ((cb->flags & PACK_REFS_PRUNE) && !do_not_prune(flags)) {
diff --git a/t/t3211-peel-ref.sh b/t/t3211-peel-ref.sh
new file mode 100755
index 0000000..dd5b48b
--- /dev/null
+++ b/t/t3211-peel-ref.sh
@@ -0,0 +1,42 @@
+#!/bin/sh
+
+test_description='tests for the peel_ref optimization of packed-refs'
+. ./test-lib.sh
+
+test_expect_success 'create annotated tag in refs/tags' '
+	test_commit base &&
+	git tag -m annotated foo
+'
+
+test_expect_success 'create annotated tag outside of refs/tags' '
+	git update-ref refs/outside/foo refs/tags/foo
+'
+
+# This matches show-ref's output
+print_ref() {
+	echo "`git rev-parse "$1"` $1"
+}
+
+test_expect_success 'set up expected show-ref output' '
+	{
+		print_ref "refs/heads/master" &&
+		print_ref "refs/outside/foo" &&
+		print_ref "refs/outside/foo^{}" &&
+		print_ref "refs/tags/base" &&
+		print_ref "refs/tags/foo" &&
+		print_ref "refs/tags/foo^{}"
+	} >expect
+'
+
+test_expect_success 'refs are peeled outside of refs/tags (loose)' '
+	git show-ref -d >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'refs are peeled outside of refs/tags (packed)' '
+	git pack-refs --all &&
+	git show-ref -d >actual &&
+	test_cmp expect actual
+'
+
+test_done
-- 
1.8.2.rc2.7.gef06216

  reply	other threads:[~2013-03-16  9:01 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-16  9:00 [PATCH 0/2] peel-ref optimization fixes Jeff King
2013-03-16  9:01 ` Jeff King [this message]
2013-03-16 13:50   ` [PATCH 1/2] pack-refs: write peeled entry for non-tags Michael Haggerty
2013-03-17  6:02     ` Jeff King
2013-03-16  9:01 ` [PATCH 2/2] pack-refs: add fully-peeled trait Jeff King
2013-03-16 14:06   ` Michael Haggerty
2013-03-17  6:04     ` Jeff King
2013-03-17  5:50   ` Junio C Hamano
2013-03-17  5:55     ` Jeff King
2013-03-17  8:21 ` [PATCH v2 0/4] peel-ref optimization fixes Jeff King
2013-03-17  8:22   ` [PATCH v2 1/4] avoid segfaults on parse_object failure Jeff King
2013-03-17  8:23   ` [PATCH v2 2/4] use parse_object_or_die instead of die("bad object") Jeff King
2013-03-17  8:23   ` [PATCH v2 3/4] pack-refs: write peeled entry for non-tags Jeff King
2013-03-17  8:28   ` [PATCH v2 4/4] pack-refs: add fully-peeled trait Jeff King
2013-03-17 20:01     ` Junio C Hamano
2013-03-18 11:37       ` [PATCH v3 " Jeff King
2013-03-18 16:26         ` Junio C Hamano
2013-03-18  3:12     ` [PATCH v2 " Michael Haggerty
2013-03-18 15:12       ` 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=20130316090103.GA26855@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mhagger@alum.mit.edu \
    /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).