Re: [PATCH v1 2/3] pack-bitmap.c: add "break" statement in "open_pack_bitmap()"

[Taylor Blau <me@ttaylorr.com>]

On Thu, Mar 24, 2022 at 07:44:00PM +0800, Teng Long wrote:
> There will be only one operant ".bitmap" file in repo, so let's
> add "break" statement in "open_pack_bitmap()" when looping all
> the packs in repo.
>
> Signed-off-by: Teng Long <dyroneteng@gmail.com>
> ---
>  pack-bitmap.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/pack-bitmap.c b/pack-bitmap.c
> index 931219adf0..b1357137bf 100644
> --- a/pack-bitmap.c
> +++ b/pack-bitmap.c
> @@ -484,8 +484,10 @@ static int open_pack_bitmap(struct repository *r,
>  	assert(!bitmap_git->map);
>
>  	for (p = get_all_packs(r); p; p = p->next) {
> -		if (open_pack_bitmap_1(bitmap_git, p) == 0)
> +		if (open_pack_bitmap_1(bitmap_git, p) == 0) {
>  			ret = 0;
> +			break;

The lack of a break here is intentional, I think, since having more than
one bitmap of the same kind in a repository is an error.

(This is behavior we inherited from the pre-MIDX bitmap days, when
having more than one pack bitmap caused Git to signal an error, since it
could only use the results from a single bitmap).

You can see in pack-bitmap.c::open_pack_bitmap_1() that we have a
condition saying:

    if (bitmap_git->pack || bitmap_git->midx) {
        /* ignore extra bitmap file; we can only handle one */
        warning("...")
        close(fd;)
        return -1;
    }

We do want to call that open_pack_bitmap_1() function on every pack we
know about to make sure that one and only one of them corresponds to a
.bitmap.

Thanks,
Taylor