Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: assign commits to closest tag #711

Draft
wants to merge 17 commits into
base: main
Choose a base branch
from
Draft
1 change: 0 additions & 1 deletion git-cliff-core/src/changelog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,6 @@ impl<'a> Changelog<'a> {
}
}
}

for release in &self.releases {
let write_result = write!(
out,
Expand Down
174 changes: 156 additions & 18 deletions git-cliff-core/src/repo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use lazy_regex::{
Lazy,
Regex,
};
use std::cmp::Reverse;
use std::io;
use std::path::PathBuf;
use url::Url;
Expand Down Expand Up @@ -354,10 +355,10 @@ impl Repository {
/// It collects lightweight and annotated tags.
pub fn tags(
&self,
pattern: &Option<Regex>,
pattern: Option<&Regex>,
topo_order: bool,
use_branch_tags: bool,
) -> Result<IndexMap<String, Tag>> {
) -> Result<TaggedCommits<'_>> {
let mut tags: Vec<(Commit, Tag)> = Vec::new();
let tag_names = self.inner.tag_names(None)?;
let head_commit = self.inner.head()?.peel_to_commit()?;
Expand Down Expand Up @@ -402,12 +403,9 @@ impl Repository {
}
}
if !topo_order {
tags.sort_by(|a, b| a.0.time().seconds().cmp(&b.0.time().seconds()));
tags.sort_by_key(|(commit, _)| commit.time().seconds());
}
Ok(tags
.into_iter()
.map(|(a, b)| (a.id().to_string(), b))
.collect())
TaggedCommits::new(self, tags)
}

/// Returns the remote of the upstream repository.
Expand Down Expand Up @@ -452,6 +450,147 @@ impl Repository {
}
}

/// Stores which commits are tagged with which tags.
#[derive(Debug)]
pub struct TaggedCommits<'a> {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move this to its own module (tagged.rs or tagged_commit.rs) and add unit tests! 🐻

/// All the commits in the repository.
pub commits: IndexMap<String, Commit<'a>>,
/// Commit ID to tag map.
tags: IndexMap<String, Tag>,
/// List of tags' commit indexes. Points into `commits`.
///
/// Sorted in reverse order, meaning the first element is the latest tag.
///
/// Used for lookups.
tag_indexes: Vec<usize>,
}

impl<'a> TaggedCommits<'a> {
fn new(
repository: &'a Repository,
tags: Vec<(Commit<'a>, Tag)>,
) -> Result<Self> {
let commits = repository.commits(None, None, None)?;
let commits: IndexMap<_, _> = commits
.into_iter()
.map(|c| (c.id().to_string(), c))
.collect();
let mut tag_ids: Vec<_> = tags
.iter()
.filter_map(|(commit, _tag)| {
let id = commit.id().to_string();
commits.get_index_of(&id)
})
.collect();
tag_ids.sort_by_key(|idx| Reverse(*idx));
let tags = tags
.into_iter()
.map(|(commit, tag)| (commit.id().to_string(), tag))
.collect();
Ok(Self {
commits,
tag_indexes: tag_ids,
tags,
})
}

/// Returns the number of tags.
pub fn len(&self) -> usize {
self.tags.len()
}

/// Returns `true` if there are no tags.
pub fn is_empty(&self) -> bool {
self.tags.is_empty()
}

/// Returns an iterator over all the tags.
pub fn tags(&self) -> impl Iterator<Item = &Tag> {
self.tags.iter().map(|(_, tag)| tag)
}

/// Returns the last tag.
pub fn last(&self) -> Option<&Tag> {
self.tags().last()
}

/// Returns the tag of the given commit.
///
/// Note that this only searches for an exact match.
/// For a more general search, use [`get_closest`](Self::get_closest)
/// instead.
pub fn get(&self, commit: &str) -> Option<&Tag> {
self.tags.get(commit)
}

/// Returns the tag at the given index.
///
/// The index can be calculated with `tags().position()`.
pub fn get_index(&self, idx: usize) -> Option<&Tag> {
self.tags.get_index(idx).map(|(_, tag)| tag)
}

/// Returns the tag closest to the given commit.
pub fn get_closest(&self, commit: &str) -> Option<&Tag> {
// Try exact match first.
if let Some(tagged) = self.get(commit) {
return Some(tagged);
}

let index = self.commits.get_index_of(commit)?;
let tag_index =
*self.tag_indexes.iter().find(|tag_idx| index >= **tag_idx)?;
self.get_tag_by_id(tag_index)
}

fn get_tag_by_id(&self, id: usize) -> Option<&Tag> {
let (commit_of_tag, _) = self.commits.get_index(id)?;
self.tags.get(commit_of_tag)
}

/// Returns the commit of the given tag.
pub fn get_commit(&self, tag_name: &str) -> Option<&str> {
self.tags
.iter()
.find(|(_, t)| t.name == tag_name)
.map(|(commit, _)| commit.as_str())
}

/// Returns `true` if the given tag exists.
pub fn contains_commit(&self, commit: &str) -> bool {
self.tags.contains_key(commit)
}

/// Inserts a new tagged commit.
pub fn insert(&mut self, commit: String, tag: Tag) {
if let Some(index) = self.commits.get_index_of(&commit) {
if let Err(idx) = self.binary_search(index) {
self.tag_indexes.insert(idx, index);
}
}
self.tags.insert(commit, tag);
}

/// Retains only the tags specified by the predicate.
pub fn retain(&mut self, mut f: impl FnMut(&Tag) -> bool) {
self.tag_indexes.retain(|&idx| {
let panic_msg = "invalid TaggedCommits state";
let (commit_of_tag, _) = self.commits.get_index(idx).expect(panic_msg);
let tag = self.tags.get(commit_of_tag).expect(panic_msg);
let retain = f(tag);
if !retain {
self.tags.shift_remove(commit_of_tag);
}
retain
});
}

fn binary_search(&self, index: usize) -> std::result::Result<usize, usize> {
self.tag_indexes
.binary_search_by_key(&Reverse(index), |tag_idx| Reverse(*tag_idx))
}
}

fn find_remote(url: &str) -> Result<Remote> {
url_path_segments(url).or_else(|err| {
if url.contains("@") && url.contains(":") && url.contains("/") {
Expand Down Expand Up @@ -522,12 +661,11 @@ fn ssh_path_segments(url: &str) -> Result<Remote> {
mod test {
use super::*;
use crate::commit::Commit as AppCommit;
use std::env;
use std::fs;
use std::path::Path;
use std::process::Command;
use std::str;
use std::{
env,
fs,
};
use temp_dir::TempDir;

fn get_last_commit_hash() -> Result<String> {
Expand Down Expand Up @@ -568,7 +706,7 @@ mod test {

fn get_repository() -> Result<Repository> {
Repository::init(
PathBuf::from(env!("CARGO_MANIFEST_DIR"))
Path::new(env!("CARGO_MANIFEST_DIR"))
.parent()
.expect("parent directory not found")
.to_path_buf(),
Expand Down Expand Up @@ -615,8 +753,8 @@ mod test {
#[test]
fn get_latest_tag() -> Result<()> {
let repository = get_repository()?;
let tags = repository.tags(&None, false, false)?;
let latest = tags.last().expect("no tags found").1.name.clone();
let tags = repository.tags(None, false, false)?;
let latest = tags.last().expect("no tags found").name.clone();
assert_eq!(get_last_tag()?, latest);

let current = repository.current_tag().expect("a current tag").name;
Expand All @@ -627,7 +765,7 @@ mod test {
#[test]
fn git_tags() -> Result<()> {
let repository = get_repository()?;
let tags = repository.tags(&None, true, false)?;
let tags = repository.tags(None, true, false)?;
assert_eq!(
tags.get("2b8b4d3535f29231e05c3572e919634b9af907b6")
.expect(
Expand All @@ -646,8 +784,8 @@ mod test {
"v0.1.0-beta.4"
);
let tags = repository.tags(
&Some(
Regex::new("^v[0-9]+\\.[0-9]+\\.[0-9]$")
Some(
&Regex::new("^v[0-9]+\\.[0-9]+\\.[0-9]$")
.expect("the regex is not valid"),
),
true,
Expand All @@ -661,7 +799,7 @@ mod test {
.name,
"v0.1.0"
);
assert!(!tags.contains_key("4ddef08debfff48117586296e49d5caa0800d1b5"));
assert!(!tags.contains_commit("4ddef08debfff48117586296e49d5caa0800d1b5"));
Ok(())
}

Expand Down
8 changes: 8 additions & 0 deletions git-cliff-core/src/tag.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,14 @@ pub struct Tag {
pub message: Option<String>,
}

impl PartialEq for Tag {
fn eq(&self, other: &Self) -> bool {
self.name == other.name
}
}

impl Eq for Tag {}

#[cfg(test)]
mod test {
use super::*;
Expand Down
Loading
Loading