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

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Dec 7, 2022, 11:53 UTC
Message-ID: <221207.86h6y7a0l7.gmgdl@evledraar.gmail.com>
In-Reply-To: <CAOLa=ZS5k=s98Bo9GE+RRa3jtanehL35y-hhLhy1DoM7GyO0cQ@mail.gmail.com>

On Wed, Dec 07 2022, Karthik Nayak wrote:

Show 19 quoted lines

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