Skip to content

Commit 0069e50

Browse files
Prevent churn calculation from returning an error on partial clones (#1736)
Co-authored-by: lawrence3699 <lawrence3699@users.noreply.github.com> Co-authored-by: Spenser Black <spenserblack01@gmail.com>
1 parent 8d4e4c8 commit 0069e50

3 files changed

Lines changed: 32 additions & 6 deletions

File tree

src/info/git/mod.rs

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,9 @@ fn get_churn_channel(
130130
if is_bot_commit(&commit, &mailmap, bot_regex_pattern.as_ref())? {
131131
continue;
132132
}
133-
compute_diff_with_parent(&mut number_of_commits_by_file_path, &commit, &repo)?;
134-
diffs_computed += 1;
133+
if compute_diff_with_parent(&mut number_of_commits_by_file_path, &commit, &repo)? {
134+
diffs_computed += 1;
135+
}
135136
if should_break(
136137
is_traversal_complete.load(Ordering::Relaxed),
137138
total_number_of_commits.load(Ordering::Relaxed),
@@ -179,11 +180,16 @@ fn update_signature_counts(
179180
Ok(())
180181
}
181182

183+
184+
/// This will get the trees for a commit and its parent and compute the diff. On success, this
185+
/// returns `Ok(true)`. If the trees are not available due to a partial clone, this will return
186+
/// `Ok(false)`. Essentially, this function will have successfully determined that there is no diff
187+
/// to compute. Other issues that prevent calculation will return `Err`.
182188
fn compute_diff_with_parent(
183189
change_map: &mut HashMap<BString, usize>,
184190
commit: &Commit,
185191
repo: &gix::Repository,
186-
) -> Result<()> {
192+
) -> Result<bool> {
187193
let mut parents = commit.parent_ids();
188194
let parents = (
189195
parents
@@ -194,8 +200,12 @@ fn compute_diff_with_parent(
194200
);
195201

196202
if let (parent_tree_id, None) = parents {
197-
let old_tree = parent_tree_id.object()?.into_tree();
198-
let new_tree = commit.tree()?;
203+
let Some(old_tree) = parent_tree_id.try_object()?.map(|tree| tree.into_tree()) else {
204+
return Ok(false);
205+
};
206+
let Some(new_tree) = commit.tree_id()?.try_object()?.map(|tree| tree.into_tree()) else {
207+
return Ok(false);
208+
};
199209
let changes =
200210
repo.diff_tree_to_tree(&old_tree, &new_tree, Options::default().with_rewrites(None))?;
201211
for change in &changes {
@@ -212,7 +222,7 @@ fn compute_diff_with_parent(
212222
}
213223
}
214224

215-
Ok(())
225+
Ok(true)
216226
}
217227

218228
fn is_bot_commit(

tests/fixtures/make_partial_repo.sh

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,8 @@ git clone --filter=blob:none file://$PWD/base partial
2828
(cd partial
2929
git config diff.renames true
3030
)
31+
32+
git clone --filter=tree:0 file://$PWD/base partial_treeless
33+
(cd partial_treeless
34+
git config diff.renames true
35+
)

tests/repo.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,17 @@ fn test_partial_repo() -> Result<()> {
8484
Ok(())
8585
}
8686

87+
#[test]
88+
fn test_treeless_partial_repo() -> Result<()> {
89+
let repo = named_repo("make_partial_repo.sh", "partial_treeless")?;
90+
let config: CliOptions = CliOptions {
91+
input: repo.path().to_path_buf(),
92+
..Default::default()
93+
};
94+
let _info = build_info(&config).expect("no error");
95+
Ok(())
96+
}
97+
8798
#[test]
8899
fn test_repo_with_pre_epoch_dates() -> Result<()> {
89100
let repo = repo("make_pre_epoch_repo.sh")?;

0 commit comments

Comments
 (0)