diff --git a/git-object/src/commit/ref_iter.rs b/git-object/src/commit/ref_iter.rs index d934c84ce99..7b3d34329b7 100644 --- a/git-object/src/commit/ref_iter.rs +++ b/git-object/src/commit/ref_iter.rs @@ -77,7 +77,6 @@ impl<'a> CommitRefIter<'a> { } /// Returns the committer signature if there is no decoding error. - /// Errors are coerced into options, hiding whether there was an error or not. The caller knows if there was an error or not. pub fn committer(mut self) -> Result, crate::decode::Error> { self.find_map(|t| match t { Ok(Token::Committer { signature }) => Some(Ok(signature)), @@ -90,7 +89,6 @@ impl<'a> CommitRefIter<'a> { /// Returns the author signature if there is no decoding error. /// /// It may contain white space surrounding it, and is exactly as parsed. - /// Errors are coerced into options, hiding whether there was an error or not. The caller knows if there was an error or not. pub fn author(mut self) -> Result, crate::decode::Error> { self.find_map(|t| match t { Ok(Token::Author { signature }) => Some(Ok(signature)), @@ -104,7 +102,6 @@ impl<'a> CommitRefIter<'a> { /// /// It may contain white space surrounding it, and is exactly as // parsed. - /// Errors are coerced into options, hiding whether there was an error or not. The caller knows if there was an error or not. pub fn message(mut self) -> Result<&'a BStr, crate::decode::Error> { self.find_map(|t| match t { Ok(Token::Message(msg)) => Some(Ok(msg)), diff --git a/git-object/src/lib.rs b/git-object/src/lib.rs index 06d8caa79c7..40551d9554b 100644 --- a/git-object/src/lib.rs +++ b/git-object/src/lib.rs @@ -148,6 +148,7 @@ pub struct TagRef<'a> { /// Like [`TagRef`], but as `Iterator` to support entirely allocation free parsing. /// It's particularly useful to dereference only the target chain. +#[derive(Copy, Clone)] pub struct TagRefIter<'a> { data: &'a [u8], state: tag::ref_iter::State, diff --git a/git-object/src/tag/ref_iter.rs b/git-object/src/tag/ref_iter.rs index 94e6f654919..76b3e4a1e18 100644 --- a/git-object/src/tag/ref_iter.rs +++ b/git-object/src/tag/ref_iter.rs @@ -9,6 +9,7 @@ use nom::{ use crate::{bstr::ByteSlice, parse, parse::NL, tag::decode, Kind, TagRefIter}; +#[derive(Copy, Clone)] pub(crate) enum State { Target, TargetKind, @@ -39,10 +40,21 @@ impl<'a> TagRefIter<'a> { /// Errors are coerced into options, hiding whether there was an error or not. The caller should assume an error if they /// call the method as intended. Such a squelched error cannot be recovered unless the objects data is retrieved and parsed again. /// `next()`. - pub fn target_id(&mut self) -> Result { + pub fn target_id(mut self) -> Result { let token = self.next().ok_or_else(missing_field)??; Token::into_id(token).ok_or_else(missing_field) } + + /// Returns the taggers signature if there is no decoding error, and if this field exists. + /// Errors are coerced into options, hiding whether there was an error or not. The caller knows if there was an error or not. + pub fn tagger(mut self) -> Result>, crate::decode::Error> { + self.find_map(|t| match t { + Ok(Token::Tagger(signature)) => Some(Ok(signature)), + Err(err) => Some(Err(err)), + _ => None, + }) + .ok_or_else(missing_field)? + } } fn missing_field() -> crate::decode::Error { diff --git a/git-object/tests/immutable/tag.rs b/git-object/tests/immutable/tag.rs index bd7687c4dd8..d3457a4ea60 100644 --- a/git-object/tests/immutable/tag.rs +++ b/git-object/tests/immutable/tag.rs @@ -26,17 +26,21 @@ mod iter { #[test] fn empty() -> crate::Result { + let tag = fixture_bytes("tag", "empty.txt"); + let tag_iter = TagRefIter::from_bytes(&tag); + let target_id = hex_to_id("01dd4e2a978a9f5bd773dae6da7aa4a5ac1cdbbc"); + let tagger = Some(signature(1592381636)); assert_eq!( - TagRefIter::from_bytes(&fixture_bytes("tag", "empty.txt")).collect::, _>>()?, + tag_iter.collect::, _>>()?, vec![ - Token::Target { - id: hex_to_id("01dd4e2a978a9f5bd773dae6da7aa4a5ac1cdbbc") - }, + Token::Target { id: target_id }, Token::TargetKind(Kind::Commit), Token::Name(b"empty".as_bstr()), - Token::Tagger(Some(signature(1592381636))), + Token::Tagger(tagger), ] ); + assert_eq!(tag_iter.target_id()?, target_id); + assert_eq!(tag_iter.tagger()?, tagger); Ok(()) } diff --git a/git-repository/src/commit.rs b/git-repository/src/commit.rs index d3175990552..e944db8e8d4 100644 --- a/git-repository/src/commit.rs +++ b/git-repository/src/commit.rs @@ -55,6 +55,7 @@ pub mod describe { } /// A selector to choose what kind of references should contribute to names. + #[derive(Debug, Clone, Copy, PartialOrd, PartialEq, Ord, Eq, Hash)] pub enum SelectRef { /// Only use annotated tags for names. AnnotatedTags, @@ -70,33 +71,60 @@ pub mod describe { repo: &Repository, ) -> Result>, Error> { let platform = repo.references()?; - let into_tuple = - |r: crate::Reference<'_>| (r.inner.target.into_id(), Cow::from(r.inner.name.shorten().to_owned())); Ok(match self { - SelectRef::AllTags => platform - .tags()? - .peeled() + SelectRef::AllTags | SelectRef::AllRefs => { + let mut refs: Vec<_> = match self { + SelectRef::AllRefs => platform.all()?, + SelectRef::AllTags => platform.tags()?, + _ => unreachable!(), + } .filter_map(Result::ok) - .map(into_tuple) - .collect(), - SelectRef::AllRefs => platform - .all()? - .peeled() - .filter_map(Result::ok) - .map(into_tuple) - .collect(), - SelectRef::AnnotatedTags => platform - .tags()? - .filter_map(Result::ok) - .filter_map(|r: crate::Reference<'_>| { - // TODO: we assume direct refs for tags, which is the common case, but it doesn't have to be - // so rather follow symrefs till the first object and then peel tags after the first object was found. - let tag = r.try_id()?.object().ok()?.try_into_tag().ok()?; - let commit_id = tag.target_id().ok()?.object().ok()?.try_into_commit().ok()?.id; - Some((commit_id, r.name().shorten().to_owned().into())) + .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 (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()?; + (1, tag.tagger().ok()??.time.seconds_since_unix_epoch) + } + _ => (0, 0), + }; + ( + peeled_id.inner, + prio, + tag_time, + Cow::from(r.inner.name.shorten().to_owned()), + ) + .into() }) - .collect(), + .collect(); + refs.sort_by(|a, b| a.2.cmp(&b.2).then_with(|| a.1.cmp(&b.1))); // by time ascending, then by priority. Older entries overwrite newer ones. + refs.into_iter().map(|(a, _, _, b)| (a, b)).collect() + } + SelectRef::AnnotatedTags => { + let mut peeled_commits_and_tag_date: Vec<_> = platform + .tags()? + .filter_map(Result::ok) + .filter_map(|r: crate::Reference<'_>| { + // TODO: we assume direct refs for tags, which is the common case, but it doesn't have to be + // so rather follow symrefs till the first object and then peel tags after the first object was found. + let tag = r.try_id()?.object().ok()?.try_into_tag().ok()?; + let tag_time = tag + .tagger() + .ok() + .and_then(|s| s.map(|s| s.time.seconds_since_unix_epoch)) + .unwrap_or(0); + let commit_id = tag.target_id().ok()?.object().ok()?.try_into_commit().ok()?.id; + Some((commit_id, tag_time, r.name().shorten().to_owned().into())) + }) + .collect(); + peeled_commits_and_tag_date.sort_by(|a, b| a.1.cmp(&b.1)); // by time, ascending, causing older names to overwrite newer ones. + peeled_commits_and_tag_date + .into_iter() + .map(|(a, _, c)| (a, c)) + .collect() + } }) } } @@ -146,7 +174,7 @@ pub mod describe { /// if one was found. /// /// Note that there will always be `Some(format)` - pub fn try_format(self) -> Result>, Error> { + pub fn try_format(&self) -> Result>, Error> { self.try_resolve()?.map(|r| r.format()).transpose() } @@ -154,7 +182,7 @@ pub mod describe { /// if one was found. /// /// The outcome provides additional information, but leaves the caller with the burden - pub fn try_resolve(self) -> Result>, Error> { + pub fn try_resolve(&self) -> Result>, Error> { // TODO: dirty suffix with respective dirty-detection let outcome = git_revision::describe( &self.id, @@ -180,7 +208,7 @@ pub mod describe { } /// Like [`try_format()`][Platform::try_format()], but turns `id_as_fallback()` on to always produce a format. - pub fn format(mut self) -> Result, Error> { + pub fn format(&mut self) -> Result, Error> { self.id_as_fallback = true; Ok(self.try_format()?.expect("BUG: fallback must always produce a format")) } diff --git a/git-repository/src/object/tag.rs b/git-repository/src/object/tag.rs index f7532aa1f27..d2684ee9820 100644 --- a/git-repository/src/object/tag.rs +++ b/git-repository/src/object/tag.rs @@ -1,10 +1,15 @@ use crate::{ext::ObjectIdExt, Tag}; impl<'repo> Tag<'repo> { - /// Decode this tag and return the id of its target. + /// Decode this tag partially and return the id of its target. pub fn target_id(&self) -> Result, git_object::decode::Error> { git_object::TagRefIter::from_bytes(&self.data) .target_id() .map(|id| id.attach(self.repo)) } + + /// Decode this tag partially and return the tagger, if the field exists. + pub fn tagger(&self) -> Result>, git_object::decode::Error> { + git_object::TagRefIter::from_bytes(&self.data).tagger() + } } diff --git a/git-repository/tests/commit/mod.rs b/git-repository/tests/commit/mod.rs new file mode 100644 index 00000000000..1f4e1e27a13 --- /dev/null +++ b/git-repository/tests/commit/mod.rs @@ -0,0 +1,14 @@ +mod describe { + use crate::named_repo; + use git_repository::commit::describe::SelectRef::{AllRefs, AllTags, AnnotatedTags}; + + #[test] + fn tags_are_sorted_by_date_and_lexigraphically() { + let repo = named_repo("make_commit_describe_multiple_tags.sh").unwrap(); + let mut describe = repo.head_commit().unwrap().describe(); + for filter in &[AnnotatedTags, AllTags, AllRefs] { + describe = describe.names(*filter); + assert_eq!(describe.format().unwrap().to_string(), "v2", "{:?}", filter); + } + } +} diff --git a/git-repository/tests/fixtures/generated-archives/make_commit_describe_multiple_tags.tar.xz b/git-repository/tests/fixtures/generated-archives/make_commit_describe_multiple_tags.tar.xz new file mode 100644 index 00000000000..19c70cb4076 --- /dev/null +++ b/git-repository/tests/fixtures/generated-archives/make_commit_describe_multiple_tags.tar.xz @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:22fd60100187fc3f4180b31d3ae1d8c5506a65c3ee059b4bbc3a40009fb23f60 +size 10408 diff --git a/git-repository/tests/fixtures/make_commit_describe_multiple_tags.sh b/git-repository/tests/fixtures/make_commit_describe_multiple_tags.sh new file mode 100644 index 00000000000..cdafd0b9892 --- /dev/null +++ b/git-repository/tests/fixtures/make_commit_describe_multiple_tags.sh @@ -0,0 +1,11 @@ +#!/bin/bash +set -eu -o pipefail + +git init -q +git commit --allow-empty -q -m c1 +git commit --allow-empty -q -m c2 + +git tag v0 -m "tag object 0" "HEAD~1" +git tag v1 -m "tag object 1" +git tag v1.5 +GIT_COMMITTER_DATE="2022-01-02 00:00:00 +0000" git tag v2 -m "tag object 2" diff --git a/git-repository/tests/git.rs b/git-repository/tests/git.rs index 6610ce02798..5bd0fdbfb48 100644 --- a/git-repository/tests/git.rs +++ b/git-repository/tests/git.rs @@ -28,6 +28,7 @@ fn basic_rw_repo() -> crate::Result<(Repository, tempfile::TempDir)> { repo_rw("make_basic_repo.sh") } +mod commit; mod discover; mod id; mod init;