Re: [PATCH] object-file: don't use object database without a repository

From: Jeff King <peff@peff.net>
Date: Apr 6, 2026, 20:06 UTC
Message-ID: <20260406200651.GA26091@coredump.intra.peff.net>
In-Reply-To: <adP0hnV7Gl08qqqf@denethor>

On Mon, Apr 06, 2026 at 01:17:17PM -0500, Justin Tobler wrote:

Show 21 quoted lines

> On 26/04/05 03:17PM, Jeff King wrote:
> > But I think the actual code change in your patch is the wrong thing, so
> > I also don't think we'd want to just squash that test in. I'm hoping
> > Justin has some insights on how to do a more complete fix.
> 
> I agree with Peff here that the correct fix should continue to use the
> object streaming mechanisms. To avoid this segfault, we really should
> avoid using ODB transactions when there isn't an ODB in the first place.
> 
> I replied in another thread[1] with how we could go about fixing. To
> summarize, it just so happens that I already have a patch[2] out on the
> list that appears to resolve this issue.
> 
> For the use case here, git-diff(1) is only interested in generating the
> hash for the "large" blobs and not actually writing anything to the ODB.
> This patch introduces a separate "hash-only" variant of
> `index_blob_packfile_transaction()` and is used to bypass creating an
> ODB transaction when object writes are not needed.
> 
> If this is the route we want to go down, I can extract this patch from
> the current series and send it as a separate fix. :)

Yeah, I think this is a good path forward. I took a look at making the transaction begin/end conditional, but that's not nearly enough anymore. The transaction object stores state which is used under the hood by index_blob_packfile_transaction(). So we'd really need some kind of fake noop transaction that understands how to stream.

Just having the caller divert to a "hash this without having an odb" interface is way simpler (especially since this is the only spot that needs it, so we are only paying the price once either way).

I gave a cursory look at the patch you linked. For a maint fix like this I think we could probably slim it down a bit: introduce the new hash-only helper but _don't_ actually rip flag support out of index_blob_packfile_transaction(), so we know that we can't accidentally break it. Though maybe that is being overly cautious; it only has one caller, and that caller would no longer be passing in any meaningful flags.

-Peff