On Tue, Mar 30, 2021 at 11:03:44AM -0400, Taylor Blau wrote: > Here is another reroll of my series to implement a reverse index in > preparation for multi-pack reachability bitmaps. Thanks, this addresses all of my comments from the last round. > This reroll differs only in the feedback I incorporated from Peff's review. They > are mostly cosmetic; the most substantial change being that the --preferred-pack > code now uses bsearch() to locate the name of the preferred pack (instead of > implementing a binary search itself). Yeah, I read over this part carefully, since it's actual new code (that isn't run yet!), but I think it is correct. One minor observation: > @@ t/t5319-multi-pack-index.sh: test_expect_success 'warn on improper hash version' > + write --preferred-pack=test-BC-$bc.idx 2>err && > + test_must_be_empty err && > + > ++ echo hi && > ++ test-tool read-midx --show-objects objects >out && > ++ > + ofs=$(git show-index <objects/pack/test-BC-$bc.idx | grep $b | > + cut -d" " -f1) && > -+ midx_expect_object_offset $b $ofs objects > ++ printf "%s %s\tobjects/pack/test-BC-%s.pack\n" \ > ++ "$b" "$ofs" "$bc" >expect && > ++ grep ^$b out >actual && > ++ > ++ test_cmp expect actual > + ) > +' I'd probably have just skipped show-index entirely, and done: grep "^$b .* objects/pack/test-BC" actual which expresses the intent ($b came from that pack). But I don't mind the more exacting version (and certainly it is not worth a re-roll even if you prefer mine). -Peff
