"brian m. carlson" <sand...@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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to