* [PATCH 0/7] odb: unify read and write streams
@ 2026-08-04 7:25 Patrick Steinhardt
2026-08-04 7:25 ` [PATCH 1/7] odb/streaming: track write stream size in the structure Patrick Steinhardt
` (7 more replies)
0 siblings, 8 replies; 27+ messages in thread
From: Patrick Steinhardt @ 2026-08-04 7:25 UTC (permalink / raw)
To: git
Hi,
we have two different kind of object database streams in our code base:
`odb_write_stream` and `odb_read_stream`. While those are used for
different use cases, the provided functionality is ultimately the exact
same.
This patch series thus refactors these streams so that we have a single
`odb_stream`, only. This allows us to reuse the streams for different
kinds of purposes and makes them more generally useful overall. For
example, it's trivially possible now to create an object stream for any
given object and then write that stream into a different source.
The series is built on top of 5b2471720c (The 10th batch, 2026-08-03).
Thanks!
Patrick
---
Patrick Steinhardt (7):
odb/streaming: track write stream size in the structure
odb/streaming: drop `is_finished` field
odb/streaming: support streaming arbitrary object types
odb/streaming: rename `struct odb_read_stream`
odb/streaming: consolidate read and write streams
odb/streaming: rename `struct read_object_fd_data`
odb/streaming: unify function names to create new streams
archive-tar.c | 8 ++--
archive-zip.c | 12 ++---
builtin/index-pack.c | 8 ++--
builtin/pack-objects.c | 18 ++++----
builtin/unpack-objects.c | 42 +++++++++--------
object-file.c | 76 +++++++++++++++---------------
object-file.h | 2 +-
object.c | 6 +--
odb.c | 4 +-
odb.h | 4 +-
odb/source-files.c | 7 ++-
odb/source-inmemory.c | 32 +++++++------
odb/source-loose.c | 33 ++++++++------
odb/source-packed.c | 5 +-
odb/source.h | 13 +++---
odb/streaming.c | 104 ++++++++++++++++++++----------------------
odb/streaming.h | 69 ++++++++++------------------
odb/transaction.c | 6 +--
odb/transaction.h | 6 +--
pack-check.c | 4 +-
packfile.c | 8 ++--
packfile.h | 4 +-
t/unit-tests/u-odb-inmemory.c | 37 ++++++++-------
23 files changed, 247 insertions(+), 261 deletions(-)
---
base-commit: 5b2471720c93ee30e5764a19f3d3b3ae9ec9712a
change-id: 20260724-pks-odb-stream-unification-334dc2a75888
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH 1/7] odb/streaming: track write stream size in the structure 2026-08-04 7:25 [PATCH 0/7] odb: unify read and write streams Patrick Steinhardt @ 2026-08-04 7:25 ` 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 ` (6 subsequent siblings) 7 siblings, 1 reply; 27+ messages in thread From: Patrick Steinhardt @ 2026-08-04 7:25 UTC (permalink / raw) To: git When passing around a `struct odb_write_stream` we typically also have to pass the number of bytes that the stream will yield. This is required because the object header itself contains that size, and consequently we cannot write the header without that information. Move this information into the stream itself so that it becomes self- describing. In addition to that, this also brings the `struct odb_write_stream` a bit closer to the `struct odb_read_stream` so that we can eventually merge both stream types. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- builtin/unpack-objects.c | 3 ++- object-file.c | 25 +++++++++++-------------- odb.c | 4 ++-- odb.h | 2 +- odb/source-files.c | 3 +-- odb/source-inmemory.c | 11 +++++------ odb/source-loose.c | 7 +++---- odb/source-packed.c | 1 - odb/source.h | 5 ++--- odb/streaming.c | 1 + odb/streaming.h | 1 + odb/transaction.c | 4 ++-- odb/transaction.h | 4 ++-- t/unit-tests/u-odb-inmemory.c | 11 +++++------ 14 files changed, 38 insertions(+), 44 deletions(-) diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c index 4263edfbec..f3e0b504f4 100644 --- a/builtin/unpack-objects.c +++ b/builtin/unpack-objects.c @@ -392,13 +392,14 @@ static void stream_blob(unsigned long size, unsigned nr) struct odb_write_stream in_stream = { .read = feed_input_zstream, .data = &data, + .size = size, }; struct obj_info *info = &obj_list[nr]; data.zstream = &zstream; git_inflate_init(&zstream); - if (odb_write_object_stream(the_repository->objects, &in_stream, size, &info->oid)) + if (odb_write_object_stream(the_repository->objects, &in_stream, &info->oid)) die(_("failed to write object in stream")); if (data.status != Z_STREAM_END) diff --git a/object-file.c b/object-file.c index ec35c318bc..b196abb596 100644 --- a/object-file.c +++ b/object-file.c @@ -704,7 +704,7 @@ static void prepare_packfile_transaction(struct odb_transaction_files *transacti static int hash_blob_stream(struct odb_write_stream *stream, const struct git_hash_algo *hash_algo, - struct object_id *result_oid, size_t size) + struct object_id *result_oid) { unsigned char buf[16384]; struct git_hash_ctx ctx; @@ -712,7 +712,7 @@ static int hash_blob_stream(struct odb_write_stream *stream, size_t bytes_hashed = 0; header_len = format_object_header((char *)buf, sizeof(buf), - OBJ_BLOB, size); + OBJ_BLOB, stream->size); git_hash_init(&ctx, hash_algo); git_hash_update(&ctx, buf, header_len); @@ -727,7 +727,7 @@ static int hash_blob_stream(struct odb_write_stream *stream, bytes_hashed += read_result; } - if (bytes_hashed != size) + if (bytes_hashed != stream->size) return -1; git_hash_final_oid(result_oid, &ctx); @@ -740,7 +740,7 @@ static int hash_blob_stream(struct odb_write_stream *stream, * packfile in state while updating the hash in ctx. */ static void stream_blob_to_pack(struct transaction_packfile *state, - struct git_hash_ctx *ctx, size_t size, + struct git_hash_ctx *ctx, struct odb_write_stream *stream) { git_zstream s; @@ -753,7 +753,7 @@ static void stream_blob_to_pack(struct transaction_packfile *state, git_deflate_init(&s, cfg->pack_compression_level); - hdrlen = encode_in_pack_object_header(obuf, sizeof(obuf), OBJ_BLOB, size); + hdrlen = encode_in_pack_object_header(obuf, sizeof(obuf), OBJ_BLOB, stream->size); s.next_out = obuf + hdrlen; s.avail_out = sizeof(obuf) - hdrlen; @@ -793,9 +793,9 @@ static void stream_blob_to_pack(struct transaction_packfile *state, } } - if (bytes_read != size) + if (bytes_read != stream->size) die("read %" PRIuMAX " bytes of blob data, but expected %" PRIuMAX " bytes", - (uintmax_t)bytes_read, (uintmax_t)size); + (uintmax_t)bytes_read, (uintmax_t)stream->size); git_deflate_end(&s); } @@ -870,7 +870,6 @@ static void flush_packfile_transaction(struct odb_transaction_files *transaction */ static int odb_transaction_files_write_object_stream(struct odb_transaction *base, struct odb_write_stream *stream, - size_t size, struct object_id *result_oid) { struct odb_transaction_files *transaction = container_of(base, @@ -884,7 +883,7 @@ static int odb_transaction_files_write_object_stream(struct odb_transaction *bas struct pack_idx_entry *idx; header_len = format_object_header((char *)obuf, sizeof(obuf), - OBJ_BLOB, size); + OBJ_BLOB, stream->size); git_hash_init(&ctx, transaction->base.source->odb->repo->hash_algo); git_hash_update(&ctx, obuf, header_len); @@ -899,7 +898,7 @@ static int odb_transaction_files_write_object_stream(struct odb_transaction *bas * to zlib compression and is sufficient for this check. */ if (state->nr_written && pack_size_limit_cfg && - pack_size_limit_cfg < state->offset + size) + pack_size_limit_cfg < state->offset + stream->size) flush_packfile_transaction(transaction); CALLOC_ARRAY(idx, 1); @@ -909,7 +908,7 @@ static int odb_transaction_files_write_object_stream(struct odb_transaction *bas hashfile_checkpoint(state->f, &checkpoint); idx->offset = state->offset; crc32_begin(state->f); - stream_blob_to_pack(state, &ctx, size, stream); + stream_blob_to_pack(state, &ctx, stream); git_hash_final_oid(result_oid, &ctx); idx->crc32 = crc32_end(state->f); @@ -962,14 +961,12 @@ int index_fd(struct index_state *istate, struct object_id *oid, odb_transaction_begin_or_die(odb, &transaction, 0); ret = odb_transaction_write_object_stream(transaction, &stream, - xsize_t(st->st_size), oid); if (!inflight) odb_transaction_commit(transaction); } else { ret = hash_blob_stream(&stream, - the_repository->hash_algo, oid, - xsize_t(st->st_size)); + the_repository->hash_algo, oid); } odb_write_stream_release(&stream); diff --git a/odb.c b/odb.c index dabd481f57..585b2b2965 100644 --- a/odb.c +++ b/odb.c @@ -1028,10 +1028,10 @@ int odb_write_object_ext(struct object_database *odb, } int odb_write_object_stream(struct object_database *odb, - struct odb_write_stream *stream, size_t len, + struct odb_write_stream *stream, struct object_id *oid) { - return odb_source_write_object_stream(odb->sources, stream, len, oid); + return odb_source_write_object_stream(odb->sources, stream, oid); } struct object_database *odb_new(struct repository *repo, diff --git a/odb.h b/odb.h index cbc2f9ced4..019d3af3e8 100644 --- a/odb.h +++ b/odb.h @@ -629,7 +629,7 @@ static inline int odb_write_object(struct object_database *odb, struct odb_write_stream; int odb_write_object_stream(struct object_database *odb, - struct odb_write_stream *stream, size_t len, + struct odb_write_stream *stream, struct object_id *oid); void parse_alternates(const char *string, diff --git a/odb/source-files.c b/odb/source-files.c index 5e086d266f..f51960bd71 100644 --- a/odb/source-files.c +++ b/odb/source-files.c @@ -175,11 +175,10 @@ static int odb_source_files_write_object(struct odb_source *source, static int odb_source_files_write_object_stream(struct odb_source *source, struct odb_write_stream *stream, - size_t len, struct object_id *oid) { struct odb_source_files *files = odb_source_files_downcast(source); - return odb_source_write_object_stream(&files->loose->base, stream, len, oid); + return odb_source_write_object_stream(&files->loose->base, stream, oid); } static int odb_source_files_begin_transaction(struct odb_source *source, diff --git a/odb/source-inmemory.c b/odb/source-inmemory.c index 3e71611b8e..398131e194 100644 --- a/odb/source-inmemory.c +++ b/odb/source-inmemory.c @@ -257,7 +257,6 @@ static int odb_source_inmemory_write_object(struct odb_source *source, static int odb_source_inmemory_write_object_stream(struct odb_source *source, struct odb_write_stream *stream, - size_t len, struct object_id *oid) { char buf[16384]; @@ -265,12 +264,12 @@ static int odb_source_inmemory_write_object_stream(struct odb_source *source, char *data; int ret; - CALLOC_ARRAY(data, len); + CALLOC_ARRAY(data, stream->size); while (!stream->is_finished) { ssize_t bytes_read; bytes_read = odb_write_stream_read(stream, buf, sizeof(buf)); - if (total_read + bytes_read > len) { + if (total_read + bytes_read > stream->size) { ret = error("object stream yielded more bytes than expected"); goto out; } @@ -279,15 +278,15 @@ static int odb_source_inmemory_write_object_stream(struct odb_source *source, total_read += bytes_read; } - if (total_read != len) { + if (total_read != stream->size) { ret = error("object stream yielded less bytes than expected"); goto out; } hash_object_file(source->odb->repo->hash_algo, data, total_read, OBJ_BLOB, oid); - ret = odb_source_inmemory_write_object(source, data, len, OBJ_BLOB, oid, - NULL, NULL, 0); + ret = odb_source_inmemory_write_object(source, data, stream->size, + OBJ_BLOB, oid, NULL, NULL, 0); if (ret < 0) goto out; diff --git a/odb/source-loose.c b/odb/source-loose.c index ef0e919277..77a2adb52a 100644 --- a/odb/source-loose.c +++ b/odb/source-loose.c @@ -846,7 +846,6 @@ static int odb_source_loose_write_object(struct odb_source *source, static int odb_source_loose_write_object_stream(struct odb_source *source, struct odb_write_stream *in_stream, - size_t len, struct object_id *oid) { struct odb_source_loose *loose = odb_source_loose_downcast(source); @@ -868,7 +867,7 @@ static int odb_source_loose_write_object_stream(struct odb_source *source, /* Since oid is not determined, save tmp file to odb path. */ strbuf_addf(&filename, "%s/", loose->base.path); - hdrlen = format_object_header(hdr, sizeof(hdr), OBJ_BLOB, len); + hdrlen = format_object_header(hdr, sizeof(hdr), OBJ_BLOB, in_stream->size); /* * Common steps for write_loose_object and stream_loose_object to @@ -916,9 +915,9 @@ static int odb_source_loose_write_object_stream(struct odb_source *source, */ } while (ret == Z_OK || ret == Z_BUF_ERROR); - if (stream.total_in != len + hdrlen) + if (stream.total_in != in_stream->size + hdrlen) die(_("write stream object %"PRIuMAX" != %"PRIuMAX), (uintmax_t)stream.total_in, - (uintmax_t)len + hdrlen); + (uintmax_t)in_stream->size + hdrlen); /* * Common steps for write_loose_object and stream_loose_object to diff --git a/odb/source-packed.c b/odb/source-packed.c index 0890704e76..e6ff74833b 100644 --- a/odb/source-packed.c +++ b/odb/source-packed.c @@ -610,7 +610,6 @@ static int odb_source_packed_write_object(struct odb_source *source UNUSED, static int odb_source_packed_write_object_stream(struct odb_source *source UNUSED, struct odb_write_stream *stream UNUSED, - size_t len UNUSED, struct object_id *oid UNUSED) { return error("packed backend cannot write object streams"); diff --git a/odb/source.h b/odb/source.h index fc04dd5cda..0080148ba7 100644 --- a/odb/source.h +++ b/odb/source.h @@ -221,7 +221,7 @@ struct odb_source { * otherwise. */ int (*write_object_stream)(struct odb_source *source, - struct odb_write_stream *stream, size_t len, + struct odb_write_stream *stream, struct object_id *oid); /* @@ -437,10 +437,9 @@ static inline int odb_source_write_object(struct odb_source *source, */ static inline int odb_source_write_object_stream(struct odb_source *source, struct odb_write_stream *stream, - size_t len, struct object_id *oid) { - return source->write_object_stream(source, stream, len, oid); + return source->write_object_stream(source, stream, oid); } /* diff --git a/odb/streaming.c b/odb/streaming.c index 20531e864c..38c2f6687c 100644 --- a/odb/streaming.c +++ b/odb/streaming.c @@ -336,5 +336,6 @@ void odb_write_stream_from_fd(struct odb_write_stream *stream, int fd, stream->data = data; stream->read = read_object_fd; + stream->size = size; stream->is_finished = 0; } diff --git a/odb/streaming.h b/odb/streaming.h index c023671780..4d7d31b5aa 100644 --- a/odb/streaming.h +++ b/odb/streaming.h @@ -55,6 +55,7 @@ ssize_t odb_read_stream_read(struct odb_read_stream *stream, void *buf, size_t l struct odb_write_stream { ssize_t (*read)(struct odb_write_stream *, unsigned char *, size_t); void *data; + size_t size; int is_finished; }; diff --git a/odb/transaction.c b/odb/transaction.c index dab7da6a9a..6aaf133812 100644 --- a/odb/transaction.c +++ b/odb/transaction.c @@ -40,9 +40,9 @@ int odb_transaction_commit(struct odb_transaction *transaction) int odb_transaction_write_object_stream(struct odb_transaction *transaction, struct odb_write_stream *stream, - size_t len, struct object_id *oid) + struct object_id *oid) { - return transaction->write_object_stream(transaction, stream, len, oid); + return transaction->write_object_stream(transaction, stream, oid); } int odb_transaction_env(struct odb_transaction *transaction, struct strvec *env) diff --git a/odb/transaction.h b/odb/transaction.h index 4cb2eafcbf..ffb279314c 100644 --- a/odb/transaction.h +++ b/odb/transaction.h @@ -31,7 +31,7 @@ struct odb_transaction { * otherwise. */ int (*write_object_stream)(struct odb_transaction *transaction, - struct odb_write_stream *stream, size_t len, + struct odb_write_stream *stream, struct object_id *oid); /* @@ -82,7 +82,7 @@ int odb_transaction_commit(struct odb_transaction *transaction); */ int odb_transaction_write_object_stream(struct odb_transaction *transaction, struct odb_write_stream *stream, - size_t len, struct object_id *oid); + struct object_id *oid); /* * Populates the provided strvec with the environment variables that a child diff --git a/t/unit-tests/u-odb-inmemory.c b/t/unit-tests/u-odb-inmemory.c index ddf2db5c81..5ccc52dccc 100644 --- a/t/unit-tests/u-odb-inmemory.c +++ b/t/unit-tests/u-odb-inmemory.c @@ -269,7 +269,6 @@ struct membuf_write_stream { struct odb_write_stream base; const char *buf; size_t offset; - size_t size; }; static ssize_t membuf_write_stream_read(struct odb_write_stream *stream, @@ -280,13 +279,13 @@ static ssize_t membuf_write_stream_read(struct odb_write_stream *stream, if (chunk_size > len) chunk_size = len; - if (chunk_size > s->size - s->offset) - chunk_size = s->size - s->offset; + if (chunk_size > s->base.size - s->offset) + chunk_size = s->base.size - s->offset; memcpy(buf, s->buf + s->offset, chunk_size); s->offset += chunk_size; - if (s->offset == s->size) + if (s->offset == s->base.size) s->base.is_finished = 1; return chunk_size; @@ -298,13 +297,13 @@ void test_odb_inmemory__write_object_stream(void) const char data[] = "foobar"; struct membuf_write_stream stream = { .base.read = membuf_write_stream_read, + .base.size = strlen(data), .buf = data, - .size = strlen(data), }; struct object_id written_oid; cl_must_pass(odb_source_write_object_stream(&source->base, &stream.base, - strlen(data), &written_oid)); + &written_oid)); cl_assert_equal_s(oid_to_hex(&written_oid), FOOBAR_OID); cl_assert_object_info(source, &written_oid, OBJ_BLOB, "foobar"); -- 2.55.0.679.g6767b8d81c.dirty ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 1/7] odb/streaming: track write stream size in the structure 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 0 siblings, 0 replies; 27+ messages in thread From: Justin Tobler @ 2026-08-04 16:47 UTC (permalink / raw) To: Patrick Steinhardt; +Cc: git On 26/08/04 09:25AM, Patrick Steinhardt wrote: > When passing around a `struct odb_write_stream` we typically also have > to pass the number of bytes that the stream will yield. This is required > because the object header itself contains that size, and consequently we > cannot write the header without that information. > > Move this information into the stream itself so that it becomes self- > describing. In addition to that, this also brings the `struct > odb_write_stream` a bit closer to the `struct odb_read_stream` so that > we can eventually merge both stream types. Storing the object size in the stream directly makes complete sense. > Signed-off-by: Patrick Steinhardt <ps@pks.im> > --- > builtin/unpack-objects.c | 3 ++- > object-file.c | 25 +++++++++++-------------- > odb.c | 4 ++-- > odb.h | 2 +- > odb/source-files.c | 3 +-- > odb/source-inmemory.c | 11 +++++------ > odb/source-loose.c | 7 +++---- > odb/source-packed.c | 1 - > odb/source.h | 5 ++--- > odb/streaming.c | 1 + > odb/streaming.h | 1 + > odb/transaction.c | 4 ++-- > odb/transaction.h | 4 ++-- > t/unit-tests/u-odb-inmemory.c | 11 +++++------ > 14 files changed, 38 insertions(+), 44 deletions(-) > [snip] > diff --git a/odb/streaming.h b/odb/streaming.h > index c023671780..4d7d31b5aa 100644 > --- a/odb/streaming.h > +++ b/odb/streaming.h > @@ -55,6 +55,7 @@ ssize_t odb_read_stream_read(struct odb_read_stream *stream, void *buf, size_t l > struct odb_write_stream { > ssize_t (*read)(struct odb_write_stream *, unsigned char *, size_t); > void *data; > + size_t size; > int is_finished; > }; The size is now stored directly in the stream, the rest of this patch is adjusting callers to use the embedded size information instead of passing it. Looks good. -Justin ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 2/7] odb/streaming: drop `is_finished` field 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 7:25 ` 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 ` (5 subsequent siblings) 7 siblings, 1 reply; 27+ messages in thread From: Patrick Steinhardt @ 2026-08-04 7:25 UTC (permalink / raw) To: git The `is_finished` field is used to track whether a write stream is done writing all of its data. Tracking this field as part of the stream itself shouldn't be required though: callers will already know when the stream is done when the stream's read function returns zero bytes, same as when reading from a file descriptor. There is one exception where it gets a bit more complicated: when consuming data in "builtin/unpack-objects.c" it may happen that we don't yield any new bytes after reading from the pipe. This is addressed by looping until we have produced at least a single byte of output. Drop the field from `struct odb_write_stream`. Again, same as in the preceding commit, this brings the structure a bit closer to its sibling `struct odb_read_stream`. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- builtin/unpack-objects.c | 15 ++++++++------- object-file.c | 13 ++++++++----- odb/source-inmemory.c | 9 ++++++++- odb/source-loose.c | 12 ++++++++---- odb/streaming.c | 5 +---- odb/streaming.h | 1 - t/unit-tests/u-odb-inmemory.c | 5 +++-- 7 files changed, 36 insertions(+), 24 deletions(-) diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c index f3e0b504f4..b7c486ea94 100644 --- a/builtin/unpack-objects.c +++ b/builtin/unpack-objects.c @@ -368,20 +368,20 @@ static ssize_t feed_input_zstream(struct odb_write_stream *in_stream, { struct input_zstream_data *data = in_stream->data; git_zstream *zstream = data->zstream; - void *in = fill(1); - if (in_stream->is_finished) + if (data->status != Z_OK) return 0; zstream->next_out = buf; zstream->avail_out = buf_len; - zstream->next_in = in; - zstream->avail_in = len; - data->status = git_inflate(zstream, 0); + while (data->status == Z_OK && zstream->avail_out == buf_len) { + zstream->next_in = fill(1); + zstream->avail_in = len; + data->status = git_inflate(zstream, 0); + use(len - zstream->avail_in); + } - in_stream->is_finished = data->status != Z_OK; - use(len - zstream->avail_in); return buf_len - zstream->avail_out; } @@ -397,6 +397,7 @@ static void stream_blob(unsigned long size, unsigned nr) struct obj_info *info = &obj_list[nr]; data.zstream = &zstream; + data.status = Z_OK; git_inflate_init(&zstream); if (odb_write_object_stream(the_repository->objects, &in_stream, &info->oid)) diff --git a/object-file.c b/object-file.c index b196abb596..317c09dff8 100644 --- a/object-file.c +++ b/object-file.c @@ -716,12 +716,13 @@ static int hash_blob_stream(struct odb_write_stream *stream, git_hash_init(&ctx, hash_algo); git_hash_update(&ctx, buf, header_len); - while (!stream->is_finished) { + while (1) { ssize_t read_result = odb_write_stream_read(stream, buf, sizeof(buf)); - if (read_result < 0) return -1; + if (!read_result) + break; git_hash_update(&ctx, buf, read_result); bytes_hashed += read_result; @@ -749,6 +750,7 @@ static void stream_blob_to_pack(struct transaction_packfile *state, unsigned hdrlen; int status = Z_OK; struct repo_config_values *cfg = repo_config_values(the_repository); + bool is_finished = false; size_t bytes_read = 0; git_deflate_init(&s, cfg->pack_compression_level); @@ -758,12 +760,13 @@ static void stream_blob_to_pack(struct transaction_packfile *state, s.avail_out = sizeof(obuf) - hdrlen; while (status != Z_STREAM_END) { - if (!stream->is_finished && !s.avail_in) { + if (!is_finished && !s.avail_in) { ssize_t rsize = odb_write_stream_read(stream, ibuf, sizeof(ibuf)); - if (rsize < 0) die("failed to read blob data"); + if (!rsize) + is_finished = true; git_hash_update(ctx, ibuf, rsize); @@ -772,7 +775,7 @@ static void stream_blob_to_pack(struct transaction_packfile *state, bytes_read += rsize; } - status = git_deflate(&s, stream->is_finished ? Z_FINISH : 0); + status = git_deflate(&s, is_finished ? Z_FINISH : 0); if (!s.avail_out || status == Z_STREAM_END) { size_t written = s.next_out - obuf; diff --git a/odb/source-inmemory.c b/odb/source-inmemory.c index 398131e194..01bb81c63c 100644 --- a/odb/source-inmemory.c +++ b/odb/source-inmemory.c @@ -265,10 +265,17 @@ static int odb_source_inmemory_write_object_stream(struct odb_source *source, int ret; CALLOC_ARRAY(data, stream->size); - while (!stream->is_finished) { + while (1) { ssize_t bytes_read; bytes_read = odb_write_stream_read(stream, buf, sizeof(buf)); + if (bytes_read < 0) { + ret = error("failed to read object stream"); + goto out; + } + if (!bytes_read) + break; + if (total_read + bytes_read > stream->size) { ret = error("object stream yielded more bytes than expected"); goto out; diff --git a/odb/source-loose.c b/odb/source-loose.c index 77a2adb52a..361b4e2a2a 100644 --- a/odb/source-loose.c +++ b/odb/source-loose.c @@ -859,6 +859,7 @@ static int odb_source_loose_write_object_stream(struct odb_source *source, struct strbuf filename = STRBUF_INIT; unsigned char buf[8192]; int dirlen; + bool is_finished = false; char hdr[MAX_HEADER_LEN]; int hdrlen; @@ -889,7 +890,7 @@ static int odb_source_loose_write_object_stream(struct odb_source *source, do { unsigned char *in0 = stream.next_in; - if (!stream.avail_in && !in_stream->is_finished) { + if (!stream.avail_in && !is_finished) { ssize_t read_len = odb_write_stream_read(in_stream, buf, sizeof(buf)); if (read_len < 0) { @@ -898,12 +899,15 @@ static int odb_source_loose_write_object_stream(struct odb_source *source, goto cleanup; } + /* All data has been read. */ + if (!read_len) { + is_finished = true; + flush = 1; + } + stream.avail_in = read_len; stream.next_in = buf; in0 = buf; - /* All data has been read. */ - if (in_stream->is_finished) - flush = 1; } ret = write_loose_object_common(loose, &c, &compat_c, &stream, flush, in0, fd, compressed, sizeof(compressed)); diff --git a/odb/streaming.c b/odb/streaming.c index 38c2f6687c..912e75e682 100644 --- a/odb/streaming.c +++ b/odb/streaming.c @@ -310,7 +310,7 @@ static ssize_t read_object_fd(struct odb_write_stream *stream, ssize_t read_result; size_t count; - if (stream->is_finished) + if (!data->remaining) return 0; count = data->remaining < len ? data->remaining : len; @@ -319,8 +319,6 @@ static ssize_t read_object_fd(struct odb_write_stream *stream, return -1; data->remaining -= count; - if (!data->remaining) - stream->is_finished = 1; return read_result; } @@ -337,5 +335,4 @@ void odb_write_stream_from_fd(struct odb_write_stream *stream, int fd, stream->data = data; stream->read = read_object_fd; stream->size = size; - stream->is_finished = 0; } diff --git a/odb/streaming.h b/odb/streaming.h index 4d7d31b5aa..5e8e6e532e 100644 --- a/odb/streaming.h +++ b/odb/streaming.h @@ -56,7 +56,6 @@ struct odb_write_stream { ssize_t (*read)(struct odb_write_stream *, unsigned char *, size_t); void *data; size_t size; - int is_finished; }; /* diff --git a/t/unit-tests/u-odb-inmemory.c b/t/unit-tests/u-odb-inmemory.c index 5ccc52dccc..4437140ed0 100644 --- a/t/unit-tests/u-odb-inmemory.c +++ b/t/unit-tests/u-odb-inmemory.c @@ -277,6 +277,9 @@ static ssize_t membuf_write_stream_read(struct odb_write_stream *stream, struct membuf_write_stream *s = container_of(stream, struct membuf_write_stream, base); size_t chunk_size = 2; + if (s->offset == s->base.size) + return 0; + if (chunk_size > len) chunk_size = len; if (chunk_size > s->base.size - s->offset) @@ -285,8 +288,6 @@ static ssize_t membuf_write_stream_read(struct odb_write_stream *stream, memcpy(buf, s->buf + s->offset, chunk_size); s->offset += chunk_size; - if (s->offset == s->base.size) - s->base.is_finished = 1; return chunk_size; } -- 2.55.0.679.g6767b8d81c.dirty ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 2/7] odb/streaming: drop `is_finished` field 2026-08-04 7:25 ` [PATCH 2/7] odb/streaming: drop `is_finished` field Patrick Steinhardt @ 2026-08-04 17:46 ` Justin Tobler 0 siblings, 0 replies; 27+ messages in thread From: Justin Tobler @ 2026-08-04 17:46 UTC (permalink / raw) To: Patrick Steinhardt; +Cc: git On 26/08/04 09:25AM, Patrick Steinhardt wrote: > The `is_finished` field is used to track whether a write stream is done > writing all of its data. Tracking this field as part of the stream > itself shouldn't be required though: callers will already know when the > stream is done when the stream's read function returns zero bytes, same > as when reading from a file descriptor. > > There is one exception where it gets a bit more complicated: when > consuming data in "builtin/unpack-objects.c" it may happen that we don't > yield any new bytes after reading from the pipe. This is addressed by > looping until we have produced at least a single byte of output. Addressing this one outlier sounds reasonable. > Drop the field from `struct odb_write_stream`. Again, same as in the > preceding commit, this brings the structure a bit closer to its sibling > `struct odb_read_stream`. This also makes the overal interface a bit simpler. Callers can trust that when `odb_write_stream_read()` returns zero, it is actually finished without having to inspect further. > Signed-off-by: Patrick Steinhardt <ps@pks.im> > --- > builtin/unpack-objects.c | 15 ++++++++------- > object-file.c | 13 ++++++++----- > odb/source-inmemory.c | 9 ++++++++- > odb/source-loose.c | 12 ++++++++---- > odb/streaming.c | 5 +---- > odb/streaming.h | 1 - > t/unit-tests/u-odb-inmemory.c | 5 +++-- > 7 files changed, 36 insertions(+), 24 deletions(-) > > diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c > index f3e0b504f4..b7c486ea94 100644 > --- a/builtin/unpack-objects.c > +++ b/builtin/unpack-objects.c > @@ -368,20 +368,20 @@ static ssize_t feed_input_zstream(struct odb_write_stream *in_stream, > { > struct input_zstream_data *data = in_stream->data; > git_zstream *zstream = data->zstream; > - void *in = fill(1); > > - if (in_stream->is_finished) > + if (data->status != Z_OK) > return 0; > > zstream->next_out = buf; > zstream->avail_out = buf_len; > - zstream->next_in = in; > - zstream->avail_in = len; > > - data->status = git_inflate(zstream, 0); > + while (data->status == Z_OK && zstream->avail_out == buf_len) { > + zstream->next_in = fill(1); > + zstream->avail_in = len; > + data->status = git_inflate(zstream, 0); > + use(len - zstream->avail_in); > + } Ok, now we call `git_inflate()` in a loop until there is an error or we get some data back. This makes it so we can trust that returning zero does mean that the stream is finished. Previously, it was the callers responsibility to check the `is_finished` stream field to be certain. I was curious if we needed to update any code documentation with this change, but it looks like the comments for `odb_write_stream_read()` already made it sound like this was the current behavior. [snip] > diff --git a/odb/streaming.h b/odb/streaming.h > index 4d7d31b5aa..5e8e6e532e 100644 > --- a/odb/streaming.h > +++ b/odb/streaming.h > @@ -56,7 +56,6 @@ struct odb_write_stream { > ssize_t (*read)(struct odb_write_stream *, unsigned char *, size_t); > void *data; > size_t size; > - int is_finished; The field is dropped. Nice. The rest of this patch looks good. -Justin ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 3/7] odb/streaming: support streaming arbitrary object types 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 7:25 ` [PATCH 2/7] odb/streaming: drop `is_finished` field Patrick Steinhardt @ 2026-08-04 7:25 ` Patrick Steinhardt 2026-08-04 18:03 ` Justin Tobler 2026-08-04 18:54 ` Junio C Hamano 2026-08-04 7:25 ` [PATCH 4/7] odb/streaming: rename `struct odb_read_stream` Patrick Steinhardt ` (4 subsequent siblings) 7 siblings, 2 replies; 27+ messages in thread From: Patrick Steinhardt @ 2026-08-04 7:25 UTC (permalink / raw) To: git 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. As we only ever write large files, the infrastructure doesn't support specifying any other object type than "blob". This limitation is quite artificial though: there is no reason why we shouldn't support writing arbitrary large objects with a stream. While it's very unlikely that we encounter a huge object other than a blob, users are known to be creative and sometimes like to inflict pain on themselves by creating commits or trees that are huge. Extend the infrastructure to support streaming arbitrary object types. For now we don't use this functionality anywhere, but it brings us a bit closer to unify `struct odb_read_stream` and `struct odb_write_stream`. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- builtin/unpack-objects.c | 1 + object-file.c | 31 +++++++++++++++---------------- odb/source-inmemory.c | 2 +- odb/source-loose.c | 2 +- odb/streaming.c | 3 ++- odb/streaming.h | 3 ++- t/unit-tests/u-odb-inmemory.c | 7 +++++-- 7 files changed, 27 insertions(+), 22 deletions(-) diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c index b7c486ea94..7439ec53be 100644 --- a/builtin/unpack-objects.c +++ b/builtin/unpack-objects.c @@ -393,6 +393,7 @@ static void stream_blob(unsigned long size, unsigned nr) .read = feed_input_zstream, .data = &data, .size = size, + .type = OBJ_BLOB, }; struct obj_info *info = &obj_list[nr]; diff --git a/object-file.c b/object-file.c index 317c09dff8..699a6a008c 100644 --- a/object-file.c +++ b/object-file.c @@ -702,9 +702,9 @@ static void prepare_packfile_transaction(struct odb_transaction_files *transacti die_errno("unable to write pack header"); } -static int hash_blob_stream(struct odb_write_stream *stream, - const struct git_hash_algo *hash_algo, - struct object_id *result_oid) +static int hash_stream(struct odb_write_stream *stream, + const struct git_hash_algo *hash_algo, + struct object_id *result_oid) { unsigned char buf[16384]; struct git_hash_ctx ctx; @@ -712,7 +712,7 @@ static int hash_blob_stream(struct odb_write_stream *stream, size_t bytes_hashed = 0; header_len = format_object_header((char *)buf, sizeof(buf), - OBJ_BLOB, stream->size); + stream->type, stream->size); git_hash_init(&ctx, hash_algo); git_hash_update(&ctx, buf, header_len); @@ -740,9 +740,9 @@ static int hash_blob_stream(struct odb_write_stream *stream, * Read the contents from the stream provided, streaming it to the * packfile in state while updating the hash in ctx. */ -static void stream_blob_to_pack(struct transaction_packfile *state, - struct git_hash_ctx *ctx, - struct odb_write_stream *stream) +static void stream_to_pack(struct transaction_packfile *state, + struct git_hash_ctx *ctx, + struct odb_write_stream *stream) { git_zstream s; unsigned char ibuf[16384]; @@ -755,7 +755,7 @@ static void stream_blob_to_pack(struct transaction_packfile *state, git_deflate_init(&s, cfg->pack_compression_level); - hdrlen = encode_in_pack_object_header(obuf, sizeof(obuf), OBJ_BLOB, stream->size); + hdrlen = encode_in_pack_object_header(obuf, sizeof(obuf), stream->type, stream->size); s.next_out = obuf + hdrlen; s.avail_out = sizeof(obuf) - hdrlen; @@ -764,7 +764,7 @@ static void stream_blob_to_pack(struct transaction_packfile *state, ssize_t rsize = odb_write_stream_read(stream, ibuf, sizeof(ibuf)); if (rsize < 0) - die("failed to read blob data"); + die("failed to read object data"); if (!rsize) is_finished = true; @@ -797,7 +797,7 @@ static void stream_blob_to_pack(struct transaction_packfile *state, } if (bytes_read != stream->size) - die("read %" PRIuMAX " bytes of blob data, but expected %" PRIuMAX " bytes", + die("read %" PRIuMAX " bytes of object data, but expected %" PRIuMAX " bytes", (uintmax_t)bytes_read, (uintmax_t)stream->size); git_deflate_end(&s); @@ -868,7 +868,7 @@ static void flush_packfile_transaction(struct odb_transaction_files *transaction * result, which we need to know beforehand when writing a git object. * Since the primary motivation for trying to stream from the working * tree file and to avoid mmaping it in core is to deal with large - * binary blobs, they generally do not want to get any conversion, and + * objects, they generally do not want to get any conversion, and * callers should avoid this code path when filters are requested. */ static int odb_transaction_files_write_object_stream(struct odb_transaction *base, @@ -886,7 +886,7 @@ static int odb_transaction_files_write_object_stream(struct odb_transaction *bas struct pack_idx_entry *idx; header_len = format_object_header((char *)obuf, sizeof(obuf), - OBJ_BLOB, stream->size); + stream->type, stream->size); git_hash_init(&ctx, transaction->base.source->odb->repo->hash_algo); git_hash_update(&ctx, obuf, header_len); @@ -911,7 +911,7 @@ static int odb_transaction_files_write_object_stream(struct odb_transaction *bas hashfile_checkpoint(state->f, &checkpoint); idx->offset = state->offset; crc32_begin(state->f); - stream_blob_to_pack(state, &ctx, stream); + stream_to_pack(state, &ctx, stream); git_hash_final_oid(result_oid, &ctx); idx->crc32 = crc32_end(state->f); @@ -953,7 +953,7 @@ int index_fd(struct index_state *istate, struct object_id *oid, type, path, flags); } else { struct odb_write_stream stream; - odb_write_stream_from_fd(&stream, fd, xsize_t(st->st_size)); + odb_write_stream_from_fd(&stream, fd, xsize_t(st->st_size), OBJ_BLOB); if (flags & INDEX_WRITE_OBJECT) { struct object_database *odb = the_repository->objects; @@ -968,8 +968,7 @@ int index_fd(struct index_state *istate, struct object_id *oid, if (!inflight) odb_transaction_commit(transaction); } else { - ret = hash_blob_stream(&stream, - the_repository->hash_algo, oid); + ret = hash_stream(&stream, the_repository->hash_algo, oid); } odb_write_stream_release(&stream); 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); if (ret < 0) goto out; diff --git a/odb/source-loose.c b/odb/source-loose.c index 361b4e2a2a..5681a38f03 100644 --- a/odb/source-loose.c +++ b/odb/source-loose.c @@ -868,7 +868,7 @@ static int odb_source_loose_write_object_stream(struct odb_source *source, /* Since oid is not determined, save tmp file to odb path. */ strbuf_addf(&filename, "%s/", loose->base.path); - hdrlen = format_object_header(hdr, sizeof(hdr), OBJ_BLOB, in_stream->size); + hdrlen = format_object_header(hdr, sizeof(hdr), in_stream->type, in_stream->size); /* * Common steps for write_loose_object and stream_loose_object to diff --git a/odb/streaming.c b/odb/streaming.c index 912e75e682..0918cad426 100644 --- a/odb/streaming.c +++ b/odb/streaming.c @@ -324,7 +324,7 @@ static ssize_t read_object_fd(struct odb_write_stream *stream, } void odb_write_stream_from_fd(struct odb_write_stream *stream, int fd, - size_t size) + size_t size, enum object_type type) { struct read_object_fd_data *data; @@ -335,4 +335,5 @@ void odb_write_stream_from_fd(struct odb_write_stream *stream, int fd, stream->data = data; stream->read = read_object_fd; stream->size = size; + stream->type = type; } diff --git a/odb/streaming.h b/odb/streaming.h index 5e8e6e532e..3c8ed55129 100644 --- a/odb/streaming.h +++ b/odb/streaming.h @@ -56,6 +56,7 @@ struct odb_write_stream { ssize_t (*read)(struct odb_write_stream *, unsigned char *, size_t); void *data; size_t size; + enum object_type type; }; /* @@ -92,6 +93,6 @@ int odb_stream_blob_to_fd(struct object_database *odb, * Sets up an ODB write stream that reads from an fd. */ void odb_write_stream_from_fd(struct odb_write_stream *stream, int fd, - size_t size); + size_t size, enum object_type type); #endif /* STREAMING_H */ diff --git a/t/unit-tests/u-odb-inmemory.c b/t/unit-tests/u-odb-inmemory.c index 4437140ed0..1ab07af6d6 100644 --- a/t/unit-tests/u-odb-inmemory.c +++ b/t/unit-tests/u-odb-inmemory.c @@ -297,8 +297,11 @@ void test_odb_inmemory__write_object_stream(void) struct odb_source_inmemory *source = odb_source_inmemory_new(odb); const char data[] = "foobar"; struct membuf_write_stream stream = { - .base.read = membuf_write_stream_read, - .base.size = strlen(data), + .base = { + .read = membuf_write_stream_read, + .size = strlen(data), + .type = OBJ_BLOB, + }, .buf = data, }; struct object_id written_oid; -- 2.55.0.679.g6767b8d81c.dirty ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 3/7] odb/streaming: support streaming arbitrary object types 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 1 sibling, 1 reply; 27+ messages in thread From: Justin Tobler @ 2026-08-04 18:03 UTC (permalink / raw) To: Patrick Steinhardt; +Cc: git On 26/08/04 09:25AM, Patrick Steinhardt wrote: > 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. > > As we only ever write large files, the infrastructure doesn't support > specifying any other object type than "blob". This limitation is quite > artificial though: there is no reason why we shouldn't support writing > arbitrary large objects with a stream. While it's very unlikely that we > encounter a huge object other than a blob, users are known to be > creative and sometimes like to inflict pain on themselves by creating > commits or trees that are huge. > > Extend the infrastructure to support streaming arbitrary object types. > For now we don't use this functionality anywhere, but it brings us a bit > closer to unify `struct odb_read_stream` and `struct odb_write_stream`. Very happy to see this change. :) > Signed-off-by: Patrick Steinhardt <ps@pks.im> > --- > builtin/unpack-objects.c | 1 + > object-file.c | 31 +++++++++++++++---------------- > odb/source-inmemory.c | 2 +- > odb/source-loose.c | 2 +- > odb/streaming.c | 3 ++- > odb/streaming.h | 3 ++- > t/unit-tests/u-odb-inmemory.c | 7 +++++-- > 7 files changed, 27 insertions(+), 22 deletions(-) Just FYI, there is also a comment in "odb/transaction.h" for the `write_object_stream` callback that is also now outdated due to this change. We may want to update that too. [snip] > @@ -953,7 +953,7 @@ int index_fd(struct index_state *istate, struct object_id *oid, > type, path, flags); > } else { > struct odb_write_stream stream; > - odb_write_stream_from_fd(&stream, fd, xsize_t(st->st_size)); > + odb_write_stream_from_fd(&stream, fd, xsize_t(st->st_size), OBJ_BLOB); We still only target large blobs for streaming here, but the underlying infrastructure is now generic which is nice. [snip] > diff --git a/odb/streaming.h b/odb/streaming.h > index 5e8e6e532e..3c8ed55129 100644 > --- a/odb/streaming.h > +++ b/odb/streaming.h > @@ -56,6 +56,7 @@ struct odb_write_stream { > ssize_t (*read)(struct odb_write_stream *, unsigned char *, size_t); > void *data; > size_t size; > + enum object_type type; We now store the object type in the stream itself. Similar to size information, the type information is always known in advance when creating the object stream. The rest of this patch is just updating call sites accordingly. Looks good. -Justin ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/7] odb/streaming: support streaming arbitrary object types 2026-08-04 18:03 ` Justin Tobler @ 2026-08-05 6:06 ` Patrick Steinhardt 0 siblings, 0 replies; 27+ messages in thread From: Patrick Steinhardt @ 2026-08-05 6:06 UTC (permalink / raw) To: Justin Tobler; +Cc: git On Tue, Aug 04, 2026 at 01:03:41PM -0500, Justin Tobler wrote: > On 26/08/04 09:25AM, Patrick Steinhardt wrote: > > builtin/unpack-objects.c | 1 + > > object-file.c | 31 +++++++++++++++---------------- > > odb/source-inmemory.c | 2 +- > > odb/source-loose.c | 2 +- > > odb/streaming.c | 3 ++- > > odb/streaming.h | 3 ++- > > t/unit-tests/u-odb-inmemory.c | 7 +++++-- > > 7 files changed, 27 insertions(+), 22 deletions(-) > > Just FYI, there is also a comment in "odb/transaction.h" for the > `write_object_stream` callback that is also now outdated due to this > change. We may want to update that too. Good catch, fixed now. Patrick ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/7] odb/streaming: support streaming arbitrary object types 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-04 18:54 ` Junio C Hamano 2026-08-05 6:06 ` Patrick Steinhardt 1 sibling, 1 reply; 27+ messages in thread From: Junio C Hamano @ 2026-08-04 18:54 UTC (permalink / raw) To: Patrick Steinhardt; +Cc: git 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. > 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? ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/7] odb/streaming: support streaming arbitrary object types 2026-08-04 18:54 ` Junio C Hamano @ 2026-08-05 6:06 ` Patrick Steinhardt 0 siblings, 0 replies; 27+ messages in thread From: Patrick Steinhardt @ 2026-08-05 6:06 UTC (permalink / raw) To: Junio C Hamano; +Cc: git 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 4/7] odb/streaming: rename `struct odb_read_stream` 2026-08-04 7:25 [PATCH 0/7] odb: unify read and write streams Patrick Steinhardt ` (2 preceding siblings ...) 2026-08-04 7:25 ` [PATCH 3/7] odb/streaming: support streaming arbitrary object types Patrick Steinhardt @ 2026-08-04 7:25 ` Patrick Steinhardt 2026-08-04 7:25 ` [PATCH 5/7] odb/streaming: consolidate read and write streams Patrick Steinhardt ` (3 subsequent siblings) 7 siblings, 0 replies; 27+ messages in thread From: Patrick Steinhardt @ 2026-08-04 7:25 UTC (permalink / raw) To: git Rename `struct odb_read_stream` to just `struct odb_stream`. This prepares for unification of the two different types of streams, as these provide the same functionality with the preceding refactorings. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- archive-tar.c | 6 +++--- archive-zip.c | 10 ++++----- builtin/index-pack.c | 6 +++--- builtin/pack-objects.c | 14 ++++++------- object-file.c | 4 ++-- object-file.h | 2 +- object.c | 6 +++--- odb/source-files.c | 2 +- odb/source-inmemory.c | 8 ++++---- odb/source-loose.c | 8 ++++---- odb/source-packed.c | 2 +- odb/source.h | 6 +++--- odb/streaming.c | 48 +++++++++++++++++++++---------------------- odb/streaming.h | 24 +++++++++++----------- pack-check.c | 4 ++-- packfile.c | 8 ++++---- packfile.h | 4 ++-- t/unit-tests/u-odb-inmemory.c | 12 +++++------ 18 files changed, 87 insertions(+), 87 deletions(-) diff --git a/archive-tar.c b/archive-tar.c index 0fc70d13a8..df2d7fb8e9 100644 --- a/archive-tar.c +++ b/archive-tar.c @@ -129,7 +129,7 @@ static void write_trailer(void) */ static int stream_blocked(struct repository *r, const struct object_id *oid) { - struct odb_read_stream *st; + struct odb_stream *st; char buf[BLOCKSIZE]; ssize_t readlen; @@ -137,12 +137,12 @@ static int stream_blocked(struct repository *r, const struct object_id *oid) if (!st) return error(_("cannot stream blob %s"), oid_to_hex(oid)); for (;;) { - readlen = odb_read_stream_read(st, buf, sizeof(buf)); + readlen = odb_stream_read(st, buf, sizeof(buf)); if (readlen <= 0) break; do_write_blocked(buf, readlen); } - odb_read_stream_close(st); + odb_stream_close(st); if (!readlen) finish_record(); return readlen; diff --git a/archive-zip.c b/archive-zip.c index 97ea8d60d6..8095fd04d5 100644 --- a/archive-zip.c +++ b/archive-zip.c @@ -309,7 +309,7 @@ static int write_zip_entry(struct archiver_args *args, enum zip_method method; unsigned char *out; void *deflated = NULL; - struct odb_read_stream *stream = NULL; + struct odb_stream *stream = NULL; unsigned long flags = 0; int is_binary = -1; const char *path_without_prefix = path + args->baselen; @@ -428,7 +428,7 @@ static int write_zip_entry(struct archiver_args *args, ssize_t readlen; for (;;) { - readlen = odb_read_stream_read(stream, buf, sizeof(buf)); + readlen = odb_stream_read(stream, buf, sizeof(buf)); if (readlen <= 0) break; crc = crc32(crc, buf, readlen); @@ -438,7 +438,7 @@ static int write_zip_entry(struct archiver_args *args, buf, readlen); write_or_die(1, buf, readlen); } - odb_read_stream_close(stream); + odb_stream_close(stream); if (readlen) return readlen; @@ -461,7 +461,7 @@ static int write_zip_entry(struct archiver_args *args, zstream.avail_out = sizeof(compressed); for (;;) { - readlen = odb_read_stream_read(stream, buf, sizeof(buf)); + readlen = odb_stream_read(stream, buf, sizeof(buf)); if (readlen <= 0) break; crc = crc32(crc, buf, readlen); @@ -485,7 +485,7 @@ static int write_zip_entry(struct archiver_args *args, } } - odb_read_stream_close(stream); + odb_stream_close(stream); if (readlen) return readlen; diff --git a/builtin/index-pack.c b/builtin/index-pack.c index bc86925ad0..7226da3e65 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -763,7 +763,7 @@ static void find_ref_delta_children(const struct object_id *oid, struct compare_data { struct object_entry *entry; - struct odb_read_stream *st; + struct odb_stream *st; unsigned char *buf; unsigned long buf_size; }; @@ -780,7 +780,7 @@ static int compare_objects(const unsigned char *buf, unsigned long size, } while (size) { - ssize_t len = odb_read_stream_read(data->st, data->buf, size); + ssize_t len = odb_stream_read(data->st, data->buf, size); if (len == 0) die(_("SHA1 COLLISION FOUND WITH %s !"), oid_to_hex(&data->entry->idx.oid)); @@ -813,7 +813,7 @@ static int check_collison(struct object_entry *entry) die(_("SHA1 COLLISION FOUND WITH %s !"), oid_to_hex(&entry->idx.oid)); unpack_data(entry, compare_objects, &data); - odb_read_stream_close(data.st); + odb_stream_close(data.st); free(data.buf); return 0; } diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 1ec5b6f206..683160c6bb 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -411,7 +411,7 @@ static unsigned long do_compress(void **pptr, unsigned long size) return stream.total_out; } -static unsigned long write_large_blob_data(struct odb_read_stream *st, struct hashfile *f, +static unsigned long write_large_blob_data(struct odb_stream *st, struct hashfile *f, const struct object_id *oid) { git_zstream stream; @@ -425,7 +425,7 @@ static unsigned long write_large_blob_data(struct odb_read_stream *st, struct ha for (;;) { ssize_t readlen; int zret = Z_OK; - readlen = odb_read_stream_read(st, ibuf, sizeof(ibuf)); + readlen = odb_stream_read(st, ibuf, sizeof(ibuf)); if (readlen == -1) die(_("unable to read %s"), oid_to_hex(oid)); @@ -521,7 +521,7 @@ static unsigned long write_no_reuse_object(struct hashfile *f, struct object_ent unsigned hdrlen; enum object_type type; void *buf; - struct odb_read_stream *st = NULL; + struct odb_stream *st = NULL; const unsigned hashsz = the_hash_algo->rawsz; if (!usable_delta) { @@ -589,7 +589,7 @@ static unsigned long write_no_reuse_object(struct hashfile *f, struct object_ent dheader[--pos] = 128 | (--ofs & 127); if (limit && hdrlen + sizeof(dheader) - pos + datalen + hashsz >= limit) { if (st) - odb_read_stream_close(st); + odb_stream_close(st); free(buf); return 0; } @@ -603,7 +603,7 @@ static unsigned long write_no_reuse_object(struct hashfile *f, struct object_ent */ if (limit && hdrlen + hashsz + datalen + hashsz >= limit) { if (st) - odb_read_stream_close(st); + odb_stream_close(st); free(buf); return 0; } @@ -613,7 +613,7 @@ static unsigned long write_no_reuse_object(struct hashfile *f, struct object_ent } else { if (limit && hdrlen + datalen + hashsz >= limit) { if (st) - odb_read_stream_close(st); + odb_stream_close(st); free(buf); return 0; } @@ -621,7 +621,7 @@ static unsigned long write_no_reuse_object(struct hashfile *f, struct object_ent } if (st) { datalen = write_large_blob_data(st, f, &entry->idx.oid); - odb_read_stream_close(st); + odb_stream_close(st); } else { hashwrite(f, buf, datalen); free(buf); diff --git a/object-file.c b/object-file.c index 699a6a008c..5f6d584c35 100644 --- a/object-file.c +++ b/object-file.c @@ -122,7 +122,7 @@ int check_object_signature(struct repository *r, const struct object_id *oid, } int stream_object_signature(struct repository *r, - struct odb_read_stream *st, + struct odb_stream *st, const struct object_id *oid) { struct object_id real_oid; @@ -138,7 +138,7 @@ int stream_object_signature(struct repository *r, git_hash_update(&c, hdr, hdrlen); for (;;) { char buf[1024 * 16]; - ssize_t readlen = odb_read_stream_read(st, buf, sizeof(buf)); + ssize_t readlen = odb_stream_read(st, buf, sizeof(buf)); if (readlen < 0) return -1; if (!readlen) diff --git a/object-file.h b/object-file.h index 805f2cfa28..f44758c4f8 100644 --- a/object-file.h +++ b/object-file.h @@ -101,7 +101,7 @@ int check_object_signature(struct repository *r, const struct object_id *oid, * the streaming interface and rehash it to do the same. */ int stream_object_signature(struct repository *r, - struct odb_read_stream *stream, + struct odb_stream *stream, const struct object_id *oid); enum finalize_object_file_flags { diff --git a/object.c b/object.c index 23b84aa7e2..37e6efee47 100644 --- a/object.c +++ b/object.c @@ -345,7 +345,7 @@ struct object *parse_object_with_flags(struct repository *r, if ((!obj || obj->type == OBJ_NONE || obj->type == OBJ_BLOB) && odb_read_object_info(r->objects, oid, NULL) == OBJ_BLOB) { if (!skip_hash) { - struct odb_read_stream *stream = odb_read_stream_open(r->objects, oid, NULL); + struct odb_stream *stream = odb_read_stream_open(r->objects, oid, NULL); if (!stream) { error(_("unable to open object stream for %s"), oid_to_hex(oid)); @@ -354,11 +354,11 @@ struct object *parse_object_with_flags(struct repository *r, if (stream_object_signature(r, stream, repl) < 0) { error(_("hash mismatch %s"), oid_to_hex(oid)); - odb_read_stream_close(stream); + odb_stream_close(stream); return NULL; } - odb_read_stream_close(stream); + odb_stream_close(stream); } parse_blob_buffer(lookup_blob(r, oid)); return lookup_object(r, oid); diff --git a/odb/source-files.c b/odb/source-files.c index f51960bd71..f7b8c76549 100644 --- a/odb/source-files.c +++ b/odb/source-files.c @@ -63,7 +63,7 @@ static int odb_source_files_read_object_info(struct odb_source *source, return -1; } -static int odb_source_files_read_object_stream(struct odb_read_stream **out, +static int odb_source_files_read_object_stream(struct odb_stream **out, struct odb_source *source, const struct object_id *oid) { diff --git a/odb/source-inmemory.c b/odb/source-inmemory.c index 4f76db5496..4bee3ae699 100644 --- a/odb/source-inmemory.c +++ b/odb/source-inmemory.c @@ -73,12 +73,12 @@ static int odb_source_inmemory_read_object_info(struct odb_source *source, } struct odb_read_stream_inmemory { - struct odb_read_stream base; + struct odb_stream base; const unsigned char *buf; size_t offset; }; -static ssize_t odb_read_stream_inmemory_read(struct odb_read_stream *stream, +static ssize_t odb_read_stream_inmemory_read(struct odb_stream *stream, char *buf, size_t buf_len) { struct odb_read_stream_inmemory *inmemory = @@ -94,12 +94,12 @@ static ssize_t odb_read_stream_inmemory_read(struct odb_read_stream *stream, return bytes; } -static int odb_read_stream_inmemory_close(struct odb_read_stream *stream UNUSED) +static int odb_read_stream_inmemory_close(struct odb_stream *stream UNUSED) { return 0; } -static int odb_source_inmemory_read_object_stream(struct odb_read_stream **out, +static int odb_source_inmemory_read_object_stream(struct odb_stream **out, struct odb_source *source, const struct object_id *oid) { diff --git a/odb/source-loose.c b/odb/source-loose.c index 5681a38f03..038defd905 100644 --- a/odb/source-loose.c +++ b/odb/source-loose.c @@ -278,7 +278,7 @@ static void *odb_source_loose_map_object(struct odb_source_loose *loose, } struct odb_loose_read_stream { - struct odb_read_stream base; + struct odb_stream base; git_zstream z; enum { ODB_LOOSE_READ_STREAM_INUSE, @@ -292,7 +292,7 @@ struct odb_loose_read_stream { int hdr_used; }; -static ssize_t read_istream_loose(struct odb_read_stream *_st, char *buf, size_t sz) +static ssize_t read_istream_loose(struct odb_stream *_st, char *buf, size_t sz) { struct odb_loose_read_stream *st = container_of(_st, struct odb_loose_read_stream, base); @@ -339,7 +339,7 @@ static ssize_t read_istream_loose(struct odb_read_stream *_st, char *buf, size_t return total_read; } -static int close_istream_loose(struct odb_read_stream *_st) +static int close_istream_loose(struct odb_stream *_st) { struct odb_loose_read_stream *st = container_of(_st, struct odb_loose_read_stream, base); @@ -350,7 +350,7 @@ static int close_istream_loose(struct odb_read_stream *_st) return 0; } -static int odb_source_loose_read_object_stream(struct odb_read_stream **out, +static int odb_source_loose_read_object_stream(struct odb_stream **out, struct odb_source *source, const struct object_id *oid) { diff --git a/odb/source-packed.c b/odb/source-packed.c index e6ff74833b..b3186ca593 100644 --- a/odb/source-packed.c +++ b/odb/source-packed.c @@ -70,7 +70,7 @@ static int odb_source_packed_read_object_info(struct odb_source *source, return 0; } -static int odb_source_packed_read_object_stream(struct odb_read_stream **out, +static int odb_source_packed_read_object_stream(struct odb_stream **out, struct odb_source *source, const struct object_id *oid) { diff --git a/odb/source.h b/odb/source.h index 0080148ba7..89b0c39682 100644 --- a/odb/source.h +++ b/odb/source.h @@ -26,7 +26,7 @@ enum odb_source_type { }; struct object_id; -struct odb_read_stream; +struct odb_stream; struct strvec; /* @@ -125,7 +125,7 @@ struct odb_source { * The callback is expected to return a negative error code in case * creating the object stream has failed, 0 otherwise. */ - int (*read_object_stream)(struct odb_read_stream **out, + int (*read_object_stream)(struct odb_stream **out, struct odb_source *source, const struct object_id *oid); @@ -339,7 +339,7 @@ static inline int odb_source_read_object_info(struct odb_source *source, * Create a new read stream for the given object ID. Returns 0 on success, a * negative error code otherwise. */ -static inline int odb_source_read_object_stream(struct odb_read_stream **out, +static inline int odb_source_read_object_stream(struct odb_stream **out, struct odb_source *source, const struct object_id *oid) { diff --git a/odb/streaming.c b/odb/streaming.c index 0918cad426..98e2152e36 100644 --- a/odb/streaming.c +++ b/odb/streaming.c @@ -20,8 +20,8 @@ *****************************************************************/ struct odb_filtered_read_stream { - struct odb_read_stream base; - struct odb_read_stream *upstream; + struct odb_stream base; + struct odb_stream *upstream; struct stream_filter *filter; char ibuf[FILTER_BUFFER]; char obuf[FILTER_BUFFER]; @@ -30,14 +30,14 @@ struct odb_filtered_read_stream { int input_finished; }; -static int close_istream_filtered(struct odb_read_stream *_fs) +static int close_istream_filtered(struct odb_stream *_fs) { struct odb_filtered_read_stream *fs = (struct odb_filtered_read_stream *)_fs; free_stream_filter(fs->filter); - return odb_read_stream_close(fs->upstream); + return odb_stream_close(fs->upstream); } -static ssize_t read_istream_filtered(struct odb_read_stream *_fs, char *buf, +static ssize_t read_istream_filtered(struct odb_stream *_fs, char *buf, size_t sz) { struct odb_filtered_read_stream *fs = (struct odb_filtered_read_stream *)_fs; @@ -86,7 +86,7 @@ static ssize_t read_istream_filtered(struct odb_read_stream *_fs, char *buf, /* refill the input from the upstream */ if (!fs->input_finished) { - fs->i_end = odb_read_stream_read(fs->upstream, fs->ibuf, FILTER_BUFFER); + fs->i_end = odb_stream_read(fs->upstream, fs->ibuf, FILTER_BUFFER); if (fs->i_end < 0) return -1; if (fs->i_end) @@ -97,8 +97,8 @@ static ssize_t read_istream_filtered(struct odb_read_stream *_fs, char *buf, return filled; } -static struct odb_read_stream *attach_stream_filter(struct odb_read_stream *st, - struct stream_filter *filter) +static struct odb_stream *attach_stream_filter(struct odb_stream *st, + struct stream_filter *filter) { struct odb_filtered_read_stream *fs; @@ -120,19 +120,19 @@ static struct odb_read_stream *attach_stream_filter(struct odb_read_stream *st, *****************************************************************/ struct odb_incore_read_stream { - struct odb_read_stream base; + struct odb_stream base; char *buf; /* from odb_read_object_info_extended() */ unsigned long read_ptr; }; -static int close_istream_incore(struct odb_read_stream *_st) +static int close_istream_incore(struct odb_stream *_st) { struct odb_incore_read_stream *st = (struct odb_incore_read_stream *)_st; free(st->buf); return 0; } -static ssize_t read_istream_incore(struct odb_read_stream *_st, char *buf, size_t sz) +static ssize_t read_istream_incore(struct odb_stream *_st, char *buf, size_t sz) { struct odb_incore_read_stream *st = (struct odb_incore_read_stream *)_st; size_t read_size = sz; @@ -147,7 +147,7 @@ static ssize_t read_istream_incore(struct odb_read_stream *_st, char *buf, size_ return read_size; } -static int open_istream_incore(struct odb_read_stream **out, +static int open_istream_incore(struct odb_stream **out, struct object_database *odb, const struct object_id *oid) { @@ -178,7 +178,7 @@ static int open_istream_incore(struct odb_read_stream **out, * static helpers variables and functions for users of streaming interface *****************************************************************************/ -static int istream_source(struct odb_read_stream **out, +static int istream_source(struct odb_stream **out, struct object_database *odb, const struct object_id *oid) { @@ -196,23 +196,23 @@ static int istream_source(struct odb_read_stream **out, * Users of streaming interface ****************************************************************/ -int odb_read_stream_close(struct odb_read_stream *st) +int odb_stream_close(struct odb_stream *st) { int r = st->close(st); free(st); return r; } -ssize_t odb_read_stream_read(struct odb_read_stream *st, void *buf, size_t sz) +ssize_t odb_stream_read(struct odb_stream *st, void *buf, size_t sz) { return st->read(st, buf, sz); } -struct odb_read_stream *odb_read_stream_open(struct object_database *odb, - const struct object_id *oid, - struct stream_filter *filter) +struct odb_stream *odb_read_stream_open(struct object_database *odb, + const struct object_id *oid, + struct stream_filter *filter) { - struct odb_read_stream *st; + struct odb_stream *st; const struct object_id *real = lookup_replace_object(odb->repo, oid); int ret = istream_source(&st, odb, real); @@ -221,9 +221,9 @@ struct odb_read_stream *odb_read_stream_open(struct object_database *odb, if (filter) { /* Add "&& !is_null_stream_filter(filter)" for performance */ - struct odb_read_stream *nst = attach_stream_filter(st, filter); + struct odb_stream *nst = attach_stream_filter(st, filter); if (!nst) { - odb_read_stream_close(st); + odb_stream_close(st); return NULL; } st = nst; @@ -248,7 +248,7 @@ int odb_stream_blob_to_fd(struct object_database *odb, struct stream_filter *filter, int can_seek) { - struct odb_read_stream *st; + struct odb_stream *st; ssize_t kept = 0; int result = -1; @@ -263,7 +263,7 @@ int odb_stream_blob_to_fd(struct object_database *odb, for (;;) { char buf[1024 * 16]; ssize_t wrote, holeto; - ssize_t readlen = odb_read_stream_read(st, buf, sizeof(buf)); + ssize_t readlen = odb_stream_read(st, buf, sizeof(buf)); if (readlen < 0) goto close_and_exit; @@ -294,7 +294,7 @@ int odb_stream_blob_to_fd(struct object_database *odb, result = 0; close_and_exit: - odb_read_stream_close(st); + odb_stream_close(st); return result; } diff --git a/odb/streaming.h b/odb/streaming.h index 3c8ed55129..037954c231 100644 --- a/odb/streaming.h +++ b/odb/streaming.h @@ -8,19 +8,19 @@ #include "odb.h" struct object_database; -struct odb_read_stream; +struct odb_stream; struct stream_filter; -typedef int (*odb_read_stream_close_fn)(struct odb_read_stream *); -typedef ssize_t (*odb_read_stream_read_fn)(struct odb_read_stream *, char *, size_t); +typedef int (*odb_stream_close_fn)(struct odb_stream *); +typedef ssize_t (*odb_stream_read_fn)(struct odb_stream *, char *, size_t); /* * A stream that can be used to read an object from the object database without * loading all of it into memory. */ -struct odb_read_stream { - odb_read_stream_close_fn close; - odb_read_stream_read_fn read; +struct odb_stream { + odb_stream_close_fn close; + odb_stream_read_fn read; enum object_type type; size_t size; /* inflated size of full object */ }; @@ -31,22 +31,22 @@ struct odb_read_stream { * * Returns the stream on success, a `NULL` pointer otherwise. */ -struct odb_read_stream *odb_read_stream_open(struct object_database *odb, - const struct object_id *oid, - struct stream_filter *filter); +struct odb_stream *odb_read_stream_open(struct object_database *odb, + const struct object_id *oid, + struct stream_filter *filter); /* - * Close the given read stream and release all resources associated with it. + * Close the given object stream and release all resources associated with it. * Returns 0 on success, a negative error code otherwise. */ -int odb_read_stream_close(struct odb_read_stream *stream); +int odb_stream_close(struct odb_stream *stream); /* * Read data from the stream into the buffer. Returns 0 on EOF and the number * of bytes read on success. Returns a negative error code in case reading from * the stream fails. */ -ssize_t odb_read_stream_read(struct odb_read_stream *stream, void *buf, size_t len); +ssize_t odb_stream_read(struct odb_stream *stream, void *buf, size_t len); /* * A stream that provides an object to be written to the object database without diff --git a/pack-check.c b/pack-check.c index c3b8db7c5c..1b5e26847d 100644 --- a/pack-check.c +++ b/pack-check.c @@ -106,7 +106,7 @@ static int verify_packfile(struct repository *r, QSORT(entries, nr_objects, compare_entries); for (i = 0; i < nr_objects; i++) { - struct odb_read_stream *stream = NULL; + struct odb_stream *stream = NULL; void *data; struct object_id oid; enum object_type type; @@ -171,7 +171,7 @@ static int verify_packfile(struct repository *r, display_progress(progress, base_count + i); if (stream) - odb_read_stream_close(stream); + odb_stream_close(stream); free(data); } diff --git a/packfile.c b/packfile.c index 0eee45055f..70254573a3 100644 --- a/packfile.c +++ b/packfile.c @@ -2115,7 +2115,7 @@ int parse_pack_header_option(const char *in, unsigned char *out, unsigned int *l } struct odb_packed_read_stream { - struct odb_read_stream base; + struct odb_stream base; struct packed_git *pack; git_zstream z; enum { @@ -2127,7 +2127,7 @@ struct odb_packed_read_stream { off_t pos; }; -static ssize_t read_istream_pack_non_delta(struct odb_read_stream *_st, char *buf, +static ssize_t read_istream_pack_non_delta(struct odb_stream *_st, char *buf, size_t sz) { struct odb_packed_read_stream *st = (struct odb_packed_read_stream *)_st; @@ -2187,7 +2187,7 @@ static ssize_t read_istream_pack_non_delta(struct odb_read_stream *_st, char *bu return total_read; } -static int close_istream_pack_non_delta(struct odb_read_stream *_st) +static int close_istream_pack_non_delta(struct odb_stream *_st) { struct odb_packed_read_stream *st = (struct odb_packed_read_stream *)_st; if (st->z_state == ODB_PACKED_READ_STREAM_INUSE) @@ -2195,7 +2195,7 @@ static int close_istream_pack_non_delta(struct odb_read_stream *_st) return 0; } -int packfile_read_object_stream(struct odb_read_stream **out, +int packfile_read_object_stream(struct odb_stream **out, const struct object_id *oid, struct packed_git *pack, off_t offset) diff --git a/packfile.h b/packfile.h index e1f77152b5..f913cb3d0c 100644 --- a/packfile.h +++ b/packfile.h @@ -12,7 +12,7 @@ /* in odb.h */ struct object_info; -struct odb_read_stream; +struct odb_stream; struct packed_git { struct pack_window *windows; @@ -306,7 +306,7 @@ off_t get_delta_base(struct packed_git *p, struct pack_window **w_curs, off_t *curpos, enum object_type type, off_t delta_obj_offset); -int packfile_read_object_stream(struct odb_read_stream **out, +int packfile_read_object_stream(struct odb_stream **out, const struct object_id *oid, struct packed_git *pack, off_t offset); diff --git a/t/unit-tests/u-odb-inmemory.c b/t/unit-tests/u-odb-inmemory.c index 1ab07af6d6..839a0fd3b7 100644 --- a/t/unit-tests/u-odb-inmemory.c +++ b/t/unit-tests/u-odb-inmemory.c @@ -100,7 +100,7 @@ void test_odb_inmemory__read_written_object(void) void test_odb_inmemory__read_stream_object(void) { struct odb_source_inmemory *source = odb_source_inmemory_new(odb); - struct odb_read_stream *stream; + struct odb_stream *stream; struct object_id written_oid; const char data[] = "foobar"; char buf[3] = { 0 }; @@ -112,15 +112,15 @@ void test_odb_inmemory__read_stream_object(void) cl_assert_equal_i(stream->type, OBJ_BLOB); cl_assert_equal_u(stream->size, 6); - cl_assert_equal_i(odb_read_stream_read(stream, buf, 2), 2); + cl_assert_equal_i(odb_stream_read(stream, buf, 2), 2); cl_assert_equal_s(buf, "fo"); - cl_assert_equal_i(odb_read_stream_read(stream, buf, 2), 2); + cl_assert_equal_i(odb_stream_read(stream, buf, 2), 2); cl_assert_equal_s(buf, "ob"); - cl_assert_equal_i(odb_read_stream_read(stream, buf, 2), 2); + cl_assert_equal_i(odb_stream_read(stream, buf, 2), 2); cl_assert_equal_s(buf, "ar"); - cl_assert_equal_i(odb_read_stream_read(stream, buf, 2), 0); + cl_assert_equal_i(odb_stream_read(stream, buf, 2), 0); - odb_read_stream_close(stream); + odb_stream_close(stream); odb_source_free(&source->base); } -- 2.55.0.679.g6767b8d81c.dirty ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 5/7] odb/streaming: consolidate read and write streams 2026-08-04 7:25 [PATCH 0/7] odb: unify read and write streams Patrick Steinhardt ` (3 preceding siblings ...) 2026-08-04 7:25 ` [PATCH 4/7] odb/streaming: rename `struct odb_read_stream` Patrick Steinhardt @ 2026-08-04 7:25 ` Patrick Steinhardt 2026-08-04 18:23 ` Justin Tobler 2026-08-04 7:25 ` [PATCH 6/7] odb/streaming: rename `struct read_object_fd_data` Patrick Steinhardt ` (2 subsequent siblings) 7 siblings, 1 reply; 27+ messages in thread From: Patrick Steinhardt @ 2026-08-04 7:25 UTC (permalink / raw) To: git The `struct odb_read_stream` and `struct odb_write_stream` both provide the same functionality: they allow a caller to read object data from an arbitrary source. Historically, the only difference was that the read stream was used to read data out of the object database, whereas the write stream was used to write data into the object database, but the interfaces were mostly the same. Over the preceding commits we have refactored the write stream to have almost exactly the same interface as the read stream. With these refactorings we can now easily merge those two streams into a single interface that's used for both use cases. While most of the changes are mechanical, there are two sites that need special mention: - "builtin/unpack-objects.c" creates a write stream from compressed object data. - "odb/streaming.c" creates a write stream from a file descriptor. Adapting these sites to yield the new stream type requires a couple more changes. Most importantly, instead of embedding the pointer to the data in `struct odb_write_stream`, we now allocate a structure that wraps the new `struct odb_stream` base. Other than that though, the changes are rather straight forward. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- builtin/unpack-objects.c | 31 ++++++++++++++++--------------- object-file.c | 25 ++++++++++++------------- odb.c | 2 +- odb.h | 4 ++-- odb/source-files.c | 2 +- odb/source-inmemory.c | 4 ++-- odb/source-loose.c | 6 +++--- odb/source-packed.c | 2 +- odb/source.h | 4 ++-- odb/streaming.c | 35 ++++++++++++++++------------------- odb/streaming.h | 31 +++---------------------------- odb/transaction.c | 2 +- odb/transaction.h | 4 ++-- t/unit-tests/u-odb-inmemory.c | 6 +++--- 14 files changed, 65 insertions(+), 93 deletions(-) diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c index 7439ec53be..05a2d48011 100644 --- a/builtin/unpack-objects.c +++ b/builtin/unpack-objects.c @@ -359,20 +359,21 @@ static void unpack_non_delta_entry(enum object_type type, unsigned long size, } struct input_zstream_data { + struct odb_stream base; git_zstream *zstream; int status; }; -static ssize_t feed_input_zstream(struct odb_write_stream *in_stream, - unsigned char *buf, size_t buf_len) +static ssize_t feed_input_zstream(struct odb_stream *in_stream, + char *buf, size_t buf_len) { - struct input_zstream_data *data = in_stream->data; + struct input_zstream_data *data = container_of(in_stream, struct input_zstream_data, base); git_zstream *zstream = data->zstream; if (data->status != Z_OK) return 0; - zstream->next_out = buf; + zstream->next_out = (unsigned char *) buf; zstream->avail_out = buf_len; while (data->status == Z_OK && zstream->avail_out == buf_len) { @@ -388,24 +389,24 @@ static ssize_t feed_input_zstream(struct odb_write_stream *in_stream, static void stream_blob(unsigned long size, unsigned nr) { git_zstream zstream = { 0 }; - struct input_zstream_data data = { 0 }; - struct odb_write_stream in_stream = { - .read = feed_input_zstream, - .data = &data, - .size = size, - .type = OBJ_BLOB, + struct input_zstream_data in_stream = { + .base = { + .read = feed_input_zstream, + .size = size, + .type = OBJ_BLOB, + }, + .zstream = &zstream, + .status = Z_OK, }; struct obj_info *info = &obj_list[nr]; - data.zstream = &zstream; - data.status = Z_OK; git_inflate_init(&zstream); - if (odb_write_object_stream(the_repository->objects, &in_stream, &info->oid)) + if (odb_write_object_stream(the_repository->objects, &in_stream.base, &info->oid)) die(_("failed to write object in stream")); - if (data.status != Z_STREAM_END) - die(_("inflate returned (%d)"), data.status); + if (in_stream.status != Z_STREAM_END) + die(_("inflate returned (%d)"), in_stream.status); git_inflate_end(&zstream); if (strict) { diff --git a/object-file.c b/object-file.c index 5f6d584c35..068c6e5672 100644 --- a/object-file.c +++ b/object-file.c @@ -702,7 +702,7 @@ static void prepare_packfile_transaction(struct odb_transaction_files *transacti die_errno("unable to write pack header"); } -static int hash_stream(struct odb_write_stream *stream, +static int hash_stream(struct odb_stream *stream, const struct git_hash_algo *hash_algo, struct object_id *result_oid) { @@ -717,8 +717,8 @@ static int hash_stream(struct odb_write_stream *stream, git_hash_update(&ctx, buf, header_len); while (1) { - ssize_t read_result = odb_write_stream_read(stream, buf, - sizeof(buf)); + ssize_t read_result = odb_stream_read(stream, buf, + sizeof(buf)); if (read_result < 0) return -1; if (!read_result) @@ -742,7 +742,7 @@ static int hash_stream(struct odb_write_stream *stream, */ static void stream_to_pack(struct transaction_packfile *state, struct git_hash_ctx *ctx, - struct odb_write_stream *stream) + struct odb_stream *stream) { git_zstream s; unsigned char ibuf[16384]; @@ -761,8 +761,8 @@ static void stream_to_pack(struct transaction_packfile *state, while (status != Z_STREAM_END) { if (!is_finished && !s.avail_in) { - ssize_t rsize = odb_write_stream_read(stream, ibuf, - sizeof(ibuf)); + ssize_t rsize = odb_stream_read(stream, ibuf, + sizeof(ibuf)); if (rsize < 0) die("failed to read object data"); if (!rsize) @@ -872,7 +872,7 @@ static void flush_packfile_transaction(struct odb_transaction_files *transaction * callers should avoid this code path when filters are requested. */ static int odb_transaction_files_write_object_stream(struct odb_transaction *base, - struct odb_write_stream *stream, + struct odb_stream *stream, struct object_id *result_oid) { struct odb_transaction_files *transaction = container_of(base, @@ -952,8 +952,8 @@ int index_fd(struct index_state *istate, struct object_id *oid, ret = index_core(istate, oid, fd, xsize_t(st->st_size), type, path, flags); } else { - struct odb_write_stream stream; - odb_write_stream_from_fd(&stream, fd, xsize_t(st->st_size), OBJ_BLOB); + struct odb_stream *stream = odb_write_stream_from_fd(fd, xsize_t(st->st_size), + OBJ_BLOB); if (flags & INDEX_WRITE_OBJECT) { struct object_database *odb = the_repository->objects; @@ -963,15 +963,14 @@ int index_fd(struct index_state *istate, struct object_id *oid, if (!inflight) odb_transaction_begin_or_die(odb, &transaction, 0); ret = odb_transaction_write_object_stream(transaction, - &stream, - oid); + stream, oid); if (!inflight) odb_transaction_commit(transaction); } else { - ret = hash_stream(&stream, the_repository->hash_algo, oid); + ret = hash_stream(stream, the_repository->hash_algo, oid); } - odb_write_stream_release(&stream); + odb_stream_close(stream); } close(fd); diff --git a/odb.c b/odb.c index 585b2b2965..eec4cc5302 100644 --- a/odb.c +++ b/odb.c @@ -1028,7 +1028,7 @@ int odb_write_object_ext(struct object_database *odb, } int odb_write_object_stream(struct object_database *odb, - struct odb_write_stream *stream, + struct odb_stream *stream, struct object_id *oid) { return odb_source_write_object_stream(odb->sources, stream, oid); diff --git a/odb.h b/odb.h index 019d3af3e8..fbe75c5a81 100644 --- a/odb.h +++ b/odb.h @@ -626,10 +626,10 @@ static inline int odb_write_object(struct object_database *odb, return odb_write_object_ext(odb, buf, len, type, oid, NULL, 0); } -struct odb_write_stream; +struct odb_stream; int odb_write_object_stream(struct object_database *odb, - struct odb_write_stream *stream, + struct odb_stream *stream, struct object_id *oid); void parse_alternates(const char *string, diff --git a/odb/source-files.c b/odb/source-files.c index f7b8c76549..6defe5ac4f 100644 --- a/odb/source-files.c +++ b/odb/source-files.c @@ -174,7 +174,7 @@ static int odb_source_files_write_object(struct odb_source *source, } static int odb_source_files_write_object_stream(struct odb_source *source, - struct odb_write_stream *stream, + struct odb_stream *stream, struct object_id *oid) { struct odb_source_files *files = odb_source_files_downcast(source); diff --git a/odb/source-inmemory.c b/odb/source-inmemory.c index 4bee3ae699..bb63cdce86 100644 --- a/odb/source-inmemory.c +++ b/odb/source-inmemory.c @@ -256,7 +256,7 @@ static int odb_source_inmemory_write_object(struct odb_source *source, } static int odb_source_inmemory_write_object_stream(struct odb_source *source, - struct odb_write_stream *stream, + struct odb_stream *stream, struct object_id *oid) { char buf[16384]; @@ -268,7 +268,7 @@ static int odb_source_inmemory_write_object_stream(struct odb_source *source, while (1) { ssize_t bytes_read; - bytes_read = odb_write_stream_read(stream, buf, sizeof(buf)); + bytes_read = odb_stream_read(stream, buf, sizeof(buf)); if (bytes_read < 0) { ret = error("failed to read object stream"); goto out; diff --git a/odb/source-loose.c b/odb/source-loose.c index 038defd905..ff1bede7fe 100644 --- a/odb/source-loose.c +++ b/odb/source-loose.c @@ -845,7 +845,7 @@ static int odb_source_loose_write_object(struct odb_source *source, } static int odb_source_loose_write_object_stream(struct odb_source *source, - struct odb_write_stream *in_stream, + struct odb_stream *in_stream, struct object_id *oid) { struct odb_source_loose *loose = odb_source_loose_downcast(source); @@ -891,8 +891,8 @@ static int odb_source_loose_write_object_stream(struct odb_source *source, unsigned char *in0 = stream.next_in; if (!stream.avail_in && !is_finished) { - ssize_t read_len = odb_write_stream_read(in_stream, buf, - sizeof(buf)); + ssize_t read_len = odb_stream_read(in_stream, buf, + sizeof(buf)); if (read_len < 0) { close(fd); err = -1; diff --git a/odb/source-packed.c b/odb/source-packed.c index b3186ca593..630d955585 100644 --- a/odb/source-packed.c +++ b/odb/source-packed.c @@ -609,7 +609,7 @@ static int odb_source_packed_write_object(struct odb_source *source UNUSED, } static int odb_source_packed_write_object_stream(struct odb_source *source UNUSED, - struct odb_write_stream *stream UNUSED, + struct odb_stream *stream UNUSED, struct object_id *oid UNUSED) { return error("packed backend cannot write object streams"); diff --git a/odb/source.h b/odb/source.h index 89b0c39682..0b99c698b5 100644 --- a/odb/source.h +++ b/odb/source.h @@ -221,7 +221,7 @@ struct odb_source { * otherwise. */ int (*write_object_stream)(struct odb_source *source, - struct odb_write_stream *stream, + struct odb_stream *stream, struct object_id *oid); /* @@ -436,7 +436,7 @@ static inline int odb_source_write_object(struct odb_source *source, * out pointer for the object ID. */ static inline int odb_source_write_object_stream(struct odb_source *source, - struct odb_write_stream *stream, + struct odb_stream *stream, struct object_id *oid) { return source->write_object_stream(source, stream, oid); diff --git a/odb/streaming.c b/odb/streaming.c index 98e2152e36..1a267e6b90 100644 --- a/odb/streaming.c +++ b/odb/streaming.c @@ -232,16 +232,6 @@ struct odb_stream *odb_read_stream_open(struct object_database *odb, return st; } -ssize_t odb_write_stream_read(struct odb_write_stream *st, void *buf, size_t sz) -{ - return st->read(st, buf, sz); -} - -void odb_write_stream_release(struct odb_write_stream *st) -{ - free(st->data); -} - int odb_stream_blob_to_fd(struct object_database *odb, int fd, const struct object_id *oid, @@ -299,14 +289,15 @@ int odb_stream_blob_to_fd(struct object_database *odb, } struct read_object_fd_data { + struct odb_stream base; int fd; size_t remaining; }; -static ssize_t read_object_fd(struct odb_write_stream *stream, - unsigned char *buf, size_t len) +static ssize_t read_object_fd(struct odb_stream *stream, + char *buf, size_t len) { - struct read_object_fd_data *data = stream->data; + struct read_object_fd_data *data = container_of(stream, struct read_object_fd_data, base); ssize_t read_result; size_t count; @@ -323,17 +314,23 @@ static ssize_t read_object_fd(struct odb_write_stream *stream, return read_result; } -void odb_write_stream_from_fd(struct odb_write_stream *stream, int fd, - size_t size, enum object_type type) +static int close_object_fd(struct odb_stream *stream UNUSED) +{ + /* The file descriptor is owned by the caller for now. */ + return 0; +} + +struct odb_stream *odb_write_stream_from_fd(int fd, size_t size, enum object_type type) { struct read_object_fd_data *data; CALLOC_ARRAY(data, 1); + data->base.read = read_object_fd; + data->base.close = close_object_fd; + data->base.size = size; + data->base.type = type; data->fd = fd; data->remaining = size; - stream->data = data; - stream->read = read_object_fd; - stream->size = size; - stream->type = type; + return &data->base; } diff --git a/odb/streaming.h b/odb/streaming.h index 037954c231..60b9803190 100644 --- a/odb/streaming.h +++ b/odb/streaming.h @@ -15,8 +15,8 @@ typedef int (*odb_stream_close_fn)(struct odb_stream *); typedef ssize_t (*odb_stream_read_fn)(struct odb_stream *, char *, size_t); /* - * A stream that can be used to read an object from the object database without - * loading all of it into memory. + * A stream that can be used to read an object from or write an object into the + * object database without loading all of it into memory. */ struct odb_stream { odb_stream_close_fn close; @@ -48,30 +48,6 @@ int odb_stream_close(struct odb_stream *stream); */ ssize_t odb_stream_read(struct odb_stream *stream, void *buf, size_t len); -/* - * A stream that provides an object to be written to the object database without - * loading all of it into memory. - */ -struct odb_write_stream { - ssize_t (*read)(struct odb_write_stream *, unsigned char *, size_t); - void *data; - size_t size; - enum object_type type; -}; - -/* - * Read data from the stream into the buffer. Returns 0 when finished and the - * number of bytes read on success. Returns a negative error code in case - * reading from the stream fails. - */ -ssize_t odb_write_stream_read(struct odb_write_stream *stream, void *buf, - size_t len); - -/* - * Releases memory allocated for underlying stream data. - */ -void odb_write_stream_release(struct odb_write_stream *stream); - /* * Look up the object by its ID and write the full contents to the file * descriptor. The object must be a blob, or the function will fail. When @@ -92,7 +68,6 @@ int odb_stream_blob_to_fd(struct object_database *odb, /* * Sets up an ODB write stream that reads from an fd. */ -void odb_write_stream_from_fd(struct odb_write_stream *stream, int fd, - size_t size, enum object_type type); +struct odb_stream *odb_write_stream_from_fd(int fd, size_t size, enum object_type type); #endif /* STREAMING_H */ diff --git a/odb/transaction.c b/odb/transaction.c index 6aaf133812..69d71b9e97 100644 --- a/odb/transaction.c +++ b/odb/transaction.c @@ -39,7 +39,7 @@ int odb_transaction_commit(struct odb_transaction *transaction) } int odb_transaction_write_object_stream(struct odb_transaction *transaction, - struct odb_write_stream *stream, + struct odb_stream *stream, struct object_id *oid) { return transaction->write_object_stream(transaction, stream, oid); diff --git a/odb/transaction.h b/odb/transaction.h index ffb279314c..b83c77c80a 100644 --- a/odb/transaction.h +++ b/odb/transaction.h @@ -31,7 +31,7 @@ struct odb_transaction { * otherwise. */ int (*write_object_stream)(struct odb_transaction *transaction, - struct odb_write_stream *stream, + struct odb_stream *stream, struct object_id *oid); /* @@ -81,7 +81,7 @@ int odb_transaction_commit(struct odb_transaction *transaction); * error code otherwise. */ int odb_transaction_write_object_stream(struct odb_transaction *transaction, - struct odb_write_stream *stream, + struct odb_stream *stream, struct object_id *oid); /* diff --git a/t/unit-tests/u-odb-inmemory.c b/t/unit-tests/u-odb-inmemory.c index 839a0fd3b7..b8b331b37d 100644 --- a/t/unit-tests/u-odb-inmemory.c +++ b/t/unit-tests/u-odb-inmemory.c @@ -266,13 +266,13 @@ void test_odb_inmemory__freshen_object(void) } struct membuf_write_stream { - struct odb_write_stream base; + struct odb_stream base; const char *buf; size_t offset; }; -static ssize_t membuf_write_stream_read(struct odb_write_stream *stream, - unsigned char *buf, size_t len) +static ssize_t membuf_write_stream_read(struct odb_stream *stream, + char *buf, size_t len) { struct membuf_write_stream *s = container_of(stream, struct membuf_write_stream, base); size_t chunk_size = 2; -- 2.55.0.679.g6767b8d81c.dirty ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 5/7] odb/streaming: consolidate read and write streams 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 0 siblings, 1 reply; 27+ messages in thread From: Justin Tobler @ 2026-08-04 18:23 UTC (permalink / raw) To: Patrick Steinhardt; +Cc: git On 26/08/04 09:25AM, Patrick Steinhardt wrote: > The `struct odb_read_stream` and `struct odb_write_stream` both provide > the same functionality: they allow a caller to read object data from an > arbitrary source. Historically, the only difference was that the read > stream was used to read data out of the object database, whereas the > write stream was used to write data into the object database, but the > interfaces were mostly the same. Ok. > Over the preceding commits we have refactored the write stream to have > almost exactly the same interface as the read stream. With these > refactorings we can now easily merge those two streams into a single > interface that's used for both use cases. Nice. > While most of the changes are mechanical, there are two sites that need > special mention: > > - "builtin/unpack-objects.c" creates a write stream from compressed > object data. > > - "odb/streaming.c" creates a write stream from a file descriptor. > > Adapting these sites to yield the new stream type requires a couple more > changes. Most importantly, instead of embedding the pointer to the data > in `struct odb_write_stream`, we now allocate a structure that wraps the > new `struct odb_stream` base. Other than that though, the changes are > rather straight forward. Ok, creating wrapper stream types for these sounds reasonable. > > Signed-off-by: Patrick Steinhardt <ps@pks.im> > --- > builtin/unpack-objects.c | 31 ++++++++++++++++--------------- > object-file.c | 25 ++++++++++++------------- > odb.c | 2 +- > odb.h | 4 ++-- > odb/source-files.c | 2 +- > odb/source-inmemory.c | 4 ++-- > odb/source-loose.c | 6 +++--- > odb/source-packed.c | 2 +- > odb/source.h | 4 ++-- > odb/streaming.c | 35 ++++++++++++++++------------------- > odb/streaming.h | 31 +++---------------------------- > odb/transaction.c | 2 +- > odb/transaction.h | 4 ++-- > t/unit-tests/u-odb-inmemory.c | 6 +++--- > 14 files changed, 65 insertions(+), 93 deletions(-) > > diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c > index 7439ec53be..05a2d48011 100644 > --- a/builtin/unpack-objects.c > +++ b/builtin/unpack-objects.c > @@ -359,20 +359,21 @@ static void unpack_non_delta_entry(enum object_type type, unsigned long size, > } > > struct input_zstream_data { > + struct odb_stream base; > git_zstream *zstream; > int status; > }; Ok, as mentioned in the commit message, we now embed the stream instead storing a pointer to the extra data. Should we also update the struct name here now that `input_zstream_data` is really itself a stream? > -static ssize_t feed_input_zstream(struct odb_write_stream *in_stream, > - unsigned char *buf, size_t buf_len) > +static ssize_t feed_input_zstream(struct odb_stream *in_stream, > + char *buf, size_t buf_len) > { > - struct input_zstream_data *data = in_stream->data; > + struct input_zstream_data *data = container_of(in_stream, struct input_zstream_data, base); Callback is updated to fetch data from the base stream. > git_zstream *zstream = data->zstream; > > if (data->status != Z_OK) > return 0; > > - zstream->next_out = buf; > + zstream->next_out = (unsigned char *) buf; > zstream->avail_out = buf_len; > > while (data->status == Z_OK && zstream->avail_out == buf_len) { > @@ -388,24 +389,24 @@ static ssize_t feed_input_zstream(struct odb_write_stream *in_stream, > static void stream_blob(unsigned long size, unsigned nr) > { > git_zstream zstream = { 0 }; > - struct input_zstream_data data = { 0 }; > - struct odb_write_stream in_stream = { > - .read = feed_input_zstream, > - .data = &data, > - .size = size, > - .type = OBJ_BLOB, > + struct input_zstream_data in_stream = { > + .base = { > + .read = feed_input_zstream, > + .size = size, > + .type = OBJ_BLOB, > + }, > + .zstream = &zstream, > + .status = Z_OK, > }; > struct obj_info *info = &obj_list[nr]; > > - data.zstream = &zstream; > - data.status = Z_OK; > git_inflate_init(&zstream); > > - if (odb_write_object_stream(the_repository->objects, &in_stream, &info->oid)) > + if (odb_write_object_stream(the_repository->objects, &in_stream.base, &info->oid)) > die(_("failed to write object in stream")); > > - if (data.status != Z_STREAM_END) > - die(_("inflate returned (%d)"), data.status); > + if (in_stream.status != Z_STREAM_END) > + die(_("inflate returned (%d)"), in_stream.status); > git_inflate_end(&zstream); > > if (strict) { Stream set up is now updated to use the wrapper stream. Looks good. [snip] > @@ -299,14 +289,15 @@ int odb_stream_blob_to_fd(struct object_database *odb, > } > > struct read_object_fd_data { > + struct odb_stream base; > int fd; > size_t remaining; > }; `read_object_fd_data` is also now set up as a wrapper stream. Should we also rename it accordingly? > -static ssize_t read_object_fd(struct odb_write_stream *stream, > - unsigned char *buf, size_t len) > +static ssize_t read_object_fd(struct odb_stream *stream, > + char *buf, size_t len) > { > - struct read_object_fd_data *data = stream->data; > + struct read_object_fd_data *data = container_of(stream, struct read_object_fd_data, base); > ssize_t read_result; > size_t count; > > @@ -323,17 +314,23 @@ static ssize_t read_object_fd(struct odb_write_stream *stream, > return read_result; > } > > -void odb_write_stream_from_fd(struct odb_write_stream *stream, int fd, > - size_t size, enum object_type type) > +static int close_object_fd(struct odb_stream *stream UNUSED) > +{ > + /* The file descriptor is owned by the caller for now. */ > + return 0; > +} > + > +struct odb_stream *odb_write_stream_from_fd(int fd, size_t size, enum object_type type) Should we also update the name of this function? The rest of this patch is just renames and call site updates to consolidate the two stream types. Looks good. -Justin ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 5/7] odb/streaming: consolidate read and write streams 2026-08-04 18:23 ` Justin Tobler @ 2026-08-05 6:06 ` Patrick Steinhardt 0 siblings, 0 replies; 27+ messages in thread From: Patrick Steinhardt @ 2026-08-05 6:06 UTC (permalink / raw) To: Justin Tobler; +Cc: git On Tue, Aug 04, 2026 at 01:23:42PM -0500, Justin Tobler wrote: > On 26/08/04 09:25AM, Patrick Steinhardt wrote: > > diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c > > index 7439ec53be..05a2d48011 100644 > > --- a/builtin/unpack-objects.c > > +++ b/builtin/unpack-objects.c > > @@ -359,20 +359,21 @@ static void unpack_non_delta_entry(enum object_type type, unsigned long size, > > } > > > > struct input_zstream_data { > > + struct odb_stream base; > > git_zstream *zstream; > > int status; > > }; > > Ok, as mentioned in the commit message, we now embed the stream instead > storing a pointer to the extra data. Should we also update the struct > name here now that `input_zstream_data` is really itself a stream? I intentionally didn't rename anything in this commit here to keep churn minimal, and deferred the renames into subsequent commits. I should've noted that in the commit message though. This one structure I didn't rename though. I'll add a commit. Patrick ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 6/7] odb/streaming: rename `struct read_object_fd_data` 2026-08-04 7:25 [PATCH 0/7] odb: unify read and write streams Patrick Steinhardt ` (4 preceding siblings ...) 2026-08-04 7:25 ` [PATCH 5/7] odb/streaming: consolidate read and write streams Patrick Steinhardt @ 2026-08-04 7:25 ` 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-05 7:44 ` [PATCH v2 0/8] odb: unify read and write streams Patrick Steinhardt 7 siblings, 1 reply; 27+ messages in thread From: Patrick Steinhardt @ 2026-08-04 7:25 UTC (permalink / raw) To: git With the preceding refactorings the `struct read_object_fd_data` is now somewhat misnamed, as it doesn't only contain the data anymore, but also the stream itself. Rename the structure to `struct fd_stream` to better match the new structure. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- odb/streaming.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/odb/streaming.c b/odb/streaming.c index 1a267e6b90..c436b18d39 100644 --- a/odb/streaming.c +++ b/odb/streaming.c @@ -288,33 +288,33 @@ int odb_stream_blob_to_fd(struct object_database *odb, return result; } -struct read_object_fd_data { +struct fd_stream { struct odb_stream base; int fd; size_t remaining; }; -static ssize_t read_object_fd(struct odb_stream *stream, +static ssize_t fd_stream_read(struct odb_stream *stream, char *buf, size_t len) { - struct read_object_fd_data *data = container_of(stream, struct read_object_fd_data, base); + struct fd_stream *fds = container_of(stream, struct fd_stream, base); ssize_t read_result; size_t count; - if (!data->remaining) + if (!fds->remaining) return 0; - count = data->remaining < len ? data->remaining : len; - read_result = read_in_full(data->fd, buf, count); + count = fds->remaining < len ? fds->remaining : len; + read_result = read_in_full(fds->fd, buf, count); if (read_result < 0 || (size_t)read_result != count) return -1; - data->remaining -= count; + fds->remaining -= count; return read_result; } -static int close_object_fd(struct odb_stream *stream UNUSED) +static int fd_stream_close(struct odb_stream *stream UNUSED) { /* The file descriptor is owned by the caller for now. */ return 0; @@ -322,15 +322,15 @@ static int close_object_fd(struct odb_stream *stream UNUSED) struct odb_stream *odb_write_stream_from_fd(int fd, size_t size, enum object_type type) { - struct read_object_fd_data *data; + struct fd_stream *fds; - CALLOC_ARRAY(data, 1); - data->base.read = read_object_fd; - data->base.close = close_object_fd; - data->base.size = size; - data->base.type = type; - data->fd = fd; - data->remaining = size; + CALLOC_ARRAY(fds, 1); + fds->base.read = fd_stream_read; + fds->base.close = fd_stream_close; + fds->base.size = size; + fds->base.type = type; + fds->fd = fd; + fds->remaining = size; - return &data->base; + return &fds->base; } -- 2.55.0.679.g6767b8d81c.dirty ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 6/7] odb/streaming: rename `struct read_object_fd_data` 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 0 siblings, 0 replies; 27+ messages in thread From: Justin Tobler @ 2026-08-04 18:25 UTC (permalink / raw) To: Patrick Steinhardt; +Cc: git On 26/08/04 09:25AM, Patrick Steinhardt wrote: > With the preceding refactorings the `struct read_object_fd_data` is now > somewhat misnamed, as it doesn't only contain the data anymore, but also > the stream itself. Rename the structure to `struct fd_stream` to better > match the new structure. Ah ok, this addresses one of my comments in the last patch. The renames here all look good. -Justin ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 7/7] odb/streaming: unify function names to create new streams 2026-08-04 7:25 [PATCH 0/7] odb: unify read and write streams Patrick Steinhardt ` (5 preceding siblings ...) 2026-08-04 7:25 ` [PATCH 6/7] odb/streaming: rename `struct read_object_fd_data` Patrick Steinhardt @ 2026-08-04 7:25 ` 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 7 siblings, 1 reply; 27+ messages in thread From: Patrick Steinhardt @ 2026-08-04 7:25 UTC (permalink / raw) To: git Unify the function names to create new streams from different sources so that they follow a common schema. While at it, document the ownership of the file descriptor passed to `odb_stream_from_fd()`. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- archive-tar.c | 2 +- archive-zip.c | 2 +- builtin/index-pack.c | 2 +- builtin/pack-objects.c | 4 ++-- object-file.c | 4 ++-- object.c | 2 +- odb/streaming.c | 10 +++++----- odb/streaming.h | 23 +++++++++++++---------- 8 files changed, 26 insertions(+), 23 deletions(-) diff --git a/archive-tar.c b/archive-tar.c index df2d7fb8e9..a1c66024d4 100644 --- a/archive-tar.c +++ b/archive-tar.c @@ -133,7 +133,7 @@ static int stream_blocked(struct repository *r, const struct object_id *oid) char buf[BLOCKSIZE]; ssize_t readlen; - st = odb_read_stream_open(r->objects, oid, NULL); + st = odb_stream_from_object(r->objects, oid, NULL); if (!st) return error(_("cannot stream blob %s"), oid_to_hex(oid)); for (;;) { diff --git a/archive-zip.c b/archive-zip.c index 8095fd04d5..1a948c2f83 100644 --- a/archive-zip.c +++ b/archive-zip.c @@ -347,7 +347,7 @@ static int write_zip_entry(struct archiver_args *args, method = ZIP_METHOD_DEFLATE; if (!buffer) { - stream = odb_read_stream_open(args->repo->objects, oid, NULL); + stream = odb_stream_from_object(args->repo->objects, oid, NULL); if (!stream) return error(_("cannot stream blob %s"), oid_to_hex(oid)); diff --git a/builtin/index-pack.c b/builtin/index-pack.c index 7226da3e65..d1761282db 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -806,7 +806,7 @@ static int check_collison(struct object_entry *entry) memset(&data, 0, sizeof(data)); data.entry = entry; - data.st = odb_read_stream_open(the_repository->objects, &entry->idx.oid, NULL); + data.st = odb_stream_from_object(the_repository->objects, &entry->idx.oid, NULL); if (!data.st) return -1; if (data.st->size != entry->size || data.st->type != entry->type) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 683160c6bb..10d00ca792 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -528,8 +528,8 @@ static unsigned long write_no_reuse_object(struct hashfile *f, struct object_ent if (oe_type(entry) == OBJ_BLOB && oe_size_greater_than(&to_pack, entry, repo_settings_get_big_file_threshold(the_repository)) && - (st = odb_read_stream_open(the_repository->objects, &entry->idx.oid, - NULL)) != NULL) { + (st = odb_stream_from_object(the_repository->objects, &entry->idx.oid, + NULL)) != NULL) { buf = NULL; type = st->type; size = st->size; diff --git a/object-file.c b/object-file.c index 068c6e5672..11d1af342e 100644 --- a/object-file.c +++ b/object-file.c @@ -952,8 +952,8 @@ int index_fd(struct index_state *istate, struct object_id *oid, ret = index_core(istate, oid, fd, xsize_t(st->st_size), type, path, flags); } else { - struct odb_stream *stream = odb_write_stream_from_fd(fd, xsize_t(st->st_size), - OBJ_BLOB); + struct odb_stream *stream = odb_stream_from_fd(fd, xsize_t(st->st_size), + OBJ_BLOB); if (flags & INDEX_WRITE_OBJECT) { struct object_database *odb = the_repository->objects; diff --git a/object.c b/object.c index 37e6efee47..97f7fc0e87 100644 --- a/object.c +++ b/object.c @@ -345,7 +345,7 @@ struct object *parse_object_with_flags(struct repository *r, if ((!obj || obj->type == OBJ_NONE || obj->type == OBJ_BLOB) && odb_read_object_info(r->objects, oid, NULL) == OBJ_BLOB) { if (!skip_hash) { - struct odb_stream *stream = odb_read_stream_open(r->objects, oid, NULL); + struct odb_stream *stream = odb_stream_from_object(r->objects, oid, NULL); if (!stream) { error(_("unable to open object stream for %s"), oid_to_hex(oid)); diff --git a/odb/streaming.c b/odb/streaming.c index c436b18d39..9c85ec54f5 100644 --- a/odb/streaming.c +++ b/odb/streaming.c @@ -208,9 +208,9 @@ ssize_t odb_stream_read(struct odb_stream *st, void *buf, size_t sz) return st->read(st, buf, sz); } -struct odb_stream *odb_read_stream_open(struct object_database *odb, - const struct object_id *oid, - struct stream_filter *filter) +struct odb_stream *odb_stream_from_object(struct object_database *odb, + const struct object_id *oid, + struct stream_filter *filter) { struct odb_stream *st; const struct object_id *real = lookup_replace_object(odb->repo, oid); @@ -242,7 +242,7 @@ int odb_stream_blob_to_fd(struct object_database *odb, ssize_t kept = 0; int result = -1; - st = odb_read_stream_open(odb, oid, filter); + st = odb_stream_from_object(odb, oid, filter); if (!st) { if (filter) free_stream_filter(filter); @@ -320,7 +320,7 @@ static int fd_stream_close(struct odb_stream *stream UNUSED) return 0; } -struct odb_stream *odb_write_stream_from_fd(int fd, size_t size, enum object_type type) +struct odb_stream *odb_stream_from_fd(int fd, size_t size, enum object_type type) { struct fd_stream *fds; diff --git a/odb/streaming.h b/odb/streaming.h index 60b9803190..b522ff513f 100644 --- a/odb/streaming.h +++ b/odb/streaming.h @@ -26,14 +26,22 @@ struct odb_stream { }; /* - * Create a new object stream for the given object database. An optional filter - * can be used to transform the object's content. + * Create a new object stream for the given object. An optional filter can be + * used to transform the object's content. * * Returns the stream on success, a `NULL` pointer otherwise. */ -struct odb_stream *odb_read_stream_open(struct object_database *odb, - const struct object_id *oid, - struct stream_filter *filter); +struct odb_stream *odb_stream_from_object(struct object_database *odb, + const struct object_id *oid, + struct stream_filter *filter); + +/* + * Create a new object stream for the given file descriptor. This can be used + * to, for example, stream an object into the object database. This function + * does _not_ take ownership of the file descriptor. It's the responsibility of + * the caller to close it after the stream has been closed. + */ +struct odb_stream *odb_stream_from_fd(int fd, size_t size, enum object_type type); /* * Close the given object stream and release all resources associated with it. @@ -65,9 +73,4 @@ int odb_stream_blob_to_fd(struct object_database *odb, struct stream_filter *filter, int can_seek); -/* - * Sets up an ODB write stream that reads from an fd. - */ -struct odb_stream *odb_write_stream_from_fd(int fd, size_t size, enum object_type type); - #endif /* STREAMING_H */ -- 2.55.0.679.g6767b8d81c.dirty ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 7/7] odb/streaming: unify function names to create new streams 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 0 siblings, 0 replies; 27+ messages in thread From: Justin Tobler @ 2026-08-04 18:30 UTC (permalink / raw) To: Patrick Steinhardt; +Cc: git On 26/08/04 09:25AM, Patrick Steinhardt wrote: > Unify the function names to create new streams from different sources so > that they follow a common schema. While at it, document the ownership of > the file descriptor passed to `odb_stream_from_fd()`. > > Signed-off-by: Patrick Steinhardt <ps@pks.im> > --- [snip] > +/* > + * Create a new object stream for the given file descriptor. This can be used > + * to, for example, stream an object into the object database. This function > + * does _not_ take ownership of the file descriptor. It's the responsibility of > + * the caller to close it after the stream has been closed. > + */ > +struct odb_stream *odb_stream_from_fd(int fd, size_t size, enum object_type type); Ah ok, here we rename `odb_write_stream_from_fd()` to `odb_stream_from_fd()`. This also addresses one of my comments from a previous patch. The renames in this patch all look sensible to me. Thanks. -Justin ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2 0/8] odb: unify read and write streams 2026-08-04 7:25 [PATCH 0/7] odb: unify read and write streams Patrick Steinhardt ` (6 preceding siblings ...) 2026-08-04 7:25 ` [PATCH 7/7] odb/streaming: unify function names to create new streams Patrick Steinhardt @ 2026-08-05 7:44 ` Patrick Steinhardt 2026-08-05 7:44 ` [PATCH v2 1/8] odb/streaming: track write stream size in the structure Patrick Steinhardt ` (7 more replies) 7 siblings, 8 replies; 27+ messages in thread From: Patrick Steinhardt @ 2026-08-05 7:44 UTC (permalink / raw) To: git; +Cc: Justin Tobler, Junio C Hamano Hi, we have two different kind of object database streams in our code base: `odb_write_stream` and `odb_read_stream`. While those are used for different use cases, the provided functionality is ultimately the exact same. This patch series thus refactors these streams so that we have a single `odb_stream`, only. This allows us to reuse the streams for different kinds of purposes and makes them more generally useful overall. For example, it's trivially possible now to create an object stream for any given object and then write that stream into a different source. The series is built on top of 5b2471720c (The 10th batch, 2026-08-03). Changes in v2: - Use the correct object type when hashing in-memory objects. - Remove a stale comment. - Adapt a commit message to mention that renames will follow in subsequent commits. - Add another commit to rename `struct input_zstream_data`. - Link to v1: https://patch.msgid.link/20260804-pks-odb-stream-unification-v1-0-86d70e82345e@pks.im Thanks! Patrick --- Patrick Steinhardt (8): odb/streaming: track write stream size in the structure odb/streaming: drop `is_finished` field odb/streaming: support streaming arbitrary object types odb/streaming: rename `struct odb_read_stream` odb/streaming: consolidate read and write streams odb/streaming: rename `struct read_object_fd_data` odb/streaming: rename `struct input_zstream_data` odb/streaming: unify function names to create new streams archive-tar.c | 8 ++-- archive-zip.c | 12 ++--- builtin/index-pack.c | 8 ++-- builtin/pack-objects.c | 18 ++++---- builtin/unpack-objects.c | 44 ++++++++++-------- object-file.c | 76 +++++++++++++++--------------- object-file.h | 2 +- object.c | 6 +-- odb.c | 4 +- odb.h | 4 +- odb/source-files.c | 7 ++- odb/source-inmemory.c | 35 ++++++++------ odb/source-loose.c | 33 ++++++++------ odb/source-packed.c | 5 +- odb/source.h | 13 +++--- odb/streaming.c | 104 ++++++++++++++++++++---------------------- odb/streaming.h | 69 ++++++++++------------------ odb/transaction.c | 6 +-- odb/transaction.h | 8 ++-- pack-check.c | 4 +- packfile.c | 8 ++-- packfile.h | 4 +- t/unit-tests/u-odb-inmemory.c | 37 ++++++++------- 23 files changed, 251 insertions(+), 264 deletions(-) Range-diff versus v1: 1: 0085df877f = 1: 1966710c12 odb/streaming: track write stream size in the structure 2: 5fbbfd9010 = 2: 87c7981a6c odb/streaming: drop `is_finished` field 3: 52e5b87761 ! 3: 9aede44fba odb/streaming: support streaming arbitrary object types @@ object-file.c: int index_fd(struct index_state *istate, struct object_id *oid, ## odb/source-inmemory.c ## @@ odb/source-inmemory.c: 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); + goto out; + } + +- hash_object_file(source->odb->repo->hash_algo, data, total_read, OBJ_BLOB, oid); ++ hash_object_file(source->odb->repo->hash_algo, data, total_read, ++ stream->type, oid); ret = odb_source_inmemory_write_object(source, data, stream->size, - OBJ_BLOB, oid, NULL, NULL, 0); @@ odb/streaming.h: int odb_stream_blob_to_fd(struct object_database *odb, #endif /* STREAMING_H */ + ## odb/transaction.h ## +@@ odb/transaction.h: struct odb_transaction { + + /* + * This callback is expected to write the given object stream into +- * the ODB transaction. Note that for now, only blobs support streaming. ++ * the ODB transaction. + * + * The resulting object ID shall be written into the out pointer. The + * callback is expected to return 0 on success, a negative error code + ## t/unit-tests/u-odb-inmemory.c ## @@ t/unit-tests/u-odb-inmemory.c: void test_odb_inmemory__write_object_stream(void) struct odb_source_inmemory *source = odb_source_inmemory_new(odb); 4: f178d441f0 = 4: ca84a2b645 odb/streaming: rename `struct odb_read_stream` 5: 0d72d27078 ! 5: 838394bffc odb/streaming: consolidate read and write streams @@ Commit message new `struct odb_stream` base. Other than that though, the changes are rather straight forward. + Some of the structures and functions are now somewhat misnamed. These + will be fixed in subsequent commits. + Signed-off-by: Patrick Steinhardt <ps@pks.im> ## builtin/unpack-objects.c ## 6: ced59bdc85 = 6: 850b7e081d odb/streaming: rename `struct read_object_fd_data` -: ---------- > 7: c3fe9f8b0c odb/streaming: rename `struct input_zstream_data` 7: f76f4350ef = 8: 4df81651ba odb/streaming: unify function names to create new streams --- base-commit: 5b2471720c93ee30e5764a19f3d3b3ae9ec9712a change-id: 20260724-pks-odb-stream-unification-334dc2a75888 ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2 1/8] odb/streaming: track write stream size in the structure 2026-08-05 7:44 ` [PATCH v2 0/8] odb: unify read and write streams Patrick Steinhardt @ 2026-08-05 7:44 ` Patrick Steinhardt 2026-08-05 7:44 ` [PATCH v2 2/8] odb/streaming: drop `is_finished` field Patrick Steinhardt ` (6 subsequent siblings) 7 siblings, 0 replies; 27+ messages in thread From: Patrick Steinhardt @ 2026-08-05 7:44 UTC (permalink / raw) To: git; +Cc: Justin Tobler, Junio C Hamano When passing around a `struct odb_write_stream` we typically also have to pass the number of bytes that the stream will yield. This is required because the object header itself contains that size, and consequently we cannot write the header without that information. Move this information into the stream itself so that it becomes self- describing. In addition to that, this also brings the `struct odb_write_stream` a bit closer to the `struct odb_read_stream` so that we can eventually merge both stream types. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- builtin/unpack-objects.c | 3 ++- object-file.c | 25 +++++++++++-------------- odb.c | 4 ++-- odb.h | 2 +- odb/source-files.c | 3 +-- odb/source-inmemory.c | 11 +++++------ odb/source-loose.c | 7 +++---- odb/source-packed.c | 1 - odb/source.h | 5 ++--- odb/streaming.c | 1 + odb/streaming.h | 1 + odb/transaction.c | 4 ++-- odb/transaction.h | 4 ++-- t/unit-tests/u-odb-inmemory.c | 11 +++++------ 14 files changed, 38 insertions(+), 44 deletions(-) diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c index 4263edfbec..f3e0b504f4 100644 --- a/builtin/unpack-objects.c +++ b/builtin/unpack-objects.c @@ -392,13 +392,14 @@ static void stream_blob(unsigned long size, unsigned nr) struct odb_write_stream in_stream = { .read = feed_input_zstream, .data = &data, + .size = size, }; struct obj_info *info = &obj_list[nr]; data.zstream = &zstream; git_inflate_init(&zstream); - if (odb_write_object_stream(the_repository->objects, &in_stream, size, &info->oid)) + if (odb_write_object_stream(the_repository->objects, &in_stream, &info->oid)) die(_("failed to write object in stream")); if (data.status != Z_STREAM_END) diff --git a/object-file.c b/object-file.c index ec35c318bc..b196abb596 100644 --- a/object-file.c +++ b/object-file.c @@ -704,7 +704,7 @@ static void prepare_packfile_transaction(struct odb_transaction_files *transacti static int hash_blob_stream(struct odb_write_stream *stream, const struct git_hash_algo *hash_algo, - struct object_id *result_oid, size_t size) + struct object_id *result_oid) { unsigned char buf[16384]; struct git_hash_ctx ctx; @@ -712,7 +712,7 @@ static int hash_blob_stream(struct odb_write_stream *stream, size_t bytes_hashed = 0; header_len = format_object_header((char *)buf, sizeof(buf), - OBJ_BLOB, size); + OBJ_BLOB, stream->size); git_hash_init(&ctx, hash_algo); git_hash_update(&ctx, buf, header_len); @@ -727,7 +727,7 @@ static int hash_blob_stream(struct odb_write_stream *stream, bytes_hashed += read_result; } - if (bytes_hashed != size) + if (bytes_hashed != stream->size) return -1; git_hash_final_oid(result_oid, &ctx); @@ -740,7 +740,7 @@ static int hash_blob_stream(struct odb_write_stream *stream, * packfile in state while updating the hash in ctx. */ static void stream_blob_to_pack(struct transaction_packfile *state, - struct git_hash_ctx *ctx, size_t size, + struct git_hash_ctx *ctx, struct odb_write_stream *stream) { git_zstream s; @@ -753,7 +753,7 @@ static void stream_blob_to_pack(struct transaction_packfile *state, git_deflate_init(&s, cfg->pack_compression_level); - hdrlen = encode_in_pack_object_header(obuf, sizeof(obuf), OBJ_BLOB, size); + hdrlen = encode_in_pack_object_header(obuf, sizeof(obuf), OBJ_BLOB, stream->size); s.next_out = obuf + hdrlen; s.avail_out = sizeof(obuf) - hdrlen; @@ -793,9 +793,9 @@ static void stream_blob_to_pack(struct transaction_packfile *state, } } - if (bytes_read != size) + if (bytes_read != stream->size) die("read %" PRIuMAX " bytes of blob data, but expected %" PRIuMAX " bytes", - (uintmax_t)bytes_read, (uintmax_t)size); + (uintmax_t)bytes_read, (uintmax_t)stream->size); git_deflate_end(&s); } @@ -870,7 +870,6 @@ static void flush_packfile_transaction(struct odb_transaction_files *transaction */ static int odb_transaction_files_write_object_stream(struct odb_transaction *base, struct odb_write_stream *stream, - size_t size, struct object_id *result_oid) { struct odb_transaction_files *transaction = container_of(base, @@ -884,7 +883,7 @@ static int odb_transaction_files_write_object_stream(struct odb_transaction *bas struct pack_idx_entry *idx; header_len = format_object_header((char *)obuf, sizeof(obuf), - OBJ_BLOB, size); + OBJ_BLOB, stream->size); git_hash_init(&ctx, transaction->base.source->odb->repo->hash_algo); git_hash_update(&ctx, obuf, header_len); @@ -899,7 +898,7 @@ static int odb_transaction_files_write_object_stream(struct odb_transaction *bas * to zlib compression and is sufficient for this check. */ if (state->nr_written && pack_size_limit_cfg && - pack_size_limit_cfg < state->offset + size) + pack_size_limit_cfg < state->offset + stream->size) flush_packfile_transaction(transaction); CALLOC_ARRAY(idx, 1); @@ -909,7 +908,7 @@ static int odb_transaction_files_write_object_stream(struct odb_transaction *bas hashfile_checkpoint(state->f, &checkpoint); idx->offset = state->offset; crc32_begin(state->f); - stream_blob_to_pack(state, &ctx, size, stream); + stream_blob_to_pack(state, &ctx, stream); git_hash_final_oid(result_oid, &ctx); idx->crc32 = crc32_end(state->f); @@ -962,14 +961,12 @@ int index_fd(struct index_state *istate, struct object_id *oid, odb_transaction_begin_or_die(odb, &transaction, 0); ret = odb_transaction_write_object_stream(transaction, &stream, - xsize_t(st->st_size), oid); if (!inflight) odb_transaction_commit(transaction); } else { ret = hash_blob_stream(&stream, - the_repository->hash_algo, oid, - xsize_t(st->st_size)); + the_repository->hash_algo, oid); } odb_write_stream_release(&stream); diff --git a/odb.c b/odb.c index dabd481f57..585b2b2965 100644 --- a/odb.c +++ b/odb.c @@ -1028,10 +1028,10 @@ int odb_write_object_ext(struct object_database *odb, } int odb_write_object_stream(struct object_database *odb, - struct odb_write_stream *stream, size_t len, + struct odb_write_stream *stream, struct object_id *oid) { - return odb_source_write_object_stream(odb->sources, stream, len, oid); + return odb_source_write_object_stream(odb->sources, stream, oid); } struct object_database *odb_new(struct repository *repo, diff --git a/odb.h b/odb.h index cbc2f9ced4..019d3af3e8 100644 --- a/odb.h +++ b/odb.h @@ -629,7 +629,7 @@ static inline int odb_write_object(struct object_database *odb, struct odb_write_stream; int odb_write_object_stream(struct object_database *odb, - struct odb_write_stream *stream, size_t len, + struct odb_write_stream *stream, struct object_id *oid); void parse_alternates(const char *string, diff --git a/odb/source-files.c b/odb/source-files.c index 5e086d266f..f51960bd71 100644 --- a/odb/source-files.c +++ b/odb/source-files.c @@ -175,11 +175,10 @@ static int odb_source_files_write_object(struct odb_source *source, static int odb_source_files_write_object_stream(struct odb_source *source, struct odb_write_stream *stream, - size_t len, struct object_id *oid) { struct odb_source_files *files = odb_source_files_downcast(source); - return odb_source_write_object_stream(&files->loose->base, stream, len, oid); + return odb_source_write_object_stream(&files->loose->base, stream, oid); } static int odb_source_files_begin_transaction(struct odb_source *source, diff --git a/odb/source-inmemory.c b/odb/source-inmemory.c index 3e71611b8e..398131e194 100644 --- a/odb/source-inmemory.c +++ b/odb/source-inmemory.c @@ -257,7 +257,6 @@ static int odb_source_inmemory_write_object(struct odb_source *source, static int odb_source_inmemory_write_object_stream(struct odb_source *source, struct odb_write_stream *stream, - size_t len, struct object_id *oid) { char buf[16384]; @@ -265,12 +264,12 @@ static int odb_source_inmemory_write_object_stream(struct odb_source *source, char *data; int ret; - CALLOC_ARRAY(data, len); + CALLOC_ARRAY(data, stream->size); while (!stream->is_finished) { ssize_t bytes_read; bytes_read = odb_write_stream_read(stream, buf, sizeof(buf)); - if (total_read + bytes_read > len) { + if (total_read + bytes_read > stream->size) { ret = error("object stream yielded more bytes than expected"); goto out; } @@ -279,15 +278,15 @@ static int odb_source_inmemory_write_object_stream(struct odb_source *source, total_read += bytes_read; } - if (total_read != len) { + if (total_read != stream->size) { ret = error("object stream yielded less bytes than expected"); goto out; } hash_object_file(source->odb->repo->hash_algo, data, total_read, OBJ_BLOB, oid); - ret = odb_source_inmemory_write_object(source, data, len, OBJ_BLOB, oid, - NULL, NULL, 0); + ret = odb_source_inmemory_write_object(source, data, stream->size, + OBJ_BLOB, oid, NULL, NULL, 0); if (ret < 0) goto out; diff --git a/odb/source-loose.c b/odb/source-loose.c index ef0e919277..77a2adb52a 100644 --- a/odb/source-loose.c +++ b/odb/source-loose.c @@ -846,7 +846,6 @@ static int odb_source_loose_write_object(struct odb_source *source, static int odb_source_loose_write_object_stream(struct odb_source *source, struct odb_write_stream *in_stream, - size_t len, struct object_id *oid) { struct odb_source_loose *loose = odb_source_loose_downcast(source); @@ -868,7 +867,7 @@ static int odb_source_loose_write_object_stream(struct odb_source *source, /* Since oid is not determined, save tmp file to odb path. */ strbuf_addf(&filename, "%s/", loose->base.path); - hdrlen = format_object_header(hdr, sizeof(hdr), OBJ_BLOB, len); + hdrlen = format_object_header(hdr, sizeof(hdr), OBJ_BLOB, in_stream->size); /* * Common steps for write_loose_object and stream_loose_object to @@ -916,9 +915,9 @@ static int odb_source_loose_write_object_stream(struct odb_source *source, */ } while (ret == Z_OK || ret == Z_BUF_ERROR); - if (stream.total_in != len + hdrlen) + if (stream.total_in != in_stream->size + hdrlen) die(_("write stream object %"PRIuMAX" != %"PRIuMAX), (uintmax_t)stream.total_in, - (uintmax_t)len + hdrlen); + (uintmax_t)in_stream->size + hdrlen); /* * Common steps for write_loose_object and stream_loose_object to diff --git a/odb/source-packed.c b/odb/source-packed.c index 0890704e76..e6ff74833b 100644 --- a/odb/source-packed.c +++ b/odb/source-packed.c @@ -610,7 +610,6 @@ static int odb_source_packed_write_object(struct odb_source *source UNUSED, static int odb_source_packed_write_object_stream(struct odb_source *source UNUSED, struct odb_write_stream *stream UNUSED, - size_t len UNUSED, struct object_id *oid UNUSED) { return error("packed backend cannot write object streams"); diff --git a/odb/source.h b/odb/source.h index fc04dd5cda..0080148ba7 100644 --- a/odb/source.h +++ b/odb/source.h @@ -221,7 +221,7 @@ struct odb_source { * otherwise. */ int (*write_object_stream)(struct odb_source *source, - struct odb_write_stream *stream, size_t len, + struct odb_write_stream *stream, struct object_id *oid); /* @@ -437,10 +437,9 @@ static inline int odb_source_write_object(struct odb_source *source, */ static inline int odb_source_write_object_stream(struct odb_source *source, struct odb_write_stream *stream, - size_t len, struct object_id *oid) { - return source->write_object_stream(source, stream, len, oid); + return source->write_object_stream(source, stream, oid); } /* diff --git a/odb/streaming.c b/odb/streaming.c index 20531e864c..38c2f6687c 100644 --- a/odb/streaming.c +++ b/odb/streaming.c @@ -336,5 +336,6 @@ void odb_write_stream_from_fd(struct odb_write_stream *stream, int fd, stream->data = data; stream->read = read_object_fd; + stream->size = size; stream->is_finished = 0; } diff --git a/odb/streaming.h b/odb/streaming.h index c023671780..4d7d31b5aa 100644 --- a/odb/streaming.h +++ b/odb/streaming.h @@ -55,6 +55,7 @@ ssize_t odb_read_stream_read(struct odb_read_stream *stream, void *buf, size_t l struct odb_write_stream { ssize_t (*read)(struct odb_write_stream *, unsigned char *, size_t); void *data; + size_t size; int is_finished; }; diff --git a/odb/transaction.c b/odb/transaction.c index dab7da6a9a..6aaf133812 100644 --- a/odb/transaction.c +++ b/odb/transaction.c @@ -40,9 +40,9 @@ int odb_transaction_commit(struct odb_transaction *transaction) int odb_transaction_write_object_stream(struct odb_transaction *transaction, struct odb_write_stream *stream, - size_t len, struct object_id *oid) + struct object_id *oid) { - return transaction->write_object_stream(transaction, stream, len, oid); + return transaction->write_object_stream(transaction, stream, oid); } int odb_transaction_env(struct odb_transaction *transaction, struct strvec *env) diff --git a/odb/transaction.h b/odb/transaction.h index 4cb2eafcbf..ffb279314c 100644 --- a/odb/transaction.h +++ b/odb/transaction.h @@ -31,7 +31,7 @@ struct odb_transaction { * otherwise. */ int (*write_object_stream)(struct odb_transaction *transaction, - struct odb_write_stream *stream, size_t len, + struct odb_write_stream *stream, struct object_id *oid); /* @@ -82,7 +82,7 @@ int odb_transaction_commit(struct odb_transaction *transaction); */ int odb_transaction_write_object_stream(struct odb_transaction *transaction, struct odb_write_stream *stream, - size_t len, struct object_id *oid); + struct object_id *oid); /* * Populates the provided strvec with the environment variables that a child diff --git a/t/unit-tests/u-odb-inmemory.c b/t/unit-tests/u-odb-inmemory.c index ddf2db5c81..5ccc52dccc 100644 --- a/t/unit-tests/u-odb-inmemory.c +++ b/t/unit-tests/u-odb-inmemory.c @@ -269,7 +269,6 @@ struct membuf_write_stream { struct odb_write_stream base; const char *buf; size_t offset; - size_t size; }; static ssize_t membuf_write_stream_read(struct odb_write_stream *stream, @@ -280,13 +279,13 @@ static ssize_t membuf_write_stream_read(struct odb_write_stream *stream, if (chunk_size > len) chunk_size = len; - if (chunk_size > s->size - s->offset) - chunk_size = s->size - s->offset; + if (chunk_size > s->base.size - s->offset) + chunk_size = s->base.size - s->offset; memcpy(buf, s->buf + s->offset, chunk_size); s->offset += chunk_size; - if (s->offset == s->size) + if (s->offset == s->base.size) s->base.is_finished = 1; return chunk_size; @@ -298,13 +297,13 @@ void test_odb_inmemory__write_object_stream(void) const char data[] = "foobar"; struct membuf_write_stream stream = { .base.read = membuf_write_stream_read, + .base.size = strlen(data), .buf = data, - .size = strlen(data), }; struct object_id written_oid; cl_must_pass(odb_source_write_object_stream(&source->base, &stream.base, - strlen(data), &written_oid)); + &written_oid)); cl_assert_equal_s(oid_to_hex(&written_oid), FOOBAR_OID); cl_assert_object_info(source, &written_oid, OBJ_BLOB, "foobar"); -- 2.55.0.679.g6767b8d81c.dirty ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 2/8] odb/streaming: drop `is_finished` field 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 ` Patrick Steinhardt 2026-08-05 7:44 ` [PATCH v2 3/8] odb/streaming: support streaming arbitrary object types Patrick Steinhardt ` (5 subsequent siblings) 7 siblings, 0 replies; 27+ messages in thread From: Patrick Steinhardt @ 2026-08-05 7:44 UTC (permalink / raw) To: git; +Cc: Justin Tobler, Junio C Hamano The `is_finished` field is used to track whether a write stream is done writing all of its data. Tracking this field as part of the stream itself shouldn't be required though: callers will already know when the stream is done when the stream's read function returns zero bytes, same as when reading from a file descriptor. There is one exception where it gets a bit more complicated: when consuming data in "builtin/unpack-objects.c" it may happen that we don't yield any new bytes after reading from the pipe. This is addressed by looping until we have produced at least a single byte of output. Drop the field from `struct odb_write_stream`. Again, same as in the preceding commit, this brings the structure a bit closer to its sibling `struct odb_read_stream`. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- builtin/unpack-objects.c | 15 ++++++++------- object-file.c | 13 ++++++++----- odb/source-inmemory.c | 9 ++++++++- odb/source-loose.c | 12 ++++++++---- odb/streaming.c | 5 +---- odb/streaming.h | 1 - t/unit-tests/u-odb-inmemory.c | 5 +++-- 7 files changed, 36 insertions(+), 24 deletions(-) diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c index f3e0b504f4..b7c486ea94 100644 --- a/builtin/unpack-objects.c +++ b/builtin/unpack-objects.c @@ -368,20 +368,20 @@ static ssize_t feed_input_zstream(struct odb_write_stream *in_stream, { struct input_zstream_data *data = in_stream->data; git_zstream *zstream = data->zstream; - void *in = fill(1); - if (in_stream->is_finished) + if (data->status != Z_OK) return 0; zstream->next_out = buf; zstream->avail_out = buf_len; - zstream->next_in = in; - zstream->avail_in = len; - data->status = git_inflate(zstream, 0); + while (data->status == Z_OK && zstream->avail_out == buf_len) { + zstream->next_in = fill(1); + zstream->avail_in = len; + data->status = git_inflate(zstream, 0); + use(len - zstream->avail_in); + } - in_stream->is_finished = data->status != Z_OK; - use(len - zstream->avail_in); return buf_len - zstream->avail_out; } @@ -397,6 +397,7 @@ static void stream_blob(unsigned long size, unsigned nr) struct obj_info *info = &obj_list[nr]; data.zstream = &zstream; + data.status = Z_OK; git_inflate_init(&zstream); if (odb_write_object_stream(the_repository->objects, &in_stream, &info->oid)) diff --git a/object-file.c b/object-file.c index b196abb596..317c09dff8 100644 --- a/object-file.c +++ b/object-file.c @@ -716,12 +716,13 @@ static int hash_blob_stream(struct odb_write_stream *stream, git_hash_init(&ctx, hash_algo); git_hash_update(&ctx, buf, header_len); - while (!stream->is_finished) { + while (1) { ssize_t read_result = odb_write_stream_read(stream, buf, sizeof(buf)); - if (read_result < 0) return -1; + if (!read_result) + break; git_hash_update(&ctx, buf, read_result); bytes_hashed += read_result; @@ -749,6 +750,7 @@ static void stream_blob_to_pack(struct transaction_packfile *state, unsigned hdrlen; int status = Z_OK; struct repo_config_values *cfg = repo_config_values(the_repository); + bool is_finished = false; size_t bytes_read = 0; git_deflate_init(&s, cfg->pack_compression_level); @@ -758,12 +760,13 @@ static void stream_blob_to_pack(struct transaction_packfile *state, s.avail_out = sizeof(obuf) - hdrlen; while (status != Z_STREAM_END) { - if (!stream->is_finished && !s.avail_in) { + if (!is_finished && !s.avail_in) { ssize_t rsize = odb_write_stream_read(stream, ibuf, sizeof(ibuf)); - if (rsize < 0) die("failed to read blob data"); + if (!rsize) + is_finished = true; git_hash_update(ctx, ibuf, rsize); @@ -772,7 +775,7 @@ static void stream_blob_to_pack(struct transaction_packfile *state, bytes_read += rsize; } - status = git_deflate(&s, stream->is_finished ? Z_FINISH : 0); + status = git_deflate(&s, is_finished ? Z_FINISH : 0); if (!s.avail_out || status == Z_STREAM_END) { size_t written = s.next_out - obuf; diff --git a/odb/source-inmemory.c b/odb/source-inmemory.c index 398131e194..01bb81c63c 100644 --- a/odb/source-inmemory.c +++ b/odb/source-inmemory.c @@ -265,10 +265,17 @@ static int odb_source_inmemory_write_object_stream(struct odb_source *source, int ret; CALLOC_ARRAY(data, stream->size); - while (!stream->is_finished) { + while (1) { ssize_t bytes_read; bytes_read = odb_write_stream_read(stream, buf, sizeof(buf)); + if (bytes_read < 0) { + ret = error("failed to read object stream"); + goto out; + } + if (!bytes_read) + break; + if (total_read + bytes_read > stream->size) { ret = error("object stream yielded more bytes than expected"); goto out; diff --git a/odb/source-loose.c b/odb/source-loose.c index 77a2adb52a..361b4e2a2a 100644 --- a/odb/source-loose.c +++ b/odb/source-loose.c @@ -859,6 +859,7 @@ static int odb_source_loose_write_object_stream(struct odb_source *source, struct strbuf filename = STRBUF_INIT; unsigned char buf[8192]; int dirlen; + bool is_finished = false; char hdr[MAX_HEADER_LEN]; int hdrlen; @@ -889,7 +890,7 @@ static int odb_source_loose_write_object_stream(struct odb_source *source, do { unsigned char *in0 = stream.next_in; - if (!stream.avail_in && !in_stream->is_finished) { + if (!stream.avail_in && !is_finished) { ssize_t read_len = odb_write_stream_read(in_stream, buf, sizeof(buf)); if (read_len < 0) { @@ -898,12 +899,15 @@ static int odb_source_loose_write_object_stream(struct odb_source *source, goto cleanup; } + /* All data has been read. */ + if (!read_len) { + is_finished = true; + flush = 1; + } + stream.avail_in = read_len; stream.next_in = buf; in0 = buf; - /* All data has been read. */ - if (in_stream->is_finished) - flush = 1; } ret = write_loose_object_common(loose, &c, &compat_c, &stream, flush, in0, fd, compressed, sizeof(compressed)); diff --git a/odb/streaming.c b/odb/streaming.c index 38c2f6687c..912e75e682 100644 --- a/odb/streaming.c +++ b/odb/streaming.c @@ -310,7 +310,7 @@ static ssize_t read_object_fd(struct odb_write_stream *stream, ssize_t read_result; size_t count; - if (stream->is_finished) + if (!data->remaining) return 0; count = data->remaining < len ? data->remaining : len; @@ -319,8 +319,6 @@ static ssize_t read_object_fd(struct odb_write_stream *stream, return -1; data->remaining -= count; - if (!data->remaining) - stream->is_finished = 1; return read_result; } @@ -337,5 +335,4 @@ void odb_write_stream_from_fd(struct odb_write_stream *stream, int fd, stream->data = data; stream->read = read_object_fd; stream->size = size; - stream->is_finished = 0; } diff --git a/odb/streaming.h b/odb/streaming.h index 4d7d31b5aa..5e8e6e532e 100644 --- a/odb/streaming.h +++ b/odb/streaming.h @@ -56,7 +56,6 @@ struct odb_write_stream { ssize_t (*read)(struct odb_write_stream *, unsigned char *, size_t); void *data; size_t size; - int is_finished; }; /* diff --git a/t/unit-tests/u-odb-inmemory.c b/t/unit-tests/u-odb-inmemory.c index 5ccc52dccc..4437140ed0 100644 --- a/t/unit-tests/u-odb-inmemory.c +++ b/t/unit-tests/u-odb-inmemory.c @@ -277,6 +277,9 @@ static ssize_t membuf_write_stream_read(struct odb_write_stream *stream, struct membuf_write_stream *s = container_of(stream, struct membuf_write_stream, base); size_t chunk_size = 2; + if (s->offset == s->base.size) + return 0; + if (chunk_size > len) chunk_size = len; if (chunk_size > s->base.size - s->offset) @@ -285,8 +288,6 @@ static ssize_t membuf_write_stream_read(struct odb_write_stream *stream, memcpy(buf, s->buf + s->offset, chunk_size); s->offset += chunk_size; - if (s->offset == s->base.size) - s->base.is_finished = 1; return chunk_size; } -- 2.55.0.679.g6767b8d81c.dirty ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 3/8] odb/streaming: support streaming arbitrary object types 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 ` Patrick Steinhardt 2026-08-05 7:44 ` [PATCH v2 4/8] odb/streaming: rename `struct odb_read_stream` Patrick Steinhardt ` (4 subsequent siblings) 7 siblings, 0 replies; 27+ messages in thread From: Patrick Steinhardt @ 2026-08-05 7:44 UTC (permalink / raw) To: git; +Cc: Justin Tobler, Junio C Hamano 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. As we only ever write large files, the infrastructure doesn't support specifying any other object type than "blob". This limitation is quite artificial though: there is no reason why we shouldn't support writing arbitrary large objects with a stream. While it's very unlikely that we encounter a huge object other than a blob, users are known to be creative and sometimes like to inflict pain on themselves by creating commits or trees that are huge. Extend the infrastructure to support streaming arbitrary object types. For now we don't use this functionality anywhere, but it brings us a bit closer to unify `struct odb_read_stream` and `struct odb_write_stream`. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- builtin/unpack-objects.c | 1 + object-file.c | 31 +++++++++++++++---------------- odb/source-inmemory.c | 5 +++-- odb/source-loose.c | 2 +- odb/streaming.c | 3 ++- odb/streaming.h | 3 ++- odb/transaction.h | 2 +- t/unit-tests/u-odb-inmemory.c | 7 +++++-- 8 files changed, 30 insertions(+), 24 deletions(-) diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c index b7c486ea94..7439ec53be 100644 --- a/builtin/unpack-objects.c +++ b/builtin/unpack-objects.c @@ -393,6 +393,7 @@ static void stream_blob(unsigned long size, unsigned nr) .read = feed_input_zstream, .data = &data, .size = size, + .type = OBJ_BLOB, }; struct obj_info *info = &obj_list[nr]; diff --git a/object-file.c b/object-file.c index 317c09dff8..699a6a008c 100644 --- a/object-file.c +++ b/object-file.c @@ -702,9 +702,9 @@ static void prepare_packfile_transaction(struct odb_transaction_files *transacti die_errno("unable to write pack header"); } -static int hash_blob_stream(struct odb_write_stream *stream, - const struct git_hash_algo *hash_algo, - struct object_id *result_oid) +static int hash_stream(struct odb_write_stream *stream, + const struct git_hash_algo *hash_algo, + struct object_id *result_oid) { unsigned char buf[16384]; struct git_hash_ctx ctx; @@ -712,7 +712,7 @@ static int hash_blob_stream(struct odb_write_stream *stream, size_t bytes_hashed = 0; header_len = format_object_header((char *)buf, sizeof(buf), - OBJ_BLOB, stream->size); + stream->type, stream->size); git_hash_init(&ctx, hash_algo); git_hash_update(&ctx, buf, header_len); @@ -740,9 +740,9 @@ static int hash_blob_stream(struct odb_write_stream *stream, * Read the contents from the stream provided, streaming it to the * packfile in state while updating the hash in ctx. */ -static void stream_blob_to_pack(struct transaction_packfile *state, - struct git_hash_ctx *ctx, - struct odb_write_stream *stream) +static void stream_to_pack(struct transaction_packfile *state, + struct git_hash_ctx *ctx, + struct odb_write_stream *stream) { git_zstream s; unsigned char ibuf[16384]; @@ -755,7 +755,7 @@ static void stream_blob_to_pack(struct transaction_packfile *state, git_deflate_init(&s, cfg->pack_compression_level); - hdrlen = encode_in_pack_object_header(obuf, sizeof(obuf), OBJ_BLOB, stream->size); + hdrlen = encode_in_pack_object_header(obuf, sizeof(obuf), stream->type, stream->size); s.next_out = obuf + hdrlen; s.avail_out = sizeof(obuf) - hdrlen; @@ -764,7 +764,7 @@ static void stream_blob_to_pack(struct transaction_packfile *state, ssize_t rsize = odb_write_stream_read(stream, ibuf, sizeof(ibuf)); if (rsize < 0) - die("failed to read blob data"); + die("failed to read object data"); if (!rsize) is_finished = true; @@ -797,7 +797,7 @@ static void stream_blob_to_pack(struct transaction_packfile *state, } if (bytes_read != stream->size) - die("read %" PRIuMAX " bytes of blob data, but expected %" PRIuMAX " bytes", + die("read %" PRIuMAX " bytes of object data, but expected %" PRIuMAX " bytes", (uintmax_t)bytes_read, (uintmax_t)stream->size); git_deflate_end(&s); @@ -868,7 +868,7 @@ static void flush_packfile_transaction(struct odb_transaction_files *transaction * result, which we need to know beforehand when writing a git object. * Since the primary motivation for trying to stream from the working * tree file and to avoid mmaping it in core is to deal with large - * binary blobs, they generally do not want to get any conversion, and + * objects, they generally do not want to get any conversion, and * callers should avoid this code path when filters are requested. */ static int odb_transaction_files_write_object_stream(struct odb_transaction *base, @@ -886,7 +886,7 @@ static int odb_transaction_files_write_object_stream(struct odb_transaction *bas struct pack_idx_entry *idx; header_len = format_object_header((char *)obuf, sizeof(obuf), - OBJ_BLOB, stream->size); + stream->type, stream->size); git_hash_init(&ctx, transaction->base.source->odb->repo->hash_algo); git_hash_update(&ctx, obuf, header_len); @@ -911,7 +911,7 @@ static int odb_transaction_files_write_object_stream(struct odb_transaction *bas hashfile_checkpoint(state->f, &checkpoint); idx->offset = state->offset; crc32_begin(state->f); - stream_blob_to_pack(state, &ctx, stream); + stream_to_pack(state, &ctx, stream); git_hash_final_oid(result_oid, &ctx); idx->crc32 = crc32_end(state->f); @@ -953,7 +953,7 @@ int index_fd(struct index_state *istate, struct object_id *oid, type, path, flags); } else { struct odb_write_stream stream; - odb_write_stream_from_fd(&stream, fd, xsize_t(st->st_size)); + odb_write_stream_from_fd(&stream, fd, xsize_t(st->st_size), OBJ_BLOB); if (flags & INDEX_WRITE_OBJECT) { struct object_database *odb = the_repository->objects; @@ -968,8 +968,7 @@ int index_fd(struct index_state *istate, struct object_id *oid, if (!inflight) odb_transaction_commit(transaction); } else { - ret = hash_blob_stream(&stream, - the_repository->hash_algo, oid); + ret = hash_stream(&stream, the_repository->hash_algo, oid); } odb_write_stream_release(&stream); diff --git a/odb/source-inmemory.c b/odb/source-inmemory.c index 01bb81c63c..139618024a 100644 --- a/odb/source-inmemory.c +++ b/odb/source-inmemory.c @@ -290,10 +290,11 @@ static int odb_source_inmemory_write_object_stream(struct odb_source *source, goto out; } - hash_object_file(source->odb->repo->hash_algo, data, total_read, OBJ_BLOB, oid); + hash_object_file(source->odb->repo->hash_algo, data, total_read, + stream->type, oid); ret = odb_source_inmemory_write_object(source, data, stream->size, - OBJ_BLOB, oid, NULL, NULL, 0); + stream->type, oid, NULL, NULL, 0); if (ret < 0) goto out; diff --git a/odb/source-loose.c b/odb/source-loose.c index 361b4e2a2a..5681a38f03 100644 --- a/odb/source-loose.c +++ b/odb/source-loose.c @@ -868,7 +868,7 @@ static int odb_source_loose_write_object_stream(struct odb_source *source, /* Since oid is not determined, save tmp file to odb path. */ strbuf_addf(&filename, "%s/", loose->base.path); - hdrlen = format_object_header(hdr, sizeof(hdr), OBJ_BLOB, in_stream->size); + hdrlen = format_object_header(hdr, sizeof(hdr), in_stream->type, in_stream->size); /* * Common steps for write_loose_object and stream_loose_object to diff --git a/odb/streaming.c b/odb/streaming.c index 912e75e682..0918cad426 100644 --- a/odb/streaming.c +++ b/odb/streaming.c @@ -324,7 +324,7 @@ static ssize_t read_object_fd(struct odb_write_stream *stream, } void odb_write_stream_from_fd(struct odb_write_stream *stream, int fd, - size_t size) + size_t size, enum object_type type) { struct read_object_fd_data *data; @@ -335,4 +335,5 @@ void odb_write_stream_from_fd(struct odb_write_stream *stream, int fd, stream->data = data; stream->read = read_object_fd; stream->size = size; + stream->type = type; } diff --git a/odb/streaming.h b/odb/streaming.h index 5e8e6e532e..3c8ed55129 100644 --- a/odb/streaming.h +++ b/odb/streaming.h @@ -56,6 +56,7 @@ struct odb_write_stream { ssize_t (*read)(struct odb_write_stream *, unsigned char *, size_t); void *data; size_t size; + enum object_type type; }; /* @@ -92,6 +93,6 @@ int odb_stream_blob_to_fd(struct object_database *odb, * Sets up an ODB write stream that reads from an fd. */ void odb_write_stream_from_fd(struct odb_write_stream *stream, int fd, - size_t size); + size_t size, enum object_type type); #endif /* STREAMING_H */ diff --git a/odb/transaction.h b/odb/transaction.h index ffb279314c..1eb74664c6 100644 --- a/odb/transaction.h +++ b/odb/transaction.h @@ -24,7 +24,7 @@ struct odb_transaction { /* * This callback is expected to write the given object stream into - * the ODB transaction. Note that for now, only blobs support streaming. + * the ODB transaction. * * The resulting object ID shall be written into the out pointer. The * callback is expected to return 0 on success, a negative error code diff --git a/t/unit-tests/u-odb-inmemory.c b/t/unit-tests/u-odb-inmemory.c index 4437140ed0..1ab07af6d6 100644 --- a/t/unit-tests/u-odb-inmemory.c +++ b/t/unit-tests/u-odb-inmemory.c @@ -297,8 +297,11 @@ void test_odb_inmemory__write_object_stream(void) struct odb_source_inmemory *source = odb_source_inmemory_new(odb); const char data[] = "foobar"; struct membuf_write_stream stream = { - .base.read = membuf_write_stream_read, - .base.size = strlen(data), + .base = { + .read = membuf_write_stream_read, + .size = strlen(data), + .type = OBJ_BLOB, + }, .buf = data, }; struct object_id written_oid; -- 2.55.0.679.g6767b8d81c.dirty ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 4/8] odb/streaming: rename `struct odb_read_stream` 2026-08-05 7:44 ` [PATCH v2 0/8] odb: unify read and write streams Patrick Steinhardt ` (2 preceding siblings ...) 2026-08-05 7:44 ` [PATCH v2 3/8] odb/streaming: support streaming arbitrary object types Patrick Steinhardt @ 2026-08-05 7:44 ` Patrick Steinhardt 2026-08-05 7:44 ` [PATCH v2 5/8] odb/streaming: consolidate read and write streams Patrick Steinhardt ` (3 subsequent siblings) 7 siblings, 0 replies; 27+ messages in thread From: Patrick Steinhardt @ 2026-08-05 7:44 UTC (permalink / raw) To: git; +Cc: Justin Tobler, Junio C Hamano Rename `struct odb_read_stream` to just `struct odb_stream`. This prepares for unification of the two different types of streams, as these provide the same functionality with the preceding refactorings. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- archive-tar.c | 6 +++--- archive-zip.c | 10 ++++----- builtin/index-pack.c | 6 +++--- builtin/pack-objects.c | 14 ++++++------- object-file.c | 4 ++-- object-file.h | 2 +- object.c | 6 +++--- odb/source-files.c | 2 +- odb/source-inmemory.c | 8 ++++---- odb/source-loose.c | 8 ++++---- odb/source-packed.c | 2 +- odb/source.h | 6 +++--- odb/streaming.c | 48 +++++++++++++++++++++---------------------- odb/streaming.h | 24 +++++++++++----------- pack-check.c | 4 ++-- packfile.c | 8 ++++---- packfile.h | 4 ++-- t/unit-tests/u-odb-inmemory.c | 12 +++++------ 18 files changed, 87 insertions(+), 87 deletions(-) diff --git a/archive-tar.c b/archive-tar.c index 0fc70d13a8..df2d7fb8e9 100644 --- a/archive-tar.c +++ b/archive-tar.c @@ -129,7 +129,7 @@ static void write_trailer(void) */ static int stream_blocked(struct repository *r, const struct object_id *oid) { - struct odb_read_stream *st; + struct odb_stream *st; char buf[BLOCKSIZE]; ssize_t readlen; @@ -137,12 +137,12 @@ static int stream_blocked(struct repository *r, const struct object_id *oid) if (!st) return error(_("cannot stream blob %s"), oid_to_hex(oid)); for (;;) { - readlen = odb_read_stream_read(st, buf, sizeof(buf)); + readlen = odb_stream_read(st, buf, sizeof(buf)); if (readlen <= 0) break; do_write_blocked(buf, readlen); } - odb_read_stream_close(st); + odb_stream_close(st); if (!readlen) finish_record(); return readlen; diff --git a/archive-zip.c b/archive-zip.c index 97ea8d60d6..8095fd04d5 100644 --- a/archive-zip.c +++ b/archive-zip.c @@ -309,7 +309,7 @@ static int write_zip_entry(struct archiver_args *args, enum zip_method method; unsigned char *out; void *deflated = NULL; - struct odb_read_stream *stream = NULL; + struct odb_stream *stream = NULL; unsigned long flags = 0; int is_binary = -1; const char *path_without_prefix = path + args->baselen; @@ -428,7 +428,7 @@ static int write_zip_entry(struct archiver_args *args, ssize_t readlen; for (;;) { - readlen = odb_read_stream_read(stream, buf, sizeof(buf)); + readlen = odb_stream_read(stream, buf, sizeof(buf)); if (readlen <= 0) break; crc = crc32(crc, buf, readlen); @@ -438,7 +438,7 @@ static int write_zip_entry(struct archiver_args *args, buf, readlen); write_or_die(1, buf, readlen); } - odb_read_stream_close(stream); + odb_stream_close(stream); if (readlen) return readlen; @@ -461,7 +461,7 @@ static int write_zip_entry(struct archiver_args *args, zstream.avail_out = sizeof(compressed); for (;;) { - readlen = odb_read_stream_read(stream, buf, sizeof(buf)); + readlen = odb_stream_read(stream, buf, sizeof(buf)); if (readlen <= 0) break; crc = crc32(crc, buf, readlen); @@ -485,7 +485,7 @@ static int write_zip_entry(struct archiver_args *args, } } - odb_read_stream_close(stream); + odb_stream_close(stream); if (readlen) return readlen; diff --git a/builtin/index-pack.c b/builtin/index-pack.c index bc86925ad0..7226da3e65 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -763,7 +763,7 @@ static void find_ref_delta_children(const struct object_id *oid, struct compare_data { struct object_entry *entry; - struct odb_read_stream *st; + struct odb_stream *st; unsigned char *buf; unsigned long buf_size; }; @@ -780,7 +780,7 @@ static int compare_objects(const unsigned char *buf, unsigned long size, } while (size) { - ssize_t len = odb_read_stream_read(data->st, data->buf, size); + ssize_t len = odb_stream_read(data->st, data->buf, size); if (len == 0) die(_("SHA1 COLLISION FOUND WITH %s !"), oid_to_hex(&data->entry->idx.oid)); @@ -813,7 +813,7 @@ static int check_collison(struct object_entry *entry) die(_("SHA1 COLLISION FOUND WITH %s !"), oid_to_hex(&entry->idx.oid)); unpack_data(entry, compare_objects, &data); - odb_read_stream_close(data.st); + odb_stream_close(data.st); free(data.buf); return 0; } diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 1ec5b6f206..683160c6bb 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -411,7 +411,7 @@ static unsigned long do_compress(void **pptr, unsigned long size) return stream.total_out; } -static unsigned long write_large_blob_data(struct odb_read_stream *st, struct hashfile *f, +static unsigned long write_large_blob_data(struct odb_stream *st, struct hashfile *f, const struct object_id *oid) { git_zstream stream; @@ -425,7 +425,7 @@ static unsigned long write_large_blob_data(struct odb_read_stream *st, struct ha for (;;) { ssize_t readlen; int zret = Z_OK; - readlen = odb_read_stream_read(st, ibuf, sizeof(ibuf)); + readlen = odb_stream_read(st, ibuf, sizeof(ibuf)); if (readlen == -1) die(_("unable to read %s"), oid_to_hex(oid)); @@ -521,7 +521,7 @@ static unsigned long write_no_reuse_object(struct hashfile *f, struct object_ent unsigned hdrlen; enum object_type type; void *buf; - struct odb_read_stream *st = NULL; + struct odb_stream *st = NULL; const unsigned hashsz = the_hash_algo->rawsz; if (!usable_delta) { @@ -589,7 +589,7 @@ static unsigned long write_no_reuse_object(struct hashfile *f, struct object_ent dheader[--pos] = 128 | (--ofs & 127); if (limit && hdrlen + sizeof(dheader) - pos + datalen + hashsz >= limit) { if (st) - odb_read_stream_close(st); + odb_stream_close(st); free(buf); return 0; } @@ -603,7 +603,7 @@ static unsigned long write_no_reuse_object(struct hashfile *f, struct object_ent */ if (limit && hdrlen + hashsz + datalen + hashsz >= limit) { if (st) - odb_read_stream_close(st); + odb_stream_close(st); free(buf); return 0; } @@ -613,7 +613,7 @@ static unsigned long write_no_reuse_object(struct hashfile *f, struct object_ent } else { if (limit && hdrlen + datalen + hashsz >= limit) { if (st) - odb_read_stream_close(st); + odb_stream_close(st); free(buf); return 0; } @@ -621,7 +621,7 @@ static unsigned long write_no_reuse_object(struct hashfile *f, struct object_ent } if (st) { datalen = write_large_blob_data(st, f, &entry->idx.oid); - odb_read_stream_close(st); + odb_stream_close(st); } else { hashwrite(f, buf, datalen); free(buf); diff --git a/object-file.c b/object-file.c index 699a6a008c..5f6d584c35 100644 --- a/object-file.c +++ b/object-file.c @@ -122,7 +122,7 @@ int check_object_signature(struct repository *r, const struct object_id *oid, } int stream_object_signature(struct repository *r, - struct odb_read_stream *st, + struct odb_stream *st, const struct object_id *oid) { struct object_id real_oid; @@ -138,7 +138,7 @@ int stream_object_signature(struct repository *r, git_hash_update(&c, hdr, hdrlen); for (;;) { char buf[1024 * 16]; - ssize_t readlen = odb_read_stream_read(st, buf, sizeof(buf)); + ssize_t readlen = odb_stream_read(st, buf, sizeof(buf)); if (readlen < 0) return -1; if (!readlen) diff --git a/object-file.h b/object-file.h index 805f2cfa28..f44758c4f8 100644 --- a/object-file.h +++ b/object-file.h @@ -101,7 +101,7 @@ int check_object_signature(struct repository *r, const struct object_id *oid, * the streaming interface and rehash it to do the same. */ int stream_object_signature(struct repository *r, - struct odb_read_stream *stream, + struct odb_stream *stream, const struct object_id *oid); enum finalize_object_file_flags { diff --git a/object.c b/object.c index 23b84aa7e2..37e6efee47 100644 --- a/object.c +++ b/object.c @@ -345,7 +345,7 @@ struct object *parse_object_with_flags(struct repository *r, if ((!obj || obj->type == OBJ_NONE || obj->type == OBJ_BLOB) && odb_read_object_info(r->objects, oid, NULL) == OBJ_BLOB) { if (!skip_hash) { - struct odb_read_stream *stream = odb_read_stream_open(r->objects, oid, NULL); + struct odb_stream *stream = odb_read_stream_open(r->objects, oid, NULL); if (!stream) { error(_("unable to open object stream for %s"), oid_to_hex(oid)); @@ -354,11 +354,11 @@ struct object *parse_object_with_flags(struct repository *r, if (stream_object_signature(r, stream, repl) < 0) { error(_("hash mismatch %s"), oid_to_hex(oid)); - odb_read_stream_close(stream); + odb_stream_close(stream); return NULL; } - odb_read_stream_close(stream); + odb_stream_close(stream); } parse_blob_buffer(lookup_blob(r, oid)); return lookup_object(r, oid); diff --git a/odb/source-files.c b/odb/source-files.c index f51960bd71..f7b8c76549 100644 --- a/odb/source-files.c +++ b/odb/source-files.c @@ -63,7 +63,7 @@ static int odb_source_files_read_object_info(struct odb_source *source, return -1; } -static int odb_source_files_read_object_stream(struct odb_read_stream **out, +static int odb_source_files_read_object_stream(struct odb_stream **out, struct odb_source *source, const struct object_id *oid) { diff --git a/odb/source-inmemory.c b/odb/source-inmemory.c index 139618024a..485d587036 100644 --- a/odb/source-inmemory.c +++ b/odb/source-inmemory.c @@ -73,12 +73,12 @@ static int odb_source_inmemory_read_object_info(struct odb_source *source, } struct odb_read_stream_inmemory { - struct odb_read_stream base; + struct odb_stream base; const unsigned char *buf; size_t offset; }; -static ssize_t odb_read_stream_inmemory_read(struct odb_read_stream *stream, +static ssize_t odb_read_stream_inmemory_read(struct odb_stream *stream, char *buf, size_t buf_len) { struct odb_read_stream_inmemory *inmemory = @@ -94,12 +94,12 @@ static ssize_t odb_read_stream_inmemory_read(struct odb_read_stream *stream, return bytes; } -static int odb_read_stream_inmemory_close(struct odb_read_stream *stream UNUSED) +static int odb_read_stream_inmemory_close(struct odb_stream *stream UNUSED) { return 0; } -static int odb_source_inmemory_read_object_stream(struct odb_read_stream **out, +static int odb_source_inmemory_read_object_stream(struct odb_stream **out, struct odb_source *source, const struct object_id *oid) { diff --git a/odb/source-loose.c b/odb/source-loose.c index 5681a38f03..038defd905 100644 --- a/odb/source-loose.c +++ b/odb/source-loose.c @@ -278,7 +278,7 @@ static void *odb_source_loose_map_object(struct odb_source_loose *loose, } struct odb_loose_read_stream { - struct odb_read_stream base; + struct odb_stream base; git_zstream z; enum { ODB_LOOSE_READ_STREAM_INUSE, @@ -292,7 +292,7 @@ struct odb_loose_read_stream { int hdr_used; }; -static ssize_t read_istream_loose(struct odb_read_stream *_st, char *buf, size_t sz) +static ssize_t read_istream_loose(struct odb_stream *_st, char *buf, size_t sz) { struct odb_loose_read_stream *st = container_of(_st, struct odb_loose_read_stream, base); @@ -339,7 +339,7 @@ static ssize_t read_istream_loose(struct odb_read_stream *_st, char *buf, size_t return total_read; } -static int close_istream_loose(struct odb_read_stream *_st) +static int close_istream_loose(struct odb_stream *_st) { struct odb_loose_read_stream *st = container_of(_st, struct odb_loose_read_stream, base); @@ -350,7 +350,7 @@ static int close_istream_loose(struct odb_read_stream *_st) return 0; } -static int odb_source_loose_read_object_stream(struct odb_read_stream **out, +static int odb_source_loose_read_object_stream(struct odb_stream **out, struct odb_source *source, const struct object_id *oid) { diff --git a/odb/source-packed.c b/odb/source-packed.c index e6ff74833b..b3186ca593 100644 --- a/odb/source-packed.c +++ b/odb/source-packed.c @@ -70,7 +70,7 @@ static int odb_source_packed_read_object_info(struct odb_source *source, return 0; } -static int odb_source_packed_read_object_stream(struct odb_read_stream **out, +static int odb_source_packed_read_object_stream(struct odb_stream **out, struct odb_source *source, const struct object_id *oid) { diff --git a/odb/source.h b/odb/source.h index 0080148ba7..89b0c39682 100644 --- a/odb/source.h +++ b/odb/source.h @@ -26,7 +26,7 @@ enum odb_source_type { }; struct object_id; -struct odb_read_stream; +struct odb_stream; struct strvec; /* @@ -125,7 +125,7 @@ struct odb_source { * The callback is expected to return a negative error code in case * creating the object stream has failed, 0 otherwise. */ - int (*read_object_stream)(struct odb_read_stream **out, + int (*read_object_stream)(struct odb_stream **out, struct odb_source *source, const struct object_id *oid); @@ -339,7 +339,7 @@ static inline int odb_source_read_object_info(struct odb_source *source, * Create a new read stream for the given object ID. Returns 0 on success, a * negative error code otherwise. */ -static inline int odb_source_read_object_stream(struct odb_read_stream **out, +static inline int odb_source_read_object_stream(struct odb_stream **out, struct odb_source *source, const struct object_id *oid) { diff --git a/odb/streaming.c b/odb/streaming.c index 0918cad426..98e2152e36 100644 --- a/odb/streaming.c +++ b/odb/streaming.c @@ -20,8 +20,8 @@ *****************************************************************/ struct odb_filtered_read_stream { - struct odb_read_stream base; - struct odb_read_stream *upstream; + struct odb_stream base; + struct odb_stream *upstream; struct stream_filter *filter; char ibuf[FILTER_BUFFER]; char obuf[FILTER_BUFFER]; @@ -30,14 +30,14 @@ struct odb_filtered_read_stream { int input_finished; }; -static int close_istream_filtered(struct odb_read_stream *_fs) +static int close_istream_filtered(struct odb_stream *_fs) { struct odb_filtered_read_stream *fs = (struct odb_filtered_read_stream *)_fs; free_stream_filter(fs->filter); - return odb_read_stream_close(fs->upstream); + return odb_stream_close(fs->upstream); } -static ssize_t read_istream_filtered(struct odb_read_stream *_fs, char *buf, +static ssize_t read_istream_filtered(struct odb_stream *_fs, char *buf, size_t sz) { struct odb_filtered_read_stream *fs = (struct odb_filtered_read_stream *)_fs; @@ -86,7 +86,7 @@ static ssize_t read_istream_filtered(struct odb_read_stream *_fs, char *buf, /* refill the input from the upstream */ if (!fs->input_finished) { - fs->i_end = odb_read_stream_read(fs->upstream, fs->ibuf, FILTER_BUFFER); + fs->i_end = odb_stream_read(fs->upstream, fs->ibuf, FILTER_BUFFER); if (fs->i_end < 0) return -1; if (fs->i_end) @@ -97,8 +97,8 @@ static ssize_t read_istream_filtered(struct odb_read_stream *_fs, char *buf, return filled; } -static struct odb_read_stream *attach_stream_filter(struct odb_read_stream *st, - struct stream_filter *filter) +static struct odb_stream *attach_stream_filter(struct odb_stream *st, + struct stream_filter *filter) { struct odb_filtered_read_stream *fs; @@ -120,19 +120,19 @@ static struct odb_read_stream *attach_stream_filter(struct odb_read_stream *st, *****************************************************************/ struct odb_incore_read_stream { - struct odb_read_stream base; + struct odb_stream base; char *buf; /* from odb_read_object_info_extended() */ unsigned long read_ptr; }; -static int close_istream_incore(struct odb_read_stream *_st) +static int close_istream_incore(struct odb_stream *_st) { struct odb_incore_read_stream *st = (struct odb_incore_read_stream *)_st; free(st->buf); return 0; } -static ssize_t read_istream_incore(struct odb_read_stream *_st, char *buf, size_t sz) +static ssize_t read_istream_incore(struct odb_stream *_st, char *buf, size_t sz) { struct odb_incore_read_stream *st = (struct odb_incore_read_stream *)_st; size_t read_size = sz; @@ -147,7 +147,7 @@ static ssize_t read_istream_incore(struct odb_read_stream *_st, char *buf, size_ return read_size; } -static int open_istream_incore(struct odb_read_stream **out, +static int open_istream_incore(struct odb_stream **out, struct object_database *odb, const struct object_id *oid) { @@ -178,7 +178,7 @@ static int open_istream_incore(struct odb_read_stream **out, * static helpers variables and functions for users of streaming interface *****************************************************************************/ -static int istream_source(struct odb_read_stream **out, +static int istream_source(struct odb_stream **out, struct object_database *odb, const struct object_id *oid) { @@ -196,23 +196,23 @@ static int istream_source(struct odb_read_stream **out, * Users of streaming interface ****************************************************************/ -int odb_read_stream_close(struct odb_read_stream *st) +int odb_stream_close(struct odb_stream *st) { int r = st->close(st); free(st); return r; } -ssize_t odb_read_stream_read(struct odb_read_stream *st, void *buf, size_t sz) +ssize_t odb_stream_read(struct odb_stream *st, void *buf, size_t sz) { return st->read(st, buf, sz); } -struct odb_read_stream *odb_read_stream_open(struct object_database *odb, - const struct object_id *oid, - struct stream_filter *filter) +struct odb_stream *odb_read_stream_open(struct object_database *odb, + const struct object_id *oid, + struct stream_filter *filter) { - struct odb_read_stream *st; + struct odb_stream *st; const struct object_id *real = lookup_replace_object(odb->repo, oid); int ret = istream_source(&st, odb, real); @@ -221,9 +221,9 @@ struct odb_read_stream *odb_read_stream_open(struct object_database *odb, if (filter) { /* Add "&& !is_null_stream_filter(filter)" for performance */ - struct odb_read_stream *nst = attach_stream_filter(st, filter); + struct odb_stream *nst = attach_stream_filter(st, filter); if (!nst) { - odb_read_stream_close(st); + odb_stream_close(st); return NULL; } st = nst; @@ -248,7 +248,7 @@ int odb_stream_blob_to_fd(struct object_database *odb, struct stream_filter *filter, int can_seek) { - struct odb_read_stream *st; + struct odb_stream *st; ssize_t kept = 0; int result = -1; @@ -263,7 +263,7 @@ int odb_stream_blob_to_fd(struct object_database *odb, for (;;) { char buf[1024 * 16]; ssize_t wrote, holeto; - ssize_t readlen = odb_read_stream_read(st, buf, sizeof(buf)); + ssize_t readlen = odb_stream_read(st, buf, sizeof(buf)); if (readlen < 0) goto close_and_exit; @@ -294,7 +294,7 @@ int odb_stream_blob_to_fd(struct object_database *odb, result = 0; close_and_exit: - odb_read_stream_close(st); + odb_stream_close(st); return result; } diff --git a/odb/streaming.h b/odb/streaming.h index 3c8ed55129..037954c231 100644 --- a/odb/streaming.h +++ b/odb/streaming.h @@ -8,19 +8,19 @@ #include "odb.h" struct object_database; -struct odb_read_stream; +struct odb_stream; struct stream_filter; -typedef int (*odb_read_stream_close_fn)(struct odb_read_stream *); -typedef ssize_t (*odb_read_stream_read_fn)(struct odb_read_stream *, char *, size_t); +typedef int (*odb_stream_close_fn)(struct odb_stream *); +typedef ssize_t (*odb_stream_read_fn)(struct odb_stream *, char *, size_t); /* * A stream that can be used to read an object from the object database without * loading all of it into memory. */ -struct odb_read_stream { - odb_read_stream_close_fn close; - odb_read_stream_read_fn read; +struct odb_stream { + odb_stream_close_fn close; + odb_stream_read_fn read; enum object_type type; size_t size; /* inflated size of full object */ }; @@ -31,22 +31,22 @@ struct odb_read_stream { * * Returns the stream on success, a `NULL` pointer otherwise. */ -struct odb_read_stream *odb_read_stream_open(struct object_database *odb, - const struct object_id *oid, - struct stream_filter *filter); +struct odb_stream *odb_read_stream_open(struct object_database *odb, + const struct object_id *oid, + struct stream_filter *filter); /* - * Close the given read stream and release all resources associated with it. + * Close the given object stream and release all resources associated with it. * Returns 0 on success, a negative error code otherwise. */ -int odb_read_stream_close(struct odb_read_stream *stream); +int odb_stream_close(struct odb_stream *stream); /* * Read data from the stream into the buffer. Returns 0 on EOF and the number * of bytes read on success. Returns a negative error code in case reading from * the stream fails. */ -ssize_t odb_read_stream_read(struct odb_read_stream *stream, void *buf, size_t len); +ssize_t odb_stream_read(struct odb_stream *stream, void *buf, size_t len); /* * A stream that provides an object to be written to the object database without diff --git a/pack-check.c b/pack-check.c index c3b8db7c5c..1b5e26847d 100644 --- a/pack-check.c +++ b/pack-check.c @@ -106,7 +106,7 @@ static int verify_packfile(struct repository *r, QSORT(entries, nr_objects, compare_entries); for (i = 0; i < nr_objects; i++) { - struct odb_read_stream *stream = NULL; + struct odb_stream *stream = NULL; void *data; struct object_id oid; enum object_type type; @@ -171,7 +171,7 @@ static int verify_packfile(struct repository *r, display_progress(progress, base_count + i); if (stream) - odb_read_stream_close(stream); + odb_stream_close(stream); free(data); } diff --git a/packfile.c b/packfile.c index 0eee45055f..70254573a3 100644 --- a/packfile.c +++ b/packfile.c @@ -2115,7 +2115,7 @@ int parse_pack_header_option(const char *in, unsigned char *out, unsigned int *l } struct odb_packed_read_stream { - struct odb_read_stream base; + struct odb_stream base; struct packed_git *pack; git_zstream z; enum { @@ -2127,7 +2127,7 @@ struct odb_packed_read_stream { off_t pos; }; -static ssize_t read_istream_pack_non_delta(struct odb_read_stream *_st, char *buf, +static ssize_t read_istream_pack_non_delta(struct odb_stream *_st, char *buf, size_t sz) { struct odb_packed_read_stream *st = (struct odb_packed_read_stream *)_st; @@ -2187,7 +2187,7 @@ static ssize_t read_istream_pack_non_delta(struct odb_read_stream *_st, char *bu return total_read; } -static int close_istream_pack_non_delta(struct odb_read_stream *_st) +static int close_istream_pack_non_delta(struct odb_stream *_st) { struct odb_packed_read_stream *st = (struct odb_packed_read_stream *)_st; if (st->z_state == ODB_PACKED_READ_STREAM_INUSE) @@ -2195,7 +2195,7 @@ static int close_istream_pack_non_delta(struct odb_read_stream *_st) return 0; } -int packfile_read_object_stream(struct odb_read_stream **out, +int packfile_read_object_stream(struct odb_stream **out, const struct object_id *oid, struct packed_git *pack, off_t offset) diff --git a/packfile.h b/packfile.h index e1f77152b5..f913cb3d0c 100644 --- a/packfile.h +++ b/packfile.h @@ -12,7 +12,7 @@ /* in odb.h */ struct object_info; -struct odb_read_stream; +struct odb_stream; struct packed_git { struct pack_window *windows; @@ -306,7 +306,7 @@ off_t get_delta_base(struct packed_git *p, struct pack_window **w_curs, off_t *curpos, enum object_type type, off_t delta_obj_offset); -int packfile_read_object_stream(struct odb_read_stream **out, +int packfile_read_object_stream(struct odb_stream **out, const struct object_id *oid, struct packed_git *pack, off_t offset); diff --git a/t/unit-tests/u-odb-inmemory.c b/t/unit-tests/u-odb-inmemory.c index 1ab07af6d6..839a0fd3b7 100644 --- a/t/unit-tests/u-odb-inmemory.c +++ b/t/unit-tests/u-odb-inmemory.c @@ -100,7 +100,7 @@ void test_odb_inmemory__read_written_object(void) void test_odb_inmemory__read_stream_object(void) { struct odb_source_inmemory *source = odb_source_inmemory_new(odb); - struct odb_read_stream *stream; + struct odb_stream *stream; struct object_id written_oid; const char data[] = "foobar"; char buf[3] = { 0 }; @@ -112,15 +112,15 @@ void test_odb_inmemory__read_stream_object(void) cl_assert_equal_i(stream->type, OBJ_BLOB); cl_assert_equal_u(stream->size, 6); - cl_assert_equal_i(odb_read_stream_read(stream, buf, 2), 2); + cl_assert_equal_i(odb_stream_read(stream, buf, 2), 2); cl_assert_equal_s(buf, "fo"); - cl_assert_equal_i(odb_read_stream_read(stream, buf, 2), 2); + cl_assert_equal_i(odb_stream_read(stream, buf, 2), 2); cl_assert_equal_s(buf, "ob"); - cl_assert_equal_i(odb_read_stream_read(stream, buf, 2), 2); + cl_assert_equal_i(odb_stream_read(stream, buf, 2), 2); cl_assert_equal_s(buf, "ar"); - cl_assert_equal_i(odb_read_stream_read(stream, buf, 2), 0); + cl_assert_equal_i(odb_stream_read(stream, buf, 2), 0); - odb_read_stream_close(stream); + odb_stream_close(stream); odb_source_free(&source->base); } -- 2.55.0.679.g6767b8d81c.dirty ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 5/8] odb/streaming: consolidate read and write streams 2026-08-05 7:44 ` [PATCH v2 0/8] odb: unify read and write streams Patrick Steinhardt ` (3 preceding siblings ...) 2026-08-05 7:44 ` [PATCH v2 4/8] odb/streaming: rename `struct odb_read_stream` Patrick Steinhardt @ 2026-08-05 7:44 ` Patrick Steinhardt 2026-08-05 7:44 ` [PATCH v2 6/8] odb/streaming: rename `struct read_object_fd_data` Patrick Steinhardt ` (2 subsequent siblings) 7 siblings, 0 replies; 27+ messages in thread From: Patrick Steinhardt @ 2026-08-05 7:44 UTC (permalink / raw) To: git; +Cc: Justin Tobler, Junio C Hamano The `struct odb_read_stream` and `struct odb_write_stream` both provide the same functionality: they allow a caller to read object data from an arbitrary source. Historically, the only difference was that the read stream was used to read data out of the object database, whereas the write stream was used to write data into the object database, but the interfaces were mostly the same. Over the preceding commits we have refactored the write stream to have almost exactly the same interface as the read stream. With these refactorings we can now easily merge those two streams into a single interface that's used for both use cases. While most of the changes are mechanical, there are two sites that need special mention: - "builtin/unpack-objects.c" creates a write stream from compressed object data. - "odb/streaming.c" creates a write stream from a file descriptor. Adapting these sites to yield the new stream type requires a couple more changes. Most importantly, instead of embedding the pointer to the data in `struct odb_write_stream`, we now allocate a structure that wraps the new `struct odb_stream` base. Other than that though, the changes are rather straight forward. Some of the structures and functions are now somewhat misnamed. These will be fixed in subsequent commits. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- builtin/unpack-objects.c | 31 ++++++++++++++++--------------- object-file.c | 25 ++++++++++++------------- odb.c | 2 +- odb.h | 4 ++-- odb/source-files.c | 2 +- odb/source-inmemory.c | 4 ++-- odb/source-loose.c | 6 +++--- odb/source-packed.c | 2 +- odb/source.h | 4 ++-- odb/streaming.c | 35 ++++++++++++++++------------------- odb/streaming.h | 31 +++---------------------------- odb/transaction.c | 2 +- odb/transaction.h | 4 ++-- t/unit-tests/u-odb-inmemory.c | 6 +++--- 14 files changed, 65 insertions(+), 93 deletions(-) diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c index 7439ec53be..05a2d48011 100644 --- a/builtin/unpack-objects.c +++ b/builtin/unpack-objects.c @@ -359,20 +359,21 @@ static void unpack_non_delta_entry(enum object_type type, unsigned long size, } struct input_zstream_data { + struct odb_stream base; git_zstream *zstream; int status; }; -static ssize_t feed_input_zstream(struct odb_write_stream *in_stream, - unsigned char *buf, size_t buf_len) +static ssize_t feed_input_zstream(struct odb_stream *in_stream, + char *buf, size_t buf_len) { - struct input_zstream_data *data = in_stream->data; + struct input_zstream_data *data = container_of(in_stream, struct input_zstream_data, base); git_zstream *zstream = data->zstream; if (data->status != Z_OK) return 0; - zstream->next_out = buf; + zstream->next_out = (unsigned char *) buf; zstream->avail_out = buf_len; while (data->status == Z_OK && zstream->avail_out == buf_len) { @@ -388,24 +389,24 @@ static ssize_t feed_input_zstream(struct odb_write_stream *in_stream, static void stream_blob(unsigned long size, unsigned nr) { git_zstream zstream = { 0 }; - struct input_zstream_data data = { 0 }; - struct odb_write_stream in_stream = { - .read = feed_input_zstream, - .data = &data, - .size = size, - .type = OBJ_BLOB, + struct input_zstream_data in_stream = { + .base = { + .read = feed_input_zstream, + .size = size, + .type = OBJ_BLOB, + }, + .zstream = &zstream, + .status = Z_OK, }; struct obj_info *info = &obj_list[nr]; - data.zstream = &zstream; - data.status = Z_OK; git_inflate_init(&zstream); - if (odb_write_object_stream(the_repository->objects, &in_stream, &info->oid)) + if (odb_write_object_stream(the_repository->objects, &in_stream.base, &info->oid)) die(_("failed to write object in stream")); - if (data.status != Z_STREAM_END) - die(_("inflate returned (%d)"), data.status); + if (in_stream.status != Z_STREAM_END) + die(_("inflate returned (%d)"), in_stream.status); git_inflate_end(&zstream); if (strict) { diff --git a/object-file.c b/object-file.c index 5f6d584c35..068c6e5672 100644 --- a/object-file.c +++ b/object-file.c @@ -702,7 +702,7 @@ static void prepare_packfile_transaction(struct odb_transaction_files *transacti die_errno("unable to write pack header"); } -static int hash_stream(struct odb_write_stream *stream, +static int hash_stream(struct odb_stream *stream, const struct git_hash_algo *hash_algo, struct object_id *result_oid) { @@ -717,8 +717,8 @@ static int hash_stream(struct odb_write_stream *stream, git_hash_update(&ctx, buf, header_len); while (1) { - ssize_t read_result = odb_write_stream_read(stream, buf, - sizeof(buf)); + ssize_t read_result = odb_stream_read(stream, buf, + sizeof(buf)); if (read_result < 0) return -1; if (!read_result) @@ -742,7 +742,7 @@ static int hash_stream(struct odb_write_stream *stream, */ static void stream_to_pack(struct transaction_packfile *state, struct git_hash_ctx *ctx, - struct odb_write_stream *stream) + struct odb_stream *stream) { git_zstream s; unsigned char ibuf[16384]; @@ -761,8 +761,8 @@ static void stream_to_pack(struct transaction_packfile *state, while (status != Z_STREAM_END) { if (!is_finished && !s.avail_in) { - ssize_t rsize = odb_write_stream_read(stream, ibuf, - sizeof(ibuf)); + ssize_t rsize = odb_stream_read(stream, ibuf, + sizeof(ibuf)); if (rsize < 0) die("failed to read object data"); if (!rsize) @@ -872,7 +872,7 @@ static void flush_packfile_transaction(struct odb_transaction_files *transaction * callers should avoid this code path when filters are requested. */ static int odb_transaction_files_write_object_stream(struct odb_transaction *base, - struct odb_write_stream *stream, + struct odb_stream *stream, struct object_id *result_oid) { struct odb_transaction_files *transaction = container_of(base, @@ -952,8 +952,8 @@ int index_fd(struct index_state *istate, struct object_id *oid, ret = index_core(istate, oid, fd, xsize_t(st->st_size), type, path, flags); } else { - struct odb_write_stream stream; - odb_write_stream_from_fd(&stream, fd, xsize_t(st->st_size), OBJ_BLOB); + struct odb_stream *stream = odb_write_stream_from_fd(fd, xsize_t(st->st_size), + OBJ_BLOB); if (flags & INDEX_WRITE_OBJECT) { struct object_database *odb = the_repository->objects; @@ -963,15 +963,14 @@ int index_fd(struct index_state *istate, struct object_id *oid, if (!inflight) odb_transaction_begin_or_die(odb, &transaction, 0); ret = odb_transaction_write_object_stream(transaction, - &stream, - oid); + stream, oid); if (!inflight) odb_transaction_commit(transaction); } else { - ret = hash_stream(&stream, the_repository->hash_algo, oid); + ret = hash_stream(stream, the_repository->hash_algo, oid); } - odb_write_stream_release(&stream); + odb_stream_close(stream); } close(fd); diff --git a/odb.c b/odb.c index 585b2b2965..eec4cc5302 100644 --- a/odb.c +++ b/odb.c @@ -1028,7 +1028,7 @@ int odb_write_object_ext(struct object_database *odb, } int odb_write_object_stream(struct object_database *odb, - struct odb_write_stream *stream, + struct odb_stream *stream, struct object_id *oid) { return odb_source_write_object_stream(odb->sources, stream, oid); diff --git a/odb.h b/odb.h index 019d3af3e8..fbe75c5a81 100644 --- a/odb.h +++ b/odb.h @@ -626,10 +626,10 @@ static inline int odb_write_object(struct object_database *odb, return odb_write_object_ext(odb, buf, len, type, oid, NULL, 0); } -struct odb_write_stream; +struct odb_stream; int odb_write_object_stream(struct object_database *odb, - struct odb_write_stream *stream, + struct odb_stream *stream, struct object_id *oid); void parse_alternates(const char *string, diff --git a/odb/source-files.c b/odb/source-files.c index f7b8c76549..6defe5ac4f 100644 --- a/odb/source-files.c +++ b/odb/source-files.c @@ -174,7 +174,7 @@ static int odb_source_files_write_object(struct odb_source *source, } static int odb_source_files_write_object_stream(struct odb_source *source, - struct odb_write_stream *stream, + struct odb_stream *stream, struct object_id *oid) { struct odb_source_files *files = odb_source_files_downcast(source); diff --git a/odb/source-inmemory.c b/odb/source-inmemory.c index 485d587036..795672adf2 100644 --- a/odb/source-inmemory.c +++ b/odb/source-inmemory.c @@ -256,7 +256,7 @@ static int odb_source_inmemory_write_object(struct odb_source *source, } static int odb_source_inmemory_write_object_stream(struct odb_source *source, - struct odb_write_stream *stream, + struct odb_stream *stream, struct object_id *oid) { char buf[16384]; @@ -268,7 +268,7 @@ static int odb_source_inmemory_write_object_stream(struct odb_source *source, while (1) { ssize_t bytes_read; - bytes_read = odb_write_stream_read(stream, buf, sizeof(buf)); + bytes_read = odb_stream_read(stream, buf, sizeof(buf)); if (bytes_read < 0) { ret = error("failed to read object stream"); goto out; diff --git a/odb/source-loose.c b/odb/source-loose.c index 038defd905..ff1bede7fe 100644 --- a/odb/source-loose.c +++ b/odb/source-loose.c @@ -845,7 +845,7 @@ static int odb_source_loose_write_object(struct odb_source *source, } static int odb_source_loose_write_object_stream(struct odb_source *source, - struct odb_write_stream *in_stream, + struct odb_stream *in_stream, struct object_id *oid) { struct odb_source_loose *loose = odb_source_loose_downcast(source); @@ -891,8 +891,8 @@ static int odb_source_loose_write_object_stream(struct odb_source *source, unsigned char *in0 = stream.next_in; if (!stream.avail_in && !is_finished) { - ssize_t read_len = odb_write_stream_read(in_stream, buf, - sizeof(buf)); + ssize_t read_len = odb_stream_read(in_stream, buf, + sizeof(buf)); if (read_len < 0) { close(fd); err = -1; diff --git a/odb/source-packed.c b/odb/source-packed.c index b3186ca593..630d955585 100644 --- a/odb/source-packed.c +++ b/odb/source-packed.c @@ -609,7 +609,7 @@ static int odb_source_packed_write_object(struct odb_source *source UNUSED, } static int odb_source_packed_write_object_stream(struct odb_source *source UNUSED, - struct odb_write_stream *stream UNUSED, + struct odb_stream *stream UNUSED, struct object_id *oid UNUSED) { return error("packed backend cannot write object streams"); diff --git a/odb/source.h b/odb/source.h index 89b0c39682..0b99c698b5 100644 --- a/odb/source.h +++ b/odb/source.h @@ -221,7 +221,7 @@ struct odb_source { * otherwise. */ int (*write_object_stream)(struct odb_source *source, - struct odb_write_stream *stream, + struct odb_stream *stream, struct object_id *oid); /* @@ -436,7 +436,7 @@ static inline int odb_source_write_object(struct odb_source *source, * out pointer for the object ID. */ static inline int odb_source_write_object_stream(struct odb_source *source, - struct odb_write_stream *stream, + struct odb_stream *stream, struct object_id *oid) { return source->write_object_stream(source, stream, oid); diff --git a/odb/streaming.c b/odb/streaming.c index 98e2152e36..1a267e6b90 100644 --- a/odb/streaming.c +++ b/odb/streaming.c @@ -232,16 +232,6 @@ struct odb_stream *odb_read_stream_open(struct object_database *odb, return st; } -ssize_t odb_write_stream_read(struct odb_write_stream *st, void *buf, size_t sz) -{ - return st->read(st, buf, sz); -} - -void odb_write_stream_release(struct odb_write_stream *st) -{ - free(st->data); -} - int odb_stream_blob_to_fd(struct object_database *odb, int fd, const struct object_id *oid, @@ -299,14 +289,15 @@ int odb_stream_blob_to_fd(struct object_database *odb, } struct read_object_fd_data { + struct odb_stream base; int fd; size_t remaining; }; -static ssize_t read_object_fd(struct odb_write_stream *stream, - unsigned char *buf, size_t len) +static ssize_t read_object_fd(struct odb_stream *stream, + char *buf, size_t len) { - struct read_object_fd_data *data = stream->data; + struct read_object_fd_data *data = container_of(stream, struct read_object_fd_data, base); ssize_t read_result; size_t count; @@ -323,17 +314,23 @@ static ssize_t read_object_fd(struct odb_write_stream *stream, return read_result; } -void odb_write_stream_from_fd(struct odb_write_stream *stream, int fd, - size_t size, enum object_type type) +static int close_object_fd(struct odb_stream *stream UNUSED) +{ + /* The file descriptor is owned by the caller for now. */ + return 0; +} + +struct odb_stream *odb_write_stream_from_fd(int fd, size_t size, enum object_type type) { struct read_object_fd_data *data; CALLOC_ARRAY(data, 1); + data->base.read = read_object_fd; + data->base.close = close_object_fd; + data->base.size = size; + data->base.type = type; data->fd = fd; data->remaining = size; - stream->data = data; - stream->read = read_object_fd; - stream->size = size; - stream->type = type; + return &data->base; } diff --git a/odb/streaming.h b/odb/streaming.h index 037954c231..60b9803190 100644 --- a/odb/streaming.h +++ b/odb/streaming.h @@ -15,8 +15,8 @@ typedef int (*odb_stream_close_fn)(struct odb_stream *); typedef ssize_t (*odb_stream_read_fn)(struct odb_stream *, char *, size_t); /* - * A stream that can be used to read an object from the object database without - * loading all of it into memory. + * A stream that can be used to read an object from or write an object into the + * object database without loading all of it into memory. */ struct odb_stream { odb_stream_close_fn close; @@ -48,30 +48,6 @@ int odb_stream_close(struct odb_stream *stream); */ ssize_t odb_stream_read(struct odb_stream *stream, void *buf, size_t len); -/* - * A stream that provides an object to be written to the object database without - * loading all of it into memory. - */ -struct odb_write_stream { - ssize_t (*read)(struct odb_write_stream *, unsigned char *, size_t); - void *data; - size_t size; - enum object_type type; -}; - -/* - * Read data from the stream into the buffer. Returns 0 when finished and the - * number of bytes read on success. Returns a negative error code in case - * reading from the stream fails. - */ -ssize_t odb_write_stream_read(struct odb_write_stream *stream, void *buf, - size_t len); - -/* - * Releases memory allocated for underlying stream data. - */ -void odb_write_stream_release(struct odb_write_stream *stream); - /* * Look up the object by its ID and write the full contents to the file * descriptor. The object must be a blob, or the function will fail. When @@ -92,7 +68,6 @@ int odb_stream_blob_to_fd(struct object_database *odb, /* * Sets up an ODB write stream that reads from an fd. */ -void odb_write_stream_from_fd(struct odb_write_stream *stream, int fd, - size_t size, enum object_type type); +struct odb_stream *odb_write_stream_from_fd(int fd, size_t size, enum object_type type); #endif /* STREAMING_H */ diff --git a/odb/transaction.c b/odb/transaction.c index 6aaf133812..69d71b9e97 100644 --- a/odb/transaction.c +++ b/odb/transaction.c @@ -39,7 +39,7 @@ int odb_transaction_commit(struct odb_transaction *transaction) } int odb_transaction_write_object_stream(struct odb_transaction *transaction, - struct odb_write_stream *stream, + struct odb_stream *stream, struct object_id *oid) { return transaction->write_object_stream(transaction, stream, oid); diff --git a/odb/transaction.h b/odb/transaction.h index 1eb74664c6..65248a409c 100644 --- a/odb/transaction.h +++ b/odb/transaction.h @@ -31,7 +31,7 @@ struct odb_transaction { * otherwise. */ int (*write_object_stream)(struct odb_transaction *transaction, - struct odb_write_stream *stream, + struct odb_stream *stream, struct object_id *oid); /* @@ -81,7 +81,7 @@ int odb_transaction_commit(struct odb_transaction *transaction); * error code otherwise. */ int odb_transaction_write_object_stream(struct odb_transaction *transaction, - struct odb_write_stream *stream, + struct odb_stream *stream, struct object_id *oid); /* diff --git a/t/unit-tests/u-odb-inmemory.c b/t/unit-tests/u-odb-inmemory.c index 839a0fd3b7..b8b331b37d 100644 --- a/t/unit-tests/u-odb-inmemory.c +++ b/t/unit-tests/u-odb-inmemory.c @@ -266,13 +266,13 @@ void test_odb_inmemory__freshen_object(void) } struct membuf_write_stream { - struct odb_write_stream base; + struct odb_stream base; const char *buf; size_t offset; }; -static ssize_t membuf_write_stream_read(struct odb_write_stream *stream, - unsigned char *buf, size_t len) +static ssize_t membuf_write_stream_read(struct odb_stream *stream, + char *buf, size_t len) { struct membuf_write_stream *s = container_of(stream, struct membuf_write_stream, base); size_t chunk_size = 2; -- 2.55.0.679.g6767b8d81c.dirty ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 6/8] odb/streaming: rename `struct read_object_fd_data` 2026-08-05 7:44 ` [PATCH v2 0/8] odb: unify read and write streams Patrick Steinhardt ` (4 preceding siblings ...) 2026-08-05 7:44 ` [PATCH v2 5/8] odb/streaming: consolidate read and write streams Patrick Steinhardt @ 2026-08-05 7:44 ` 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 7 siblings, 0 replies; 27+ messages in thread From: Patrick Steinhardt @ 2026-08-05 7:44 UTC (permalink / raw) To: git; +Cc: Justin Tobler, Junio C Hamano With the preceding refactorings the `struct read_object_fd_data` is now somewhat misnamed, as it doesn't only contain the data anymore, but also the stream itself. Rename the structure to `struct fd_stream` to better match the new structure. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- odb/streaming.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/odb/streaming.c b/odb/streaming.c index 1a267e6b90..c436b18d39 100644 --- a/odb/streaming.c +++ b/odb/streaming.c @@ -288,33 +288,33 @@ int odb_stream_blob_to_fd(struct object_database *odb, return result; } -struct read_object_fd_data { +struct fd_stream { struct odb_stream base; int fd; size_t remaining; }; -static ssize_t read_object_fd(struct odb_stream *stream, +static ssize_t fd_stream_read(struct odb_stream *stream, char *buf, size_t len) { - struct read_object_fd_data *data = container_of(stream, struct read_object_fd_data, base); + struct fd_stream *fds = container_of(stream, struct fd_stream, base); ssize_t read_result; size_t count; - if (!data->remaining) + if (!fds->remaining) return 0; - count = data->remaining < len ? data->remaining : len; - read_result = read_in_full(data->fd, buf, count); + count = fds->remaining < len ? fds->remaining : len; + read_result = read_in_full(fds->fd, buf, count); if (read_result < 0 || (size_t)read_result != count) return -1; - data->remaining -= count; + fds->remaining -= count; return read_result; } -static int close_object_fd(struct odb_stream *stream UNUSED) +static int fd_stream_close(struct odb_stream *stream UNUSED) { /* The file descriptor is owned by the caller for now. */ return 0; @@ -322,15 +322,15 @@ static int close_object_fd(struct odb_stream *stream UNUSED) struct odb_stream *odb_write_stream_from_fd(int fd, size_t size, enum object_type type) { - struct read_object_fd_data *data; + struct fd_stream *fds; - CALLOC_ARRAY(data, 1); - data->base.read = read_object_fd; - data->base.close = close_object_fd; - data->base.size = size; - data->base.type = type; - data->fd = fd; - data->remaining = size; + CALLOC_ARRAY(fds, 1); + fds->base.read = fd_stream_read; + fds->base.close = fd_stream_close; + fds->base.size = size; + fds->base.type = type; + fds->fd = fd; + fds->remaining = size; - return &data->base; + return &fds->base; } -- 2.55.0.679.g6767b8d81c.dirty ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 7/8] odb/streaming: rename `struct input_zstream_data` 2026-08-05 7:44 ` [PATCH v2 0/8] odb: unify read and write streams Patrick Steinhardt ` (5 preceding siblings ...) 2026-08-05 7:44 ` [PATCH v2 6/8] odb/streaming: rename `struct read_object_fd_data` Patrick Steinhardt @ 2026-08-05 7:44 ` Patrick Steinhardt 2026-08-05 7:44 ` [PATCH v2 8/8] odb/streaming: unify function names to create new streams Patrick Steinhardt 7 siblings, 0 replies; 27+ messages in thread From: Patrick Steinhardt @ 2026-08-05 7:44 UTC (permalink / raw) To: git; +Cc: Justin Tobler, Junio C Hamano With the preceding refactorings the `struct input_zstream_data` is now somewhat misnamed, as it doesn't only contain the data anymore, but also the stream itself. Rename the structure to `struct zlib_stream` to better match the new structure. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- builtin/unpack-objects.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c index 05a2d48011..3392a3b87d 100644 --- a/builtin/unpack-objects.c +++ b/builtin/unpack-objects.c @@ -358,16 +358,16 @@ static void unpack_non_delta_entry(enum object_type type, unsigned long size, write_object(nr, type, buf, size); } -struct input_zstream_data { +struct zlib_stream { struct odb_stream base; git_zstream *zstream; int status; }; -static ssize_t feed_input_zstream(struct odb_stream *in_stream, - char *buf, size_t buf_len) +static ssize_t zlib_stream_read(struct odb_stream *in_stream, + char *buf, size_t buf_len) { - struct input_zstream_data *data = container_of(in_stream, struct input_zstream_data, base); + struct zlib_stream *data = container_of(in_stream, struct zlib_stream, base); git_zstream *zstream = data->zstream; if (data->status != Z_OK) @@ -389,9 +389,9 @@ static ssize_t feed_input_zstream(struct odb_stream *in_stream, static void stream_blob(unsigned long size, unsigned nr) { git_zstream zstream = { 0 }; - struct input_zstream_data in_stream = { + struct zlib_stream in_stream = { .base = { - .read = feed_input_zstream, + .read = zlib_stream_read, .size = size, .type = OBJ_BLOB, }, -- 2.55.0.679.g6767b8d81c.dirty ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 8/8] odb/streaming: unify function names to create new streams 2026-08-05 7:44 ` [PATCH v2 0/8] odb: unify read and write streams Patrick Steinhardt ` (6 preceding siblings ...) 2026-08-05 7:44 ` [PATCH v2 7/8] odb/streaming: rename `struct input_zstream_data` Patrick Steinhardt @ 2026-08-05 7:44 ` Patrick Steinhardt 7 siblings, 0 replies; 27+ messages in thread From: Patrick Steinhardt @ 2026-08-05 7:44 UTC (permalink / raw) To: git; +Cc: Justin Tobler, Junio C Hamano Unify the function names to create new streams from different sources so that they follow a common schema. While at it, document the ownership of the file descriptor passed to `odb_stream_from_fd()`. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- archive-tar.c | 2 +- archive-zip.c | 2 +- builtin/index-pack.c | 2 +- builtin/pack-objects.c | 4 ++-- object-file.c | 4 ++-- object.c | 2 +- odb/streaming.c | 10 +++++----- odb/streaming.h | 23 +++++++++++++---------- 8 files changed, 26 insertions(+), 23 deletions(-) diff --git a/archive-tar.c b/archive-tar.c index df2d7fb8e9..a1c66024d4 100644 --- a/archive-tar.c +++ b/archive-tar.c @@ -133,7 +133,7 @@ static int stream_blocked(struct repository *r, const struct object_id *oid) char buf[BLOCKSIZE]; ssize_t readlen; - st = odb_read_stream_open(r->objects, oid, NULL); + st = odb_stream_from_object(r->objects, oid, NULL); if (!st) return error(_("cannot stream blob %s"), oid_to_hex(oid)); for (;;) { diff --git a/archive-zip.c b/archive-zip.c index 8095fd04d5..1a948c2f83 100644 --- a/archive-zip.c +++ b/archive-zip.c @@ -347,7 +347,7 @@ static int write_zip_entry(struct archiver_args *args, method = ZIP_METHOD_DEFLATE; if (!buffer) { - stream = odb_read_stream_open(args->repo->objects, oid, NULL); + stream = odb_stream_from_object(args->repo->objects, oid, NULL); if (!stream) return error(_("cannot stream blob %s"), oid_to_hex(oid)); diff --git a/builtin/index-pack.c b/builtin/index-pack.c index 7226da3e65..d1761282db 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -806,7 +806,7 @@ static int check_collison(struct object_entry *entry) memset(&data, 0, sizeof(data)); data.entry = entry; - data.st = odb_read_stream_open(the_repository->objects, &entry->idx.oid, NULL); + data.st = odb_stream_from_object(the_repository->objects, &entry->idx.oid, NULL); if (!data.st) return -1; if (data.st->size != entry->size || data.st->type != entry->type) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 683160c6bb..10d00ca792 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -528,8 +528,8 @@ static unsigned long write_no_reuse_object(struct hashfile *f, struct object_ent if (oe_type(entry) == OBJ_BLOB && oe_size_greater_than(&to_pack, entry, repo_settings_get_big_file_threshold(the_repository)) && - (st = odb_read_stream_open(the_repository->objects, &entry->idx.oid, - NULL)) != NULL) { + (st = odb_stream_from_object(the_repository->objects, &entry->idx.oid, + NULL)) != NULL) { buf = NULL; type = st->type; size = st->size; diff --git a/object-file.c b/object-file.c index 068c6e5672..11d1af342e 100644 --- a/object-file.c +++ b/object-file.c @@ -952,8 +952,8 @@ int index_fd(struct index_state *istate, struct object_id *oid, ret = index_core(istate, oid, fd, xsize_t(st->st_size), type, path, flags); } else { - struct odb_stream *stream = odb_write_stream_from_fd(fd, xsize_t(st->st_size), - OBJ_BLOB); + struct odb_stream *stream = odb_stream_from_fd(fd, xsize_t(st->st_size), + OBJ_BLOB); if (flags & INDEX_WRITE_OBJECT) { struct object_database *odb = the_repository->objects; diff --git a/object.c b/object.c index 37e6efee47..97f7fc0e87 100644 --- a/object.c +++ b/object.c @@ -345,7 +345,7 @@ struct object *parse_object_with_flags(struct repository *r, if ((!obj || obj->type == OBJ_NONE || obj->type == OBJ_BLOB) && odb_read_object_info(r->objects, oid, NULL) == OBJ_BLOB) { if (!skip_hash) { - struct odb_stream *stream = odb_read_stream_open(r->objects, oid, NULL); + struct odb_stream *stream = odb_stream_from_object(r->objects, oid, NULL); if (!stream) { error(_("unable to open object stream for %s"), oid_to_hex(oid)); diff --git a/odb/streaming.c b/odb/streaming.c index c436b18d39..9c85ec54f5 100644 --- a/odb/streaming.c +++ b/odb/streaming.c @@ -208,9 +208,9 @@ ssize_t odb_stream_read(struct odb_stream *st, void *buf, size_t sz) return st->read(st, buf, sz); } -struct odb_stream *odb_read_stream_open(struct object_database *odb, - const struct object_id *oid, - struct stream_filter *filter) +struct odb_stream *odb_stream_from_object(struct object_database *odb, + const struct object_id *oid, + struct stream_filter *filter) { struct odb_stream *st; const struct object_id *real = lookup_replace_object(odb->repo, oid); @@ -242,7 +242,7 @@ int odb_stream_blob_to_fd(struct object_database *odb, ssize_t kept = 0; int result = -1; - st = odb_read_stream_open(odb, oid, filter); + st = odb_stream_from_object(odb, oid, filter); if (!st) { if (filter) free_stream_filter(filter); @@ -320,7 +320,7 @@ static int fd_stream_close(struct odb_stream *stream UNUSED) return 0; } -struct odb_stream *odb_write_stream_from_fd(int fd, size_t size, enum object_type type) +struct odb_stream *odb_stream_from_fd(int fd, size_t size, enum object_type type) { struct fd_stream *fds; diff --git a/odb/streaming.h b/odb/streaming.h index 60b9803190..b522ff513f 100644 --- a/odb/streaming.h +++ b/odb/streaming.h @@ -26,14 +26,22 @@ struct odb_stream { }; /* - * Create a new object stream for the given object database. An optional filter - * can be used to transform the object's content. + * Create a new object stream for the given object. An optional filter can be + * used to transform the object's content. * * Returns the stream on success, a `NULL` pointer otherwise. */ -struct odb_stream *odb_read_stream_open(struct object_database *odb, - const struct object_id *oid, - struct stream_filter *filter); +struct odb_stream *odb_stream_from_object(struct object_database *odb, + const struct object_id *oid, + struct stream_filter *filter); + +/* + * Create a new object stream for the given file descriptor. This can be used + * to, for example, stream an object into the object database. This function + * does _not_ take ownership of the file descriptor. It's the responsibility of + * the caller to close it after the stream has been closed. + */ +struct odb_stream *odb_stream_from_fd(int fd, size_t size, enum object_type type); /* * Close the given object stream and release all resources associated with it. @@ -65,9 +73,4 @@ int odb_stream_blob_to_fd(struct object_database *odb, struct stream_filter *filter, int can_seek); -/* - * Sets up an ODB write stream that reads from an fd. - */ -struct odb_stream *odb_write_stream_from_fd(int fd, size_t size, enum object_type type); - #endif /* STREAMING_H */ -- 2.55.0.679.g6767b8d81c.dirty ^ permalink raw reply related [flat|nested] 27+ messages in thread
end of thread, other threads:[~2026-08-05 7:45 UTC | newest] Thread overview: 27+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 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 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
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox