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 <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/bloom.c b/bloom.c
index 196cda0a1bd7aaf80098b4a8de480c1243cda13b..e2ede44126c7fd2a1ecf1fd829b8e199cfab4736 100644 (file)
--- a/bloom.c
+++ b/bloom.c
@@ -215,7 +215,7 @@ struct bloom_filter *get_bloom_filter(struct repository *r,
                diff_tree_oid(NULL, &c->object.oid, "", &diffopt);
        diffcore_std(&diffopt);
 
-       if (diff_queued_diff.nr <= max_changes) {
+       if (diffopt.num_changes <= max_changes) {
                struct hashmap pathmap;
                struct pathmap_hash_entry *e;
                struct hashmap_iter iter;

diff --git a/t/t0095-bloom.sh b/t/t0095-bloom.sh
index 6defeb544f1451cb70ea17015af84903774b70ca..48a906255967c3a9451ad7093c5b9860c23c149e 100755 (executable)
--- a/t/t0095-bloom.sh
+++ b/t/t0095-bloom.sh
@@ -100,7 +100,7 @@ test_expect_success EXPENSIVE 'get bloom filter for commit with 513 changes' '
        rm actual &&
        rm expect &&
        mkdir bigDir &&
-       for i in $(test_seq 0 512)
+       for i in $(test_seq 0 511)
        do
                echo $i >bigDir/$i
        done &&