Re: git log --no-walk --tags produces strange result for certain user

Hi everybody,

I received one more complaint for this issue, and now it appears in a public repository https://github.com/spray/spray 

To reproduce:

# git clone https://github.com/spray/spray 
# cd spray
# git log --no-walk --tags --pretty="%H %d" --decorate=full | tail -3
3273edafcd9f9701d62e061c5257c0a09e2e1fb7  (tag: refs/tags/v0.8.0-RC1)
ff3a2946bc54da76ddb47e82c81419cc7ae3db6b  (tag: refs/tags/v0.7.0)
8b4043428b90b7f45b7241b3c2c032cf785479ce 

So here the last hash doesn't have a decoration.

Thanks for any help.
Kirill. 

On Mon, Dec 16, 2013 at 03:52:35PM +0400, Kirill Likhodedov wrote:

Hi everybody,

I received one more complaint for this issue, and now it appears in a public repository https://github.com/spray/spray 

To reproduce:

# git clone https://github.com/spray/spray 
# cd spray
# git log --no-walk --tags --pretty="%H %d" --decorate=full | tail -3
3273edafcd9f9701d62e061c5257c0a09e2e1fb7  (tag: refs/tags/v0.8.0-RC1)
ff3a2946bc54da76ddb47e82c81419cc7ae3db6b  (tag: refs/tags/v0.7.0)
8b4043428b90b7f45b7241b3c2c032cf785479ce 

So here the last hash doesn't have a decoration.

This looks like a bug:

vauxhall ok % git describe 8b4043428b90b7f45b7241b3c2c032cf785479ce
v0.5.0

I'm looking at it.

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187

git log did not correctly handle decorations when a tag object referenced
another tag object that was no longer a ref, such as when the second tag was
deleted.  The commit would not be decorated correctly because parse_object had
not been called on the second tag and therefore its tagged field had not been
filled in, resulting in none of the tags being associated with the relevant
commit.

Call parse_object to fill in this field if it is absent so that the chain of
tags can be dereferenced and the commit can be properly decorated.  Include
tests as well to prevent future regressions.

Signed-off-by: brian m. carlson <redacted>
---
 log-tree.c                    | 13 ++++++++++---
 t/t4205-log-pretty-formats.sh | 15 +++++++++++++++
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index 642faff..a6b60b7 100644
--- a/log-tree.c
+++ b/log-tree.c

@@ -131,9 +131,16 @@ static int add_ref_decoration(const char *refname, const unsigned char *sha1, in
 		refname = prettify_refname(refname);
 	add_name_decoration(type, refname, obj);
 	while (obj->type == OBJ_TAG) {
-		obj = ((struct tag *)obj)->tagged;
-		if (!obj)
-			break;
+		struct object *tagged = ((struct tag *)obj)->tagged;
+		if (!tagged) {
+			obj = parse_object(obj->sha1);
+			if (!obj)
+				break;
+			tagged = ((struct tag *)obj)->tagged;
+			if (!tagged)
+				break;
+		}
+		obj = tagged;
 		add_name_decoration(DECORATION_REF_TAG, refname, obj);
 	}
 	return 0;

diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index fb00041..2a6278b 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh

@@ -310,4 +310,19 @@ EOF
 	test_cmp expected actual
 '
 
+test_expect_success 'log decoration properly follows tag chain' '
+	git tag -a tag1 -m tag1 &&
+	git tag -a tag2 -m tag2 tag1 &&
+	git tag -d tag1 &&
+	git commit --amend -m shorter &&
+	git log --no-walk --tags --pretty="%H %d" --decorate=full >actual &&
+	cat <<EOF >expected &&
+6a908c10688b2503073c39c9ba26322c73902bb5  (tag: refs/tags/tag2)
+9f716384d92283fb915a4eee5073f030638e05f9  (tag: refs/tags/message-one)
+b87e4cccdb77336ea79d89224737be7ea8e95367  (tag: refs/tags/message-two)
+EOF
+	sort actual >actual1 &&
+	test_cmp expected actual1
+'
+
 test_done

-- 
1.8.5.1

On 12/16/2013 12:52 PM, Kirill Likhodedov wrote:

I received one more complaint for this issue, and now it appears in a public repository https://github.com/spray/spray 

To reproduce:

# git clone https://github.com/spray/spray 
# cd spray
# git log --no-walk --tags --pretty="%H %d" --decorate=full | tail -3
3273edafcd9f9701d62e061c5257c0a09e2e1fb7  (tag: refs/tags/v0.8.0-RC1)
ff3a2946bc54da76ddb47e82c81419cc7ae3db6b  (tag: refs/tags/v0.7.0)
8b4043428b90b7f45b7241b3c2c032cf785479ce 

So here the last hash doesn't have a decoration.

The problem is that reference refs/tags/v0.5.0 points at a tag object
8f6ca98087 which itself points at another tag object 2eddbcbff4 which
finally points at commit 8b4043428b.  Probably we should handle
recursive tag objects like this, but OTOH I can't think of a reason why
one would want to create them in the first place.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

On 01/16/2014 11:31 AM, Michael Haggerty wrote:

On 12/16/2013 12:52 PM, Kirill Likhodedov wrote:

quoted

I received one more complaint for this issue, and now it appears in a public repository https://github.com/spray/spray 

To reproduce:

# git clone https://github.com/spray/spray 
# cd spray
# git log --no-walk --tags --pretty="%H %d" --decorate=full | tail -3
3273edafcd9f9701d62e061c5257c0a09e2e1fb7  (tag: refs/tags/v0.8.0-RC1)
ff3a2946bc54da76ddb47e82c81419cc7ae3db6b  (tag: refs/tags/v0.7.0)
8b4043428b90b7f45b7241b3c2c032cf785479ce 

So here the last hash doesn't have a decoration.

The problem is that reference refs/tags/v0.5.0 points at a tag object
8f6ca98087 which itself points at another tag object 2eddbcbff4 which
finally points at commit 8b4043428b.  Probably we should handle
recursive tag objects like this, but OTOH I can't think of a reason why
one would want to create them in the first place.

Junio just pointed out to me that this bug has been fixed already, by
Brian Carlson, in 5e1361cc, which is already in master.  Sorry for the
noise.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/