Re: [PATCH 1/2] pack-objects: Teach --use-bitmap-index codepath to respect --local, --honor-pack-keep and --incremental

> Since 6b8fda2d (pack-objects: use bitmaps when packing objects) there
> are two codepaths in pack-objects: with & without using bitmap
> reachability index.
>
> However add_object_entry_from_bitmap(), despite its non-bitmapped
> counterpart add_object_entry(), in no way does check for whether --local
> or --honor-pack-keep or --incremental should be respected. In
> non-bitmapped codepath this is handled in want_object_in_pack(), but
> bitmapped codepath has simply no such checking at all.
>
> The bitmapped codepath however was allowing to pass in all those options
> and with bitmap indices still being used under such conditions -
> potentially giving wrong output (e.g. including objects from non-local or
> .keep'ed pack).
>
> We can easily fix this by noting the following: when an object comes to
> add_object_entry_from_bitmap() it can come for two reasons:
>
>     1. entries coming from main pack covered by bitmap index, and
>     2. object coming from, possibly alternate, loose or other packs.
>
> For "2" we always have pack not yet found by bitmap traversal code, and
> thus we can simply reuse non-bitmapped want_object_in_pack() to find in
> which pack an object lives and also for taking omitting decision.
>
> For "1" we always have pack already found by bitmap traversal code and we
> only need to check that pack for same criteria used in
> want_object_in_pack() for found_pack.
>
> Suggested-by: Junio C Hamano <gitster@pobox.com>
> Discussed-with: Jeff King <peff@peff.net>
> ---

> +/* Like want_object_in_pack() but for objects coming from-under bitmapped traversal */
> +static int want_object_in_pack_bitmap(const unsigned char *sha1,
> +				      struct packed_git **found_pack,
> +				      off_t *found_offset)
> +{
> +	struct packed_git *p = *found_pack;
> +
> +	/*
> +	 * There are two types of requests coming here:
> +	 * 1. entries coming from main pack covered by bitmap index, and
> +	 * 2. object coming from, possibly alternate, loose or other packs.
> +	 *
> +	 * For "1" we always have *found_pack != NULL passed here from
> +	 * traverse_bitmap_commit_list(). (*found_pack is bitmap_git.pack
> +	 * actually).
> +	 *
> +	 * For "2" we always have *found_pack == NULL passed here from
> +	 * traverse_bitmap_commit_list() - since this is the way bitmap
> +	 * traversal passes here "extended" bitmap entries.
> +	 */
> +
> +	/* objects not covered by bitmap */
> +	if (!p)
> +		return want_object_in_pack(sha1, 0, found_pack, found_offset);
> +	/* objects covered by bitmap - we only have to check p wrt local and .keep */

> +	if (incremental)
> +		return 0;
> +	if (local && !p->pack_local)
> +		return 0;
> +	if (ignore_packed_keep && p->pack_local && p->pack_keep)
> +		return 0;
> +
> +	return 1;
> +}
> +