[PATCH] pack-bitmap: handle objects at bitmap position zero

[David Lin <davidzylin@gmail.com>]

`bitmap_position()` only returns a negative value when an object is not present in the bitmap index.

In `find_objects()`, we have added a check in 11d45a6e6a to avoid processing a root whose reachability is already represented by the base bitmap, but accidentally uses `pos > 0`. Consequently, it never performs the membership test for an object at position zero.

If that object has an individual reachability bitmap, we unnecessarily load and OR that bitmap into the base again. Otherwise, we add the object to the not-mapped list, only for the subsequent pass to recognize that it is already present. The latter pass correctly treats all non-negative positions as valid, so this does not change the resulting object set, but an off-by-one edge case.

Treat position zero as valid by changing the condition to `pos >= 0`.

The existing pseudo-merge traversal test exercises this case. Its position-zero commit is presented through multiple roots. Before this change, each occurrence is counted as a bitmap hit; afterwards, only the first occurrence loads the bitmap. Assert the resulting hit count to cover the boundary condition.

Thanks in advance for the review!

Signed-off-by: David Lin <davidlin@stripe.com>
---
 pack-bitmap.c                   | 2 +-
 t/t5333-pseudo-merge-bitmaps.sh | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/pack-bitmap.c b/pack-bitmap.c
index d8dc4ae8d1..e85bd69ba4 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -1569,7 +1569,7 @@ static struct bitmap *find_objects(struct bitmap_index *bitmap_git,
 
 		if (base) {
 			int pos = bitmap_position(bitmap_git, &object->oid);
-			if (pos > 0 && bitmap_get(base, pos)) {
+			if (pos >= 0 && bitmap_get(base, pos)) {
 				object->flags |= SEEN;
 				continue;
 			}
diff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh
index 305d677108..2a6c0e2318 100755
--- a/t/t5333-pseudo-merge-bitmaps.sh
+++ b/t/t5333-pseudo-merge-bitmaps.sh
@@ -85,6 +85,10 @@ test_expect_success 'bitmap traversal with pseudo-merges' '
 
 	test_pseudo_merges_satisfied 8 <trace2.txt &&
 	test_pseudo_merges_cascades 1 <trace2.txt &&
+
+	# Position zero is named by HEAD, its branch, and its tag, but its
+	# bitmap should only be loaded once.
+	test_trace2_data bitmap bitmap/hits 1 <trace2.txt &&
 	test_cmp expect actual
 '
 

base-commit: 9a0c4701dcd5725c4184599322b52933ff5005ca
-- 
2.54.0