Skip to content

Commit 2f6775f

Browse files
derrickstoleegitster
authored andcommitted
bloom: use num_changes not nr for limit detection
As diff_tree_oid() computes a diff, it will terminate early if the total number of changed paths is strictly larger than max_changes. This includes the directories that changed, not just the file paths. However, only the file paths are reflected in the resulting diff queue's "nr" value. Use the "num_changes" from diffopt to check if the diff terminated early. This is incredibly important, as it can result in incorrect filters! For example, the first commit in the Linux kernel repo reports only 471 changes, but since these are nested inside several directories they expand to 513 "real" changes, and in fact the total list of changes is not reported. Thus, the computed filter for this commit is incorrect. Demonstrate the subtle difference by using one fewer file change in the 'get bloom filter for commit with 513 changes' test. Before, this edited 513 files inside "bigDir" which hit this inequality. However, dropping the file count by one demonstrates how the previous inequality was incorrect but the new one is correct. Signed-off-by: Derrick Stolee <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 65c1a28 commit 2f6775f

File tree

2 files changed

+2
-2
lines changed

2 files changed

+2
-2
lines changed

bloom.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ struct bloom_filter *get_bloom_filter(struct repository *r,
215215
diff_tree_oid(NULL, &c->object.oid, "", &diffopt);
216216
diffcore_std(&diffopt);
217217

218-
if (diff_queued_diff.nr <= max_changes) {
218+
if (diffopt.num_changes <= max_changes) {
219219
struct hashmap pathmap;
220220
struct pathmap_hash_entry *e;
221221
struct hashmap_iter iter;

t/t0095-bloom.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ test_expect_success EXPENSIVE 'get bloom filter for commit with 513 changes' '
100100
rm actual &&
101101
rm expect &&
102102
mkdir bigDir &&
103-
for i in $(test_seq 0 512)
103+
for i in $(test_seq 0 511)
104104
do
105105
echo $i >bigDir/$i
106106
done &&

0 commit comments

Comments
 (0)