feat(008): replace git binary shell-outs with in-process gix implementation#147
Conversation
…tation
Replace all four std::process::Command::new("git") call sites in
http_git_server.rs with a new HttpPackServer that implements the HTTP
smart protocol (upload-pack and receive-pack) entirely in-process using
gix-pack, gix-packetline, and gix-protocol sub-crates.
- Add HttpPackServer in src/git/pack_server.rs with advertise_upload_pack_refs,
handle_upload_pack, advertise_receive_pack_refs, and handle_receive_pack
- Enforce configurable max pack size via DefaultBodyLimit (HTTP 413 on oversize)
- Atomic ref-update transactions with fast-forward checks and rollback on failure
- Remove git binary from docker/git-service.Dockerfile runtime stage
- Delete scripts/init-demo-catalog.sh and remove all doc references to it
- Add CI grep gates: no Command::new("git") in src/, no init-demo-catalog refs
- Add 8 integration tests covering clone, fetch, push, rejection, size limit,
and atomicity (all pass without git binary on the test host PATH)
Closes GH#109
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9d7d7e123b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Without symref=HEAD:refs/heads/main in the capability string, git clients on Linux do not set up a local tracking branch after clone, leaving them in a detached HEAD state. This caused clone tests to fail (README.md missing) and push tests to fail with "src refspec main does not match any" on x86_64-linux CI runners. Build the capability string dynamically so HEAD symbolic targets are always reflected correctly regardless of the default branch name. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
On Linux, gix's platform.all() does not yield HEAD as a regular ref (HEAD is a special pseudo-ref, not stored in packed-refs or loose refs). On macOS it happened to appear, masking the bug locally. Without HEAD in the advertisement, git clone prints "remote HEAD refers to nonexistent ref, unable to checkout" and creates an empty working tree (exits 0 but no files). This caused three integration tests to fail on x86_64-linux CI. Fix: resolve HEAD explicitly via repo.head_id() and prepend it to the ref list, skipping it if encountered again during platform.all(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
P1: stream pack data in ≤65511-byte sideband-64k chunks so large packs (> 64 KiB) are not truncated by the pkt-line 65516-byte limit. P2: write incoming pack bytes to a temp quarantine directory before touching refs; only move the pack/index into the live ODB after the atomic ref transaction commits, so a rejected push leaves no orphan objects on disk. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
juliuskrah
left a comment
juliuskrah
left a comment
There was a problem hiding this comment.
Both P1 and P2 issues resolved in commit 3f7dce2.
P1 (sideband chunking): handle_upload_pack now iterates pack_data.chunks(65511) and emits one sideband pkt-line per chunk, staying within the 65516-byte pkt-line hard limit regardless of pack size.
P2 (object quarantine): handle_receive_pack now stages incoming pack bytes into a tempfile::TempDir quarantine directory via stage_pack_to_quarantine before touching any refs. The temp pack/index files are only moved into the live ODB pack directory (promote_quarantine) after repo.edit_references commits successfully. A failed ref transaction (stale old_oid, non-fast-forward, etc.) causes the TempDir to be dropped automatically, leaving the repository object store unchanged.
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f7dce2f47
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
P1 (line 136): ref-delete commands (new_oid == 000…000) now produce Change::Delete instead of Change::Update, so `git push origin :branch` is handled correctly. P1 (line 152): pre-receive / update / post-receive lifecycle points are now called in handle_receive_pack. All three are no-op skeletons for now; future tasks will route them to gitstore-api over gRPC. P2 (line 378): parse_wants_and_haves replaces parse_wants so `have` lines from the client are collected; build_pack_for_wants uses rev_walk().with_boundary(haves) to exclude already-known commits, restoring incremental fetch behaviour. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 participant
Summary
std::process::Command::new("git")shell-out call sites inhttp_git_server.rswithHttpPackServer— a new in-process implementation of the HTTP smart protocol (upload-pack and receive-pack) usinggix-pack,gix-packetline, andgix-protocolgitbinary from thedocker/git-service.Dockerfileruntime stage (~30 MB image reduction)scripts/init-demo-catalog.shand remove all documentation references to itWhat changed
src/git/pack_server.rsHttpPackServerwith 4 methods: advertise/handle for upload-pack and receive-packsrc/http_git_server.rsDefaultBodyLimitapplied to receive-pack route (HTTP 413 on oversize)docker/git-service.Dockerfilegitremoved from runtimeapk add; stalesafe.directoryconfig line removedscripts/init-demo-catalog.shdocs/(3 files)init-demo-catalog.shreferences removed.github/workflows/ci.ymlshellout-guardsjob with grep gatestests/integration/http_smart_protocol.rsTest plan
cargo test(60 unit + 8 HTTP smart-protocol integration + 4 gRPC integration)cargo fmt --all -- --check— cleancargo clippy --all-targets --all-features -- -D warnings— cleangrep -rn 'Command::new("git")' gitstore-git-service/src/— no matcheswhich gitinside container exits non-zeroCloses #109
🤖 Generated with Claude Code