Subject: Re: [PATCH 3/3] cat-file: avoid verifying submodules' OIDs

From: Junio C Hamano <[email-masked]>
Date: Tue, 12 Mar 2024 11:35:16 -0700

"Johannes Schindelin via GitGitGadget" <[email-masked]>
writes:

> +Submodules are handled specially in `git cat-file`, as the objects
> +corresponding to the recorded OIDs are not expected to be present in the
> +current repository. For that reason, submodules are reported as having
> +type `submodule` and mode 1600000 and all other fields are zeroed out.

While the above may not be technically wrong per-se, I am not sure
if that is the more important part of what we want to tell our
users.  For example, "git ls-tree HEAD -- sha1collisiondetection"
reports "160000 commit ...object.name.... sha1collisiondetection".
Is it correct to say ...

    Submodules are handled specially in `git ls-tree`, as the
    objects corresponding to the recorded OIDs are not expected to
    be present in the current repository.

...?  I do not think so.

For the same reason, as an explanation for the reason why "git
cat-file -t :sha1collisiondetection" just reports "submodule", the
new text does not sit well.

I actually have to wonder if the new behaviour proposed by this
patch is a solution that is in search of a problem, or trying to
solve an unstated problem in a wrong way.

    O=$(git rev-parse --verify :sha1collisiondetection)
    git cat-file -t "$O"
    
should fail because the object whose name is $O is not available.
Why should then this succeed and give a different result?

    git cat-file -t :sha1collisiondetection

The "cat-file" command is about objects.  While object's type may
sometimes be inferrable (by being contained in a tree), if the user
asks us to determine the type of the object, we should actually hit
the object store, whether the commit object in question happens to
be on our history or somebody else's history that our gitlink points
at.

So, I am not yet convinced that I should take this patch.  Previous
two steps looked good, though.

Thanks.

> index 73bd78c0b63..c59ad682d1f 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -128,7 +128,9 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
>  	switch (opt) {
>  	case 't':
>  		oi.type_name = &sb;
> -		if (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0)
> +		if (obj_context.mode == S_IFGITLINK)
> +			strbuf_addstr(&sb, "submodule");
> +		else if (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0)
>  			die("git cat-file: could not get object info");