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

From: Jeff King <peff@peff.net>
Date: Aug 17, 2018, 22:45 UTC
Message-ID: <20180817224539.GA4158@sigill.intra.peff.net>
In-Reply-To: <CAGZ79kaP3GPZi_5vSCspgbim5VwsTMRa-fLYbjgECqsx9BTaeg@mail.gmail.com>

On Fri, Aug 17, 2018 at 03:39:29PM -0700, Stefan Beller wrote:

Show 15 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?

If the traversal hasn't been performed, the results are not defined (though I suspect yeah, it happens to say "no").

Show 6 quoted lines

> 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");

That seems like a reasonable precaution.

> > +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?

See the discussion in the commit message.

-Peff