Re: [PATCH 05/13] upload-pack: convert to use `reference_get_peeled_oid()`

From: Karthik Nayak <karthik.188@gmail.com>
Date: Oct 7, 2025, 16:18 UTC
Message-ID: <CAOLa=ZRdcXUQLXK1s1JLgZAcEYx=kT-eS6CMzCocJ9Oenia_Jw@mail.gmail.com>
In-Reply-To: <20251007-b4-pks-ref-filter-skip-parsing-objects-v1-5-916cc7c6886b@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> The `write_v0_ref()` callback is invoked from two callsites:
>

Okay so this function does multiple things based on whether the capabilities are already advertised or not.

- If not, we propagate the capabilities and set the static variable
  `capabilities` to NULL and also set `data->sent_capabilities = 1;`.
  - We receive `ref->oid` as a zero oid for the hash algorithm being
    used, we convert it to the hex format with `oid_to_hex()`.
- If already advertised, we simply propagate the reference and if it can
  be peeled, also propagate the peeled reference.

Not for your series: but this feels like the capabilities should be an independent function.

>   - Once via `send_ref()` which is a callback passed to
>     `for_each_namespaced_ref_1()`.
>

and passed to `refs_head_ref_namespaced()`

Show 12 quoted lines

>   - Once manually to announce capabilities.
>
> When sending references to the client we also send the peeled value of
> tags. As we don't have a `struct reference` available in the second
> case, we cannot easily peel by calling `reference_get_peeled_oid()`, but
> we instead have to depend on on global state via `peel_iterated_oid()`.
>
> We do have a reference available though in the first case, it's only the
> second case that keeps us from using `reference_get_peeled_oid()`. But
> that second case only announces capabilities anyway, so we're not really
> handling a reference at all here.
>

Yup, this was my inference above as well.

> Adapt that case to construct a reference manually and pass that to
> `write_v0_ref()`. Start to use `reference_get_peeled_oid()` now that we
> always have a `struct reference` available.
>

This is a fair solution for now. I think this also shows that these two operation modes should definitely be separated out.

[snip]