Skip to content

send-pack: add gvfs.negativeRefCheck to skip missing negatives#967

Draft
derrickstolee wants to merge 1 commit into
microsoft:vfs-2.55.0from
derrickstolee:gvfs-negative-ref-check
Draft

send-pack: add gvfs.negativeRefCheck to skip missing negatives#967
derrickstolee wants to merge 1 commit into
microsoft:vfs-2.55.0from
derrickstolee:gvfs-negative-ref-check

Conversation

@derrickstolee

Copy link
Copy Markdown

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.

  • This change only applies to interactions with Azure DevOps and the
    GVFS Protocol.

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 <dstolee@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant