anvilsign in

collin/anvil · a5967525

Peel annotated tag wants during fetch negotiation

Collin Richards · 2026-08-27 01:07 UTC · a5967525842c191da6222ccc680929935e41a68c · parent 2c82299b · browse files

modifiedvendor/gitserver-core/src/pack.rs+1 −1
⋯ 429 unchanged lines
430430 /// commits as walk tips, trees/blobs (rare tag targets) collected directly.
431431 /// `git fetch`/`push` send tag *object* ids whenever annotated tags are
432432 /// involved, so every pack path needs this.
433-fn peel_wants(
433+pub(crate) fn peel_wants(
434434 repo: &gix::Repository,
435435 wants: &[gix::ObjectId],
436436 ) -> std::result::Result<
⋯ 575 unchanged lines
modifiedvendor/gitserver-core/src/protocol_v2.rs+139 −2
⋯ 562 unchanged lines
563563 Ok(out)
564564 }
565565
566+/// The set of objects reachable from `wants`, used to filter the client's
567+/// `have` lines down to the ones we actually share.
568+///
569+/// Wants may be annotated tag *objects* (protocol-v2 `ls-refs` advertises
570+/// unpeeled ids), so they are peeled before the commit walk — feeding a tag id
571+/// to `rev_walk` fails the whole fetch with "Expected object of kind commit".
566572 fn collect_want_closure(
567573 repo: &gix::Repository,
568574 wants: &[gix::ObjectId],
569575 ) -> Result<Vec<gix::ObjectId>> {
576+ let (commit_wants, extras) =
577+ crate::pack::peel_wants(repo, wants).map_err(|e| Error::Protocol(e.to_string()))?;
570578 let mut seen = HashSet::new();
571579 let mut out = Vec::new();
572580
573581 let walk = repo
574- .rev_walk(wants.iter().copied())
582+ .rev_walk(commit_wants.iter().copied())
575583 .all()
576584 .map_err(|e| Error::Protocol(e.to_string()))?;
577585 for info_result in walk {
⋯ 5 unchanged lines
583591 out.push(commit_oid);
584592 }
585593
594+ // Tag objects (and direct tree/blob tag targets) are part of the closure
595+ // too, so a `have` naming one counts as common.
596+ out.extend(extras.into_iter().filter(|oid| !seen.contains(oid)));
597+
586598 Ok(out)
587599 }
588600
⋯ 20 unchanged lines
609621 };
610622 let limit = base_depth + depth;
611623
612- for want in &request.wants {
624+ // Wants may be annotated tag objects; walk from the peeled commit and let
625+ // the tag chain itself ride along in the pack.
626+ let (commit_wants, extras) = crate::pack::peel_wants(repo, &request.wants)
627+ .map_err(|e| Error::Protocol(e.to_string()))?;
628+ for want in &commit_wants {
613629 queue.push_back((*want, 1usize));
614630 }
615631
⋯ 27 unchanged lines
643659 }
644660 }
645661
662+ included_objects.extend(extras.into_iter().filter(|oid| !seen.contains(oid)));
663+
646664 Ok(DepthState {
647665 included_objects,
648666 shallow_boundary,
⋯ 38 unchanged lines
687705
688706 #[cfg(test)]
689707 mod tests {
708+ use std::path::PathBuf;
709+
690710 use super::*;
711+ use crate::pack::ShallowRequest;
691712
692713 fn pkt(data: &str) -> Vec<u8> {
693714 pktline::encode(data.as_bytes())
⋯ 61 unchanged lines
755776 let text = String::from_utf8(out).unwrap();
756777 assert!(text.contains("unborn HEAD symref-target:refs/heads/main"));
757778 }
779+ /// Build a bare repo with two commits on `main` and an annotated tag on
780+ /// the tip. Returns (bare repo path, tag object id, first commit id).
781+ fn repo_with_annotated_tag(root: &Path) -> (PathBuf, gix::ObjectId, gix::ObjectId) {
782+ use std::process::Command as Proc;
783+
784+ let bare_path = root.join("test.git");
785+ let work_path = root.join("workdir");
786+ let git = |args: &[&str], dir: &Path| {
787+ let out = Proc::new("git")
788+ .args(args)
789+ .current_dir(dir)
790+ .env("GIT_AUTHOR_NAME", "t")
791+ .env("GIT_AUTHOR_EMAIL", "t@t")
792+ .env("GIT_COMMITTER_NAME", "t")
793+ .env("GIT_COMMITTER_EMAIL", "t@t")
794+ .output()
795+ .expect("git failed to run");
796+ assert!(out.status.success(), "git {args:?} failed: {out:?}");
797+ };
798+
799+ let out = Proc::new("git")
800+ .args(["init", "--bare", "-b", "main", bare_path.to_str().unwrap()])
801+ .output()
802+ .unwrap();
803+ assert!(out.status.success());
804+ let out = Proc::new("git")
805+ .args([
806+ "clone",
807+ bare_path.to_str().unwrap(),
808+ work_path.to_str().unwrap(),
809+ ])
810+ .output()
811+ .unwrap();
812+ assert!(out.status.success());
813+
814+ std::fs::write(work_path.join("a.txt"), "one\n").unwrap();
815+ git(&["add", "a.txt"], &work_path);
816+ git(&["commit", "-m", "one"], &work_path);
817+ let first = gix::open(&work_path).unwrap().head_id().unwrap().detach();
818+
819+ std::fs::write(work_path.join("a.txt"), "two\n").unwrap();
820+ git(&["add", "a.txt"], &work_path);
821+ git(&["commit", "-m", "two"], &work_path);
822+ git(&["tag", "-a", "-m", "annotated", "v1"], &work_path);
823+ git(&["push", "-q", "origin", "main", "v1"], &work_path);
824+
825+ let repo = gix::open(&bare_path).unwrap();
826+ let tag_oid = repo
827+ .find_reference("refs/tags/v1")
828+ .unwrap()
829+ .try_id()
830+ .unwrap()
831+ .detach();
832+ assert_eq!(
833+ repo.find_object(tag_oid).unwrap().kind,
834+ gix::object::Kind::Tag,
835+ "fixture must produce a real tag object"
836+ );
837+
838+ (bare_path, tag_oid, first)
839+ }
840+
841+ /// Regression: `git pull` of a repo with annotated tags sends the tag
842+ /// *object* id as a want (that is what `ls-refs` advertises). Negotiation
843+ /// used to feed it straight to `rev_walk`, failing every fetch with
844+ /// "Expected object of kind commit but got tag".
845+ #[test]
846+ fn negotiation_accepts_annotated_tag_wants() {
847+ let root = tempfile::TempDir::new().unwrap();
848+ let (repo_path, tag_oid, first) = repo_with_annotated_tag(root.path());
849+
850+ let request = FetchRequest {
851+ upload_request: UploadPackRequest {
852+ wants: vec![tag_oid],
853+ haves: vec![first],
854+ done: false,
855+ capabilities: Default::default(),
856+ shallow: Default::default(),
857+ object_ids: None,
858+ },
859+ };
860+
861+ let common = common_haves(&repo_path, &request).unwrap();
862+ assert_eq!(common, vec![first], "the have is reachable from the tag");
863+ }
864+
865+ /// The shallow (`--depth`) path peels tag wants too, and packs the tag
866+ /// object itself alongside the depth-limited closure.
867+ #[test]
868+ fn shallow_boundaries_accept_annotated_tag_wants() {
869+ let root = tempfile::TempDir::new().unwrap();
870+ let (repo_path, tag_oid, first) = repo_with_annotated_tag(root.path());
871+
872+ let mut request = FetchRequest {
873+ upload_request: UploadPackRequest {
874+ wants: vec![tag_oid],
875+ haves: vec![],
876+ done: true,
877+ capabilities: Default::default(),
878+ shallow: ShallowRequest {
879+ depth: Some(1),
880+ ..Default::default()
881+ },
882+ object_ids: None,
883+ },
884+ };
885+
886+ let update = apply_shallow_boundaries(&repo_path, &mut request).unwrap();
887+ assert_eq!(update.shallow.len(), 1, "depth 1 stops at the tip");
888+ assert!(
889+ !update.shallow.contains(&first),
890+ "the parent commit is beyond the boundary"
891+ );
892+ let objects = request.upload_request.object_ids.unwrap();
893+ assert!(objects.contains(&tag_oid), "tag object rides along");
894+ }
758895 }