Re: [PATCH 5/5] load_ref_decorations(): avoid parsing non-tag objects

[Taylor Blau <me@ttaylorr.com>]

On Tue, Jun 22, 2021 at 12:08:40PM -0400, Jeff King wrote:
> If the packed-refs file ever learns to store all of the peeled layers,
> then we could switch to it. Or even if it stored a flag to indicate the
> peel was not multi-layer (because most of them aren't), then we could
> use it most of the time and fall back to a manual peel for the rare
> cases.

Yeah, I would be in favor of either of these. Of the two, the latter
seems like the simplest thing, but the former provides you all of the
information you could hope for.

I suppose that if you are already changing the format of packed-refs,
then we might as well do the thing which provides the most information
and allows us to optimize *all* cases, not just the vast majority of
them.

Of course, that's all way outside of the scope of this patch, which
shouldn't have to deal with either of those.

> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  log-tree.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/log-tree.c b/log-tree.c
> index 7b823786c2..8b700e9c14 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -134,6 +134,7 @@ static int add_ref_decoration(const char *refname, const struct object_id *oid,
>  			      int flags, void *cb_data)
>  {
>  	struct object *obj;
> +	enum object_type objtype;
>  	enum decoration_type type = DECORATION_NONE;
>  	struct decoration_filter *filter = (struct decoration_filter *)cb_data;
>
> @@ -155,9 +156,10 @@ static int add_ref_decoration(const char *refname, const struct object_id *oid,
>  		return 0;
>  	}
>
> -	obj = parse_object(the_repository, oid);
> -	if (!obj)
> +	objtype = oid_object_info(the_repository, oid, NULL);
> +	if (type < 0)
>  		return 0;
> +	obj = lookup_object_by_type(the_repository, oid, objtype);

The comments about s/type/obj&/ aside, this looks good to me (and the
amended version below looks ready to be picked up, at least in my eyes).

One thing I did want to note which is elided by the limited context is
that we *do* parse tags like you say, it's just hidden in the "while
(obj->type == OBJ_TAG)" loop below.

So that's doing the right thing, but it wasn't clear from the limited
context here that this patch was immediately correct.

Thanks,
Taylor