Repository navigation
Check pushed tips even when the pack is empty - #42
Merged
Merged
Conversation
A ref-only push carries a 32-byte pack with zero objects, and receive-pack skipped the connectivity check whenever ingest returned Ok(None). A command line naming an object the server does not have was therefore accepted, the ref was published to the WAL, and every clone that walked it failed with "missing object" (tobi#37, reported with a repro by czk-aa). The check now runs whenever unpack succeeded and any update names a non-zero tip. With wal.check_connectivity the existing walk covers the tips and everything new under them and stops at existing refs, so a legitimate ref-only push costs a few lookups; with it turned off the tips are still looked up one by one before anything is published. The e2e test posts the exact empty pack from the report against both settings and expects ng refs/heads/ghost, then pushes main to a new branch, which sends the same zero-object pack, and expects it to land. The test fails on main at the ng assertion and passes here. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CSbhRh6UEjFrgBYYzucvDe
|
Quick note from a follower of #37: the current CI failures look unrelated to this PR.
Both fail on code paths this PR does not touch. A workflow rerun should be enough to get a green check here. Thanks for the fix! |
brightsparc
added a commit
to introspection-org/walgit
that referenced
this pull request
Sep 12, 2026
Upstream main (tobi#42-tobi#45, tobi#49) plus the Azure and IRSA branches as updated for PRs tobi#50 and tobi#48, plus the store-plugin and notify-transport lines the production image was already built from (a6ce265). The two conflicts were additive: the Event Grid handshake test and its doc note now sit beside the loopback notify-transport test and its bullet. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T6UZ2AaFQ6m7xgdZKRyHEV
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #37. Thanks @czk-aa for the repro and for tracing both halves.
A ref-only push sends a 32-byte pack with zero objects, and
receive_pack_processincrates/walgit-server/src/smart.rsonly ran the connectivity check wheningest_packreturnedOk(Some(_)). So a command line pointingrefs/heads/ghostat an object the server doesn't have gotok, the ref went into the WAL, and every clone walking it died withmissing object.The check now runs whenever unpack succeeded and any update names a non-zero tip, pack or no pack. With
wal.check_connectivityon (the default) it's the existing walk, which stops at existing refs, so a legitimate ref-only push costs a few lookups. With it off, the tips are still looked up one by one withhas_objectbefore anything is published, so the hole is closed either way. I leftverify_txninpublish.rsalone: the receive path is the one place a pusher's oid enters, and the walk there already has the objects in hand.Test:
empty_pack_push_to_a_missing_object_is_refusedintests/e2e.rsposts the exact 32-byte pack from the report against both settings and expectsng refs/heads/ghostwith no ref published, then pushesmainto a new branch (git sends the same zero-object pack) and expects it to land. It fails onmainat thengassertion and passes here. Full e2e suite: 44 passed.