Re: [PATCH 2/2] attr: add flag `-r|--revisions` to work with revisions

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Wed, Dec 07 2022, Karthik Nayak wrote:

> On Wed, Dec 7, 2022 at 1:12 AM Junio C Hamano <gitster@pobox.com> wrote:
> [...]
>> Also, at the C API level, I suspect that we strongly prefer to pass
>> around either the "struct object_id *" or "struct tree *", not working
>> with end-user supplied character strings without any sanity-checking
>> or parsing.
>>
>
> I must admit, I did take the path of least resistance here. So we finally need
> to parse the `revision:<pathname>` where the `<pathname>` is generated
> dynamically as we move through the check-attr stack.
>
> My question is, if I generate an `object_id` at the start (in
> builtin/check-attr.c)
> with only the `revision`, is there a way to traverse to find the blob
> for each of
> the different <pathnames>? I haven't looked at Git code for a while now, and
> I'm not sure what the best way to do this. Maybe I've missed some API which
> would make this simple, any help is appreciated.

The get_oid() that you're doing now happens in a loop, and should be
pulled out of it.

I suggested making that a feature in
https://lore.kernel.org/git/221207.86lenja0zi.gmgdl@evledraar.gmail.com/;
but if you keep the interface you've got where you only support a single
<rev> it would make the most sense to do that get_oid() in the builtin/
code at the start, and then pass the oid/path pair down.

You'd still need to call read_object_file() or equivalent for each
<rev>:<path> pair though.