From 7f582faa060f958410fe53091553c13edd953376 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 14 May 2026 16:25:25 +0000 Subject: [PATCH] promisor-remote: document caller filtering contract promisor_remote_get_direct() does not, on its happy path, filter out OIDs that are already present in the local object store: every OID the caller supplies is written to the fetch subprocess's stdin and ends up in the response pack. The only filtering it performs is in remove_fetched_oids(), and that only runs after a fetch failure when falling back to a different configured promisor remote. Almost every existing caller already filters locally-present OIDs out itself (typically with odb_read_object_info_extended() and OBJECT_INFO_FOR_PREFETCH, or odb_has_object() with no fetch flag). But the existing API comment does not state this expectation, so a new caller is easy to write incorrectly (I missed this originally and wrote two problematic callers). Omitting the filter still "works" in the sense that the desired objects end up local, but it silently makes the fetch request -- and the response pack -- larger than necessary, defeating part of the point of batching. Spell the contract out so future callers know to filter (and deduplicate) themselves, and point them at the helpers they should use to check local presence without accidentally triggering a lazy fetch. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- promisor-remote.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/promisor-remote.h b/promisor-remote.h index 3d4d2de018..301f5ac5cb 100644 --- a/promisor-remote.h +++ b/promisor-remote.h @@ -29,6 +29,17 @@ int repo_has_promisor_remote(struct repository *r); * Fetches all requested objects from all promisor remotes, trying them one at * a time until all objects are fetched. * + * Callers are responsible for filtering out OIDs that are already present + * locally before calling this function: every supplied OID is sent in the + * fetch request, even if the object already exists in the local object + * store. (Only after a fetch failure does this function fall back to + * stripping already-present OIDs from the list before trying the next + * configured promisor remote.) Callers should also deduplicate the OIDs. + * + * To test for local presence without triggering a lazy fetch (which would + * defeat the purpose of batching), use odb_has_object(..., 0) or + * odb_read_object_info_extended() with OBJECT_INFO_FOR_PREFETCH. + * * If oid_nr is 0, this function returns immediately. */ void promisor_remote_get_direct(struct repository *repo,