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

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 8, 2025, 15:17 UTC
Message-ID: <xmqqjzb5pc6t.fsf@gitster.g>
In-Reply-To: <20250108034012.211043-1-jltobler@gmail.com>

Justin Tobler <jltobler@gmail.com> writes:

Show 8 quoted lines

> 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).

Show 11 quoted lines

> 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.