diff --git a/gix-blame/tests/blame.rs b/gix-blame/tests/blame.rs index d32d316ee53..6b1f33edfb6 100644 --- a/gix-blame/tests/blame.rs +++ b/gix-blame/tests/blame.rs @@ -166,10 +166,10 @@ impl Fixture { let mut reference = gix_ref::file::Store::find(&store, "HEAD")?; - // Needed for `peel_to_id_in_place`. + // Needed for `peel_to_id`. use gix_ref::file::ReferenceExt; - let head_id = reference.peel_to_id_in_place(&store, &odb)?; + let head_id = reference.peel_to_id(&store, &odb)?; let git_dir = worktree_path.join(".git"); let index = gix_index::File::at(git_dir.join("index"), gix_hash::Kind::Sha1, false, Default::default())?; diff --git a/gix-negotiate/tests/baseline/mod.rs b/gix-negotiate/tests/baseline/mod.rs index 4c4df125516..a2505aeed3f 100644 --- a/gix-negotiate/tests/baseline/mod.rs +++ b/gix-negotiate/tests/baseline/mod.rs @@ -36,7 +36,7 @@ fn run() -> crate::Result { .iter() .filter_map(|name| { refs.try_find(*name).expect("one tag per commit").map(|mut r| { - r.peel_to_id_in_place(&refs, &store).expect("works"); + r.peel_to_id(&refs, &store).expect("works"); r.target.into_id() }) }) diff --git a/gix-protocol/src/fetch/negotiate.rs b/gix-protocol/src/fetch/negotiate.rs index dcefaa6760c..767b76e4d63 100644 --- a/gix-protocol/src/fetch/negotiate.rs +++ b/gix-protocol/src/fetch/negotiate.rs @@ -373,11 +373,7 @@ fn mark_all_refs_in_repo( let _span = gix_trace::detail!("mark_all_refs"); for local_ref in store.iter()?.all()? { let mut local_ref = local_ref?; - let id = local_ref.peel_to_id_in_place_packed( - store, - objects, - store.cached_packed_buffer()?.as_ref().map(|b| &***b), - )?; + let id = local_ref.peel_to_id_packed(store, objects, store.cached_packed_buffer()?.as_ref().map(|b| &***b))?; let mut is_complete = false; if let Some(commit) = graph .get_or_insert_commit(id, |md| { diff --git a/gix-ref/src/store/file/raw_ext.rs b/gix-ref/src/store/file/raw_ext.rs index 5ec7aef7538..4fca9b33ee7 100644 --- a/gix-ref/src/store/file/raw_ext.rs +++ b/gix-ref/src/store/file/raw_ext.rs @@ -26,14 +26,31 @@ pub trait ReferenceExt: Sealed { /// /// This is useful to learn where this reference is ultimately pointing to after following all symbolic /// refs and all annotated tags to the first non-tag object. + #[deprecated = "Use `peel_to_id()` instead"] fn peel_to_id_in_place( &mut self, store: &file::Store, objects: &dyn gix_object::Find, ) -> Result; + /// Follow all symbolic targets this reference might point to and peel the underlying object + /// to the end of the tag-chain, returning the first non-tag object the annotated tag points to, + /// using `objects` to access them and `store` to lookup symbolic references. + /// + /// This is useful to learn where this reference is ultimately pointing to after following all symbolic + /// refs and all annotated tags to the first non-tag object. + /// + /// Note that this method mutates `self` in place if it does not already point to a + /// non-symbolic object. + fn peel_to_id( + &mut self, + store: &file::Store, + objects: &dyn gix_object::Find, + ) -> Result; + /// Like [`ReferenceExt::peel_to_id_in_place()`], but with support for a known stable `packed` buffer /// to use for resolving symbolic links. + #[deprecated = "Use `peel_to_id_packed()` instead"] fn peel_to_id_in_place_packed( &mut self, store: &file::Store, @@ -41,14 +58,32 @@ pub trait ReferenceExt: Sealed { packed: Option<&packed::Buffer>, ) -> Result; + /// Like [`ReferenceExt::peel_to_id_in_place()`], but with support for a known stable `packed` buffer + /// to use for resolving symbolic links. + fn peel_to_id_packed( + &mut self, + store: &file::Store, + objects: &dyn gix_object::Find, + packed: Option<&packed::Buffer>, + ) -> Result; + /// Like [`ReferenceExt::follow()`], but follows all symbolic references while gracefully handling loops, /// altering this instance in place. + #[deprecated = "Use `follow_to_object_packed()` instead"] fn follow_to_object_in_place_packed( &mut self, store: &file::Store, packed: Option<&packed::Buffer>, ) -> Result; + /// Like [`ReferenceExt::follow()`], but follows all symbolic references while gracefully handling loops, + /// altering this instance in place. + fn follow_to_object_packed( + &mut self, + store: &file::Store, + packed: Option<&packed::Buffer>, + ) -> Result; + /// Follow this symbolic reference one level and return the ref it refers to. /// /// Returns `None` if this is not a symbolic reference, hence the leaf of the chain. @@ -84,13 +119,21 @@ impl ReferenceExt for Reference { &mut self, store: &file::Store, objects: &dyn gix_object::Find, + ) -> Result { + self.peel_to_id(store, objects) + } + + fn peel_to_id( + &mut self, + store: &file::Store, + objects: &dyn gix_object::Find, ) -> Result { let packed = store.assure_packed_refs_uptodate().map_err(|err| { peel::to_id::Error::FollowToObject(peel::to_object::Error::Follow(file::find::existing::Error::Find( file::find::Error::PackedOpen(err), ))) })?; - self.peel_to_id_in_place_packed(store, objects, packed.as_ref().map(|b| &***b)) + self.peel_to_id_packed(store, objects, packed.as_ref().map(|b| &***b)) } fn peel_to_id_in_place_packed( @@ -98,6 +141,15 @@ impl ReferenceExt for Reference { store: &file::Store, objects: &dyn gix_object::Find, packed: Option<&packed::Buffer>, + ) -> Result { + self.peel_to_id_packed(store, objects, packed) + } + + fn peel_to_id_packed( + &mut self, + store: &file::Store, + objects: &dyn gix_object::Find, + packed: Option<&packed::Buffer>, ) -> Result { match self.peeled { Some(peeled) => { @@ -105,7 +157,7 @@ impl ReferenceExt for Reference { Ok(peeled) } None => { - let mut oid = self.follow_to_object_in_place_packed(store, packed)?; + let mut oid = self.follow_to_object_packed(store, packed)?; let mut buf = Vec::new(); let peeled_id = loop { let gix_object::Data { kind, data } = @@ -138,6 +190,14 @@ impl ReferenceExt for Reference { &mut self, store: &file::Store, packed: Option<&packed::Buffer>, + ) -> Result { + self.follow_to_object_packed(store, packed) + } + + fn follow_to_object_packed( + &mut self, + store: &file::Store, + packed: Option<&packed::Buffer>, ) -> Result { match self.target { Target::Object(id) => Ok(id), diff --git a/gix-ref/tests/refs/file/reference.rs b/gix-ref/tests/refs/file/reference.rs index 0539e9ecb7e..33a83bd57ec 100644 --- a/gix-ref/tests/refs/file/reference.rs +++ b/gix-ref/tests/refs/file/reference.rs @@ -81,11 +81,11 @@ mod peel { let store = store_with_packed_refs()?; let mut head: Reference = store.find_loose("HEAD")?.into(); let expected = hex_to_id("134385f6d781b7e97062102c6a483440bfda2a03"); - assert_eq!(head.peel_to_id_in_place(&store, &EmptyCommit)?, expected); + assert_eq!(head.peel_to_id(&store, &EmptyCommit)?, expected); assert_eq!(head.target.try_id().map(ToOwned::to_owned), Some(expected)); let mut head = store.find("dt1")?; - assert_eq!(head.peel_to_id_in_place(&store, &gix_object::find::Never)?, expected); + assert_eq!(head.peel_to_id(&store, &gix_object::find::Never)?, expected); assert_eq!(head.target.into_id(), expected); Ok(()) } @@ -113,7 +113,7 @@ mod peel { "but following doesn't do that, only real peeling does" ); - head.peel_to_id_in_place(&store, &EmptyCommit)?; + head.peel_to_id(&store, &EmptyCommit)?; assert_eq!( head.target.try_id().map(ToOwned::to_owned), Some(final_stop), @@ -135,23 +135,19 @@ mod peel { assert_eq!(r.kind(), gix_ref::Kind::Symbolic, "there is something to peel"); let commit = hex_to_id("134385f6d781b7e97062102c6a483440bfda2a03"); - assert_eq!(r.peel_to_id_in_place(&store, &EmptyCommit)?, commit); + assert_eq!(r.peel_to_id(&store, &EmptyCommit)?, commit); assert_eq!(r.name.as_bstr(), "refs/remotes/origin/multi-link-target3"); let mut r: Reference = store.find_loose("dt1")?.into(); assert_eq!( - r.peel_to_id_in_place(&store, &EmptyCommit)?, + r.peel_to_id(&store, &EmptyCommit)?, hex_to_id("4c3f4cce493d7beb45012e478021b5f65295e5a3"), "points to a tag object without actual object lookup" ); let odb = gix_odb::at(store.git_dir().join("objects"))?; let mut r: Reference = store.find_loose("dt1")?.into(); - assert_eq!( - r.peel_to_id_in_place(&store, &odb)?, - commit, - "points to the commit with lookup" - ); + assert_eq!(r.peel_to_id(&store, &odb)?, commit, "points to the commit with lookup"); Ok(()) } @@ -162,7 +158,7 @@ mod peel { let store = file::store_at_with_args("make_multi_hop_ref.sh", packed)?; let odb = gix_odb::at(store.git_dir().join("objects"))?; let mut r: Reference = store.find("multi-hop")?; - r.peel_to_id_in_place(&store, &odb)?; + r.peel_to_id(&store, &odb)?; let commit_id = hex_to_id("134385f6d781b7e97062102c6a483440bfda2a03"); assert_eq!(r.peeled, Some(commit_id)); @@ -172,8 +168,7 @@ mod peel { assert_eq!(obj.kind, gix_object::Kind::Commit, "always peeled to the first non-tag"); let mut r: Reference = store.find("multi-hop")?; - let tag_id = - r.follow_to_object_in_place_packed(&store, store.cached_packed_buffer()?.as_ref().map(|p| &***p))?; + let tag_id = r.follow_to_object_packed(&store, store.cached_packed_buffer()?.as_ref().map(|p| &***p))?; let obj = odb.find(&tag_id, &mut buf)?; assert_eq!(obj.kind, gix_object::Kind::Tag, "the first direct object target"); assert_eq!( @@ -183,7 +178,7 @@ mod peel { ); let mut r: Reference = store.find("multi-hop2")?; let other_tag_id = - r.follow_to_object_in_place_packed(&store, store.cached_packed_buffer()?.as_ref().map(|p| &***p))?; + r.follow_to_object_packed(&store, store.cached_packed_buffer()?.as_ref().map(|p| &***p))?; assert_eq!(other_tag_id, tag_id, "it can follow with multiple hops as well"); } Ok(()) @@ -197,14 +192,14 @@ mod peel { assert_eq!(r.name.as_bstr(), "refs/loop-a"); assert!(matches!( - r.peel_to_id_in_place(&store, &gix_object::find::Never).unwrap_err(), + r.peel_to_id(&store, &gix_object::find::Never).unwrap_err(), gix_ref::peel::to_id::Error::FollowToObject(gix_ref::peel::to_object::Error::Cycle { .. }) )); assert_eq!(r.name.as_bstr(), "refs/loop-a", "the ref is not changed on error"); let mut r: Reference = store.find_loose("loop-a")?.into(); let err = r - .follow_to_object_in_place_packed(&store, store.cached_packed_buffer()?.as_ref().map(|p| &***p)) + .follow_to_object_packed(&store, store.cached_packed_buffer()?.as_ref().map(|p| &***p)) .unwrap_err(); assert!(matches!(err, gix_ref::peel::to_object::Error::Cycle { .. })); Ok(()) diff --git a/gix-ref/tests/refs/file/worktree.rs b/gix-ref/tests/refs/file/worktree.rs index 5d23c779f8f..634a39ea6f8 100644 --- a/gix-ref/tests/refs/file/worktree.rs +++ b/gix-ref/tests/refs/file/worktree.rs @@ -60,7 +60,7 @@ fn into_peel( store: &gix_ref::file::Store, odb: gix_odb::Handle, ) -> impl Fn(gix_ref::Reference) -> gix_hash::ObjectId + '_ { - move |mut r: gix_ref::Reference| r.peel_to_id_in_place(store, &odb).unwrap() + move |mut r: gix_ref::Reference| r.peel_to_id(store, &odb).unwrap() } enum Mode { diff --git a/gix/src/clone/checkout.rs b/gix/src/clone/checkout.rs index 920550168ff..75b70db1bd7 100644 --- a/gix/src/clone/checkout.rs +++ b/gix/src/clone/checkout.rs @@ -97,7 +97,7 @@ pub mod main_worktree { })?; let root_tree_id = match &self.ref_name { - Some(reference_val) => Some(repo.find_reference(reference_val)?.peel_to_id_in_place()?), + Some(reference_val) => Some(repo.find_reference(reference_val)?.peel_to_id()?), None => repo.head()?.try_peel_to_id_in_place()?, }; diff --git a/gix/src/commit.rs b/gix/src/commit.rs index caeecc54416..6b940e6cb80 100644 --- a/gix/src/commit.rs +++ b/gix/src/commit.rs @@ -122,7 +122,7 @@ pub mod describe { .filter_map(Result::ok) .filter_map(|mut r: crate::Reference<'_>| { let target_id = r.target().try_id().map(ToOwned::to_owned); - let peeled_id = r.peel_to_id_in_place().ok()?; + let peeled_id = r.peel_to_id().ok()?; let (prio, tag_time) = match target_id { Some(target_id) if peeled_id != *target_id => { let tag = repo.find_object(target_id).ok()?.try_into_tag().ok()?; diff --git a/gix/src/head/peel.rs b/gix/src/head/peel.rs index 4ee116ed0db..a706a193c1e 100644 --- a/gix/src/head/peel.rs +++ b/gix/src/head/peel.rs @@ -128,7 +128,7 @@ impl<'repo> Head<'repo> { } Kind::Symbolic(r) => { let mut nr = r.clone().attach(self.repo); - let peeled = nr.peel_to_id_in_place(); + let peeled = nr.peel_to_id(); *r = nr.detach(); peeled? } diff --git a/gix/src/reference/iter.rs b/gix/src/reference/iter.rs index 89dc13c19db..29076417fcd 100644 --- a/gix/src/reference/iter.rs +++ b/gix/src/reference/iter.rs @@ -112,13 +112,9 @@ impl<'r> Iterator for Iter<'_, 'r> { .and_then(|mut r| { if self.peel { let repo = &self.repo; - r.peel_to_id_in_place_packed( - &repo.refs, - &repo.objects, - self.peel_with_packed.as_ref().map(|p| &***p), - ) - .map_err(|err| Box::new(err) as Box) - .map(|_| r) + r.peel_to_id_packed(&repo.refs, &repo.objects, self.peel_with_packed.as_ref().map(|p| &***p)) + .map_err(|err| Box::new(err) as Box) + .map(|_| r) } else { Ok(r) } diff --git a/gix/src/reference/mod.rs b/gix/src/reference/mod.rs index 924f3f6105f..7f944d7168a 100644 --- a/gix/src/reference/mod.rs +++ b/gix/src/reference/mod.rs @@ -65,12 +65,26 @@ impl<'repo> Reference<'repo> { /// Peeling impl<'repo> Reference<'repo> { /// Follow all symbolic targets this reference might point to and peel all annotated tags - /// to their first non-tag target, and return it, + /// to their first non-tag target, and return it. /// /// This is useful to learn where this reference is ultimately pointing to after following /// the chain of symbolic refs and annotated tags. + #[deprecated = "Use `peel_to_id()` instead"] pub fn peel_to_id_in_place(&mut self) -> Result, peel::Error> { - let oid = self.inner.peel_to_id_in_place(&self.repo.refs, &self.repo.objects)?; + let oid = self.inner.peel_to_id(&self.repo.refs, &self.repo.objects)?; + Ok(Id::from_id(oid, self.repo)) + } + + /// Follow all symbolic targets this reference might point to and peel all annotated tags + /// to their first non-tag target, and return it. + /// + /// This is useful to learn where this reference is ultimately pointing to after following + /// the chain of symbolic refs and annotated tags. + /// + /// Note that this method mutates `self` in place if it does not already point to non-symbolic + /// object. + pub fn peel_to_id(&mut self) -> Result, peel::Error> { + let oid = self.inner.peel_to_id(&self.repo.refs, &self.repo.objects)?; Ok(Id::from_id(oid, self.repo)) } @@ -85,13 +99,13 @@ impl<'repo> Reference<'repo> { ) -> Result, peel::Error> { let oid = self .inner - .peel_to_id_in_place_packed(&self.repo.refs, &self.repo.objects, packed)?; + .peel_to_id_packed(&self.repo.refs, &self.repo.objects, packed)?; Ok(Id::from_id(oid, self.repo)) } - /// Similar to [`peel_to_id_in_place()`](Reference::peel_to_id_in_place()), but consumes this instance. + /// Similar to [`peel_to_id()`](Reference::peel_to_id()), but consumes this instance. pub fn into_fully_peeled_id(mut self) -> Result, peel::Error> { - self.peel_to_id_in_place() + self.peel_to_id() } /// Follow this reference's target until it points at an object directly, and peel that object until @@ -99,6 +113,9 @@ impl<'repo> Reference<'repo> { /// /// Note that this ref will point to the first target object afterward, which may be a tag. This is different /// from [`peel_to_id_in_place()`](Self::peel_to_id_in_place()) where it will point to the first non-tag object. + /// + /// Note that `git2::Reference::peel` does not "peel in place", but returns a new object + /// instead. #[doc(alias = "peel", alias = "git2")] pub fn peel_to_kind(&mut self, kind: gix_object::Kind) -> Result, peel::to_kind::Error> { let packed = self.repo.refs.cached_packed_buffer().map_err(|err| { @@ -147,7 +164,7 @@ impl<'repo> Reference<'repo> { ) -> Result, peel::to_kind::Error> { let target = self .inner - .follow_to_object_in_place_packed(&self.repo.refs, packed)? + .follow_to_object_packed(&self.repo.refs, packed)? .attach(self.repo); Ok(target.object()?.peel_to_kind(kind)?) } @@ -175,7 +192,7 @@ impl<'repo> Reference<'repo> { ) -> Result, follow::to_object::Error> { Ok(self .inner - .follow_to_object_in_place_packed(&self.repo.refs, packed)? + .follow_to_object_packed(&self.repo.refs, packed)? .attach(self.repo)) } diff --git a/gix/src/remote/connection/fetch/update_refs/mod.rs b/gix/src/remote/connection/fetch/update_refs/mod.rs index c14d5ae26ce..4d3fd7c2906 100644 --- a/gix/src/remote/connection/fetch/update_refs/mod.rs +++ b/gix/src/remote/connection/fetch/update_refs/mod.rs @@ -127,7 +127,7 @@ pub(crate) fn update( match existing .try_id() - .map_or_else(|| existing.clone().peel_to_id_in_place(), Ok) + .map_or_else(|| existing.clone().peel_to_id(), Ok) .map(crate::Id::detach) { Ok(local_id) => { diff --git a/gix/src/remote/connection/fetch/update_refs/tests.rs b/gix/src/remote/connection/fetch/update_refs/tests.rs index e448f3525d4..8f25e495cc6 100644 --- a/gix/src/remote/connection/fetch/update_refs/tests.rs +++ b/gix/src/remote/connection/fetch/update_refs/tests.rs @@ -945,7 +945,7 @@ mod update { }, TargetRef::Symbolic(name) => { let target = name.as_bstr().into(); - match r.peel_to_id_in_place() { + match r.peel_to_id() { Ok(id) => gix_protocol::handshake::Ref::Symbolic { full_ref_name, target, diff --git a/gix/src/revision/spec/parse/delegate/mod.rs b/gix/src/revision/spec/parse/delegate/mod.rs index 8baf3c03213..d0c2611a8c7 100644 --- a/gix/src/revision/spec/parse/delegate/mod.rs +++ b/gix/src/revision/spec/parse/delegate/mod.rs @@ -201,7 +201,7 @@ impl Delegate<'_> { for (r, obj) in self.refs.iter().zip(self.objs.iter_mut()) { if let (Some(ref_), obj_opt @ None) = (r, obj) { if let Some(id) = ref_.target.try_id().map(ToOwned::to_owned).or_else(|| { - match ref_.clone().attach(repo).peel_to_id_in_place() { + match ref_.clone().attach(repo).peel_to_id() { Err(err) => { self.err.push(Error::PeelToId { name: ref_.name.clone(), diff --git a/gix/src/revision/spec/parse/delegate/revision.rs b/gix/src/revision/spec/parse/delegate/revision.rs index 3e021deec19..8cecf217fa8 100644 --- a/gix/src/revision/spec/parse/delegate/revision.rs +++ b/gix/src/revision/spec/parse/delegate/revision.rs @@ -214,7 +214,7 @@ impl delegate::Revision for Delegate<'_> { Ok(Some((ref_name, id))) => { let id = match self.repo.find_reference(ref_name.as_bstr()) { Ok(mut r) => { - let id = r.peel_to_id_in_place().map(crate::Id::detach).unwrap_or(id); + let id = r.peel_to_id().map(crate::Id::detach).unwrap_or(id); self.refs[self.idx] = Some(r.detach()); id } diff --git a/gix/tests/gix/reference/mod.rs b/gix/tests/gix/reference/mod.rs index 1ef725bd511..05339230b70 100644 --- a/gix/tests/gix/reference/mod.rs +++ b/gix/tests/gix/reference/mod.rs @@ -66,12 +66,12 @@ mod find { "it points to a tag object" ); - let object = packed_tag_ref.peel_to_id_in_place()?; + let object = packed_tag_ref.peel_to_id()?; let the_commit = hex_to_id("134385f6d781b7e97062102c6a483440bfda2a03"); assert_eq!(object, the_commit, "it is assumed to be fully peeled"); assert_eq!( object, - packed_tag_ref.peel_to_id_in_place()?, + packed_tag_ref.peel_to_id()?, "peeling again yields the same object" ); @@ -79,7 +79,7 @@ mod find { let expected: &FullNameRef = "refs/heads/multi-link-target1".try_into()?; assert_eq!(symbolic_ref.name(), expected); - assert_eq!(symbolic_ref.peel_to_id_in_place()?, the_commit); + assert_eq!(symbolic_ref.peel_to_id()?, the_commit); let expected: &FullNameRef = "refs/remotes/origin/multi-link-target3".try_into()?; assert_eq!(symbolic_ref.name(), expected, "it follows symbolic refs, too"); diff --git a/gix/tests/gix/repository/object.rs b/gix/tests/gix/repository/object.rs index 8c0c3234ad6..040238591ec 100644 --- a/gix/tests/gix/repository/object.rs +++ b/gix/tests/gix/repository/object.rs @@ -648,7 +648,7 @@ mod commit { fn multi_line_commit_message_uses_first_line_in_ref_log_ref_nonexisting() -> crate::Result { let _env = freeze_time(); let (repo, _keep) = crate::repo_rw_opts("make_basic_repo.sh", restricted_and_git())?; - let parent = repo.find_reference("HEAD")?.peel_to_id_in_place()?; + let parent = repo.find_reference("HEAD")?.peel_to_id()?; let empty_tree_id = parent.object()?.to_commit_ref_iter().tree_id().expect("tree to be set"); assert_eq!( parent @@ -697,7 +697,7 @@ mod commit { ); let mut branch = repo.find_reference("new-branch")?; - let current_commit = branch.peel_to_id_in_place()?; + let current_commit = branch.peel_to_id()?; assert_eq!(current_commit, second_commit_id, "the commit was set"); let mut log = branch.log_iter();