Subject: Re: [PATCH v2 10/10] tag: don't misreport type of tagged objects in errors

From: Junio C Hamano <[email-masked]>
Date: Mon, 29 Mar 2021 22:50:18 -0700

Ævar Arnfjörð Bjarmason  <[email-masked]> writes:

> Fix a regression in 89e4202f982 ([PATCH] Parse tags for absent
> objects, 2005-06-21) (yes, that ancient!) and correctly report an
> error on a tag like:
>
>     object <a tree hash>
>     type commit
>
> As:
>
>     error: object <a tree hash> is tree, not a commit
>
> Instead of our long-standing misbehavior of inverting the two, and
> reporting:
>
>     error: object <a tree hash> is commit, not a tree
>
> Which, as can be trivially seen with 'git cat-file -t <a tree hash>'
> is incorrect.

Hmph, I've always thought it is just "supposed to be a" missing in
the sentence ;-)

> Hence the non-intuitive solution of adding a
> lookup_{blob,commit,tag,tree}_type() function. It's to distinguish
> calls from parse_object_buffer() where we actually know the type, from
> a parse_tag_buffer() where we're just guessing about the type.

I think it makes sense to allow the caller to express distinction
between "I know that this object is a blob, because I just read its
object header" and "Another object tells me that this object must be
a blob, because it is in a tree entry whose mode bits are 100644".

I wish we found a set of names better than lookup_<type>_type() for
that, though.  It's just between

      lookup_tag_type(r, oid, OBJ_NONE);
      lookup_tag_type(r, oid, OBJ_TAG);

I cannot quite tell which one is which.  I also wonder if the last
arg should just be a boolean ("I know it is a tag" vs "I heard it
must be a tag").