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

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

[Kirill Likhodedov]

Hey,

I use the following commands to receive the list of tags together with hashes the point to:
  
    git log --tags --no-walk --format=%H%d%x01 --decorate=full

Generally it works fine, however a user reported that for his repository this command returns the list containing several hashes without tag references. Something like this:

    05c9a3a6247698ff740ca3a79828456347afdcef (HEAD, tag: refs/tags/2.33, refs/remotes/origin/master, refs/remotes/origin/HEAD, refs/heads/master)\x01
    a7fda708d76d7f83d5a160b6b137b98b7677f771 (tag: refs/tags/2.44)
    f119c2e7c69bb5ed1da5bae29c8754550ab96bde\x01
    07385a6ebe5a2e01e6ba9c8d0cb7b15c9a13f65d (tag: refs/tags/1.69)\x01

Here third hash doesn't have a reference. There are 3 such hashes in his repository.

How can this happen? Is it a bug or some special scenario?

* I've already asked the user to execute `git tag --points-at` on these "suspiciously tagged" hashes: nothing was returned.
* `git show --decorate=full` executed on these hashes return commit details, and no references on them.
* From the log user sees that these hashes indicate some "normal" commits, nothing special at first glance.

Git version that he uses is 1.8.4.0.

Thanks!

-- Kirill.

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

[Kirill Likhodedov]

Sorry for the duplication, but does anyone have an idea about this issue?

I'm worried because I've already received reports from 3 users, and I'm not sure if just skipping these "pseudo-tags" is a correct decision.

Thanks for any help!

-- Kirill.

On Dec 3, 2013, at 14:08 , Kirill Likhodedov <Kirill.Likhodedov@jetbrains.com> wrote:

> Hey,
> 
> I use the following commands to receive the list of tags together with hashes the point to:
> 
>    git log --tags --no-walk --format=%H%d%x01 --decorate=full
> 
> Generally it works fine, however a user reported that for his repository this command returns the list containing several hashes without tag references. Something like this:
> 
>    05c9a3a6247698ff740ca3a79828456347afdcef (HEAD, tag: refs/tags/2.33, refs/remotes/origin/master, refs/remotes/origin/HEAD, refs/heads/master)\x01
>    a7fda708d76d7f83d5a160b6b137b98b7677f771 (tag: refs/tags/2.44)
>    f119c2e7c69bb5ed1da5bae29c8754550ab96bde\x01
>    07385a6ebe5a2e01e6ba9c8d0cb7b15c9a13f65d (tag: refs/tags/1.69)\x01
> 
> Here third hash doesn't have a reference. There are 3 such hashes in his repository.
> 
> How can this happen? Is it a bug or some special scenario?
> 
> * I've already asked the user to execute `git tag --points-at` on these "suspiciously tagged" hashes: nothing was returned.
> * `git show --decorate=full` executed on these hashes return commit details, and no references on them.
> * From the log user sees that these hashes indicate some "normal" commits, nothing special at first glance.
> 
> Git version that he uses is 1.8.4.0.
> 
> Thanks!
> 
> -- Kirill.

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

[Kirill Likhodedov]

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. 

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

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 959 bytes --]

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH] log: properly handle decorations with chained tags

[brian m. carlson]

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 <sandals@crustytoothpaste.net>
---
 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

Re: [PATCH] log: properly handle decorations with chained tags

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

>  		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);
>  	}

OK, the above is not wrong per-se but it took me three reads to
convince myself that I understood what was going on.

Before entering this loop, obj has already been parsed, it is known
to be an annotated tag object, and its obj->tagged field is valid,
but the object pointed at by the tag may still not be parsed yet.
The object given to add_name_decoration() before we enter the loop
has been parsed, but the one given at the end of this loop is not.

I think all we need to do, in addition to what the existing code
does, is to make sure that we _parse_ the object that the tag points
at, to avoid this problem.  Something like this, perhaps, instead?

diff --git a/log-tree.c b/log-tree.c
index 8534d91..1982631 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -132,10 +132,12 @@ static int add_ref_decoration(const char *refname, const unsigned char *sha1, in
 	add_name_decoration(type, refname, obj);
 	while (obj->type == OBJ_TAG) {
 		obj = ((struct tag *)obj)->tagged;
 		if (!obj)
 			break;
+		if (!obj->parsed)
+			parse_object(obj->sha1);
 		add_name_decoration(DECORATION_REF_TAG, refname, obj);
 	}
 	return 0;
 }
 
It seems to me that the above is not just sufficient, but also shows
what the breakage was really about a lot more clearly, at least to
me.

Hmm?

> 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

Re: [PATCH] log: properly handle decorations with chained tags

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 679 bytes --]

On Tue, Dec 17, 2013 at 04:36:06PM -0800, Junio C Hamano wrote:
> I think all we need to do, in addition to what the existing code
> does, is to make sure that we _parse_ the object that the tag points
> at, to avoid this problem.  Something like this, perhaps, instead?

Yeah, that's the clean fix I was looking for, but couldn't quite come up
with.  I'm going to re-roll with your fix instead of mine and my tests.
Any objections to adding your sign-off?

-- 
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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] log: properly handle decorations with chained tags

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> On Tue, Dec 17, 2013 at 04:36:06PM -0800, Junio C Hamano wrote:
>> I think all we need to do, in addition to what the existing code
>> does, is to make sure that we _parse_ the object that the tag points
>> at, to avoid this problem.  Something like this, perhaps, instead?
>
> Yeah, that's the clean fix I was looking for, but couldn't quite come up
> with.  I'm going to re-roll with your fix instead of mine and my tests.
> Any objections to adding your sign-off?

I was actually planning to just squash 35a34ce281 as a fixup! to
your 5e65a201 and call it a day.  I think your proposed log message
in the latter clearly describes why the fix is correct.

Re: [PATCH] log: properly handle decorations with chained tags

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 773 bytes --]

On Thu, Dec 19, 2013 at 10:44:22AM -0800, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> > Yeah, that's the clean fix I was looking for, but couldn't quite come up
> > with.  I'm going to re-roll with your fix instead of mine and my tests.
> > Any objections to adding your sign-off?
> 
> I was actually planning to just squash 35a34ce281 as a fixup! to
> your 5e65a201 and call it a day.  I think your proposed log message
> in the latter clearly describes why the fix is correct.

Okay, sounds good.  Thanks.

-- 
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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Michael Haggerty]

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/

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

[Michael Haggerty]

On 01/16/2014 11:31 AM, Michael Haggerty wrote:
> 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.

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/