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

From: Karthik Nayak <karthik.188@gmail.com>
Date: Dec 7, 2022, 12:29 UTC
Message-ID: <CAOLa=ZTQ9R1dyCb5xp0c5JBLjE33dJFVPg8jQfprU1iSGq+O4g@mail.gmail.com>
In-Reply-To: <221207.86h6y7a0l7.gmgdl@evledraar.gmail.com>

On Wed, Dec 7, 2022 at 12:56 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 35 quoted lines

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

That's exactly my question, now we're calling `get_oid()` with the pathname. Which would directly give us the object_id for the blob. Whereas if we call `get_oid()` _without_ the pathname, we would still need to get the object_id for the blob.

My specific question being, how do I, given:

struct object_id oid_1; get_oid(revision, &oid)

and pathname, get the equivalent of:

struct object_id oid_2; strbuf_addf(&sb, "%s:%s", revision, path); get_oid(sb.buf, &oid)

Basically, I fail to see how to transform oid_1 to oid_2, using pathname. I agree this would eventually be fed into `read_object_file`.

-- 
- Karthik