# git log --no-walk --tags produces strange result for certain user 11 messages from 2013-12-03 to 2014-01-17. Participants: Kirill Likhodedov, brian m. carlson, Junio C Hamano, Michael Haggerty. Thread: https://gitlist.dev/t/35461 ## Kirill Likhodedov, 2013-12-03 10:08 Subject: git log --no-walk --tags produces strange result for certain user Message-ID: <5EE449B7-AB75-4EFF-85F9-292727FA1C53@jetbrains.com> URL: https://gitlist.dev/e/5EE449B7-AB75-4EFF-85F9-292727FA1C53%40jetbrains.com ``` 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) a7fda708d76d7f83d5a160b6b137b98b7677f771 (tag: refs/tags/2.44) f119c2e7c69bb5ed1da5bae29c8754550ab96bde 07385a6ebe5a2e01e6ba9c8d0cb7b15c9a13f65d (tag: refs/tags/1.69) 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. ``` ## Kirill Likhodedov, 2013-12-07 15:04 Subject: Re: git log --no-walk --tags produces strange result for certain user Message-ID: <01D3F259-94E1-4084-9210-34E2DB7B7545@jetbrains.com> URL: https://gitlist.dev/e/01D3F259-94E1-4084-9210-34E2DB7B7545%40jetbrains.com In-Reply-To: <5EE449B7-AB75-4EFF-85F9-292727FA1C53@jetbrains.com> ``` 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 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) > a7fda708d76d7f83d5a160b6b137b98b7677f771 (tag: refs/tags/2.44) > f119c2e7c69bb5ed1da5bae29c8754550ab96bde > 07385a6ebe5a2e01e6ba9c8d0cb7b15c9a13f65d (tag: refs/tags/1.69) > > 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. ``` ## Kirill Likhodedov, 2013-12-16 11:52 Subject: Re: git log --no-walk --tags produces strange result for certain user Message-ID: URL: https://gitlist.dev/e/AEF2BC21-6EE1-479F-BC63-7A5B95406A7F%40jetbrains.com In-Reply-To: <52AEB181.9020006@alum.mit.edu> ``` 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. ``` ## brian m. carlson, 2013-12-17 00:40 Subject: Re: git log --no-walk --tags produces strange result for certain user Message-ID: <20131217004044.GB259467@vauxhall.crustytoothpaste.net> URL: https://gitlist.dev/e/20131217004044.GB259467%40vauxhall.crustytoothpaste.net In-Reply-To: ``` 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 ``` ## brian m. carlson, 2013-12-17 04:28 Subject: [PATCH] log: properly handle decorations with chained tags Message-ID: <1387254501-319329-1-git-send-email-sandals@crustytoothpaste.net> URL: https://gitlist.dev/e/1387254501-319329-1-git-send-email-sandals%40crustytoothpaste.net In-Reply-To: <20131217004044.GB259467@vauxhall.crustytoothpaste.net> ``` 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 --- 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 <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 ``` ## Junio C Hamano, 2013-12-18 00:36 Subject: Re: [PATCH] log: properly handle decorations with chained tags Message-ID: URL: https://gitlist.dev/e/xmqq38lrknbd.fsf%40gitster.dls.corp.google.com In-Reply-To: <1387254501-319329-1-git-send-email-sandals@crustytoothpaste.net> ``` "brian m. carlson" 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 <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 ``` ## brian m. carlson, 2013-12-19 03:18 Subject: Re: [PATCH] log: properly handle decorations with chained tags Message-ID: <20131219031809.GG259467@vauxhall.crustytoothpaste.net> URL: https://gitlist.dev/e/20131219031809.GG259467%40vauxhall.crustytoothpaste.net In-Reply-To: ``` 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 ``` ## Junio C Hamano, 2013-12-19 18:44 Subject: Re: [PATCH] log: properly handle decorations with chained tags Message-ID: URL: https://gitlist.dev/e/xmqqfvpofzp5.fsf%40gitster.dls.corp.google.com In-Reply-To: <20131219031809.GG259467@vauxhall.crustytoothpaste.net> ``` "brian m. carlson" 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. ``` ## brian m. carlson, 2013-12-19 23:44 Subject: Re: [PATCH] log: properly handle decorations with chained tags Message-ID: <20131219234442.GI259467@vauxhall.crustytoothpaste.net> URL: https://gitlist.dev/e/20131219234442.GI259467%40vauxhall.crustytoothpaste.net In-Reply-To: ``` On Thu, Dec 19, 2013 at 10:44:22AM -0800, Junio C Hamano wrote: > "brian m. carlson" 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 ``` ## Michael Haggerty, 2014-01-16 10:31 Subject: Re: git log --no-walk --tags produces strange result for certain user Message-ID: <52D7B4F8.1010209@alum.mit.edu> URL: https://gitlist.dev/e/52D7B4F8.1010209%40alum.mit.edu In-Reply-To: ``` 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/ ``` ## Michael Haggerty, 2014-01-17 08:28 Subject: Re: git log --no-walk --tags produces strange result for certain user Message-ID: <52D8E992.3010304@alum.mit.edu> URL: https://gitlist.dev/e/52D8E992.3010304%40alum.mit.edu In-Reply-To: <52D7B4F8.1010209@alum.mit.edu> ``` 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/ ```