Re: [PATCH] log: properly handle decorations with chained tags
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 18, 2013, 00:36 UTC
- Message-ID
- <xmqq38lrknbd.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <1387254501-319329-1-git-send-email-sandals@crustytoothpaste.net>
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
Show 18 quoted lines
> 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