Re: [PATCH 10/11] pack-bitmap.c: record whether the result was filtered

From: Jeff King <peff@peff.net>
Date: Oct 11, 2024, 08:35 UTC
Message-ID: <20241011083545.GH18010@coredump.intra.peff.net>
In-Reply-To: <dffdd794de3255719028a5f2c64d43048a5a5f60.1728505840.git.me@ttaylorr.com>

On Wed, Oct 09, 2024 at 04:31:31PM -0400, Taylor Blau wrote:

Show 11 quoted lines

> In a following commit, we will prepare to mark objects for reuse which
> are stored as deltas, but whose base object wasn't included in the
> output pack (e.g., because the client already has it).
> 
> We can't, however, take advantage of that optimization when the
> traversal removed objects from the result due to either (a) object
> filtering, or (b) --unpacked.
> 
> That's because in either case, we can't rely on the principle that "if
> an object appears in the 'haves' bitmap, that the client already has
> that object", since they may not have it as a result of filtering.

I think this is a reasonable precaution, though it does make me wonder if the non-reuse code paths are so careful. That is, if we have object Y which is a delta against object X, but we know the other side _could_ have X (because it's reachable from some commit they claim to have), would we ever send a thin delta against X?

I don't recall seeing any protections for that, though I also wouldn't be too surprised if it somehow just works out because we never figure out whether they have X or not. :-/

I wonder, though: should thin deltas just be turned off entirely when filtering is in play?

Likewise for --unpacked (though in practice I think it would never be used with --thin, as it is about generating new on-disk packs).

-Peff