Git development
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 3/7] odb/streaming: support streaming arbitrary object types
Date: Wed, 5 Aug 2026 08:06:31 +0200	[thread overview]
Message-ID: <anLS5z22CpF82cd7@pks.im> (raw)
In-Reply-To: <xmqqpkzxvhqm.fsf@gitster.g>

On Tue, Aug 04, 2026 at 11:54:41AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > The object database supports the ability to write object streams into
> > it. This functionality is used when we encounter a blob that is larger
> > than "core.bigFileThreshold" so that we don't have to soak large files
> > into memory.
> 
> I am still not sold the benefit of using a single "stream" type both
> for reading and writing yet at this point in my reading (I am not
> yet done 50% of the series yet at step 3/7), but I agree that it
> would be a good thing to be able to stream objects that are not
> blobs.

The reason why I want to unify these two streams is mostly that despite
their name, they basically do the exact same thing: both stream types
allow the user to read data from them in a streaming fashion. The only
thing that's different about the "write" stream is that it doesn't
encode its information as part of the stream itself, whereas the "read"
stream does. So having two types is quite pointless in the first place.

> > diff --git a/odb/source-inmemory.c b/odb/source-inmemory.c
> > index 01bb81c63c..4f76db5496 100644
> > --- a/odb/source-inmemory.c
> > +++ b/odb/source-inmemory.c
> > @@ -293,7 +293,7 @@ static int odb_source_inmemory_write_object_stream(struct odb_source *source,
> >  	hash_object_file(source->odb->repo->hash_algo, data, total_read, OBJ_BLOB, oid);
> >  
> >  	ret = odb_source_inmemory_write_object(source, data, stream->size,
> > -					       OBJ_BLOB, oid, NULL, NULL, 0);
> > +					       stream->type, oid, NULL, NULL, 0);
> 
> It is a bit annoying that we treat 'inmemory' as if it were a valid
> single word both in the filename and in the function name, but more
> importantly, hash_object_file() (used to compute the object name of
> the object we are writing into the variable 'oid') still hashes
> assuming that the object is a blob.  What is the implication of
> feeding the data to odb_source_in_memory_write_object() as
> stream->type (which is not necessarily OBJ_BLOB) with that 'oid'
> whose object name was computed as OBJ_BLOB?

Oh, that's an oversight on my part. We'd use the wrong object header,
thus arrive at a wrong hash and then ultimately store the object under
the wrong hash in the in-memory source. Which doesn't really matter
after this patch series as we still only write blobs via streams, but
it's a bug waiting to happen. Fixed now.

Patrick

  reply	other threads:[~2026-08-05  6:06 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-04  7:25 [PATCH 0/7] odb: unify read and write streams Patrick Steinhardt
2026-08-04  7:25 ` [PATCH 1/7] odb/streaming: track write stream size in the structure Patrick Steinhardt
2026-08-04 16:47   ` Justin Tobler
2026-08-04  7:25 ` [PATCH 2/7] odb/streaming: drop `is_finished` field Patrick Steinhardt
2026-08-04 17:46   ` Justin Tobler
2026-08-04  7:25 ` [PATCH 3/7] odb/streaming: support streaming arbitrary object types Patrick Steinhardt
2026-08-04 18:03   ` Justin Tobler
2026-08-05  6:06     ` Patrick Steinhardt
2026-08-04 18:54   ` Junio C Hamano
2026-08-05  6:06     ` Patrick Steinhardt [this message]
2026-08-04  7:25 ` [PATCH 4/7] odb/streaming: rename `struct odb_read_stream` Patrick Steinhardt
2026-08-04  7:25 ` [PATCH 5/7] odb/streaming: consolidate read and write streams Patrick Steinhardt
2026-08-04 18:23   ` Justin Tobler
2026-08-05  6:06     ` Patrick Steinhardt
2026-08-04  7:25 ` [PATCH 6/7] odb/streaming: rename `struct read_object_fd_data` Patrick Steinhardt
2026-08-04 18:25   ` Justin Tobler
2026-08-04  7:25 ` [PATCH 7/7] odb/streaming: unify function names to create new streams Patrick Steinhardt
2026-08-04 18:30   ` Justin Tobler
2026-08-05  7:44 ` [PATCH v2 0/8] odb: unify read and write streams Patrick Steinhardt
2026-08-05  7:44   ` [PATCH v2 1/8] odb/streaming: track write stream size in the structure Patrick Steinhardt
2026-08-05  7:44   ` [PATCH v2 2/8] odb/streaming: drop `is_finished` field Patrick Steinhardt
2026-08-05  7:44   ` [PATCH v2 3/8] odb/streaming: support streaming arbitrary object types Patrick Steinhardt
2026-08-05  7:44   ` [PATCH v2 4/8] odb/streaming: rename `struct odb_read_stream` Patrick Steinhardt
2026-08-05  7:44   ` [PATCH v2 5/8] odb/streaming: consolidate read and write streams Patrick Steinhardt
2026-08-05  7:44   ` [PATCH v2 6/8] odb/streaming: rename `struct read_object_fd_data` Patrick Steinhardt
2026-08-05  7:44   ` [PATCH v2 7/8] odb/streaming: rename `struct input_zstream_data` Patrick Steinhardt
2026-08-05  7:44   ` [PATCH v2 8/8] odb/streaming: unify function names to create new streams Patrick Steinhardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=anLS5z22CpF82cd7@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox