Re: [PATCH 5/6] pack-bitmap: save "have" bitmap from walk

From: Stefan Beller <sbeller@google.com>
Date: Aug 17, 2018, 22:39 UTC
Message-ID: <CAGZ79kaP3GPZi_5vSCspgbim5VwsTMRa-fLYbjgECqsx9BTaeg@mail.gmail.com>
In-Reply-To: <20180817205920.GE20088@sigill.intra.peff.net>

Show 12 quoted lines

> diff --git a/pack-bitmap.h b/pack-bitmap.h
> index 4555907dee..02a60ce670 100644
> --- a/pack-bitmap.h
> +++ b/pack-bitmap.h
> @@ -50,6 +50,13 @@ int rebuild_existing_bitmaps(struct bitmap_index *, struct packing_data *mapping
>                              khash_sha1 *reused_bitmaps, int show_progress);
>  void free_bitmap_index(struct bitmap_index *);
>
> +/*
> + * After a traversal has been performed on the bitmap_index, this can be
> + * queried to see if a particular object was reachable from any of the
> + * objects flagged as UNINTERESTING.

If the traversal has not been performed, we pretend the object was not reachable?

Is this a good API design, as it can be used when you do not have done all preparations? similarly to prepare_bitmap_walk we could have

    if (!bitmap_git->result)
        BUG("failed to perform bitmap walk before querying");

> +int bitmap_has_sha1_in_uninteresting(struct bitmap_index *, const unsigned char *sha1);

You seem to have rebased it to master resolving conflicts only. ;-) Do we want to talk about object ids here instead?

(This is what I get to think about when reviewing this series "bottom up". I use "git log -w -p master..HEAD" after applying the patches, probably I should also use --reverse, such that I get to see the commit message before the code for each commit and yet only need to scroll in one direction.)