collin/anvil · 5997c273
fix: resolve ref deltas in received thin packs
Collin Richards · 2026-06-09 22:16 UTC · 5997c273e4bf6c128b52be91277145f7e477eb5d · parent 836274ec · browse files
modifiedTODO.md+53 −0
| 1 | + | # Misc TODO | |
| 2 | + | ||
| 3 | + | - [ ] should show subject line of latest commit in repo page view | |
| 4 | + | - we should mirror certain things like this from github ui | |
| 1 | 5 | - [ ] don't expose a users email on their profile page | |
| 2 | 6 | - [ ] don't include anvil before the breadcrumbs in the repo name i.e. anvil/collin/repo just do collin/repo | |
| 3 | 7 | - [x] implement github action style CI feature _(core done — see "Session notes" below; the "isolated workers" idea is the (c) sandboxed broker, still TODO)_ | |
| ⋯ 4 unchanged lines | |||
| 8 | 12 | ||
| 9 | 13 | --- | |
| 10 | 14 | ||
| 15 | + | # Error in git push _(FIXED 2026-06-10, uncommitted)_ | |
| 16 | + | ||
| 17 | + | **Root cause:** every push after the first sends a *thin pack* — deltas whose | |
| 18 | + | base objects aren't in the pack (the server already has them), referenced by | |
| 19 | + | object id (`REF_DELTA`). `vendor/gitserver-core/src/receive_pack.rs::write_pack` | |
| 20 | + | passed `None` as `thin_pack_base_object_lookup` to | |
| 21 | + | `gix_pack::Bundle::write_to_directory`, so gix couldn't resolve the bases and | |
| 22 | + | aborted. First-push-to-empty-repo worked because that pack is self-contained. | |
| 23 | + | ||
| 24 | + | **Fix:** pass the already-open `gix::Repository` as the lookup (it implements | |
| 25 | + | `gix_object::Find`). Regression test | |
| 26 | + | `receive_thin_pack_with_ref_deltas` builds a real thin pack via | |
| 27 | + | `git pack-objects --thin`, asserts it contains ref-deltas, and pushes it | |
| 28 | + | through `receive_pack`. Verified the test fails without the fix. | |
| 29 | + | ||
| 30 | + | Original report: | |
| 31 | + | ||
| 32 | + | ``` | |
| 33 | + | collin@mini ~/C/anvil (main)> git push | |
| 34 | + | Enter passphrase for key '/Users/collin/.ssh/id_ed25519': | |
| 35 | + | Enumerating objects: 117, done. | |
| 36 | + | Counting objects: 100% (117/117), done. | |
| 37 | + | Delta compression using up to 8 threads | |
| 38 | + | Compressing objects: 100% (62/62), done. | |
| 39 | + | Writing objects: 100% (66/66), 27.57 KiB | 3.94 MiB/s, done. | |
| 40 | + | Total 66 (delta 39), reused 0 (delta 0), pack-reused 0 (from 0) | |
| 41 | + | send-pack: unexpected disconnect while reading sideband packet | |
| 42 | + | fatal: the remote end hung up unexpectedly | |
| 43 | + | collin@mini ~/C/anvil (main) [128]> | |
| 44 | + | ``` | |
| 45 | + | ||
| 46 | + | server logs: | |
| 47 | + | ||
| 48 | + | ``` | |
| 49 | + | 26-06-09T21:53:46.466577Z INFO anvil_ssh: ssh auth: accepted key SHA256:Rg41caN7vw2WYYxiJN6lrIlX0DTXYF0rC2QzKZW1tB0 (user 1) | |
| 50 | + | 2026-06-09T21:53:46.635946Z INFO anvil_ssh: ssh git-receive-pack on collin/anvil.git (user Some(1)) | |
| 51 | + | 2026-06-09T21:53:46.805146Z ERROR anvil_ssh: git ssh git-receive-pack: protocol error: failed to write incoming pack: Ref delta objects are not supported as there is no way to look them up. Resolve them beforehand. | |
| 52 | + | 2026-06-09T21:54:28.571834Z INFO anvil_ssh: ssh auth: rejected unknown key SHA256:ZlWZyHqspqFeUQV84qaXtDQq4gcA33dR7y8dYbeg9u8 | |
| 53 | + | 2026-06-09T21:54:35.565597Z INFO anvil_ssh: ssh auth: accepted key SHA256:Rg41caN7vw2WYYxiJN6lrIlX0DTXYF0rC2QzKZW1tB0 (user 1) | |
| 54 | + | 2026-06-09T21:54:35.676113Z INFO anvil_ssh: ssh git-receive-pack on collin/anvil.git (user Some(1)) | |
| 55 | + | 2026-06-09T21:54:36.268520Z ERROR anvil_ssh: git ssh git-receive-pack: protocol error: failed to write incoming pack: Ref delta objects are not supported as there is no way to look them up. Resolve them beforehand. | |
| 56 | + | 2026-06-09T21:55:54.223033Z INFO anvil_ssh: ssh auth: rejected unknown key SHA256:ZlWZyHqspqFeUQV84qaXtDQq4gcA33dR7y8dYbeg9u8 | |
| 57 | + | 2026-06-09T21:56:00.758384Z INFO anvil_ssh: ssh auth: accepted key SHA256:Rg41caN7vw2WYYxiJN6lrIlX0DTXYF0rC2QzKZW1tB0 (user 1) | |
| 58 | + | 2026-06-09T21:56:00.906553Z INFO anvil_ssh: ssh git-receive-pack on collin/anvil.git (user Some(1)) | |
| 59 | + | 2026-06-09T21:56:01.067903Z ERROR anvil_ssh: git ssh git-receive-pack: protocol error: failed to write incoming pack: Ref delta objects are not supported as there is no way to look them up. Resolve them beforehand. | |
| 60 | + | ``` | |
| 61 | + | ||
| 62 | + | --- | |
| 63 | + | ||
| 11 | 64 | # Session notes / resume point | |
| 12 | 65 | ||
| 13 | 66 | _Last updated: 2026-06-10. Working state is clean: `cargo build`, `cargo clippy | |
| ⋯ 79 unchanged lines | |||
modifiedvendor/gitserver-core/src/receive_pack.rs+116 −3
| ⋯ 194 unchanged lines | |||
| 195 | 195 | ) -> Result<Vec<CommandStatus>> { | |
| 196 | 196 | check_interrupt(interrupt)?; | |
| 197 | 197 | if request.pack.fill_buf().map(|buf: &[u8]| !buf.is_empty())? { | |
| 198 | - | write_pack(repo_path, &mut request.pack, interrupt)?; | |
| 198 | + | write_pack(repo, repo_path, &mut request.pack, interrupt)?; | |
| 199 | 199 | } | |
| 200 | 200 | ||
| 201 | 201 | let mut edits = Vec::with_capacity(request.commands.len()); | |
| ⋯ 37 unchanged lines | |||
| 239 | 239 | } | |
| 240 | 240 | } | |
| 241 | 241 | ||
| 242 | - | fn write_pack<R: BufRead>(repo_path: &Path, pack: &mut R, interrupt: &AtomicBool) -> Result<()> { | |
| 242 | + | fn write_pack<R: BufRead>( | |
| 243 | + | repo: &gix::Repository, | |
| 244 | + | repo_path: &Path, | |
| 245 | + | pack: &mut R, | |
| 246 | + | interrupt: &AtomicBool, | |
| 247 | + | ) -> Result<()> { | |
| 243 | 248 | let mut progress = Discard; | |
| 249 | + | // Pushes after the first send a thin pack whose delta bases (ref-deltas) live | |
| 250 | + | // only in the existing object database; the lookup lets gix resolve them. | |
| 244 | 251 | let outcome = gix_pack::Bundle::write_to_directory( | |
| 245 | 252 | pack, | |
| 246 | 253 | Some(repo_path.join("objects/pack").as_path()), | |
| 247 | 254 | &mut progress, | |
| 248 | 255 | interrupt, | |
| 249 | - | None::<&gix::Repository>, | |
| 256 | + | Some(repo), | |
| 250 | 257 | Default::default(), | |
| 251 | 258 | ); | |
| 252 | 259 | if interrupt.load(std::sync::atomic::Ordering::Relaxed) { | |
| ⋯ 279 unchanged lines | |||
| 532 | 539 | } | |
| 533 | 540 | ||
| 534 | 541 | #[test] | |
| 542 | + | fn receive_thin_pack_with_ref_deltas() { | |
| 543 | + | let root = TempDir::new().unwrap(); | |
| 544 | + | let repo_path = root.path().join("test.git"); | |
| 545 | + | let work_dir = root.path().join("work"); | |
| 546 | + | std::fs::create_dir(&work_dir).unwrap(); | |
| 547 | + | Command::new("git") | |
| 548 | + | .args(["init", "--bare", repo_path.to_str().unwrap()]) | |
| 549 | + | .output() | |
| 550 | + | .unwrap(); | |
| 551 | + | Command::new("git") | |
| 552 | + | .args(["symbolic-ref", "HEAD", "refs/heads/main"]) | |
| 553 | + | .current_dir(&repo_path) | |
| 554 | + | .output() | |
| 555 | + | .unwrap(); | |
| 556 | + | Command::new("git") | |
| 557 | + | .args([ | |
| 558 | + | "clone", | |
| 559 | + | repo_path.to_str().unwrap(), | |
| 560 | + | work_dir.to_str().unwrap(), | |
| 561 | + | ]) | |
| 562 | + | .output() | |
| 563 | + | .unwrap(); | |
| 564 | + | ||
| 565 | + | let git = |args: &[&str]| { | |
| 566 | + | let output = Command::new("git") | |
| 567 | + | .current_dir(&work_dir) | |
| 568 | + | .args(args) | |
| 569 | + | .env("GIT_AUTHOR_NAME", "Test") | |
| 570 | + | .env("GIT_AUTHOR_EMAIL", "t@t.com") | |
| 571 | + | .env("GIT_COMMITTER_NAME", "Test") | |
| 572 | + | .env("GIT_COMMITTER_EMAIL", "t@t.com") | |
| 573 | + | .output() | |
| 574 | + | .unwrap(); | |
| 575 | + | assert!(output.status.success(), "git {args:?}: {output:?}"); | |
| 576 | + | String::from_utf8(output.stdout).unwrap().trim().to_string() | |
| 577 | + | }; | |
| 578 | + | ||
| 579 | + | // A large repetitive blob so the follow-up commit deltas against it. | |
| 580 | + | let base_content = "this line repeats to make the blob delta-friendly\n".repeat(200); | |
| 581 | + | std::fs::write(work_dir.join("data.txt"), &base_content).unwrap(); | |
| 582 | + | git(&["add", "data.txt"]); | |
| 583 | + | git(&["commit", "-m", "base"]); | |
| 584 | + | git(&["push", "origin", "main"]); | |
| 585 | + | let old_id = git(&["rev-parse", "HEAD"]); | |
| 586 | + | ||
| 587 | + | std::fs::write( | |
| 588 | + | work_dir.join("data.txt"), | |
| 589 | + | format!("{base_content}one more line\n"), | |
| 590 | + | ) | |
| 591 | + | .unwrap(); | |
| 592 | + | git(&["add", "data.txt"]); | |
| 593 | + | git(&["commit", "-m", "append"]); | |
| 594 | + | let new_id = git(&["rev-parse", "HEAD"]); | |
| 595 | + | ||
| 596 | + | // Build a thin pack exactly like a push would: bases from old_id stay out. | |
| 597 | + | let mut pack_objects = Command::new("git") | |
| 598 | + | .current_dir(&work_dir) | |
| 599 | + | .args(["pack-objects", "--thin", "--stdout", "--revs", "-q"]) | |
| 600 | + | .stdin(std::process::Stdio::piped()) | |
| 601 | + | .stdout(std::process::Stdio::piped()) | |
| 602 | + | .spawn() | |
| 603 | + | .unwrap(); | |
| 604 | + | use std::io::Write as _; | |
| 605 | + | pack_objects | |
| 606 | + | .stdin | |
| 607 | + | .take() | |
| 608 | + | .unwrap() | |
| 609 | + | .write_all(format!("{new_id}\n^{old_id}\n").as_bytes()) | |
| 610 | + | .unwrap(); | |
| 611 | + | let pack = pack_objects.wait_with_output().unwrap(); | |
| 612 | + | assert!(pack.status.success()); | |
| 613 | + | let pack = pack.stdout; | |
| 614 | + | ||
| 615 | + | // The fix only matters if the pack really contains ref deltas. | |
| 616 | + | let entries = gix_pack::data::input::BytesToEntriesIter::new_from_header( | |
| 617 | + | std::io::Cursor::new(&pack), | |
| 618 | + | gix_pack::data::input::Mode::Verify, | |
| 619 | + | gix_pack::data::input::EntryDataMode::Ignore, | |
| 620 | + | gix::hash::Kind::Sha1, | |
| 621 | + | ) | |
| 622 | + | .unwrap(); | |
| 623 | + | let has_ref_delta = entries | |
| 624 | + | .map(|e| e.unwrap()) | |
| 625 | + | .any(|entry| matches!(entry.header, gix_pack::data::entry::Header::RefDelta { .. })); | |
| 626 | + | assert!(has_ref_delta, "test pack should contain ref delta objects"); | |
| 627 | + | ||
| 628 | + | let mut body = Vec::new(); | |
| 629 | + | body.extend_from_slice(&pktline::encode( | |
| 630 | + | format!("{old_id} {new_id} refs/heads/main\0 report-status\n").as_bytes(), | |
| 631 | + | )); | |
| 632 | + | body.extend_from_slice(pktline::flush()); | |
| 633 | + | body.extend_from_slice(&pack); | |
| 634 | + | ||
| 635 | + | let response = receive_pack(&repo_path, std::io::Cursor::new(body)).unwrap(); | |
| 636 | + | let response = String::from_utf8_lossy(&response); | |
| 637 | + | assert!(response.contains("unpack ok"), "response: {response}"); | |
| 638 | + | assert!( | |
| 639 | + | response.contains("ok refs/heads/main"), | |
| 640 | + | "response: {response}" | |
| 641 | + | ); | |
| 642 | + | ||
| 643 | + | let repo = gix::open(&repo_path).unwrap(); | |
| 644 | + | assert_eq!(repo.head_id().unwrap().detach().to_string(), new_id); | |
| 645 | + | } | |
| 646 | + | ||
| 647 | + | #[test] | |
| 535 | 648 | fn ensure_fast_forward_respects_interrupt() { | |
| 536 | 649 | let root = TempDir::new().unwrap(); | |
| 537 | 650 | let repo_path = create_repo_with_commit(root.path()); | |
| ⋯ 12 unchanged lines | |||