From: Karthik Nayak Date: Tue, 07 Oct 2025 16:18:52 GMT Subject: Re: [PATCH 05/13] upload-pack: convert to use `reference_get_peeled_oid()` Message-ID: In-Reply-To: <20251007-b4-pks-ref-filter-skip-parsing-objects-v1-5-916cc7c6886b@pks.im> Patrick Steinhardt 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()` > - 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]