Subject: Re: [PATCH] rev-list: print missing object type with --missing=print-type

From: Junio C Hamano <[email-masked]>
Date: Wed, 08 Jan 2025 07:17:30 -0800

Justin Tobler <[email-masked]> writes:

> Handling of missing objects encounted by git-rev-list(1) can be
> configured with the `--missing=<action>` option and specifying the
> desired action. Of the available missing actions, none provide a way to
> print additional information about the missing object such as its type.
>
> Add a new missing action called `print-type`. Similar to `print`, this
> action prints a list of missing objects but also includes the object
> type if available in the form: `?<oid> [type]`.

This part needs to explain where the type information comes from and
what its significance is (see below for more details).

> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
> index 459e5a02f5..277a0b645e 100644
> --- a/Documentation/rev-list-options.txt
> +++ b/Documentation/rev-list-options.txt
> @@ -1024,6 +1024,9 @@ Unexpected missing objects will raise an error.
>  The form '--missing=print' is like 'allow-any', but will also print a
>  list of the missing objects.  Object IDs are prefixed with a ``?'' character.
>  +
> +The form '--missing=print-type' is like 'print', but will also print the
> +missing object type information if available in the form `?<oid> [type]`.
> ++

The users need to be told what this "type" information really means,
as its meaning is quite different from what "git cat-file -t <oid>"
would give them.  We do not have the object, so we are not learning
its type from the object itself.  How much trust should the users
put in this information, for example?

That comes back to the "where does it come from" that the future
readers of "git log" and reviewers need to be told by the proposed
log message.  Knowing the internals, I know you'd be getting it from
the "containing" objects, e.g., an object name that was found on the
"parent" object header field of another commit, which is _expected_
to be a commit, or an object name that was found in a tree entry
whose mode bits were 100644, which is _expected_ to be a blob, etc.

There are other places that you _could_ glean information about
(possibly missing) objects.  An object that is found during
"rev-list --objects" traversal (which is the topic of this patch
after all) but turned out to be missing may not just have an
expected type (because it was found in a tree object that we
successfully read) but also the full path to the object in the
top-level tree, for example.

In modern Git, there are even more places that you may be able to
use, like commit-graph that not just hints the object itself is a
commit, but what its parents are and when the commit was created.

Note that I am not suggesting to implement more code to learn "type"
information from more places than the current patch is doing.  At
least not in this iteration of the patch.  What I am getting at is
that it would help us to avoid unnecessarily limiting ourselves by
stressing on "type" too much if we at least imagine what the
possible sources of these extra pieces of information are and what
they could provide.

As I suspect that we would want to leave the door open for us to
extend this later, I would perhaps suggest an output format format
like:

    ?<object name> [<token>=<value>]...

where <token> tells what kind of extra information it is.  I expect
that the initial implementation only knows about "type" as the
<token>.  For future extensibility, we only need to say that under
the syntax:

 (1) How multiple attributes are shown?
 (2) How would a <value> with SP or LF in it is represented?

My suggestion is to have multiple <token>=<value> on the same line,
with a SP in between, and problematic bytes in <value> are quoted,
using cquote(). i.e. a <token>=<value> whose <value> part does not
begin with a double-quote ends at the first SP after it, otherwise
<value> is taken as a C-quoted string inside a pair of double-quote.

If you are adventurous, I would not mind seeing "path" implemented
as another token, since that would be fairly easily obtainable, but
it does not have to be in the initial attempt.

Thanks.