From 5019e81962610c162f30f31de7bdaec407b62aa5 Mon Sep 17 00:00:00 2001 From: o2sh Date: Sat, 3 Jun 2023 20:27:41 +0200 Subject: [PATCH 01/28] add churn metric --- src/info/author.rs | 54 ++++++----------- src/info/churn.rs | 103 ++++++++++++++++++++++++++++++++ src/info/commits.rs | 8 +-- src/info/contributors.rs | 13 ++-- src/info/created.rs | 10 ++-- src/info/last_change.rs | 10 ++-- src/info/mod.rs | 50 ++++++++++------ src/info/utils/git.rs | 112 +++++++++++++++++++++++++++++++---- src/info/utils/info_field.rs | 1 + 9 files changed, 275 insertions(+), 86 deletions(-) create mode 100644 src/info/churn.rs diff --git a/src/info/author.rs b/src/info/author.rs index 0769178c1..c2e5378e4 100644 --- a/src/info/author.rs +++ b/src/info/author.rs @@ -1,6 +1,6 @@ use crate::{ cli::NumberSeparator, - info::{format_number, utils::git::Commits, utils::info_field::InfoField}, + info::{format_number, utils::git::CommitMetrics, utils::info_field::InfoField}, }; use owo_colors::{DynColors, OwoColorize}; use serde::Serialize; @@ -10,32 +10,28 @@ use std::fmt::Write; #[serde(rename_all = "camelCase")] pub struct Author { pub name: String, - email: String, + email: Option, nbr_of_commits: usize, contribution: usize, #[serde(skip_serializing)] - show_email: bool, - #[serde(skip_serializing)] number_separator: NumberSeparator, } impl Author { pub fn new( - name: gix::bstr::BString, - email: gix::bstr::BString, + name: String, + email: Option, nbr_of_commits: usize, total_nbr_of_commits: usize, - show_email: bool, number_separator: NumberSeparator, ) -> Self { let contribution = (nbr_of_commits as f32 * 100. / total_nbr_of_commits as f32).round() as usize; Self { - name: name.to_string(), - email: email.to_string(), + name, + email, nbr_of_commits, contribution, - show_email, number_separator, } } @@ -43,13 +39,13 @@ impl Author { impl std::fmt::Display for Author { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - if self.show_email { + if let Some(email) = &self.email { write!( f, "{}% {} <{}> {}", self.contribution, self.name, - self.email, + email, format_number(&self.nbr_of_commits, self.number_separator) ) } else { @@ -72,8 +68,8 @@ pub struct AuthorsInfo { } impl AuthorsInfo { - pub fn new(info_color: DynColors, commits: &Commits) -> Self { - let authors = commits.authors_to_display.clone(); + pub fn new(info_color: DynColors, commit_metrics: &CommitMetrics) -> Self { + let authors = commit_metrics.authors_to_display.clone(); Self { authors, info_color, @@ -128,10 +124,9 @@ mod test { fn test_display_author() { let author = Author::new( "John Doe".into(), - "john.doe@email.com".into(), + Some("john.doe@email.com".into()), 1500, 2000, - true, NumberSeparator::Plain, ); @@ -140,14 +135,7 @@ mod test { #[test] fn test_display_author_with_no_email() { - let author = Author::new( - "John Doe".into(), - "john.doe@email.com".into(), - 1500, - 2000, - false, - NumberSeparator::Plain, - ); + let author = Author::new("John Doe".into(), None, 1500, 2000, NumberSeparator::Plain); assert_eq!(author.to_string(), "75% John Doe 1500"); } @@ -156,10 +144,9 @@ mod test { fn test_authors_info_title_with_one_author() { let author = Author::new( "John Doe".into(), - "john.doe@email.com".into(), + Some("john.doe@email.com".into()), 1500, 2000, - true, NumberSeparator::Plain, ); @@ -175,19 +162,17 @@ mod test { fn test_authors_info_title_with_two_authors() { let author = Author::new( "John Doe".into(), - "john.doe@email.com".into(), + Some("john.doe@email.com".into()), 1500, 2000, - true, NumberSeparator::Plain, ); let author_2 = Author::new( "Roberto Berto".into(), - "bertolone2000@email.com".into(), + None, 240, 300, - false, NumberSeparator::Plain, ); @@ -203,10 +188,9 @@ mod test { fn test_author_info_value_with_one_author() { let author = Author::new( "John Doe".into(), - "john.doe@email.com".into(), + Some("john.doe@email.com".into()), 1500, 2000, - true, NumberSeparator::Plain, ); @@ -227,19 +211,17 @@ mod test { fn test_author_info_value_with_two_authors() { let author = Author::new( "John Doe".into(), - "john.doe@email.com".into(), + Some("john.doe@email.com".into()), 1500, 2000, - true, NumberSeparator::Plain, ); let author_2 = Author::new( "Roberto Berto".into(), - "bertolone2000@email.com".into(), + None, 240, 300, - false, NumberSeparator::Plain, ); diff --git a/src/info/churn.rs b/src/info/churn.rs new file mode 100644 index 000000000..bc6fe34ff --- /dev/null +++ b/src/info/churn.rs @@ -0,0 +1,103 @@ +use super::utils::{git::CommitMetrics, info_field::InfoField}; +use crate::{cli::NumberSeparator, info::format_number}; +use owo_colors::{DynColors, OwoColorize}; +use serde::Serialize; +use std::fmt::Write; + +#[derive(Serialize, Clone)] +#[serde(rename_all = "camelCase")] +pub struct Churn { + file_path: String, + nbr_of_commits: usize, + #[serde(skip_serializing)] + number_separator: NumberSeparator, +} + +impl Churn { + pub fn new( + file_path: String, + nbr_of_commits: usize, + number_separator: NumberSeparator, + ) -> Self { + Self { + file_path, + nbr_of_commits, + number_separator, + } + } +} + +impl std::fmt::Display for Churn { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!( + f, + "{} {}", + truncate_file_path(&self.file_path, 2), + format_number(&self.nbr_of_commits, self.number_separator) + ) + } +} + +#[derive(Serialize)] +pub struct ChurnInfo { + pub churn: Vec, + #[serde(skip_serializing)] + pub info_color: DynColors, +} +impl ChurnInfo { + pub fn new(info_color: DynColors, commit_metrics: &CommitMetrics) -> Self { + let churn = commit_metrics.churns_to_display.clone(); + Self { churn, info_color } + } +} +impl std::fmt::Display for ChurnInfo { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + let mut churn_info = String::new(); + + let pad = self.title().len() + 2; + + for (i, churn) in self.churn.iter().enumerate() { + let churn_str = churn.color(self.info_color); + + if i == 0 { + let _ = write!(churn_info, "{churn_str}"); + } else { + let _ = write!(churn_info, "\n{: String { + self.to_string() + } + + fn title(&self) -> String { + "Churn".into() + } + + fn should_color(&self) -> bool { + false + } +} + +fn truncate_file_path(path: &str, depth: usize) -> String { + let components: Vec<&str> = path.split('/').collect(); + + if components.len() <= depth { + return path.to_string(); + } + + let truncated_components: Vec<&str> = components + .iter() + .skip(components.len() - depth) + .copied() + .collect(); + let truncated_path = format!(".../{}", truncated_components.join("/")); + + truncated_path +} diff --git a/src/info/commits.rs b/src/info/commits.rs index 4a61bd148..c3256dfbb 100644 --- a/src/info/commits.rs +++ b/src/info/commits.rs @@ -2,7 +2,7 @@ use serde::Serialize; use crate::{ cli::NumberSeparator, - info::{format_number, utils::git::Commits, utils::info_field::InfoField}, + info::{format_number, utils::git::CommitMetrics, utils::info_field::InfoField}, }; #[derive(Serialize)] @@ -15,10 +15,10 @@ pub struct CommitsInfo { } impl CommitsInfo { - pub fn new(commits: &Commits, number_separator: NumberSeparator) -> Self { + pub fn new(commit_metrics: &CommitMetrics, number_separator: NumberSeparator) -> Self { Self { - number_of_commits: commits.total_number_of_commits, - is_shallow: commits.is_shallow, + number_of_commits: commit_metrics.total_number_of_commits, + is_shallow: commit_metrics.is_shallow, number_separator, } } diff --git a/src/info/contributors.rs b/src/info/contributors.rs index 9c2f8a7f4..ba3656415 100644 --- a/src/info/contributors.rs +++ b/src/info/contributors.rs @@ -2,7 +2,7 @@ use serde::Serialize; use crate::{ cli::NumberSeparator, - info::{format_number, utils::git::Commits, utils::info_field::InfoField}, + info::{format_number, utils::git::CommitMetrics, utils::info_field::InfoField}, }; #[derive(Serialize)] @@ -17,12 +17,12 @@ pub struct ContributorsInfo { impl ContributorsInfo { pub fn new( - commits: &Commits, + commit_metrics: &CommitMetrics, number_of_authors_to_display: usize, number_separator: NumberSeparator, ) -> Self { Self { - total_number_of_authors: commits.total_number_of_authors, + total_number_of_authors: commit_metrics.total_number_of_authors, number_of_authors_to_display, number_separator, } @@ -50,12 +50,13 @@ mod test { #[test] fn test_display_contributors_info() { - use crate::info::utils::git::Commits; + use crate::info::utils::git::CommitMetrics; use gix::actor::Time; let timestamp = Time::now_utc(); - let commits = Commits { + let commit_metrics = CommitMetrics { authors_to_display: vec![], + churns_to_display: vec![], total_number_of_authors: 12, total_number_of_commits: 2, is_shallow: true, @@ -63,7 +64,7 @@ mod test { time_of_first_commit: timestamp, }; - let contributors_info = ContributorsInfo::new(&commits, 2, NumberSeparator::Plain); + let contributors_info = ContributorsInfo::new(&commit_metrics, 2, NumberSeparator::Plain); assert_eq!(contributors_info.value(), "12".to_string()); assert_eq!(contributors_info.title(), "Contributors".to_string()); } diff --git a/src/info/created.rs b/src/info/created.rs index af95a5e26..de6898a23 100644 --- a/src/info/created.rs +++ b/src/info/created.rs @@ -1,5 +1,5 @@ use super::utils::format_time; -use crate::info::{utils::git::Commits, utils::info_field::InfoField}; +use crate::info::{utils::git::CommitMetrics, utils::info_field::InfoField}; use serde::Serialize; #[derive(Serialize)] @@ -9,14 +9,14 @@ pub struct CreatedInfo { } impl CreatedInfo { - pub fn new(iso_time: bool, commits: &Commits) -> Self { - let creation_date = get_creation_date(commits, iso_time); + pub fn new(iso_time: bool, commit_metrics: &CommitMetrics) -> Self { + let creation_date = get_creation_date(commit_metrics, iso_time); Self { creation_date } } } -fn get_creation_date(commits: &Commits, iso_time: bool) -> String { - format_time(commits.time_of_first_commit, iso_time) +fn get_creation_date(commit_metrics: &CommitMetrics, iso_time: bool) -> String { + format_time(commit_metrics.time_of_first_commit, iso_time) } #[typetag::serialize] diff --git a/src/info/last_change.rs b/src/info/last_change.rs index 4055d11b1..b38681712 100644 --- a/src/info/last_change.rs +++ b/src/info/last_change.rs @@ -1,5 +1,5 @@ use super::utils::format_time; -use crate::info::{utils::git::Commits, utils::info_field::InfoField}; +use crate::info::{utils::git::CommitMetrics, utils::info_field::InfoField}; use serde::Serialize; #[derive(Serialize)] @@ -9,15 +9,15 @@ pub struct LastChangeInfo { } impl LastChangeInfo { - pub fn new(iso_time: bool, commits: &Commits) -> Self { - let last_change = get_date_of_last_commit(commits, iso_time); + pub fn new(iso_time: bool, commit_metrics: &CommitMetrics) -> Self { + let last_change = get_date_of_last_commit(commit_metrics, iso_time); Self { last_change } } } -fn get_date_of_last_commit(commits: &Commits, iso_time: bool) -> String { - format_time(commits.time_of_most_recent_commit, iso_time) +fn get_date_of_last_commit(commit_metrics: &CommitMetrics, iso_time: bool) -> String { + format_time(commit_metrics.time_of_most_recent_commit, iso_time) } #[typetag::serialize] diff --git a/src/info/mod.rs b/src/info/mod.rs index bc677bd56..6e4ab3a87 100644 --- a/src/info/mod.rs +++ b/src/info/mod.rs @@ -1,4 +1,5 @@ use self::author::AuthorsInfo; +use self::churn::ChurnInfo; use self::commits::CommitsInfo; use self::contributors::ContributorsInfo; use self::created::CreatedInfo; @@ -16,7 +17,7 @@ use self::size::SizeInfo; use self::title::Title; use self::url::get_repo_url; use self::url::UrlInfo; -use self::utils::git::Commits; +use self::utils::git::CommitMetrics; use self::utils::info_field::{InfoField, InfoType}; use self::version::VersionInfo; use crate::cli::{is_truecolor_terminal, CliOptions, NumberSeparator, When}; @@ -32,6 +33,7 @@ use serde::Serialize; use std::path::Path; mod author; +mod churn; mod commits; mod contributors; mod created; @@ -147,7 +149,7 @@ pub fn build_info(cli_options: &CliOptions) -> Result { let manifest = get_manifest(&repo_path)?; let repo_url = get_repo_url(&repo); - let commits = Commits::new( + let commit_metrics = CommitMetrics::new( &repo, cli_options.info.no_merges, &cli_options.info.no_bots, @@ -182,7 +184,7 @@ pub fn build_info(cli_options: &CliOptions) -> Result { .head(&repo)? .pending(&repo)? .version(&repo, &manifest)? - .created(&commits, iso_time) + .created(&commit_metrics, iso_time) .languages( &loc_by_language, true_color, @@ -190,11 +192,12 @@ pub fn build_info(cli_options: &CliOptions) -> Result { &text_colors, ) .dependencies(&manifest, number_separator) - .authors(&commits, &text_colors) - .last_change(&commits, iso_time) - .contributors(&commits, number_of_authors, number_separator) + .authors(&commit_metrics, &text_colors) + .last_change(&commit_metrics, iso_time) + .contributors(&commit_metrics, number_of_authors, number_separator) .url(&repo_url) - .commits(&commits, number_separator) + .commits(&commit_metrics, number_separator) + .churn(&commit_metrics, &text_colors) .loc(&loc_by_language, number_separator) .size(&repo, number_separator) .license(&repo_path, &manifest)? @@ -295,9 +298,9 @@ impl InfoBuilder { Ok(self) } - fn created(mut self, commits: &Commits, iso_time: bool) -> Self { + fn created(mut self, commit_metrics: &CommitMetrics, iso_time: bool) -> Self { if !self.disabled_fields.contains(&InfoType::Created) { - let created = CreatedInfo::new(iso_time, commits); + let created = CreatedInfo::new(iso_time, commit_metrics); self.info_fields.push(Box::new(created)); } self @@ -334,17 +337,17 @@ impl InfoBuilder { self } - fn authors(mut self, commits: &Commits, text_colors: &TextColors) -> Self { + fn authors(mut self, commit_metrics: &CommitMetrics, text_colors: &TextColors) -> Self { if !self.disabled_fields.contains(&InfoType::Authors) { - let authors = AuthorsInfo::new(text_colors.info, commits); + let authors = AuthorsInfo::new(text_colors.info, commit_metrics); self.info_fields.push(Box::new(authors)); } self } - fn last_change(mut self, commits: &Commits, iso_time: bool) -> Self { + fn last_change(mut self, commit_metrics: &CommitMetrics, iso_time: bool) -> Self { if !self.disabled_fields.contains(&InfoType::LastChange) { - let last_change = LastChangeInfo::new(iso_time, commits); + let last_change = LastChangeInfo::new(iso_time, commit_metrics); self.info_fields.push(Box::new(last_change)); } self @@ -352,25 +355,38 @@ impl InfoBuilder { fn contributors( mut self, - commits: &Commits, + commit_metrics: &CommitMetrics, number_of_authors: usize, number_separator: NumberSeparator, ) -> Self { if !self.disabled_fields.contains(&InfoType::Contributors) { - let contributors = ContributorsInfo::new(commits, number_of_authors, number_separator); + let contributors = + ContributorsInfo::new(commit_metrics, number_of_authors, number_separator); self.info_fields.push(Box::new(contributors)); } self } - fn commits(mut self, commits: &Commits, number_separator: NumberSeparator) -> Self { + fn commits( + mut self, + commit_metrics: &CommitMetrics, + number_separator: NumberSeparator, + ) -> Self { if !self.disabled_fields.contains(&InfoType::Commits) { - let commits = CommitsInfo::new(commits, number_separator); + let commits = CommitsInfo::new(commit_metrics, number_separator); self.info_fields.push(Box::new(commits)); } self } + fn churn(mut self, commit_metrics: &CommitMetrics, text_colors: &TextColors) -> Self { + if !self.disabled_fields.contains(&InfoType::Churn) { + let churn = ChurnInfo::new(text_colors.info, commit_metrics); + self.info_fields.push(Box::new(churn)); + } + self + } + fn loc( mut self, loc_by_language: &[(Language, usize)], diff --git a/src/info/utils/git.rs b/src/info/utils/git.rs index 4e1a8bcfd..441cd1f05 100644 --- a/src/info/utils/git.rs +++ b/src/info/utils/git.rs @@ -1,14 +1,20 @@ use crate::cli::{MyRegex, NumberSeparator}; use crate::info::author::Author; +use crate::info::churn::Churn; use anyhow::Result; -use gix::bstr::BString; use gix::bstr::ByteSlice; +use gix::bstr::{BString, Utf8Error}; +use gix::object::tree::diff::change::Event; +use gix::object::tree::diff::{Action, Change}; +use gix::objs::tree::EntryMode; +use gix::Commit; use regex::Regex; use std::collections::HashMap; use std::str::FromStr; -pub struct Commits { +pub struct CommitMetrics { pub authors_to_display: Vec, + pub churns_to_display: Vec, pub total_number_of_authors: usize, pub total_number_of_commits: usize, /// false if we have found the first commit that started it all, true if the repository is shallow. @@ -29,7 +35,7 @@ impl From for Sig { } } -impl Commits { +impl CommitMetrics { pub fn new( repo: &gix::Repository, no_merges: bool, @@ -47,17 +53,17 @@ impl Commits { let mailmap_config = repo.open_mailmap(); let mut number_of_commits_by_signature: HashMap = HashMap::new(); let mut total_number_of_commits = 0; + let mut number_of_commits_by_file_path: HashMap = HashMap::new(); // From newest to oldest while let Some(commit_id) = commit_iter_peekable.next() { let commit = commit_id?.object()?.into_commit(); - let commit = commit.decode()?; - if no_merges && commit.parents().take(2).count() > 1 { + if no_merges && commit.parent_ids().count() > 1 { continue; } - let sig = Sig::from(mailmap_config.resolve(commit.author)); + let sig = Sig::from(mailmap_config.resolve(commit.author()?)); if is_bot(&sig.name, &bot_regex_pattern) { continue; @@ -65,9 +71,18 @@ impl Commits { *number_of_commits_by_signature.entry(sig).or_insert(0) += 1; - time_of_most_recent_commit.get_or_insert_with(|| commit.time()); + if total_number_of_commits <= 100 { + compute_diff_with_parents(&mut number_of_commits_by_file_path, &commit, repo)?; + } + + let commit_time = commit + .time() + .expect("Could not read commit's creation time"); + + time_of_most_recent_commit.get_or_insert(commit_time); + if commit_iter_peekable.peek().is_none() { - time_of_first_commit = commit.time().into(); + time_of_first_commit = commit_time.into(); } total_number_of_commits += 1; @@ -81,14 +96,16 @@ impl Commits { number_separator, ); + let churns_to_display = compute_churns(number_of_commits_by_file_path, number_separator); + // This could happen if a branch pointed to non-commit object, so no traversal actually happens. let (time_of_first_commit, time_of_most_recent_commit) = time_of_first_commit .and_then(|a| time_of_most_recent_commit.map(|b| (a, b))) .unwrap_or_default(); - drop(commit_iter); Ok(Self { authors_to_display, + churns_to_display, total_number_of_authors, total_number_of_commits, is_shallow: repo.is_shallow(), @@ -98,6 +115,23 @@ impl Commits { } } +fn compute_churns( + number_of_commits_by_file_path: HashMap, + number_separator: NumberSeparator, +) -> Vec { + let mut number_of_commits_by_file_path_sorted: Vec<(String, usize)> = + number_of_commits_by_file_path.into_iter().collect(); + + number_of_commits_by_file_path_sorted + .sort_by(|(_, a_count), (_, b_count)| b_count.cmp(a_count)); + + number_of_commits_by_file_path_sorted + .into_iter() + .map(|(file_path, nbr_of_commits)| Churn::new(file_path, nbr_of_commits, number_separator)) + .take(3) + .collect() +} + fn compute_authors( number_of_commits_by_signature: HashMap, total_number_of_commits: usize, @@ -116,13 +150,15 @@ fn compute_authors( let authors: Vec = signature_with_number_of_commits_sorted .into_iter() .map(|(author, author_nbr_of_commits)| { - let email = author.email; Author::new( - author.name, - email, + author.name.to_string(), + if show_email { + Some(author.email.to_string()) + } else { + None + }, author_nbr_of_commits, total_number_of_commits, - show_email, number_separator, ) }) @@ -131,6 +167,56 @@ fn compute_authors( (authors, total_number_of_authors) } +fn compute_diff_with_parents( + change_map: &mut HashMap, + commit: &Commit, + repo: &gix::Repository, +) -> Result<()> { + // Handles the very first commit + if commit.parent_ids().count() == 0 { + repo.empty_tree() + .changes()? + .track_path() + .for_each_to_obtain_tree(&commit.tree()?, |change| { + for_each_change(change, change_map) + })?; + } + // Ignore merge commits + else if commit.parent_ids().count() == 1 { + for parent_id in commit.parent_ids() { + parent_id + .object()? + .into_commit() + .tree()? + .changes()? + .track_path() + .for_each_to_obtain_tree(&commit.tree()?, |change| { + for_each_change(change, change_map) + })?; + } + } + + Ok(()) +} + +fn for_each_change( + change: Change, + change_map: &mut HashMap, +) -> Result { + let is_file_change = match change.event { + Event::Addition { entry_mode, .. } | Event::Modification { entry_mode, .. } => { + entry_mode == EntryMode::Blob + } + Event::Deletion { .. } | Event::Rewrite { .. } => false, + }; + if is_file_change { + let path = change.location.to_os_str()?.to_string_lossy(); + *change_map.entry(path.into_owned()).or_insert(0) += 1; + } + + Ok::(Action::Continue) +} + fn get_no_bots_regex(no_bots: &Option>) -> Result> { let reg = if let Some(r) = no_bots.clone() { match r { diff --git a/src/info/utils/info_field.rs b/src/info/utils/info_field.rs index bcaa445be..d40809504 100644 --- a/src/info/utils/info_field.rs +++ b/src/info/utils/info_field.rs @@ -22,6 +22,7 @@ pub enum InfoType { Contributors, URL, Commits, + Churn, LinesOfCode, Size, License, From 205dd367d0f38f4750b0baabb85fdbf1405951ae Mon Sep 17 00:00:00 2001 From: o2sh Date: Sat, 3 Jun 2023 21:05:28 +0200 Subject: [PATCH 02/28] add diff_count --- src/info/utils/git.rs | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/src/info/utils/git.rs b/src/info/utils/git.rs index 441cd1f05..a553d7fc5 100644 --- a/src/info/utils/git.rs +++ b/src/info/utils/git.rs @@ -53,6 +53,7 @@ impl CommitMetrics { let mailmap_config = repo.open_mailmap(); let mut number_of_commits_by_signature: HashMap = HashMap::new(); let mut total_number_of_commits = 0; + let mut diff_count = 0; let mut number_of_commits_by_file_path: HashMap = HashMap::new(); // From newest to oldest @@ -71,8 +72,13 @@ impl CommitMetrics { *number_of_commits_by_signature.entry(sig).or_insert(0) += 1; - if total_number_of_commits <= 100 { - compute_diff_with_parents(&mut number_of_commits_by_file_path, &commit, repo)?; + if diff_count <= 100 { + compute_diff_with_parents( + &mut number_of_commits_by_file_path, + &commit, + repo, + &mut diff_count, + )?; } let commit_time = commit @@ -171,6 +177,7 @@ fn compute_diff_with_parents( change_map: &mut HashMap, commit: &Commit, repo: &gix::Repository, + diff_count: &mut usize, ) -> Result<()> { // Handles the very first commit if commit.parent_ids().count() == 0 { @@ -181,19 +188,17 @@ fn compute_diff_with_parents( for_each_change(change, change_map) })?; } - // Ignore merge commits - else if commit.parent_ids().count() == 1 { - for parent_id in commit.parent_ids() { - parent_id - .object()? - .into_commit() - .tree()? - .changes()? - .track_path() - .for_each_to_obtain_tree(&commit.tree()?, |change| { - for_each_change(change, change_map) - })?; - } + for parent_id in commit.parent_ids() { + parent_id + .object()? + .into_commit() + .tree()? + .changes()? + .track_path() + .for_each_to_obtain_tree(&commit.tree()?, |change| { + for_each_change(change, change_map) + })?; + *diff_count += 1; } Ok(()) From 6dadbf804e20e8a34f182bab457d00efd8030ee7 Mon Sep 17 00:00:00 2001 From: o2sh Date: Sat, 3 Jun 2023 21:19:47 +0200 Subject: [PATCH 03/28] revert --- src/info/utils/git.rs | 35 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/src/info/utils/git.rs b/src/info/utils/git.rs index a553d7fc5..441cd1f05 100644 --- a/src/info/utils/git.rs +++ b/src/info/utils/git.rs @@ -53,7 +53,6 @@ impl CommitMetrics { let mailmap_config = repo.open_mailmap(); let mut number_of_commits_by_signature: HashMap = HashMap::new(); let mut total_number_of_commits = 0; - let mut diff_count = 0; let mut number_of_commits_by_file_path: HashMap = HashMap::new(); // From newest to oldest @@ -72,13 +71,8 @@ impl CommitMetrics { *number_of_commits_by_signature.entry(sig).or_insert(0) += 1; - if diff_count <= 100 { - compute_diff_with_parents( - &mut number_of_commits_by_file_path, - &commit, - repo, - &mut diff_count, - )?; + if total_number_of_commits <= 100 { + compute_diff_with_parents(&mut number_of_commits_by_file_path, &commit, repo)?; } let commit_time = commit @@ -177,7 +171,6 @@ fn compute_diff_with_parents( change_map: &mut HashMap, commit: &Commit, repo: &gix::Repository, - diff_count: &mut usize, ) -> Result<()> { // Handles the very first commit if commit.parent_ids().count() == 0 { @@ -188,17 +181,19 @@ fn compute_diff_with_parents( for_each_change(change, change_map) })?; } - for parent_id in commit.parent_ids() { - parent_id - .object()? - .into_commit() - .tree()? - .changes()? - .track_path() - .for_each_to_obtain_tree(&commit.tree()?, |change| { - for_each_change(change, change_map) - })?; - *diff_count += 1; + // Ignore merge commits + else if commit.parent_ids().count() == 1 { + for parent_id in commit.parent_ids() { + parent_id + .object()? + .into_commit() + .tree()? + .changes()? + .track_path() + .for_each_to_obtain_tree(&commit.tree()?, |change| { + for_each_change(change, change_map) + })?; + } } Ok(()) From 5f941dd1bfc40da7fdb47984b88630cd29112521 Mon Sep 17 00:00:00 2001 From: o2sh Date: Sat, 3 Jun 2023 21:40:35 +0200 Subject: [PATCH 04/28] rename --- src/info/utils/git.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/info/utils/git.rs b/src/info/utils/git.rs index 441cd1f05..4b82c5043 100644 --- a/src/info/utils/git.rs +++ b/src/info/utils/git.rs @@ -72,7 +72,7 @@ impl CommitMetrics { *number_of_commits_by_signature.entry(sig).or_insert(0) += 1; if total_number_of_commits <= 100 { - compute_diff_with_parents(&mut number_of_commits_by_file_path, &commit, repo)?; + compute_diff(&mut number_of_commits_by_file_path, &commit, repo)?; } let commit_time = commit @@ -167,7 +167,7 @@ fn compute_authors( (authors, total_number_of_authors) } -fn compute_diff_with_parents( +fn compute_diff( change_map: &mut HashMap, commit: &Commit, repo: &gix::Repository, From 667ca9af31c39eb92c601b0099ddc8e5673e5113 Mon Sep 17 00:00:00 2001 From: o2sh Date: Sun, 4 Jun 2023 01:06:28 +0200 Subject: [PATCH 05/28] add churn cli flags --- src/cli.rs | 8 ++++++ src/info/churn.rs | 17 +++++++----- src/info/contributors.rs | 2 +- src/info/mod.rs | 9 +------ src/info/utils/git.rs | 46 ++++++++++++++++----------------- tests/snapshots/repo__repo.snap | 16 +++++++++--- 6 files changed, 56 insertions(+), 42 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 1397e4482..12242e66d 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -63,6 +63,12 @@ pub struct InfoCliOptions { /// Maximum NUM of languages to be shown #[arg(long, default_value_t = 6usize, value_name = "NUM")] pub number_of_languages: usize, + /// Maximum NUM of file churns to be shown + #[arg(long, default_value_t = 3usize, value_name = "NUM")] + pub number_of_file_churns: usize, + ///NUM of commits from HEAD used to compute file churns + #[arg(long, default_value_t = 100usize, value_name = "NUM")] + pub churn_commit_limit: usize, /// Ignore all files & directories matching EXCLUDE #[arg(long, short, num_args = 1.., value_hint = ValueHint::AnyPath)] pub exclude: Vec, @@ -228,6 +234,8 @@ impl Default for InfoCliOptions { InfoCliOptions { number_of_authors: 3, number_of_languages: 6, + number_of_file_churns: 3, + churn_commit_limit: 100, exclude: Vec::default(), no_bots: Option::default(), no_merges: Default::default(), diff --git a/src/info/churn.rs b/src/info/churn.rs index bc6fe34ff..ff3ae3260 100644 --- a/src/info/churn.rs +++ b/src/info/churn.rs @@ -6,14 +6,14 @@ use std::fmt::Write; #[derive(Serialize, Clone)] #[serde(rename_all = "camelCase")] -pub struct Churn { +pub struct FileChurn { file_path: String, nbr_of_commits: usize, #[serde(skip_serializing)] number_separator: NumberSeparator, } -impl Churn { +impl FileChurn { pub fn new( file_path: String, nbr_of_commits: usize, @@ -27,7 +27,7 @@ impl Churn { } } -impl std::fmt::Display for Churn { +impl std::fmt::Display for FileChurn { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { write!( f, @@ -40,14 +40,17 @@ impl std::fmt::Display for Churn { #[derive(Serialize)] pub struct ChurnInfo { - pub churn: Vec, + pub file_churns: Vec, #[serde(skip_serializing)] pub info_color: DynColors, } impl ChurnInfo { pub fn new(info_color: DynColors, commit_metrics: &CommitMetrics) -> Self { - let churn = commit_metrics.churns_to_display.clone(); - Self { churn, info_color } + let file_churns = commit_metrics.file_churns_to_display.clone(); + Self { + file_churns, + info_color, + } } } impl std::fmt::Display for ChurnInfo { @@ -56,7 +59,7 @@ impl std::fmt::Display for ChurnInfo { let pad = self.title().len() + 2; - for (i, churn) in self.churn.iter().enumerate() { + for (i, churn) in self.file_churns.iter().enumerate() { let churn_str = churn.color(self.info_color); if i == 0 { diff --git a/src/info/contributors.rs b/src/info/contributors.rs index ba3656415..082a60cf9 100644 --- a/src/info/contributors.rs +++ b/src/info/contributors.rs @@ -56,7 +56,7 @@ mod test { let timestamp = Time::now_utc(); let commit_metrics = CommitMetrics { authors_to_display: vec![], - churns_to_display: vec![], + file_churns_to_display: vec![], total_number_of_authors: 12, total_number_of_commits: 2, is_shallow: true, diff --git a/src/info/mod.rs b/src/info/mod.rs index 6e4ab3a87..9814eae69 100644 --- a/src/info/mod.rs +++ b/src/info/mod.rs @@ -149,14 +149,7 @@ pub fn build_info(cli_options: &CliOptions) -> Result { let manifest = get_manifest(&repo_path)?; let repo_url = get_repo_url(&repo); - let commit_metrics = CommitMetrics::new( - &repo, - cli_options.info.no_merges, - &cli_options.info.no_bots, - cli_options.info.number_of_authors, - cli_options.info.email, - cli_options.text_formatting.number_separator, - )?; + let commit_metrics = CommitMetrics::new(&repo, cli_options)?; let true_color = match cli_options.ascii.true_color { When::Always => true, When::Never => false, diff --git a/src/info/utils/git.rs b/src/info/utils/git.rs index 4b82c5043..09c77a6b2 100644 --- a/src/info/utils/git.rs +++ b/src/info/utils/git.rs @@ -1,6 +1,6 @@ -use crate::cli::{MyRegex, NumberSeparator}; +use crate::cli::{CliOptions, MyRegex, NumberSeparator}; use crate::info::author::Author; -use crate::info::churn::Churn; +use crate::info::churn::FileChurn; use anyhow::Result; use gix::bstr::ByteSlice; use gix::bstr::{BString, Utf8Error}; @@ -14,7 +14,7 @@ use std::str::FromStr; pub struct CommitMetrics { pub authors_to_display: Vec, - pub churns_to_display: Vec, + pub file_churns_to_display: Vec, pub total_number_of_authors: usize, pub total_number_of_commits: usize, /// false if we have found the first commit that started it all, true if the repository is shallow. @@ -36,15 +36,8 @@ impl From for Sig { } impl CommitMetrics { - pub fn new( - repo: &gix::Repository, - no_merges: bool, - no_bots: &Option>, - number_of_authors_to_display: usize, - show_email: bool, - number_separator: NumberSeparator, - ) -> Result { - let bot_regex_pattern = get_no_bots_regex(no_bots)?; + pub fn new(repo: &gix::Repository, options: &CliOptions) -> Result { + let bot_regex_pattern = get_no_bots_regex(&options.info.no_bots)?; let mut time_of_most_recent_commit = None; let mut time_of_first_commit = None; let mut commit_iter = repo.head_commit()?.ancestors().all()?; @@ -59,7 +52,7 @@ impl CommitMetrics { while let Some(commit_id) = commit_iter_peekable.next() { let commit = commit_id?.object()?.into_commit(); - if no_merges && commit.parent_ids().count() > 1 { + if options.info.no_merges && commit.parent_ids().count() > 1 { continue; } @@ -71,7 +64,7 @@ impl CommitMetrics { *number_of_commits_by_signature.entry(sig).or_insert(0) += 1; - if total_number_of_commits <= 100 { + if total_number_of_commits <= options.info.churn_commit_limit { compute_diff(&mut number_of_commits_by_file_path, &commit, repo)?; } @@ -91,12 +84,16 @@ impl CommitMetrics { let (authors_to_display, total_number_of_authors) = compute_authors( number_of_commits_by_signature, total_number_of_commits, - number_of_authors_to_display, - show_email, - number_separator, + options.info.number_of_authors, + options.info.email, + options.text_formatting.number_separator, ); - let churns_to_display = compute_churns(number_of_commits_by_file_path, number_separator); + let file_churns_to_display = compute_file_churns( + number_of_commits_by_file_path, + options.info.number_of_file_churns, + options.text_formatting.number_separator, + ); // This could happen if a branch pointed to non-commit object, so no traversal actually happens. let (time_of_first_commit, time_of_most_recent_commit) = time_of_first_commit @@ -105,7 +102,7 @@ impl CommitMetrics { Ok(Self { authors_to_display, - churns_to_display, + file_churns_to_display, total_number_of_authors, total_number_of_commits, is_shallow: repo.is_shallow(), @@ -115,10 +112,11 @@ impl CommitMetrics { } } -fn compute_churns( +fn compute_file_churns( number_of_commits_by_file_path: HashMap, + number_of_file_churns_to_display: usize, number_separator: NumberSeparator, -) -> Vec { +) -> Vec { let mut number_of_commits_by_file_path_sorted: Vec<(String, usize)> = number_of_commits_by_file_path.into_iter().collect(); @@ -127,8 +125,10 @@ fn compute_churns( number_of_commits_by_file_path_sorted .into_iter() - .map(|(file_path, nbr_of_commits)| Churn::new(file_path, nbr_of_commits, number_separator)) - .take(3) + .map(|(file_path, nbr_of_commits)| { + FileChurn::new(file_path, nbr_of_commits, number_separator) + }) + .take(number_of_file_churns_to_display) .collect() } diff --git a/tests/snapshots/repo__repo.snap b/tests/snapshots/repo__repo.snap index 323caa5b7..2ed063e8c 100644 --- a/tests/snapshots/repo__repo.snap +++ b/tests/snapshots/repo__repo.snap @@ -65,19 +65,19 @@ expression: info "authors": [ { "name": "Author Four", - "email": "author4@example.org", + "email": null, "nbrOfCommits": 1, "contribution": 25 }, { "name": "Author One", - "email": "author1@example.org", + "email": null, "nbrOfCommits": 1, "contribution": 25 }, { "name": "Author Three", - "email": "author3@example.org", + "email": null, "nbrOfCommits": 1, "contribution": 25 } @@ -105,6 +105,16 @@ expression: info "isShallow": false } }, + { + "ChurnInfo": { + "file_churns": [ + { + "filePath": "code.rs", + "nbrOfCommits": 4 + } + ] + } + }, { "LocInfo": { "linesOfCode": 4 From 6850da51ba85b2f9fbe58aa38ab6a2db62388696 Mon Sep 17 00:00:00 2001 From: o2sh Date: Sun, 4 Jun 2023 01:09:12 +0200 Subject: [PATCH 06/28] fix integration test --- tests/repo.rs | 6 +++++- tests/snapshots/repo__repo.snap | 6 +++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/repo.rs b/tests/repo.rs index c1cc9c6e2..cd5a5cb65 100644 --- a/tests/repo.rs +++ b/tests/repo.rs @@ -1,6 +1,6 @@ use anyhow::Result; use gix::{open, Repository, ThreadSafeRepository}; -use onefetch::cli::{CliOptions, TextForamttingCliOptions}; +use onefetch::cli::{CliOptions, InfoCliOptions, TextForamttingCliOptions}; use onefetch::info::{build_info, get_work_dir}; fn repo(name: &str) -> Result { @@ -30,6 +30,10 @@ fn test_repo() -> Result<()> { let repo = repo("repo.sh")?; let config: CliOptions = CliOptions { input: repo.path().to_path_buf(), + info: InfoCliOptions { + email: true, + ..Default::default() + }, text_formatting: TextForamttingCliOptions { iso_time: true, ..Default::default() diff --git a/tests/snapshots/repo__repo.snap b/tests/snapshots/repo__repo.snap index 2ed063e8c..eee772796 100644 --- a/tests/snapshots/repo__repo.snap +++ b/tests/snapshots/repo__repo.snap @@ -65,19 +65,19 @@ expression: info "authors": [ { "name": "Author Four", - "email": null, + "email": "author4@example.org", "nbrOfCommits": 1, "contribution": 25 }, { "name": "Author One", - "email": null, + "email": "author1@example.org", "nbrOfCommits": 1, "contribution": 25 }, { "name": "Author Three", - "email": null, + "email": "author3@example.org", "nbrOfCommits": 1, "contribution": 25 } From 3d58e2642401ac091157f7e2af65c927d0abddd8 Mon Sep 17 00:00:00 2001 From: o2sh Date: Sun, 4 Jun 2023 01:39:51 +0200 Subject: [PATCH 07/28] add unit tests --- src/info/churn.rs | 55 ++++++++++++++++++++++++++++++++++++++++--- src/info/utils/git.rs | 24 +++++++++++++++++++ 2 files changed, 76 insertions(+), 3 deletions(-) diff --git a/src/info/churn.rs b/src/info/churn.rs index ff3ae3260..ae2723bbc 100644 --- a/src/info/churn.rs +++ b/src/info/churn.rs @@ -7,8 +7,8 @@ use std::fmt::Write; #[derive(Serialize, Clone)] #[serde(rename_all = "camelCase")] pub struct FileChurn { - file_path: String, - nbr_of_commits: usize, + pub file_path: String, + pub nbr_of_commits: usize, #[serde(skip_serializing)] number_separator: NumberSeparator, } @@ -91,7 +91,7 @@ impl InfoField for ChurnInfo { fn truncate_file_path(path: &str, depth: usize) -> String { let components: Vec<&str> = path.split('/').collect(); - if components.len() <= depth { + if depth == 0 || components.len() <= depth { return path.to_string(); } @@ -104,3 +104,52 @@ fn truncate_file_path(path: &str, depth: usize) -> String { truncated_path } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_display_file_churn() { + let file_churn = FileChurn::new("path/to/file.txt".into(), 50, NumberSeparator::Plain); + + assert_eq!(file_churn.to_string(), ".../to/file.txt 50"); + } + + #[test] + fn test_churn_info_value_with_two_file_churns() { + let file_churn_1 = FileChurn::new("path/to/file.txt".into(), 50, NumberSeparator::Plain); + let file_churn_2 = FileChurn::new("file_2.txt".into(), 30, NumberSeparator::Plain); + + let churn_info = ChurnInfo { + info_color: DynColors::Rgb(255, 0, 0), + file_churns: vec![file_churn_1, file_churn_2], + }; + + assert!(churn_info.value().contains( + &".../to/file.txt 50" + .color(DynColors::Rgb(255, 0, 0)) + .to_string() + )); + + assert!(churn_info + .value() + .contains(&"file_2.txt 30".color(DynColors::Rgb(255, 0, 0)).to_string())); + } + + #[test] + fn test_truncate_file_path() { + assert_eq!( + truncate_file_path("path/to/file.txt", 3), + "path/to/file.txt" + ); + assert_eq!( + truncate_file_path("another/file.txt", 2), + "another/file.txt" + ); + assert_eq!(truncate_file_path("file.txt", 1), "file.txt"); + assert_eq!(truncate_file_path("path/to/file.txt", 2), ".../to/file.txt"); + assert_eq!(truncate_file_path("another/file.txt", 1), ".../file.txt"); + assert_eq!(truncate_file_path("file.txt", 0), "file.txt"); + } +} diff --git a/src/info/utils/git.rs b/src/info/utils/git.rs index 09c77a6b2..1ecfb4758 100644 --- a/src/info/utils/git.rs +++ b/src/info/utils/git.rs @@ -300,4 +300,28 @@ mod tests { assert_eq!(authors.len(), 2); assert_eq!(authors.get(0).unwrap().name, "Ellen Smith".to_string()); } + + #[test] + fn test_compute_file_churns() { + let mut number_of_commits_by_file_path = HashMap::new(); + number_of_commits_by_file_path.insert("path/to/file1.txt".to_string(), 2); + number_of_commits_by_file_path.insert("path/to/file2.txt".to_string(), 5); + number_of_commits_by_file_path.insert("path/to/file3.txt".to_string(), 3); + number_of_commits_by_file_path.insert("path/to/file4.txt".to_string(), 7); + + let number_of_file_churns_to_display = 3; + let number_separator = NumberSeparator::Comma; + let file_churns = compute_file_churns( + number_of_commits_by_file_path, + number_of_file_churns_to_display, + number_separator, + ); + + assert_eq!(file_churns.len(), 3); + assert_eq!( + file_churns.get(0).unwrap().file_path, + "path/to/file4.txt".to_string() + ); + assert_eq!(file_churns.get(0).unwrap().nbr_of_commits, 7); + } } From 6ffd49c31b6c8e5c9781de804bfb3fb9d121e57f Mon Sep 17 00:00:00 2001 From: o2sh Date: Sun, 4 Jun 2023 12:59:40 +0200 Subject: [PATCH 08/28] try fix codeowners --- .github/CODEOWNERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index ab5905ae4..dc2cc7213 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1,6 +1,6 @@ /.github/ @spenserblack @o2sh /image/src/ @cephalonRho @yoichi -/src/info/utils/git.rs @byron +/src/info/utils/git.rs @spenserblack /src/info/ @o2sh /languages.yaml @spenserblack @o2sh /src/info/langs/language.tera @spenserblack From 5f91e6e34c5d437f30cdffa6fc93683320de5e2a Mon Sep 17 00:00:00 2001 From: o2sh Date: Sun, 4 Jun 2023 13:02:59 +0200 Subject: [PATCH 09/28] fix codeowners --- .github/CODEOWNERS | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index dc2cc7213..a487dfbfa 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1,7 +1,6 @@ /.github/ @spenserblack @o2sh /image/src/ @cephalonRho @yoichi -/src/info/utils/git.rs @spenserblack -/src/info/ @o2sh +/src/info/utils/git.rs @byron /languages.yaml @spenserblack @o2sh /src/info/langs/language.tera @spenserblack /src/cli.rs @spenserblack @o2sh From afa578849e9ee66e1ffe821d2457896bdd25bbf3 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 4 Jun 2023 16:07:30 +0200 Subject: [PATCH 10/28] Optimize diff implementation * delay conversion to String for filepaths to the last moment. That way, only the paths that are displayed will be converted in an operation that isn't free. * change diff implementation to decode parents only once, instead of three times in the commmon case. * setup an object cache in the `Repository` for faster traversals and much faster diffs. --- src/info/mod.rs | 4 ++- src/info/utils/git.rs | 60 +++++++++++++++++++++---------------------- 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/src/info/mod.rs b/src/info/mod.rs index 9814eae69..4d673709d 100644 --- a/src/info/mod.rs +++ b/src/info/mod.rs @@ -116,7 +116,7 @@ impl std::fmt::Display for Info { } pub fn build_info(cli_options: &CliOptions) -> Result { - let repo = gix::ThreadSafeRepository::discover_opts( + let mut repo = gix::ThreadSafeRepository::discover_opts( &cli_options.input, gix::discover::upwards::Options { dot_git_only: true, @@ -125,6 +125,8 @@ pub fn build_info(cli_options: &CliOptions) -> Result { Mapping::default(), )? .to_thread_local(); + // Having an object cache is important for getting much better traversal and diff performance. + repo.object_cache_size_if_unset(4 * 1024 * 1024); let repo_path = get_work_dir(&repo)?; let loc_by_language_sorted_handle = std::thread::spawn({ diff --git a/src/info/utils/git.rs b/src/info/utils/git.rs index 1ecfb4758..f1dd42ee6 100644 --- a/src/info/utils/git.rs +++ b/src/info/utils/git.rs @@ -7,6 +7,7 @@ use gix::bstr::{BString, Utf8Error}; use gix::object::tree::diff::change::Event; use gix::object::tree::diff::{Action, Change}; use gix::objs::tree::EntryMode; +use gix::prelude::ObjectIdExt; use gix::Commit; use regex::Regex; use std::collections::HashMap; @@ -46,7 +47,7 @@ impl CommitMetrics { let mailmap_config = repo.open_mailmap(); let mut number_of_commits_by_signature: HashMap = HashMap::new(); let mut total_number_of_commits = 0; - let mut number_of_commits_by_file_path: HashMap = HashMap::new(); + let mut number_of_commits_by_file_path: HashMap = HashMap::new(); // From newest to oldest while let Some(commit_id) = commit_iter_peekable.next() { @@ -113,12 +114,11 @@ impl CommitMetrics { } fn compute_file_churns( - number_of_commits_by_file_path: HashMap, + number_of_commits_by_file_path: HashMap, number_of_file_churns_to_display: usize, number_separator: NumberSeparator, ) -> Vec { - let mut number_of_commits_by_file_path_sorted: Vec<(String, usize)> = - number_of_commits_by_file_path.into_iter().collect(); + let mut number_of_commits_by_file_path_sorted = Vec::from_iter(number_of_commits_by_file_path); number_of_commits_by_file_path_sorted .sort_by(|(_, a_count), (_, b_count)| b_count.cmp(a_count)); @@ -126,7 +126,7 @@ fn compute_file_churns( number_of_commits_by_file_path_sorted .into_iter() .map(|(file_path, nbr_of_commits)| { - FileChurn::new(file_path, nbr_of_commits, number_separator) + FileChurn::new(file_path.to_string(), nbr_of_commits, number_separator) }) .take(number_of_file_churns_to_display) .collect() @@ -168,40 +168,38 @@ fn compute_authors( } fn compute_diff( - change_map: &mut HashMap, + change_map: &mut HashMap, commit: &Commit, repo: &gix::Repository, ) -> Result<()> { - // Handles the very first commit - if commit.parent_ids().count() == 0 { - repo.empty_tree() + let mut parents = commit.parent_ids(); + let parents = ( + parents + .next() + .map(|parent_id| -> Result<_> { Ok(parent_id.object()?.into_commit().tree_id()?) }) + .unwrap_or_else(|| { + Ok(gix::hash::ObjectId::empty_tree(repo.object_hash()).attach(repo)) + })?, + parents.next(), + ); + // Ignore merge commits + if let (tree_id, None) = parents { + tree_id + .object()? + .into_tree() .changes()? .track_path() .for_each_to_obtain_tree(&commit.tree()?, |change| { for_each_change(change, change_map) })?; } - // Ignore merge commits - else if commit.parent_ids().count() == 1 { - for parent_id in commit.parent_ids() { - parent_id - .object()? - .into_commit() - .tree()? - .changes()? - .track_path() - .for_each_to_obtain_tree(&commit.tree()?, |change| { - for_each_change(change, change_map) - })?; - } - } Ok(()) } fn for_each_change( change: Change, - change_map: &mut HashMap, + change_map: &mut HashMap, ) -> Result { let is_file_change = match change.event { Event::Addition { entry_mode, .. } | Event::Modification { entry_mode, .. } => { @@ -210,11 +208,11 @@ fn for_each_change( Event::Deletion { .. } | Event::Rewrite { .. } => false, }; if is_file_change { - let path = change.location.to_os_str()?.to_string_lossy(); - *change_map.entry(path.into_owned()).or_insert(0) += 1; + let path = change.location; + *change_map.entry(path.to_owned()).or_insert(0) += 1; } - Ok::(Action::Continue) + Ok(Action::Continue) } fn get_no_bots_regex(no_bots: &Option>) -> Result> { @@ -304,10 +302,10 @@ mod tests { #[test] fn test_compute_file_churns() { let mut number_of_commits_by_file_path = HashMap::new(); - number_of_commits_by_file_path.insert("path/to/file1.txt".to_string(), 2); - number_of_commits_by_file_path.insert("path/to/file2.txt".to_string(), 5); - number_of_commits_by_file_path.insert("path/to/file3.txt".to_string(), 3); - number_of_commits_by_file_path.insert("path/to/file4.txt".to_string(), 7); + number_of_commits_by_file_path.insert("path/to/file1.txt".into(), 2); + number_of_commits_by_file_path.insert("path/to/file2.txt".into(), 5); + number_of_commits_by_file_path.insert("path/to/file3.txt".into(), 3); + number_of_commits_by_file_path.insert("path/to/file4.txt".into(), 7); let number_of_file_churns_to_display = 3; let number_separator = NumberSeparator::Comma; From 033958103f97750a946c14536b74c1f0ba27420b Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 4 Jun 2023 16:36:50 +0200 Subject: [PATCH 11/28] Don't fail on missing parent as we want to work in shallow repos, too --- src/info/utils/git.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/info/utils/git.rs b/src/info/utils/git.rs index f1dd42ee6..b7a279ac6 100644 --- a/src/info/utils/git.rs +++ b/src/info/utils/git.rs @@ -176,10 +176,8 @@ fn compute_diff( let parents = ( parents .next() - .map(|parent_id| -> Result<_> { Ok(parent_id.object()?.into_commit().tree_id()?) }) - .unwrap_or_else(|| { - Ok(gix::hash::ObjectId::empty_tree(repo.object_hash()).attach(repo)) - })?, + .and_then(|parent_id| parent_id.object().ok()?.into_commit().tree_id().ok()) + .unwrap_or_else(|| gix::hash::ObjectId::empty_tree(repo.object_hash()).attach(repo)), parents.next(), ); // Ignore merge commits From c36fe41b783058ba3a43a04c3cdacaaf46a471c4 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 4 Jun 2023 17:07:55 +0200 Subject: [PATCH 12/28] Increase performance by decoding the commit only once Previously, each time we would query commit information, the commit would lazily be decoded until the point of interest under the hood. Now we decode everything once, which is faster than what happened before. Note that diffs are still causing the parents to be decoded even we *could* pass them in, but it's not worth the complexity for just 100 commits (the default value for churn). --- src/info/utils/git.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/info/utils/git.rs b/src/info/utils/git.rs index b7a279ac6..4103d05ed 100644 --- a/src/info/utils/git.rs +++ b/src/info/utils/git.rs @@ -52,12 +52,13 @@ impl CommitMetrics { // From newest to oldest while let Some(commit_id) = commit_iter_peekable.next() { let commit = commit_id?.object()?.into_commit(); + let commit_ref = commit.decode()?; - if options.info.no_merges && commit.parent_ids().count() > 1 { + if options.info.no_merges && commit_ref.parents.len() > 1 { continue; } - let sig = Sig::from(mailmap_config.resolve(commit.author()?)); + let sig = Sig::from(mailmap_config.resolve(commit_ref.author())); if is_bot(&sig.name, &bot_regex_pattern) { continue; @@ -69,10 +70,7 @@ impl CommitMetrics { compute_diff(&mut number_of_commits_by_file_path, &commit, repo)?; } - let commit_time = commit - .time() - .expect("Could not read commit's creation time"); - + let commit_time = commit_ref.time(); time_of_most_recent_commit.get_or_insert(commit_time); if commit_iter_peekable.peek().is_none() { From fee2db327a24d7a2efbe457fd3f52b6847a8f02d Mon Sep 17 00:00:00 2001 From: o2sh Date: Sun, 4 Jun 2023 21:22:37 +0200 Subject: [PATCH 13/28] track changes on executable files --- src/info/utils/git.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/info/utils/git.rs b/src/info/utils/git.rs index 4103d05ed..d248d8d6c 100644 --- a/src/info/utils/git.rs +++ b/src/info/utils/git.rs @@ -199,7 +199,7 @@ fn for_each_change( ) -> Result { let is_file_change = match change.event { Event::Addition { entry_mode, .. } | Event::Modification { entry_mode, .. } => { - entry_mode == EntryMode::Blob + entry_mode == EntryMode::Blob || entry_mode == EntryMode::BlobExecutable } Event::Deletion { .. } | Event::Rewrite { .. } => false, }; From ce547464174673de41b2200d2f66af25d4e1d2c2 Mon Sep 17 00:00:00 2001 From: o2sh Date: Sun, 4 Jun 2023 21:56:05 +0200 Subject: [PATCH 14/28] remove for_each method --- src/info/utils/git.rs | 39 ++++++++++++++++----------------------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/src/info/utils/git.rs b/src/info/utils/git.rs index d248d8d6c..394e858fd 100644 --- a/src/info/utils/git.rs +++ b/src/info/utils/git.rs @@ -5,7 +5,7 @@ use anyhow::Result; use gix::bstr::ByteSlice; use gix::bstr::{BString, Utf8Error}; use gix::object::tree::diff::change::Event; -use gix::object::tree::diff::{Action, Change}; +use gix::object::tree::diff::Action; use gix::objs::tree::EntryMode; use gix::prelude::ObjectIdExt; use gix::Commit; @@ -67,7 +67,7 @@ impl CommitMetrics { *number_of_commits_by_signature.entry(sig).or_insert(0) += 1; if total_number_of_commits <= options.info.churn_commit_limit { - compute_diff(&mut number_of_commits_by_file_path, &commit, repo)?; + compute_diff_with_parent(&mut number_of_commits_by_file_path, &commit, repo)?; } let commit_time = commit_ref.time(); @@ -165,7 +165,7 @@ fn compute_authors( (authors, total_number_of_authors) } -fn compute_diff( +fn compute_diff_with_parent( change_map: &mut HashMap, commit: &Commit, repo: &gix::Repository, @@ -178,7 +178,7 @@ fn compute_diff( .unwrap_or_else(|| gix::hash::ObjectId::empty_tree(repo.object_hash()).attach(repo)), parents.next(), ); - // Ignore merge commits + if let (tree_id, None) = parents { tree_id .object()? @@ -186,31 +186,24 @@ fn compute_diff( .changes()? .track_path() .for_each_to_obtain_tree(&commit.tree()?, |change| { - for_each_change(change, change_map) + let is_file_change = match change.event { + Event::Addition { entry_mode, .. } | Event::Modification { entry_mode, .. } => { + entry_mode == EntryMode::Blob || entry_mode == EntryMode::BlobExecutable + } + Event::Deletion { .. } | Event::Rewrite { .. } => false, + }; + if is_file_change { + let path = change.location; + *change_map.entry(path.to_owned()).or_insert(0) += 1; + } + + Ok::(Action::Continue) })?; } Ok(()) } -fn for_each_change( - change: Change, - change_map: &mut HashMap, -) -> Result { - let is_file_change = match change.event { - Event::Addition { entry_mode, .. } | Event::Modification { entry_mode, .. } => { - entry_mode == EntryMode::Blob || entry_mode == EntryMode::BlobExecutable - } - Event::Deletion { .. } | Event::Rewrite { .. } => false, - }; - if is_file_change { - let path = change.location; - *change_map.entry(path.to_owned()).or_insert(0) += 1; - } - - Ok(Action::Continue) -} - fn get_no_bots_regex(no_bots: &Option>) -> Result> { let reg = if let Some(r) = no_bots.clone() { match r { From f9e0777516f91ab3069b579faa4d6fd345a5f7fb Mon Sep 17 00:00:00 2001 From: o2sh Date: Sun, 4 Jun 2023 22:25:43 +0200 Subject: [PATCH 15/28] use horizontal ellipsis --- src/info/churn.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/info/churn.rs b/src/info/churn.rs index ae2723bbc..0f999fe58 100644 --- a/src/info/churn.rs +++ b/src/info/churn.rs @@ -100,7 +100,7 @@ fn truncate_file_path(path: &str, depth: usize) -> String { .skip(components.len() - depth) .copied() .collect(); - let truncated_path = format!(".../{}", truncated_components.join("/")); + let truncated_path = format!("\u{2026}/{}", truncated_components.join("/")); truncated_path } @@ -113,7 +113,7 @@ mod tests { fn test_display_file_churn() { let file_churn = FileChurn::new("path/to/file.txt".into(), 50, NumberSeparator::Plain); - assert_eq!(file_churn.to_string(), ".../to/file.txt 50"); + assert_eq!(file_churn.to_string(), "\u{2026}/to/file.txt 50"); } #[test] @@ -127,7 +127,7 @@ mod tests { }; assert!(churn_info.value().contains( - &".../to/file.txt 50" + &"\u{2026}/to/file.txt 50" .color(DynColors::Rgb(255, 0, 0)) .to_string() )); @@ -148,8 +148,14 @@ mod tests { "another/file.txt" ); assert_eq!(truncate_file_path("file.txt", 1), "file.txt"); - assert_eq!(truncate_file_path("path/to/file.txt", 2), ".../to/file.txt"); - assert_eq!(truncate_file_path("another/file.txt", 1), ".../file.txt"); + assert_eq!( + truncate_file_path("path/to/file.txt", 2), + "\u{2026}/to/file.txt" + ); + assert_eq!( + truncate_file_path("another/file.txt", 1), + "\u{2026}/file.txt" + ); assert_eq!(truncate_file_path("file.txt", 0), "file.txt"); } } From 27246fe2d339d7d83208b8d15fc7297adb8f8f5a Mon Sep 17 00:00:00 2001 From: o2sh Date: Mon, 5 Jun 2023 22:58:37 +0200 Subject: [PATCH 16/28] review --- src/info/author.rs | 4 ++-- src/info/churn.rs | 7 +++---- src/info/langs/language.rs | 4 ++-- src/main.rs | 2 +- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/info/author.rs b/src/info/author.rs index c2e5378e4..b16bdcd4e 100644 --- a/src/info/author.rs +++ b/src/info/author.rs @@ -87,9 +87,9 @@ impl std::fmt::Display for AuthorsInfo { let author_str = author.color(self.info_color); if i == 0 { - let _ = write!(authors_info, "{author_str}"); + write!(authors_info, "{author_str}")?; } else { - let _ = write!(authors_info, "\n{: String { .skip(components.len() - depth) .copied() .collect(); - let truncated_path = format!("\u{2026}/{}", truncated_components.join("/")); - truncated_path + format!("\u{2026}/{}", truncated_components.join("/")) } #[cfg(test)] diff --git a/src/info/langs/language.rs b/src/info/langs/language.rs index 1d1cd6e93..86a64108a 100644 --- a/src/info/langs/language.rs +++ b/src/info/langs/language.rs @@ -129,13 +129,13 @@ impl std::fmt::Display for LanguagesInfo { format!("{language} ({formatted_number} %)").color(self.info_color) ); if i % 2 == 0 { - let _ = write!( + write!( languages_info, "\n{: Result<()> { setup_panic!(); #[cfg(windows)] - let _ = enable_ansi_support::enable_ansi_support(); + enable_ansi_support::enable_ansi_support()?; let cli_options = cli::CliOptions::parse(); From 7cae61b71ab549d28e09c7fc76e05fe60f59037c Mon Sep 17 00:00:00 2001 From: o2sh Date: Mon, 5 Jun 2023 23:16:34 +0200 Subject: [PATCH 17/28] use MAIN_SEPERATOR when building path --- src/info/churn.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/info/churn.rs b/src/info/churn.rs index 4966817be..7ce44eea0 100644 --- a/src/info/churn.rs +++ b/src/info/churn.rs @@ -2,7 +2,7 @@ use super::utils::{git::CommitMetrics, info_field::InfoField}; use crate::{cli::NumberSeparator, info::format_number}; use owo_colors::{DynColors, OwoColorize}; use serde::Serialize; -use std::fmt::Write; +use std::{fmt::Write, path::MAIN_SEPARATOR_STR}; #[derive(Serialize, Clone)] #[serde(rename_all = "camelCase")] @@ -101,7 +101,11 @@ fn truncate_file_path(path: &str, depth: usize) -> String { .copied() .collect(); - format!("\u{2026}/{}", truncated_components.join("/")) + format!( + "\u{2026}{}{}", + MAIN_SEPARATOR_STR, + truncated_components.join(MAIN_SEPARATOR_STR) + ) } #[cfg(test)] From 195fe7396b9ed0270bcd5360ef59a7e8de4a1698 Mon Sep 17 00:00:00 2001 From: o2sh Date: Mon, 5 Jun 2023 23:18:09 +0200 Subject: [PATCH 18/28] revert --- src/info/churn.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/info/churn.rs b/src/info/churn.rs index 7ce44eea0..4966817be 100644 --- a/src/info/churn.rs +++ b/src/info/churn.rs @@ -2,7 +2,7 @@ use super::utils::{git::CommitMetrics, info_field::InfoField}; use crate::{cli::NumberSeparator, info::format_number}; use owo_colors::{DynColors, OwoColorize}; use serde::Serialize; -use std::{fmt::Write, path::MAIN_SEPARATOR_STR}; +use std::fmt::Write; #[derive(Serialize, Clone)] #[serde(rename_all = "camelCase")] @@ -101,11 +101,7 @@ fn truncate_file_path(path: &str, depth: usize) -> String { .copied() .collect(); - format!( - "\u{2026}{}{}", - MAIN_SEPARATOR_STR, - truncated_components.join(MAIN_SEPARATOR_STR) - ) + format!("\u{2026}/{}", truncated_components.join("/")) } #[cfg(test)] From 627b9f2c45d7c0c321248c319de6685a49251bcd Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 6 Jun 2023 08:22:35 +0200 Subject: [PATCH 19/28] run expensive diffs in parallel and abort them once we run out of time. That way, we could even use higher amounts of diffs if we wanted to, or could warn if there was not enough time to reach the desired amount of diffs. --- src/info/utils/git.rs | 62 ++++++++++++++++++++++++++++--------------- tests/repo.rs | 4 ++- 2 files changed, 44 insertions(+), 22 deletions(-) diff --git a/src/info/utils/git.rs b/src/info/utils/git.rs index 394e858fd..ff6a145b9 100644 --- a/src/info/utils/git.rs +++ b/src/info/utils/git.rs @@ -12,6 +12,8 @@ use gix::Commit; use regex::Regex; use std::collections::HashMap; use std::str::FromStr; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::Arc; pub struct CommitMetrics { pub authors_to_display: Vec, @@ -47,38 +49,56 @@ impl CommitMetrics { let mailmap_config = repo.open_mailmap(); let mut number_of_commits_by_signature: HashMap = HashMap::new(); let mut total_number_of_commits = 0; - let mut number_of_commits_by_file_path: HashMap = HashMap::new(); + let (tx, rx) = std::sync::mpsc::channel::(); + let cancel_calc = Arc::new(AtomicBool::default()); + let calc_diffs = std::thread::spawn({ + let repo = repo.clone(); + let cancel_calc = cancel_calc.clone(); + move || -> Result<_> { + let mut number_of_commits_by_file_path: HashMap = HashMap::new(); + for commit in rx { + if cancel_calc.load(Ordering::Relaxed) { + break; + } + let commit = commit.attach(&repo).into_commit(); + compute_diff_with_parent(&mut number_of_commits_by_file_path, &commit, &repo)?; + } + Ok(number_of_commits_by_file_path) + } + }); // From newest to oldest while let Some(commit_id) = commit_iter_peekable.next() { let commit = commit_id?.object()?.into_commit(); - let commit_ref = commit.decode()?; + { + let commit_ref = commit.decode()?; - if options.info.no_merges && commit_ref.parents.len() > 1 { - continue; - } + if options.info.no_merges && commit_ref.parents.len() > 1 { + continue; + } - let sig = Sig::from(mailmap_config.resolve(commit_ref.author())); + let sig = Sig::from(mailmap_config.resolve(commit_ref.author())); - if is_bot(&sig.name, &bot_regex_pattern) { - continue; - } + if is_bot(&sig.name, &bot_regex_pattern) { + continue; + } - *number_of_commits_by_signature.entry(sig).or_insert(0) += 1; + *number_of_commits_by_signature.entry(sig).or_insert(0) += 1; + let commit_time = commit_ref.time(); + time_of_most_recent_commit.get_or_insert(commit_time); - if total_number_of_commits <= options.info.churn_commit_limit { - compute_diff_with_parent(&mut number_of_commits_by_file_path, &commit, repo)?; - } - - let commit_time = commit_ref.time(); - time_of_most_recent_commit.get_or_insert(commit_time); + if commit_iter_peekable.peek().is_none() { + time_of_first_commit = commit_time.into(); + } - if commit_iter_peekable.peek().is_none() { - time_of_first_commit = commit_time.into(); + total_number_of_commits += 1; + } + if total_number_of_commits <= options.info.churn_commit_limit { + tx.send(commit.detach()).ok(); } - - total_number_of_commits += 1; } + drop(tx); + cancel_calc.store(true, Ordering::SeqCst); let (authors_to_display, total_number_of_authors) = compute_authors( number_of_commits_by_signature, @@ -89,7 +109,7 @@ impl CommitMetrics { ); let file_churns_to_display = compute_file_churns( - number_of_commits_by_file_path, + calc_diffs.join().expect("never panics")?, options.info.number_of_file_churns, options.text_formatting.number_separator, ); diff --git a/tests/repo.rs b/tests/repo.rs index cd5a5cb65..be8d926f3 100644 --- a/tests/repo.rs +++ b/tests/repo.rs @@ -41,11 +41,13 @@ fn test_repo() -> Result<()> { ..Default::default() }; let info = build_info(&config)?; + // Note that `nbrOfCommits` is hard-coded here as its actual value is non-deterministic due to time-based computation. insta::assert_json_snapshot!( info, { ".title.gitVersion" => "git version", - ".infoFields[].HeadInfo.headRefs.shortCommitId" => "short commit" + ".infoFields[].HeadInfo.headRefs.shortCommitId" => "short commit", + ".infoFields[].ChurnInfo.file_churns[].nbrOfCommits" => 4 } ); From 1433c1702b7b4089420a4c9cb64b113d3650ee04 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 6 Jun 2023 10:58:12 +0200 Subject: [PATCH 20/28] Always calculate at least one diff for 'churn' That way, there is always some data to work with. This is important in case the repo is very small and the thread needs some time to start-up and finish. --- src/info/utils/git.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/info/utils/git.rs b/src/info/utils/git.rs index ff6a145b9..3a1be0256 100644 --- a/src/info/utils/git.rs +++ b/src/info/utils/git.rs @@ -57,11 +57,11 @@ impl CommitMetrics { move || -> Result<_> { let mut number_of_commits_by_file_path: HashMap = HashMap::new(); for commit in rx { + let commit = commit.attach(&repo).into_commit(); + compute_diff_with_parent(&mut number_of_commits_by_file_path, &commit, &repo)?; if cancel_calc.load(Ordering::Relaxed) { break; } - let commit = commit.attach(&repo).into_commit(); - compute_diff_with_parent(&mut number_of_commits_by_file_path, &commit, &repo)?; } Ok(number_of_commits_by_file_path) } From 27af8a5650ca6834c35b6cf3687b5b8d79eef1cd Mon Sep 17 00:00:00 2001 From: o2sh Date: Wed, 7 Jun 2023 19:12:17 +0200 Subject: [PATCH 21/28] improved readability + churn_pool_size CLI flag The churn_pool_size allow the user to force onefetch to be deterministic in the number of commits used to create the churn summary --- src/cli.rs | 16 ++++++++++++---- src/info/churn.rs | 33 +++++++++++++++------------------ src/info/contributors.rs | 1 + src/info/utils/git.rs | 36 +++++++++++++++++++++++------------- 4 files changed, 51 insertions(+), 35 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 12242e66d..b086149dc 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -66,9 +66,17 @@ pub struct InfoCliOptions { /// Maximum NUM of file churns to be shown #[arg(long, default_value_t = 3usize, value_name = "NUM")] pub number_of_file_churns: usize, - ///NUM of commits from HEAD used to compute file churns - #[arg(long, default_value_t = 100usize, value_name = "NUM")] - pub churn_commit_limit: usize, + /// Maximum NUM of commits from HEAD used to compute the churn summary + /// + /// By default, the actual value is non-deterministic due to time-based computation + /// and will be displayed under the info title "Churn (NUM)" + #[arg( + long, + default_value_t = 0usize, + hide_default_value = true, + value_name = "NUM" + )] + pub churn_pool_size: usize, /// Ignore all files & directories matching EXCLUDE #[arg(long, short, num_args = 1.., value_hint = ValueHint::AnyPath)] pub exclude: Vec, @@ -235,7 +243,7 @@ impl Default for InfoCliOptions { number_of_authors: 3, number_of_languages: 6, number_of_file_churns: 3, - churn_commit_limit: 100, + churn_pool_size: 100, exclude: Vec::default(), no_bots: Option::default(), no_merges: Default::default(), diff --git a/src/info/churn.rs b/src/info/churn.rs index 4966817be..226f17fc6 100644 --- a/src/info/churn.rs +++ b/src/info/churn.rs @@ -32,7 +32,7 @@ impl std::fmt::Display for FileChurn { write!( f, "{} {}", - truncate_file_path(&self.file_path, 2), + shorten_file_path(&self.file_path, 2), format_number(&self.nbr_of_commits, self.number_separator) ) } @@ -41,6 +41,7 @@ impl std::fmt::Display for FileChurn { #[derive(Serialize)] pub struct ChurnInfo { pub file_churns: Vec, + pub churn_pool_size: usize, #[serde(skip_serializing)] pub info_color: DynColors, } @@ -49,6 +50,7 @@ impl ChurnInfo { let file_churns = commit_metrics.file_churns_to_display.clone(); Self { file_churns, + churn_pool_size: commit_metrics.churn_pool_size, info_color, } } @@ -80,7 +82,7 @@ impl InfoField for ChurnInfo { } fn title(&self) -> String { - "Churn".into() + format!("Churn ({})", self.churn_pool_size) } fn should_color(&self) -> bool { @@ -88,11 +90,11 @@ impl InfoField for ChurnInfo { } } -fn truncate_file_path(path: &str, depth: usize) -> String { - let components: Vec<&str> = path.split('/').collect(); +fn shorten_file_path(file_path: &str, depth: usize) -> String { + let components: Vec<&str> = file_path.split('/').collect(); if depth == 0 || components.len() <= depth { - return path.to_string(); + return file_path.to_string(); } let truncated_components: Vec<&str> = components @@ -121,8 +123,9 @@ mod tests { let file_churn_2 = FileChurn::new("file_2.txt".into(), 30, NumberSeparator::Plain); let churn_info = ChurnInfo { - info_color: DynColors::Rgb(255, 0, 0), file_churns: vec![file_churn_1, file_churn_2], + churn_pool_size: 5, + info_color: DynColors::Rgb(255, 0, 0), }; assert!(churn_info.value().contains( @@ -138,23 +141,17 @@ mod tests { #[test] fn test_truncate_file_path() { + assert_eq!(shorten_file_path("path/to/file.txt", 3), "path/to/file.txt"); + assert_eq!(shorten_file_path("another/file.txt", 2), "another/file.txt"); + assert_eq!(shorten_file_path("file.txt", 1), "file.txt"); assert_eq!( - truncate_file_path("path/to/file.txt", 3), - "path/to/file.txt" - ); - assert_eq!( - truncate_file_path("another/file.txt", 2), - "another/file.txt" - ); - assert_eq!(truncate_file_path("file.txt", 1), "file.txt"); - assert_eq!( - truncate_file_path("path/to/file.txt", 2), + shorten_file_path("path/to/file.txt", 2), "\u{2026}/to/file.txt" ); assert_eq!( - truncate_file_path("another/file.txt", 1), + shorten_file_path("another/file.txt", 1), "\u{2026}/file.txt" ); - assert_eq!(truncate_file_path("file.txt", 0), "file.txt"); + assert_eq!(shorten_file_path("file.txt", 0), "file.txt"); } } diff --git a/src/info/contributors.rs b/src/info/contributors.rs index 082a60cf9..045b39202 100644 --- a/src/info/contributors.rs +++ b/src/info/contributors.rs @@ -59,6 +59,7 @@ mod test { file_churns_to_display: vec![], total_number_of_authors: 12, total_number_of_commits: 2, + churn_pool_size: 0, is_shallow: true, time_of_most_recent_commit: timestamp, time_of_first_commit: timestamp, diff --git a/src/info/utils/git.rs b/src/info/utils/git.rs index 3a1be0256..2faaa808a 100644 --- a/src/info/utils/git.rs +++ b/src/info/utils/git.rs @@ -20,6 +20,7 @@ pub struct CommitMetrics { pub file_churns_to_display: Vec, pub total_number_of_authors: usize, pub total_number_of_commits: usize, + pub churn_pool_size: usize, /// false if we have found the first commit that started it all, true if the repository is shallow. pub is_shallow: bool, pub time_of_most_recent_commit: gix::actor::Time, @@ -49,21 +50,27 @@ impl CommitMetrics { let mailmap_config = repo.open_mailmap(); let mut number_of_commits_by_signature: HashMap = HashMap::new(); let mut total_number_of_commits = 0; - let (tx, rx) = std::sync::mpsc::channel::(); - let cancel_calc = Arc::new(AtomicBool::default()); - let calc_diffs = std::thread::spawn({ + let (sender, receiver) = std::sync::mpsc::channel::(); + let cancellation_token = Arc::new(AtomicBool::default()); + + let churn_results = std::thread::spawn({ let repo = repo.clone(); - let cancel_calc = cancel_calc.clone(); + let cancellation_token = cancellation_token.clone(); + let churn_pool_size = options.info.churn_pool_size; move || -> Result<_> { let mut number_of_commits_by_file_path: HashMap = HashMap::new(); - for commit in rx { + let mut number_of_diffs_computed = 0; + while let Ok(commit) = receiver.recv() { let commit = commit.attach(&repo).into_commit(); compute_diff_with_parent(&mut number_of_commits_by_file_path, &commit, &repo)?; - if cancel_calc.load(Ordering::Relaxed) { + number_of_diffs_computed += 1; + if cancellation_token.load(Ordering::Relaxed) + && number_of_diffs_computed >= churn_pool_size + { break; } } - Ok(number_of_commits_by_file_path) + Ok((number_of_commits_by_file_path, number_of_diffs_computed)) } }); @@ -93,12 +100,11 @@ impl CommitMetrics { total_number_of_commits += 1; } - if total_number_of_commits <= options.info.churn_commit_limit { - tx.send(commit.detach()).ok(); - } + + sender.send(commit.detach()).ok(); } - drop(tx); - cancel_calc.store(true, Ordering::SeqCst); + + cancellation_token.store(true, Ordering::SeqCst); let (authors_to_display, total_number_of_authors) = compute_authors( number_of_commits_by_signature, @@ -108,8 +114,11 @@ impl CommitMetrics { options.text_formatting.number_separator, ); + let (number_of_commits_by_file_path, churn_pool_size) = + churn_results.join().expect("never panics")?; + let file_churns_to_display = compute_file_churns( - calc_diffs.join().expect("never panics")?, + number_of_commits_by_file_path, options.info.number_of_file_churns, options.text_formatting.number_separator, ); @@ -124,6 +133,7 @@ impl CommitMetrics { file_churns_to_display, total_number_of_authors, total_number_of_commits, + churn_pool_size, is_shallow: repo.is_shallow(), time_of_first_commit, time_of_most_recent_commit, From cc6b3ef10756d4ea6329289358b0a60138378883 Mon Sep 17 00:00:00 2001 From: o2sh Date: Wed, 7 Jun 2023 23:27:35 +0200 Subject: [PATCH 22/28] fix test --- src/cli.rs | 2 +- tests/repo.rs | 2 -- tests/snapshots/repo__repo.snap | 5 +++-- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index b086149dc..b5d55c248 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -243,7 +243,7 @@ impl Default for InfoCliOptions { number_of_authors: 3, number_of_languages: 6, number_of_file_churns: 3, - churn_pool_size: 100, + churn_pool_size: 0, exclude: Vec::default(), no_bots: Option::default(), no_merges: Default::default(), diff --git a/tests/repo.rs b/tests/repo.rs index be8d926f3..9af546295 100644 --- a/tests/repo.rs +++ b/tests/repo.rs @@ -41,13 +41,11 @@ fn test_repo() -> Result<()> { ..Default::default() }; let info = build_info(&config)?; - // Note that `nbrOfCommits` is hard-coded here as its actual value is non-deterministic due to time-based computation. insta::assert_json_snapshot!( info, { ".title.gitVersion" => "git version", ".infoFields[].HeadInfo.headRefs.shortCommitId" => "short commit", - ".infoFields[].ChurnInfo.file_churns[].nbrOfCommits" => 4 } ); diff --git a/tests/snapshots/repo__repo.snap b/tests/snapshots/repo__repo.snap index eee772796..98b0dfb69 100644 --- a/tests/snapshots/repo__repo.snap +++ b/tests/snapshots/repo__repo.snap @@ -110,9 +110,10 @@ expression: info "file_churns": [ { "filePath": "code.rs", - "nbrOfCommits": 4 + "nbrOfCommits": 1 } - ] + ], + "churn_pool_size": 1 } }, { From ddeaea38496d2b812e7b1b0c955c25378b178d8d Mon Sep 17 00:00:00 2001 From: o2sh Date: Thu, 8 Jun 2023 01:24:34 +0200 Subject: [PATCH 23/28] halt if the churn pool size is bigger than the total number of commits --- src/info/utils/git.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/info/utils/git.rs b/src/info/utils/git.rs index 2faaa808a..913896370 100644 --- a/src/info/utils/git.rs +++ b/src/info/utils/git.rs @@ -12,7 +12,7 @@ use gix::Commit; use regex::Regex; use std::collections::HashMap; use std::str::FromStr; -use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; pub struct CommitMetrics { @@ -49,9 +49,9 @@ impl CommitMetrics { let mailmap_config = repo.open_mailmap(); let mut number_of_commits_by_signature: HashMap = HashMap::new(); - let mut total_number_of_commits = 0; + let mut total_number_of_commits: usize = 0; let (sender, receiver) = std::sync::mpsc::channel::(); - let cancellation_token = Arc::new(AtomicBool::default()); + let cancellation_token = Arc::new(AtomicUsize::default()); let churn_results = std::thread::spawn({ let repo = repo.clone(); @@ -64,12 +64,15 @@ impl CommitMetrics { let commit = commit.attach(&repo).into_commit(); compute_diff_with_parent(&mut number_of_commits_by_file_path, &commit, &repo)?; number_of_diffs_computed += 1; - if cancellation_token.load(Ordering::Relaxed) - && number_of_diffs_computed >= churn_pool_size + if cancellation_token.load(Ordering::Relaxed) > 0 + && (number_of_diffs_computed >= churn_pool_size + || number_of_diffs_computed + == cancellation_token.load(Ordering::Relaxed)) { break; } } + Ok((number_of_commits_by_file_path, number_of_diffs_computed)) } }); @@ -104,7 +107,7 @@ impl CommitMetrics { sender.send(commit.detach()).ok(); } - cancellation_token.store(true, Ordering::SeqCst); + cancellation_token.store(total_number_of_commits, Ordering::SeqCst); let (authors_to_display, total_number_of_authors) = compute_authors( number_of_commits_by_signature, From 2876860a7bf1ee3a41e9ab990c56893ea1e09637 Mon Sep 17 00:00:00 2001 From: o2sh Date: Fri, 9 Jun 2023 00:04:06 +0200 Subject: [PATCH 24/28] improve readability --- src/cli.rs | 11 ++----- src/info/utils/git.rs | 54 ++++++++++++++++++++++++--------- tests/repo.rs | 1 + tests/snapshots/repo__repo.snap | 4 +-- 4 files changed, 45 insertions(+), 25 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index b5d55c248..969ab1b42 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -70,13 +70,8 @@ pub struct InfoCliOptions { /// /// By default, the actual value is non-deterministic due to time-based computation /// and will be displayed under the info title "Churn (NUM)" - #[arg( - long, - default_value_t = 0usize, - hide_default_value = true, - value_name = "NUM" - )] - pub churn_pool_size: usize, + #[arg(long, value_name = "NUM")] + pub churn_pool_size: Option, /// Ignore all files & directories matching EXCLUDE #[arg(long, short, num_args = 1.., value_hint = ValueHint::AnyPath)] pub exclude: Vec, @@ -243,7 +238,7 @@ impl Default for InfoCliOptions { number_of_authors: 3, number_of_languages: 6, number_of_file_churns: 3, - churn_pool_size: 0, + churn_pool_size: Option::default(), exclude: Vec::default(), no_bots: Option::default(), no_merges: Default::default(), diff --git a/src/info/utils/git.rs b/src/info/utils/git.rs index 913896370..a59e14229 100644 --- a/src/info/utils/git.rs +++ b/src/info/utils/git.rs @@ -12,7 +12,7 @@ use gix::Commit; use regex::Regex; use std::collections::HashMap; use std::str::FromStr; -use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use std::sync::Arc; pub struct CommitMetrics { @@ -49,14 +49,15 @@ impl CommitMetrics { let mailmap_config = repo.open_mailmap(); let mut number_of_commits_by_signature: HashMap = HashMap::new(); - let mut total_number_of_commits: usize = 0; let (sender, receiver) = std::sync::mpsc::channel::(); - let cancellation_token = Arc::new(AtomicUsize::default()); + let has_graph_commit_traversal_ended = Arc::new(AtomicBool::default()); + let total_number_of_commits = Arc::new(AtomicUsize::default()); let churn_results = std::thread::spawn({ let repo = repo.clone(); - let cancellation_token = cancellation_token.clone(); - let churn_pool_size = options.info.churn_pool_size; + let has_commit_graph_traversal_ended = has_graph_commit_traversal_ended.clone(); + let total_number_of_commits = total_number_of_commits.clone(); + let churn_pool_size_opt = options.info.churn_pool_size; move || -> Result<_> { let mut number_of_commits_by_file_path: HashMap = HashMap::new(); let mut number_of_diffs_computed = 0; @@ -64,12 +65,16 @@ impl CommitMetrics { let commit = commit.attach(&repo).into_commit(); compute_diff_with_parent(&mut number_of_commits_by_file_path, &commit, &repo)?; number_of_diffs_computed += 1; - if cancellation_token.load(Ordering::Relaxed) > 0 - && (number_of_diffs_computed >= churn_pool_size - || number_of_diffs_computed - == cancellation_token.load(Ordering::Relaxed)) - { - break; + if has_commit_graph_traversal_ended.load(Ordering::Relaxed) { + let total_number_of_commits = + total_number_of_commits.load(Ordering::Relaxed); + if should_break( + churn_pool_size_opt, + number_of_diffs_computed, + total_number_of_commits, + ) { + break; + } } } @@ -77,6 +82,7 @@ impl CommitMetrics { } }); + let mut count = 0; // From newest to oldest while let Some(commit_id) = commit_iter_peekable.next() { let commit = commit_id?.object()?.into_commit(); @@ -101,17 +107,18 @@ impl CommitMetrics { time_of_first_commit = commit_time.into(); } - total_number_of_commits += 1; + count += 1; } sender.send(commit.detach()).ok(); } - cancellation_token.store(total_number_of_commits, Ordering::SeqCst); + has_graph_commit_traversal_ended.store(true, Ordering::SeqCst); + total_number_of_commits.store(count, Ordering::SeqCst); let (authors_to_display, total_number_of_authors) = compute_authors( number_of_commits_by_signature, - total_number_of_commits, + count, options.info.number_of_authors, options.info.email, options.text_formatting.number_separator, @@ -135,7 +142,7 @@ impl CommitMetrics { authors_to_display, file_churns_to_display, total_number_of_authors, - total_number_of_commits, + total_number_of_commits: count, churn_pool_size, is_shallow: repo.is_shallow(), time_of_first_commit, @@ -144,6 +151,23 @@ impl CommitMetrics { } } +fn should_break( + churn_pool_size_opt: Option, + number_of_diffs_computed: usize, + total_number_of_commits: usize, +) -> bool { + if let Some(mut churn_pool_size) = churn_pool_size_opt { + if churn_pool_size > total_number_of_commits { + churn_pool_size = total_number_of_commits; + } + if number_of_diffs_computed == churn_pool_size { + return true; + } + return false; + } + true +} + fn compute_file_churns( number_of_commits_by_file_path: HashMap, number_of_file_churns_to_display: usize, diff --git a/tests/repo.rs b/tests/repo.rs index 9af546295..da73d4f5d 100644 --- a/tests/repo.rs +++ b/tests/repo.rs @@ -32,6 +32,7 @@ fn test_repo() -> Result<()> { input: repo.path().to_path_buf(), info: InfoCliOptions { email: true, + churn_pool_size: Some(10), ..Default::default() }, text_formatting: TextForamttingCliOptions { diff --git a/tests/snapshots/repo__repo.snap b/tests/snapshots/repo__repo.snap index 98b0dfb69..4ace263d3 100644 --- a/tests/snapshots/repo__repo.snap +++ b/tests/snapshots/repo__repo.snap @@ -110,10 +110,10 @@ expression: info "file_churns": [ { "filePath": "code.rs", - "nbrOfCommits": 1 + "nbrOfCommits": 4 } ], - "churn_pool_size": 1 + "churn_pool_size": 4 } }, { From 0b0abae8ea352f29f20f6a9b7d55d1d5c281bbab Mon Sep 17 00:00:00 2001 From: o2sh Date: Fri, 9 Jun 2023 01:08:01 +0200 Subject: [PATCH 25/28] add unit test --- Cargo.lock | 158 ++++++++++++++++++++++++++++++++++++++++-- Cargo.toml | 1 + src/info/utils/git.rs | 65 ++++++++++++----- 3 files changed, 199 insertions(+), 25 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 17876b7fc..b52fbacb4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -908,17 +908,100 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2022715d62ab30faffd124d40b76f4134a550a87792276512b18d63272333394" +[[package]] +name = "futures" +version = "0.3.28" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "23342abe12aba583913b2e62f22225ff9c950774065e4bfb61a19cd9770fec40" +dependencies = [ + "futures-channel", + "futures-core", + "futures-executor", + "futures-io", + "futures-sink", + "futures-task", + "futures-util", +] + +[[package]] +name = "futures-channel" +version = "0.3.28" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "955518d47e09b25bbebc7a18df10b81f0c766eaf4c4f1cccef2fca5f2a4fb5f2" +dependencies = [ + "futures-core", + "futures-sink", +] + [[package]] name = "futures-core" -version = "0.3.21" +version = "0.3.28" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0c09fd04b7e4073ac7156a9539b57a484a8ea920f79c7c675d05d289ab6110d3" +checksum = "4bca583b7e26f571124fe5b7561d49cb2868d79116cfa0eefce955557c6fee8c" + +[[package]] +name = "futures-executor" +version = "0.3.28" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ccecee823288125bd88b4d7f565c9e58e41858e47ab72e8ea2d64e93624386e0" +dependencies = [ + "futures-core", + "futures-task", + "futures-util", +] + +[[package]] +name = "futures-io" +version = "0.3.28" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4fff74096e71ed47f8e023204cfd0aa1289cd54ae5430a9523be060cdb849964" + +[[package]] +name = "futures-macro" +version = "0.3.28" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "89ca545a94061b6365f2c7355b4b32bd20df3ff95f02da9329b34ccc3bd6ee72" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.13", +] [[package]] name = "futures-sink" -version = "0.3.21" +version = "0.3.28" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f43be4fe21a13b9781a69afa4985b0f6ee0e1afab2c6f454a8cf30e2b2237b6e" + +[[package]] +name = "futures-task" +version = "0.3.28" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "76d3d132be6c0e6aa1534069c705a74a5997a356c0dc2f86a47765e5617c5b65" + +[[package]] +name = "futures-timer" +version = "3.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "21163e139fa306126e6eedaf49ecdb4588f939600f0b1e770f4205ee4b7fa868" +checksum = "e64b03909df88034c26dc1547e8970b91f98bdb65165d6a4e9110d94263dbb2c" + +[[package]] +name = "futures-util" +version = "0.3.28" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "26b01e40b772d54cf6c6d721c1d1abd0647a0106a12ecaa1c186273392a69533" +dependencies = [ + "futures-channel", + "futures-core", + "futures-io", + "futures-macro", + "futures-sink", + "futures-task", + "memchr", + "pin-project-lite", + "pin-utils", + "slab", +] [[package]] name = "generic-array" @@ -2197,6 +2280,7 @@ dependencies = [ "owo-colors", "pretty_assertions", "regex", + "rstest", "serde", "serde_json", "serde_yaml", @@ -2449,6 +2533,18 @@ dependencies = [ "syn 1.0.109", ] +[[package]] +name = "pin-project-lite" +version = "0.2.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e0a7ae3ac2f1173085d398531c705756c94a4c56843785df85a60c1a0afac116" + +[[package]] +name = "pin-utils" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" + [[package]] name = "pkg-config" version = "0.3.25" @@ -2665,12 +2761,47 @@ dependencies = [ "serde", ] +[[package]] +name = "rstest" +version = "0.17.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "de1bb486a691878cd320c2f0d319ba91eeaa2e894066d8b5f8f117c000e9d962" +dependencies = [ + "futures", + "futures-timer", + "rstest_macros", + "rustc_version", +] + +[[package]] +name = "rstest_macros" +version = "0.17.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "290ca1a1c8ca7edb7c3283bd44dc35dd54fdec6253a3912e201ba1072018fca8" +dependencies = [ + "cfg-if", + "proc-macro2", + "quote", + "rustc_version", + "syn 1.0.109", + "unicode-ident", +] + [[package]] name = "rustc-demangle" version = "0.1.21" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7ef03e0a2b150c7a90d01faf6254c9c48a41e95fb2a8c2ac1c6f0d2b9aefc342" +[[package]] +name = "rustc_version" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bfa0f585226d2e68097d4f95d113b15b83a82e819ab25717ec0590d9584ef366" +dependencies = [ + "semver", +] + [[package]] name = "rustix" version = "0.36.4" @@ -2726,6 +2857,12 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d29ab0c6d3fc0ee92fe66e2d99f700eab17a8d57d1c1d3b748380fb20baa78cd" +[[package]] +name = "semver" +version = "1.0.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bebd363326d05ec3e2f532ab7660680f3b02130d780c299bca73469d521bc0ed" + [[package]] name = "serde" version = "1.0.160" @@ -2827,6 +2964,15 @@ version = "0.3.10" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7bd3e3206899af3f8b12af284fafc038cc1dc2b41d1b89dd17297221c5d225de" +[[package]] +name = "slab" +version = "0.4.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6528351c9bc8ab22353f9d776db39a20288e8d6c37ef8cfe3317cf875eecfc2d" +dependencies = [ + "autocfg", +] + [[package]] name = "slug" version = "0.1.4" @@ -3293,9 +3439,9 @@ checksum = "98e90c70c9f0d4d1ee6d0a7d04aa06cb9bbd53d8cfbdd62a0269a7c2eb640552" [[package]] name = "unicode-ident" -version = "1.0.0" +version = "1.0.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d22af068fba1eb5edcb4aea19d382b2a3deb4c8f9d475c589b6ada9e0fd493ee" +checksum = "b15811caf2415fb889178633e7724bad2509101cde276048e013b9def5e51fa0" [[package]] name = "unicode-normalization" diff --git a/Cargo.toml b/Cargo.toml index 3cdef64d9..f96fb6523 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -55,6 +55,7 @@ criterion = "0.4.0" gix-testtools = "0.12.0" insta = { version = "1.29.0", features = ["json", "redactions"] } pretty_assertions = "1.3.0" +rstest = "0.17.0" [[bench]] name = "repo" diff --git a/src/info/utils/git.rs b/src/info/utils/git.rs index a59e14229..7b599fe62 100644 --- a/src/info/utils/git.rs +++ b/src/info/utils/git.rs @@ -65,16 +65,13 @@ impl CommitMetrics { let commit = commit.attach(&repo).into_commit(); compute_diff_with_parent(&mut number_of_commits_by_file_path, &commit, &repo)?; number_of_diffs_computed += 1; - if has_commit_graph_traversal_ended.load(Ordering::Relaxed) { - let total_number_of_commits = - total_number_of_commits.load(Ordering::Relaxed); - if should_break( - churn_pool_size_opt, - number_of_diffs_computed, - total_number_of_commits, - ) { - break; - } + if should_break( + has_commit_graph_traversal_ended.load(Ordering::Relaxed), + total_number_of_commits.load(Ordering::Relaxed), + churn_pool_size_opt, + number_of_diffs_computed, + ) { + break; } } @@ -152,20 +149,25 @@ impl CommitMetrics { } fn should_break( + has_commit_graph_traversal_ended: bool, + total_number_of_commits: usize, churn_pool_size_opt: Option, number_of_diffs_computed: usize, - total_number_of_commits: usize, ) -> bool { - if let Some(mut churn_pool_size) = churn_pool_size_opt { - if churn_pool_size > total_number_of_commits { - churn_pool_size = total_number_of_commits; - } - if number_of_diffs_computed == churn_pool_size { - return true; + if has_commit_graph_traversal_ended { + let total_number_of_commits = total_number_of_commits; + if let Some(mut churn_pool_size) = churn_pool_size_opt { + if churn_pool_size > total_number_of_commits { + churn_pool_size = total_number_of_commits; + } + if number_of_diffs_computed == churn_pool_size { + return true; + } + return false; } - return false; + return true; } - true + false } fn compute_file_churns( @@ -283,6 +285,7 @@ fn is_bot(author_name: &BString, bot_regex_pattern: &Option) -> bool { #[cfg(test)] mod tests { use super::*; + use rstest::rstest; #[test] fn test_get_no_bots_regex() { @@ -368,4 +371,28 @@ mod tests { ); assert_eq!(file_churns.get(0).unwrap().nbr_of_commits, 7); } + + #[rstest] + #[case(true, 10, Some(8), 4, false)] + #[case(false, 10, Some(10), 10, false)] + #[case(true, 10, Some(5), 5, true)] + #[case(true, 5, Some(10), 5, true)] + #[case(true, 5, Some(10), 3, false)] + #[case(true, 10, Some(5), 3, false)] + fn test_should_break( + #[case] has_commit_graph_traversal_ended: bool, + #[case] total_number_of_commits: usize, + #[case] churn_pool_size_opt: Option, + #[case] number_of_diffs_computed: usize, + #[case] expected: bool, + ) { + let result = should_break( + has_commit_graph_traversal_ended, + total_number_of_commits, + churn_pool_size_opt, + number_of_diffs_computed, + ); + + assert_eq!(result, expected); + } } From 16d227409a5270d5cb0ec789193a6ce52b36bc42 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 9 Jun 2023 07:28:54 +0200 Subject: [PATCH 26/28] refactor * simplify `should_break()` --- src/info/utils/git.rs | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/src/info/utils/git.rs b/src/info/utils/git.rs index 7b599fe62..b94349e9e 100644 --- a/src/info/utils/git.rs +++ b/src/info/utils/git.rs @@ -154,20 +154,12 @@ fn should_break( churn_pool_size_opt: Option, number_of_diffs_computed: usize, ) -> bool { - if has_commit_graph_traversal_ended { - let total_number_of_commits = total_number_of_commits; - if let Some(mut churn_pool_size) = churn_pool_size_opt { - if churn_pool_size > total_number_of_commits { - churn_pool_size = total_number_of_commits; - } - if number_of_diffs_computed == churn_pool_size { - return true; - } - return false; - } - return true; + if !has_commit_graph_traversal_ended { + return false; } - false + churn_pool_size_opt.map_or(true, |churn_pool_size| { + number_of_diffs_computed == churn_pool_size.min(total_number_of_commits) + }) } fn compute_file_churns( From c8a8cb7998d66197ec200e6d242405092e7f7951 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 9 Jun 2023 07:35:01 +0200 Subject: [PATCH 27/28] update to latest `gix` version --- Cargo.lock | 469 +++++++++++++++++++++++++++++++++++++++++------------ Cargo.toml | 2 +- 2 files changed, 362 insertions(+), 109 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b52fbacb4..fc3cbd9de 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1068,41 +1068,43 @@ dependencies = [ [[package]] name = "gix" -version = "0.44.1" +version = "0.45.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6bf41b61f7df395284f7a579c0fa1a7e012c5aede655174d4e91299ef1cac643" +checksum = "bf2a03ec66ee24d1b2bae3ab718f8d14f141613810cb7ff6756f7db667f1cd82" dependencies = [ - "gix-actor", - "gix-attributes", + "gix-actor 0.21.0", + "gix-attributes 0.13.0", + "gix-commitgraph", "gix-config", "gix-credentials", "gix-date", "gix-diff", - "gix-discover", - "gix-features", - "gix-fs", - "gix-glob", + "gix-discover 0.19.0", + "gix-features 0.30.0", + "gix-fs 0.2.0", + "gix-glob 0.8.0", "gix-hash", "gix-hashtable", - "gix-ignore", - "gix-index", - "gix-lock", + "gix-ignore 0.3.0", + "gix-index 0.17.0", + "gix-lock 6.0.0", "gix-mailmap", - "gix-object", + "gix-negotiate", + "gix-object 0.30.0", "gix-odb", "gix-pack", "gix-path", "gix-prompt", - "gix-ref", + "gix-ref 0.30.0", "gix-refspec", "gix-revision", "gix-sec", - "gix-tempfile", - "gix-traverse", + "gix-tempfile 6.0.0", + "gix-traverse 0.26.0", "gix-url", "gix-utils", "gix-validate", - "gix-worktree", + "gix-worktree 0.18.0", "log", "once_cell", "signal-hook", @@ -1125,6 +1127,20 @@ dependencies = [ "thiserror", ] +[[package]] +name = "gix-actor" +version = "0.21.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9fe73f9f6be1afbf1bd5be919a9636fa560e2f14d42262a934423ed6760cd838" +dependencies = [ + "bstr 1.3.0", + "btoi", + "gix-date", + "itoa", + "nom", + "thiserror", +] + [[package]] name = "gix-attributes" version = "0.12.0" @@ -1132,7 +1148,24 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3015baa01ad2122fbcaab7863c857a603eb7b7ec12ac8141207c42c6439805e2" dependencies = [ "bstr 1.3.0", - "gix-glob", + "gix-glob 0.7.0", + "gix-path", + "gix-quote", + "kstring", + "log", + "smallvec", + "thiserror", + "unicode-bom", +] + +[[package]] +name = "gix-attributes" +version = "0.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "644d4e1182dd21af10f455265eb15cb10ca3ae9c63475d7247e538e62ebacc56" +dependencies = [ + "bstr 1.3.0", + "gix-glob 0.8.0", "gix-path", "gix-quote", "kstring", @@ -1144,43 +1177,57 @@ dependencies = [ [[package]] name = "gix-bitmap" -version = "0.2.3" +version = "0.2.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "55a95f4942360766c3880bdb2b4b57f1ef73b190fc424755e7fdf480430af618" +checksum = "fc02feb20ad313d52a450852f2005c2205d24f851e74d82b7807cbe12c371667" dependencies = [ "thiserror", ] [[package]] name = "gix-chunk" -version = "0.4.1" +version = "0.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b0d39583cab06464b8bf73b3f1707458270f0e7383cb24c3c9c1a16e6f792978" +checksum = "a7acf3bc6c4b91e8fb260086daf5e105ea3a6d913f5fd3318137f7e309d6e540" dependencies = [ "thiserror", ] [[package]] name = "gix-command" -version = "0.2.4" +version = "0.2.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5f6141b70cfb21255223e42f3379855037cbbe8673b58dd8318d2f09b516fad1" +dependencies = [ + "bstr 1.3.0", +] + +[[package]] +name = "gix-commitgraph" +version = "0.16.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b2c6f75c1e0f924de39e750880a6e21307194bb1ab773efe3c7d2d787277f8ab" +checksum = "e8490ae1b3d55c47e6a71d247c082304a2f79f8d0332c1a2f5693d42a2021a09" dependencies = [ "bstr 1.3.0", + "gix-chunk", + "gix-features 0.30.0", + "gix-hash", + "memmap2 0.5.3", + "thiserror", ] [[package]] name = "gix-config" -version = "0.22.0" +version = "0.23.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1d252a0eddb6df74600d3d8872dc9fe98835a7da43110411d705b682f49d4ac1" +checksum = "51f310120ae1ba8f0ca52fb22876ce9bad5b15c8ffb3eb7302e4b64a3b9f681c" dependencies = [ "bstr 1.3.0", "gix-config-value", - "gix-features", - "gix-glob", + "gix-features 0.30.0", + "gix-glob 0.8.0", "gix-path", - "gix-ref", + "gix-ref 0.30.0", "gix-sec", "log", "memchr", @@ -1193,9 +1240,9 @@ dependencies = [ [[package]] name = "gix-config-value" -version = "0.12.0" +version = "0.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "786861e84a5793ad5f863d846de5eb064cd23b87e61ad708c8c402608202e7be" +checksum = "6f216df1c33e6e1555923eff0096858a879e8aaadd35b5d788641e4e8064c892" dependencies = [ "bitflags 2.2.1", "bstr 1.3.0", @@ -1206,9 +1253,9 @@ dependencies = [ [[package]] name = "gix-credentials" -version = "0.14.0" +version = "0.15.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4874a4fc11ffa844a3c2b87a66957bda30a73b577ef1acf15ac34df5745de5ff" +checksum = "c6f89fea8acd28f5ef8fa5042146f1637afd4d834bc8f13439d8fd1e5aca0d65" dependencies = [ "bstr 1.3.0", "gix-command", @@ -1222,9 +1269,9 @@ dependencies = [ [[package]] name = "gix-date" -version = "0.5.0" +version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "99056f37270715f5c7584fd8b46899a2296af9cae92463bf58b8bd1f5a78e553" +checksum = "bc164145670e9130a60a21670d9b6f0f4f8de04e5dd256c51fa5a0340c625902" dependencies = [ "bstr 1.3.0", "itoa", @@ -1234,12 +1281,12 @@ dependencies = [ [[package]] name = "gix-diff" -version = "0.29.0" +version = "0.30.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "644a0f2768bc42d7a69289ada80c9e15c589caefc6a315d2307202df83ed1186" +checksum = "bed89e910e19b48d31132b2b5392cef60786dd081cca5d0e31de32064f7300eb" dependencies = [ "gix-hash", - "gix-object", + "gix-object 0.30.0", "imara-diff", "thiserror", ] @@ -1254,7 +1301,22 @@ dependencies = [ "dunce", "gix-hash", "gix-path", - "gix-ref", + "gix-ref 0.29.0", + "gix-sec", + "thiserror", +] + +[[package]] +name = "gix-discover" +version = "0.19.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "aba9c6c0d1f2b2efe65581de73de4305004612d49c83773e783202a7ef204f46" +dependencies = [ + "bstr 1.3.0", + "dunce", + "gix-hash", + "gix-path", + "gix-ref 0.30.0", "gix-sec", "thiserror", ] @@ -1264,6 +1326,21 @@ name = "gix-features" version = "0.29.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf69b0f5c701cc3ae22d3204b671907668f6437ca88862d355eaf9bc47a4f897" +dependencies = [ + "flate2", + "gix-hash", + "libc", + "prodash 23.1.2", + "sha1_smol", + "thiserror", + "walkdir", +] + +[[package]] +name = "gix-features" +version = "0.30.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3a8c493409bf6060d408eec9bbdd1b12ea351266b50012e2a522f75dfc7b8314" dependencies = [ "crc32fast", "crossbeam-channel", @@ -1273,7 +1350,7 @@ dependencies = [ "libc", "once_cell", "parking_lot 0.12.1", - "prodash", + "prodash 25.0.0", "sha1_smol", "thiserror", "walkdir", @@ -1285,7 +1362,16 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9b37a1832f691fdc09910bd267f9a2e413737c1f9ec68c6e31f9e802616278a9" dependencies = [ - "gix-features", + "gix-features 0.29.0", +] + +[[package]] +name = "gix-fs" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "30da8997008adb87f94e15beb7ee229f8a48e97af585a584bfee4a5a1880aab5" +dependencies = [ + "gix-features 0.30.0", ] [[package]] @@ -1296,15 +1382,27 @@ checksum = "c07c98204529ac3f24b34754540a852593d2a4c7349008df389240266627a72a" dependencies = [ "bitflags 2.2.1", "bstr 1.3.0", - "gix-features", + "gix-features 0.29.0", + "gix-path", +] + +[[package]] +name = "gix-glob" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cd0ade1e80ab1f079703d1824e1daf73009096386aa7fd2f0477f6e4ac0a558e" +dependencies = [ + "bitflags 2.2.1", + "bstr 1.3.0", + "gix-features 0.30.0", "gix-path", ] [[package]] name = "gix-hash" -version = "0.11.1" +version = "0.11.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "078eec3ac2808cc03f0bddd2704cb661da5c5dc33b41a9d7947b141d499c7c42" +checksum = "ee181c85d3955f54c4426e6bfaeeada4428692e1a39b8788c2ac7785fc301dd8" dependencies = [ "hex", "thiserror", @@ -1312,9 +1410,9 @@ dependencies = [ [[package]] name = "gix-hashtable" -version = "0.2.0" +version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "afebb85691c6a085b114e01a27f4a61364519298c5826cb87a45c304802299bc" +checksum = "bd259bd0d96e6153e357a8cdaca76c48e103fd34208b6c0ce77b1ad995834bd2" dependencies = [ "gix-hash", "hashbrown 0.13.1", @@ -1328,7 +1426,19 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ba205b6df563e2906768bb22834c82eb46c5fdfcd86ba2c347270bc8309a05b2" dependencies = [ "bstr 1.3.0", - "gix-glob", + "gix-glob 0.7.0", + "gix-path", + "unicode-bom", +] + +[[package]] +name = "gix-ignore" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fc6f7f101a0ccce808dbf7008ba131dede94e20257e7bde7a44cbb2f8c775625" +dependencies = [ + "bstr 1.3.0", + "gix-glob 0.8.0", "gix-path", "unicode-bom", ] @@ -1344,11 +1454,33 @@ dependencies = [ "btoi", "filetime", "gix-bitmap", - "gix-features", + "gix-features 0.29.0", "gix-hash", - "gix-lock", - "gix-object", - "gix-traverse", + "gix-lock 5.0.1", + "gix-object 0.29.1", + "gix-traverse 0.25.0", + "itoa", + "memmap2 0.5.3", + "smallvec", + "thiserror", +] + +[[package]] +name = "gix-index" +version = "0.17.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "616ba958fabfb11263fa042c35690d48a6c7be4e9277e2c7e24ff263b3fe7b82" +dependencies = [ + "bitflags 2.2.1", + "bstr 1.3.0", + "btoi", + "filetime", + "gix-bitmap", + "gix-features 0.30.0", + "gix-hash", + "gix-lock 6.0.0", + "gix-object 0.30.0", + "gix-traverse 0.26.0", "itoa", "memmap2 0.5.3", "smallvec", @@ -1361,19 +1493,45 @@ version = "5.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2c693d7f05730fa74a7c467150adc7cea393518410c65f0672f80226b8111555" dependencies = [ - "gix-tempfile", + "gix-tempfile 5.0.3", + "gix-utils", + "thiserror", +] + +[[package]] +name = "gix-lock" +version = "6.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3ec5d5e6f07316d3553aa7425e3ecd935ec29882556021fe1696297a448af8d2" +dependencies = [ + "gix-tempfile 6.0.0", "gix-utils", "thiserror", ] [[package]] name = "gix-mailmap" -version = "0.12.0" +version = "0.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e8856cec3bdc3610c06970d28b6cb20a0c6621621cf9a8ec48cbd23f2630f362" +checksum = "4653701922c920e009f1bc4309feaff14882ade017770788f9a150928da3fa6a" dependencies = [ "bstr 1.3.0", - "gix-actor", + "gix-actor 0.21.0", + "thiserror", +] + +[[package]] +name = "gix-negotiate" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "82297847a7ad2d920707da5fc9ca8bb5eadf2891948dbe65625db1ffaa9803f9" +dependencies = [ + "bitflags 2.2.1", + "gix-commitgraph", + "gix-hash", + "gix-object 0.30.0", + "gix-revision", + "smallvec", "thiserror", ] @@ -1385,8 +1543,27 @@ checksum = "c9bb30ce0818d37096daa29efe361a4bc6dd0b51a5726598898be7e9a40a01e1" dependencies = [ "bstr 1.3.0", "btoi", - "gix-actor", - "gix-features", + "gix-actor 0.20.0", + "gix-features 0.29.0", + "gix-hash", + "gix-validate", + "hex", + "itoa", + "nom", + "smallvec", + "thiserror", +] + +[[package]] +name = "gix-object" +version = "0.30.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8926c8f51c44dec3e709cb5dbc93deb9e8d4064c43c9efc54c158dcdfe8446c7" +dependencies = [ + "bstr 1.3.0", + "btoi", + "gix-actor 0.21.0", + "gix-features 0.30.0", "gix-hash", "gix-validate", "hex", @@ -1398,14 +1575,14 @@ dependencies = [ [[package]] name = "gix-odb" -version = "0.45.0" +version = "0.46.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bca2f324aa67672b6d0f2c0fa93f96eb6a7029d260e4c1df5dce3c015f5e5add" +checksum = "4b234d806278eeac2f907c8b5a105c4ba537230c1a9d9236d822bf0db291f8f3" dependencies = [ "arc-swap", - "gix-features", + "gix-features 0.30.0", "gix-hash", - "gix-object", + "gix-object 0.30.0", "gix-pack", "gix-path", "gix-quote", @@ -1416,20 +1593,20 @@ dependencies = [ [[package]] name = "gix-pack" -version = "0.35.0" +version = "0.36.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "164a515900a83257ae4aa80e741655bee7a2e39113fb535d7a5ac623b445ff20" +checksum = "7d2a14cb3156037eedb17d6cb7209b7180522b8949b21fd0fe3184c0a1d0af88" dependencies = [ "clru", "gix-chunk", "gix-diff", - "gix-features", + "gix-features 0.30.0", "gix-hash", "gix-hashtable", - "gix-object", + "gix-object 0.30.0", "gix-path", - "gix-tempfile", - "gix-traverse", + "gix-tempfile 6.0.0", + "gix-traverse 0.26.0", "memmap2 0.5.3", "parking_lot 0.12.1", "smallvec", @@ -1439,9 +1616,9 @@ dependencies = [ [[package]] name = "gix-path" -version = "0.8.0" +version = "0.8.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4fc78f47095a0c15aea0e66103838f0748f4494bf7a9555dfe0f00425400396c" +checksum = "c1226f2e50adeb4d76c754c1856c06f13a24cad1624801653fbf09b869e5b808" dependencies = [ "bstr 1.3.0", "home", @@ -1451,9 +1628,9 @@ dependencies = [ [[package]] name = "gix-prompt" -version = "0.5.0" +version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "330d11fdf88fff3366c2491efde2f3e454958efe7d5ddf60272e8fb1d944bb01" +checksum = "e15fe57fa48572b7d3bf465d6a2a0351cd3c55cba74fd5f0b9c23689f9c1a31e" dependencies = [ "gix-command", "gix-config-value", @@ -1464,9 +1641,9 @@ dependencies = [ [[package]] name = "gix-quote" -version = "0.4.3" +version = "0.4.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a282f5a8d9ee0b09ec47390ac727350c48f2f5c76d803cd8da6b3e7ad56e0bcb" +checksum = "29d59489bff95b06dcdabe763b7266d3dc0a628cac1ac1caf65a7ca0a43eeae0" dependencies = [ "bstr 1.3.0", "btoi", @@ -1479,14 +1656,34 @@ version = "0.29.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b8212ecfe41815a2f1b059d82171d6276758cfac5506a5e0f04ad45ef0b1924a" dependencies = [ - "gix-actor", - "gix-features", - "gix-fs", + "gix-actor 0.20.0", + "gix-features 0.29.0", + "gix-fs 0.1.1", "gix-hash", - "gix-lock", - "gix-object", + "gix-lock 5.0.1", + "gix-object 0.29.1", "gix-path", - "gix-tempfile", + "gix-tempfile 5.0.3", + "gix-validate", + "memmap2 0.5.3", + "nom", + "thiserror", +] + +[[package]] +name = "gix-ref" +version = "0.30.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ebdd999256f4ce8a5eefa89999879c159c263f3493a951d62aa5ce42c0397e1c" +dependencies = [ + "gix-actor 0.21.0", + "gix-features 0.30.0", + "gix-fs 0.2.0", + "gix-hash", + "gix-lock 6.0.0", + "gix-object 0.30.0", + "gix-path", + "gix-tempfile 6.0.0", "gix-validate", "memmap2 0.5.3", "nom", @@ -1495,9 +1692,9 @@ dependencies = [ [[package]] name = "gix-refspec" -version = "0.10.1" +version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0a6ea733820df67e4cd7797deb12727905824d8f5b7c59d943c456d314475892" +checksum = "72bfd622abc86dd8ad1ec51b9eb77b4f1a766b94e3a1b87cf4a022c5b5570cf4" dependencies = [ "bstr 1.3.0", "gix-hash", @@ -1509,23 +1706,25 @@ dependencies = [ [[package]] name = "gix-revision" -version = "0.13.0" +version = "0.15.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "810f35e9afeccca999d5d348b239f9c162353127d2e13ff3240e31b919e35476" +checksum = "9abc4f68f85f42029ade0bece087aef7071016335772f0c7cb7d425aaaed3b33" dependencies = [ "bstr 1.3.0", + "gix-commitgraph", "gix-date", "gix-hash", "gix-hashtable", - "gix-object", + "gix-object 0.30.0", + "smallvec", "thiserror", ] [[package]] name = "gix-sec" -version = "0.8.0" +version = "0.8.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "794520043d5a024dfeac335c6e520cb616f6963e30dab995892382e998c12897" +checksum = "b2b7b38b766eb95dcc5350a9c450030b69892c0902fa35f4a6d0809273bd9dae" dependencies = [ "bitflags 2.2.1", "gix-path", @@ -1539,7 +1738,22 @@ version = "5.0.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d71a0d32f34e71e86586124225caefd78dabc605d0486de580d717653addf182" dependencies = [ - "gix-fs", + "gix-fs 0.1.1", + "libc", + "once_cell", + "parking_lot 0.12.1", + "signal-hook", + "signal-hook-registry", + "tempfile", +] + +[[package]] +name = "gix-tempfile" +version = "6.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b3785cb010e9dc5c446dfbf02bc1119fc17d3a48a27c029efcb3a3c32953eb10" +dependencies = [ + "gix-fs 0.2.0", "libc", "once_cell", "parking_lot 0.12.1", @@ -1558,12 +1772,12 @@ dependencies = [ "crc", "fastrand", "fs_extra", - "gix-discover", - "gix-fs", - "gix-ignore", - "gix-lock", - "gix-tempfile", - "gix-worktree", + "gix-discover 0.18.0", + "gix-fs 0.1.1", + "gix-ignore 0.2.0", + "gix-lock 5.0.1", + "gix-tempfile 5.0.3", + "gix-worktree 0.17.0", "io-close", "is_ci", "nom", @@ -1582,18 +1796,30 @@ checksum = "a5be1e807f288c33bb005075111886cceb43ed8a167b3182a0f62c186e2a0dd1" dependencies = [ "gix-hash", "gix-hashtable", - "gix-object", + "gix-object 0.29.1", + "thiserror", +] + +[[package]] +name = "gix-traverse" +version = "0.26.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b0842e984cb4bf26339dc559f3a1b8bf8cdb83547799b2b096822a59f87f33d9" +dependencies = [ + "gix-hash", + "gix-hashtable", + "gix-object 0.30.0", "thiserror", ] [[package]] name = "gix-url" -version = "0.18.0" +version = "0.19.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dfc77f89054297cc81491e31f1bab4027e554b5ef742a44bd7035db9a0f78b76" +checksum = "f1663df25ac42047a2547618d2a6979a26f478073f6306997429235d2cd4c863" dependencies = [ "bstr 1.3.0", - "gix-features", + "gix-features 0.30.0", "gix-path", "home", "thiserror", @@ -1602,18 +1828,18 @@ dependencies = [ [[package]] name = "gix-utils" -version = "0.1.1" +version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c10b69beac219acb8df673187a1f07dde2d74092f974fb3f9eb385aeb667c909" +checksum = "dbcfcb150c7ef553d76988467d223254045bdcad0dc6724890f32fbe96415da5" dependencies = [ "fastrand", ] [[package]] name = "gix-validate" -version = "0.7.4" +version = "0.7.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7bd629d3680773e1785e585d76fd4295b740b559cad9141517300d99a0c8c049" +checksum = "57ea5845b506c7728b9d89f4227cc369a5fc5a1d5b26c3add0f0d323413a3a60" dependencies = [ "bstr 1.3.0", "thiserror", @@ -1627,14 +1853,35 @@ checksum = "10bf56a1f5037d84293ea6cece61d9f27c4866b1e13c1c95f37cf56b7da7af25" dependencies = [ "bstr 1.3.0", "filetime", - "gix-attributes", - "gix-features", - "gix-fs", - "gix-glob", + "gix-attributes 0.12.0", + "gix-features 0.29.0", + "gix-fs 0.1.1", + "gix-glob 0.7.0", "gix-hash", - "gix-ignore", - "gix-index", - "gix-object", + "gix-ignore 0.2.0", + "gix-index 0.16.0", + "gix-object 0.29.1", + "gix-path", + "io-close", + "thiserror", +] + +[[package]] +name = "gix-worktree" +version = "0.18.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d388ad962e8854402734a7387af8790f6bdbc8d05349052dab16ca4a0def50f6" +dependencies = [ + "bstr 1.3.0", + "filetime", + "gix-attributes 0.13.0", + "gix-features 0.30.0", + "gix-fs 0.2.0", + "gix-glob 0.8.0", + "gix-hash", + "gix-ignore 0.3.0", + "gix-index 0.17.0", + "gix-object 0.30.0", "gix-path", "io-close", "thiserror", @@ -2267,7 +2514,7 @@ dependencies = [ "enable-ansi-support", "git2", "gix", - "gix-features", + "gix-features 0.29.0", "gix-testtools", "human-panic", "image", @@ -2624,6 +2871,12 @@ version = "23.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9516b775656bc3e8985e19cd4b8c0c0de045095074e453d2c0a513b5f978392d" +[[package]] +name = "prodash" +version = "25.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3236ce1618b6da4c7b618e0143c4d5b5dc190f75f81c49f248221382f7e9e9ae" + [[package]] name = "qoi" version = "0.4.1" diff --git a/Cargo.toml b/Cargo.toml index f96fb6523..d95317424 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,7 +26,7 @@ clap_complete = "4.2.3" gix-features-for-configuration-only = { package = "gix-features", version = "0.29.0", features = [ "zlib-ng", ] } -gix = { version = "0.44.1", default-features = false, features = [ +gix = { version = "0.45.1", default-features = false, features = [ "max-performance-safe", ] } git2 = { version = "0.17.1", default-features = false } From b767374952abf9e70542dd2ce2bd08c0777484b1 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 9 Jun 2023 07:45:34 +0200 Subject: [PATCH 28/28] Avoid exhaustive memory consumption by sending the commit-id instead of its buffer. The delta-processing happens by referring to a commit, and previously we could send the whole commit buffer (which is expensive) as the overall amount of buffers in flight would be bounded. Now that the bound was removed, it's necessary to limit the costs of the commit, and we do this by referring to it by id instead. That way, on the linux kernel, we get these values for memory consumption: * bounded: 960MB * unbounded buffer: 2156 * unbounded id: 1033 --- src/info/utils/git.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/info/utils/git.rs b/src/info/utils/git.rs index b94349e9e..cc6cefd3d 100644 --- a/src/info/utils/git.rs +++ b/src/info/utils/git.rs @@ -49,7 +49,7 @@ impl CommitMetrics { let mailmap_config = repo.open_mailmap(); let mut number_of_commits_by_signature: HashMap = HashMap::new(); - let (sender, receiver) = std::sync::mpsc::channel::(); + let (sender, receiver) = std::sync::mpsc::channel::(); let has_graph_commit_traversal_ended = Arc::new(AtomicBool::default()); let total_number_of_commits = Arc::new(AtomicUsize::default()); @@ -61,8 +61,8 @@ impl CommitMetrics { move || -> Result<_> { let mut number_of_commits_by_file_path: HashMap = HashMap::new(); let mut number_of_diffs_computed = 0; - while let Ok(commit) = receiver.recv() { - let commit = commit.attach(&repo).into_commit(); + while let Ok(commit_id) = receiver.recv() { + let commit = repo.find_object(commit_id)?.into_commit(); compute_diff_with_parent(&mut number_of_commits_by_file_path, &commit, &repo)?; number_of_diffs_computed += 1; if should_break( @@ -82,7 +82,8 @@ impl CommitMetrics { let mut count = 0; // From newest to oldest while let Some(commit_id) = commit_iter_peekable.next() { - let commit = commit_id?.object()?.into_commit(); + let commit_id = commit_id?; + let commit = commit_id.object()?.into_commit(); { let commit_ref = commit.decode()?; @@ -107,7 +108,7 @@ impl CommitMetrics { count += 1; } - sender.send(commit.detach()).ok(); + sender.send(commit_id.detach()).ok(); } has_graph_commit_traversal_ended.store(true, Ordering::SeqCst);