git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Justin Tobler <jltobler@gmail.com>
Date
Apr 6, 2026, 20:38 UTC
Message-ID
<adQYS_ThpOzxCLTi@denethor>
In-Reply-To
<20260406200651.GA26091@coredump.intra.peff.net>
On 26/04/06 04:06PM, Jeff King wrote:
Show 36 quoted lines
> On Mon, Apr 06, 2026 at 01:17:17PM -0500, Justin Tobler wrote:
> 
> > 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.

Ya, I think slimming down the patch probably makes sense. I'll start working on it and make sure to include some tests too. :)

Thanks, -Justin

Previous: Jeff King
Message 10 of 10 in “object-file: don't use object database without a repository”
  1. object-file: don't use object database without a repositoryLuca Stefani, Apr 4, 2026
  2. Pushkar SinghApr 5, 2026
  3. Jeff KingApr 5, 2026
  4. Luca StefaniApr 5, 2026
  5. Jeff KingApr 5, 2026
  6. Justin ToblerApr 6, 2026
  7. Luca StefaniApr 6, 2026
  8. Justin ToblerApr 6, 2026
  9. Jeff KingApr 6, 2026
  10. Justin ToblerApr 6, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.