From 8a3eba9ed54af858b8bde0b88f193cdcf1eb6d79 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Fri, 24 Jul 2026 10:05:50 -0700 Subject: [PATCH] send-pack: add gvfs.negativeRefCheck to skip missing negatives When pushing from a repository that uses the GVFS Protocol, `git push` spawns `git pack-objects --all-progress-implied --revs --stdout --thin -q` and feeds it revision parameters on stdin: the advertised refs and negotiated objects as negative (exclusion) tips, plus each ref's old and new tips. Normally feed_object() omits a negative object that is not present locally, but the core.gvfs GVFS_MISSING_OK bit disables that check so that missing negatives are still fed. That bypass is actively harmful under the GVFS Protocol. pack-objects adds `--objects-edge` for `--thin` and, while marking edges uninteresting and hunting for preferred delta bases, reads the tree of every fed exclusion. For an exclusion the client does not have locally, that read lazily downloads the object, issuing one gh_client__get_ immediate request per advertised ref. A scalar clone against a server that advertises many refs therefore triggers a storm of immediate object fetches during an ordinary push. Partial clone deliberately assumes the objects behind our refs are already present and never fetches them for a push. Restore that behavior for the GVFS Protocol behind a new opt-in config, gvfs.negativeRefCheck. When set, `git push` performs a non-fetching existence check -- odb_has_object() with flags 0 implies OBJECT_INFO_QUICK | OBJECT_INFO_SKIP_FETCH_OBJECT, so the probe never fetches -- and omits any advertised object it does not have, exactly as Git does without the GVFS_MISSING_OK bit. The decision is computed once in pack_objects() because advertised ref lists can be large. The config defaults to false, so the legacy GVFS_MISSING_OK behavior is unchanged unless an operator opts in, keeping the fallout controllable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Derrick Stolee --- Documentation/config/gvfs.adoc | 12 ++++++++++++ send-pack.c | 33 +++++++++++++++++++++++++++------ 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/Documentation/config/gvfs.adoc b/Documentation/config/gvfs.adoc index b38c7ac9eb8937..dead6f00d794da 100644 --- a/Documentation/config/gvfs.adoc +++ b/Documentation/config/gvfs.adoc @@ -27,6 +27,18 @@ gvfs.fallback:: server fails to connect. This will alert users to failures with the cache server, but avoid causing throttling on the origin server. +gvfs.negativeRefCheck:: + When using the GVFS Protocol, the `core.gvfs` `GVFS_MISSING_OK` bit + allows objects behind local refs to be missing, so `git push` + normally feeds every advertised ref to `git pack-objects` as an + exclusion, even ones that are absent locally. `pack-objects` then + lazily downloads each missing exclusion while searching for delta + bases, issuing one object request per advertised ref. If set to + `true`, `git push` instead performs a non-fetching existence check + and omits any advertised object it does not have locally, matching + the behavior of Git without the `GVFS_MISSING_OK` bit. Defaults to + `false`. + gvfs.sessionKey:: If set to a string, then the value is a config key name that will be used to set a prefix in the `X-Session-Id` header sent to the server for all diff --git a/send-pack.c b/send-pack.c index 9297ad680335ce..c1c61ded76fdf8 100644 --- a/send-pack.c +++ b/send-pack.c @@ -44,9 +44,11 @@ int option_parse_push_signed(const struct option *opt, } static void feed_object(struct repository *r, - const struct object_id *oid, FILE *fh, int negative) + const struct object_id *oid, FILE *fh, int negative, + int check_missing) { - if (negative && !gvfs_config_is_set(r, GVFS_MISSING_OK) && !odb_has_object(r->objects, oid, 0)) + if (negative && check_missing && + !odb_has_object(r->objects, oid, 0)) return; if (negative) @@ -71,6 +73,8 @@ static int pack_objects(struct repository *r, struct child_process po = CHILD_PROCESS_INIT; FILE *po_in; int rc; + int negative_ref_check = 0; + int check_missing; trace2_region_enter("send_pack", "pack_objects", r); strvec_push(&po.args, "pack-objects"); @@ -103,16 +107,33 @@ static int pack_objects(struct repository *r, * parameters by writing to the pipe. */ po_in = xfdopen(po.in, "w"); + + /* + * Normally we omit a negative (exclusion) object that we do not have + * locally. The core.gvfs GVFS_MISSING_OK bit disables that check, so + * missing negatives are still fed to pack-objects. Under the GVFS + * protocol that is harmful: pack-objects treats each fed exclusion as + * an edge and lazily downloads every one that is absent while hunting + * for preferred delta bases -- one immediate object request per + * advertised ref. Setting gvfs.negativeRefCheck restores the vanilla + * behavior of omitting a negative object we do not have, using a + * non-fetching existence check (odb_has_object() with flags 0 implies + * OBJECT_INFO_QUICK | OBJECT_INFO_SKIP_FETCH_OBJECT). + */ + repo_config_get_bool(r, "gvfs.negativeRefCheck", &negative_ref_check); + check_missing = negative_ref_check || + !gvfs_config_is_set(r, GVFS_MISSING_OK); + for (size_t i = 0; i < advertised->nr; i++) - feed_object(r, &advertised->oid[i], po_in, 1); + feed_object(r, &advertised->oid[i], po_in, 1, check_missing); for (size_t i = 0; i < negotiated->nr; i++) - feed_object(r, &negotiated->oid[i], po_in, 1); + feed_object(r, &negotiated->oid[i], po_in, 1, check_missing); while (refs) { if (!is_null_oid(&refs->old_oid)) - feed_object(r, &refs->old_oid, po_in, 1); + feed_object(r, &refs->old_oid, po_in, 1, check_missing); if (!is_null_oid(&refs->new_oid)) - feed_object(r, &refs->new_oid, po_in, 0); + feed_object(r, &refs->new_oid, po_in, 0, check_missing); refs = refs->next; }