From b1b008fa2b3c7591ad0094e45fd6fc9b8fac5bff Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 5 Aug 2026 09:44:45 +0200 Subject: [PATCH 01/43] odb/streaming: track write stream size in the structure 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 Signed-off-by: Junio C Hamano --- 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 4263edfbecdd39..f3e0b504f43f13 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 ec35c318bc9fe7..b196abb596e87b 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 dabd481f57dbc4..585b2b2965bb91 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 cbc2f9ced42338..019d3af3e8d212 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 5e086d266fac4f..f51960bd71bb11 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 3e71611b8e0071..398131e194f87c 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 ef0e9192777c4a..77a2adb52abf47 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 0890704e76879b..e6ff74833b8bec 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 fc04dd5cda8800..0080148ba71078 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 20531e864c9561..38c2f6687c432d 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 c0236717802301..4d7d31b5aa04f6 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 dab7da6a9a4f55..6aaf1338127534 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 4cb2eafcbf08f5..ffb279314cfd21 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 ddf2db5c811fb8..5ccc52dccc06f9 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"); From 726782254239706bc91276537450b7a21b81f99e Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 5 Aug 2026 09:44:46 +0200 Subject: [PATCH 02/43] odb/streaming: drop `is_finished` field 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 Signed-off-by: Junio C Hamano --- 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 f3e0b504f43f13..b7c486ea949995 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 b196abb596e87b..317c09dff8653f 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 398131e194f87c..01bb81c63cc2a4 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 77a2adb52abf47..361b4e2a2a4574 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 38c2f6687c432d..912e75e682e6a5 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 4d7d31b5aa04f6..5e8e6e532e5660 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 5ccc52dccc06f9..4437140ed04e9b 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; } From 8a51f6e8c5e1e1b849f41069dabc7bdfc1177d9b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 5 Aug 2026 09:44:47 +0200 Subject: [PATCH 03/43] odb/streaming: support streaming arbitrary object types 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 Signed-off-by: Junio C Hamano --- 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 b7c486ea949995..7439ec53be310d 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 317c09dff8653f..699a6a008ce5ba 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 01bb81c63cc2a4..139618024a6023 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 361b4e2a2a4574..5681a38f03d4d5 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 912e75e682e6a5..0918cad4267bd3 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 5e8e6e532e5660..3c8ed551293fd4 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 ffb279314cfd21..1eb74664c6bb1d 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 4437140ed04e9b..1ab07af6d666cd 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; From a59e4798f0b3edc59326312b0891521d6886ee7c Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 5 Aug 2026 09:44:48 +0200 Subject: [PATCH 04/43] odb/streaming: rename `struct odb_read_stream` 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 Signed-off-by: Junio C Hamano --- 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 0fc70d13a8807e..df2d7fb8e936a8 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 97ea8d60d6187b..8095fd04d5b9ba 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 bc86925ad04340..7226da3e65ad8d 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 1ec5b6f206366e..683160c6bbb6ab 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 699a6a008ce5ba..5f6d584c356f28 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 805f2cfa289661..f44758c4f8ba01 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 23b84aa7e29531..37e6efee47ff2a 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 f51960bd71bb11..f7b8c76549c393 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 139618024a6023..485d58703658b0 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 5681a38f03d4d5..038defd9059408 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 e6ff74833b8bec..b3186ca5933ae9 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 0080148ba71078..89b0c396822c95 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 0918cad4267bd3..98e2152e364741 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 3c8ed551293fd4..037954c2315895 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 c3b8db7c5c41a6..1b5e26847d0b2a 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 0eee45055f833e..70254573a3f4dc 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 e1f77152b5c4bf..f913cb3d0c589c 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 1ab07af6d666cd..839a0fd3b753b2 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); } From e837b812fe3521cadab2923ec7457c1ebcaeda43 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 5 Aug 2026 09:44:49 +0200 Subject: [PATCH 05/43] odb/streaming: consolidate read and write streams 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 Signed-off-by: Junio C Hamano --- 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 7439ec53be310d..05a2d48011fc73 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 5f6d584c356f28..068c6e56726db9 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 585b2b2965bb91..eec4cc53022e9d 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 019d3af3e8d212..fbe75c5a811a55 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 f7b8c76549c393..6defe5ac4f94ca 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 485d58703658b0..795672adf255c6 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 038defd9059408..ff1bede7fef24a 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 b3186ca5933ae9..630d9555856d7c 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 89b0c396822c95..0b99c698b5d625 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 98e2152e364741..1a267e6b90702a 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 037954c2315895..60b98031908f35 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 6aaf1338127534..69d71b9e97c61d 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 1eb74664c6bb1d..65248a409c820d 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 839a0fd3b753b2..b8b331b37d3beb 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; From 93dd24603b76ccc26322e48ba6c3e1dfc95ef30e Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 5 Aug 2026 09:44:50 +0200 Subject: [PATCH 06/43] odb/streaming: rename `struct read_object_fd_data` 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 Signed-off-by: Junio C Hamano --- odb/streaming.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/odb/streaming.c b/odb/streaming.c index 1a267e6b90702a..c436b18d39acdf 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; } From 3f8290ea8552bb028c14e5be75315c2b90221802 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 5 Aug 2026 09:44:51 +0200 Subject: [PATCH 07/43] odb/streaming: rename `struct input_zstream_data` 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 Signed-off-by: Junio C Hamano --- 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 05a2d48011fc73..3392a3b87ddb0c 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, }, From ebdd7e10d6935c510154bcfff03b92cd7e972830 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 5 Aug 2026 09:44:52 +0200 Subject: [PATCH 08/43] odb/streaming: unify function names to create new streams 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 Signed-off-by: Junio C Hamano --- 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 df2d7fb8e936a8..a1c66024d4dff4 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 8095fd04d5b9ba..1a948c2f83c919 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 7226da3e65ad8d..d1761282db8915 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 683160c6bbb6ab..10d00ca7922260 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 068c6e56726db9..11d1af342e8086 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 37e6efee47ff2a..97f7fc0e87a1db 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 c436b18d39acdf..9c85ec54f59bb1 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 60b98031908f35..b522ff513f26d2 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 */ From c3a8f4303c9e7b72ab506d58177648cd90026be6 Mon Sep 17 00:00:00 2001 From: Pablo Sabater Date: Sat, 8 Aug 2026 02:02:16 +0200 Subject: [PATCH 09/43] t5701: use test_file_size() to get the size of a file The 'basics of object-info' test runs 'wc -c | xargs' twice to get the size of two.t. The pipe to xargs is only there to strip the blanks that some platforms pad the output of wc with. Use the test_file_size() helper, which outputs the size directly, and store the result in a variable. Because 'git rev-parse two:two.t' is also run multiple times, store its output in a variable as well. Storing them in variables outside the HERE-document has the added benefit of preserving their exit statuses. Mentored-by: Karthik Nayak Mentored-by: Chandra Pratap Signed-off-by: Pablo Sabater Signed-off-by: Junio C Hamano --- t/t5701-git-serve.sh | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh index 9a575aa098afd3..51d5dd1ae6f389 100755 --- a/t/t5701-git-serve.sh +++ b/t/t5701-git-serve.sh @@ -344,20 +344,23 @@ test_expect_success 'unexpected lines are not allowed in fetch request' ' test_expect_success 'basics of object-info' ' test_config transfer.advertiseObjectInfo true && + two_oid=$(git rev-parse two:two.t) && + two_size=$(test_file_size two.t) && + test-tool pkt-line pack >in <<-EOF && command=object-info object-format=$(test_oid algo) 0001 size - oid $(git rev-parse two:two.t) - oid $(git rev-parse two:two.t) + oid $two_oid + oid $two_oid 0000 EOF cat >expect <<-EOF && size - $(git rev-parse two:two.t) $(wc -c Date: Sat, 8 Aug 2026 02:02:17 +0200 Subject: [PATCH 10/43] fetch-object-info: detect malformed server responses The loop reading the object-info response stops as soon as the reader returns something other than PACKET_READ_NORMAL, or once it has read as many lines as we requested. Neither end is checked. A server that answers with fewer objects leaves the end of the result arrays empty, and the caller trusts that every requested object was filled in. A server that answers with more leaves the extra packets unread. On stateless transports check_stateless_delimiter() notices, but on the others it passes unnoticed. Check both limits by extracting the packet_reader_read() from the loop condition, so the loop no longer consumes the last packet (flush). If while looping the read is different from a PACKET_READ_NORMAL, die() meaning there are fewer objects than expected. After iterating, we only expect a flush, so if the last packet is not a flush, die(). Helped-by: Junio C Hamano Mentored-by: Karthik Nayak Mentored-by: Chandra Pratap Signed-off-by: Pablo Sabater Signed-off-by: Junio C Hamano --- fetch-object-info.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/fetch-object-info.c b/fetch-object-info.c index ba7e179c44ee54..287f668a3cc07b 100644 --- a/fetch-object-info.c +++ b/fetch-object-info.c @@ -106,12 +106,13 @@ int fetch_object_info(const enum protocol_version version, struct object_info_ar } } - for (size_t i = 0; - packet_reader_read(reader) == PACKET_READ_NORMAL && - i < args->oids->nr; - i++) { + for (size_t i = 0; i < args->oids->nr; i++) { struct string_list object_info_values = STRING_LIST_INIT_DUP; + if (packet_reader_read(reader) != PACKET_READ_NORMAL) + die(_("object-info: expected %" PRIuMAX " objects, got %" PRIuMAX), + (uintmax_t)args->oids->nr, (uintmax_t)i); + string_list_split(&object_info_values, reader->line, " ", -1); if (strcmp(object_info_values.items[0].string, @@ -150,6 +151,11 @@ int fetch_object_info(const enum protocol_version version, struct object_info_ar string_list_clear(&object_info_values, 0); } + + if (packet_reader_read(reader) != PACKET_READ_FLUSH) + die(_("object-info: expected flush after %" PRIuMAX " objects"), + (uintmax_t)args->oids->nr); + check_stateless_delimiter(stateless_rpc, reader, "stateless delimiter expected"); return 0; From d43b4d3fa10ee596c1bb6517f30d6f4daad906e4 Mon Sep 17 00:00:00 2001 From: Pablo Sabater Date: Sat, 8 Aug 2026 02:02:18 +0200 Subject: [PATCH 11/43] fetch-object-info: pass arguments directly instead of a struct struct object_info_args groups three pointers that already live in the transport and are given to fetch_object_info(). Grouping them into a struct reduces the number of parameters, but it suggests that the three belong together, when they are unrelated and end up being accessed as args->* independently. Drop the struct and pass those parameters directly to fetch_object_info() and send_object_info_request(). This should have no change in behavior. Helped-by: Jeff King Helped-by: Junio C Hamano Mentored-by: Karthik Nayak Mentored-by: Chandra Pratap Signed-off-by: Pablo Sabater Signed-off-by: Junio C Hamano --- fetch-object-info.c | 53 ++++++++++++++++++++++++++------------------- fetch-object-info.h | 17 +++++++-------- transport.c | 11 +++++----- 3 files changed, 44 insertions(+), 37 deletions(-) diff --git a/fetch-object-info.c b/fetch-object-info.c index 287f668a3cc07b..53eec88cf0de10 100644 --- a/fetch-object-info.c +++ b/fetch-object-info.c @@ -9,20 +9,24 @@ #include "string-list.h" /* Sends object-info command and its arguments into the request buffer. */ -static void send_object_info_request(const int fd_out, struct object_info_args *args) +static void send_object_info_request(const int fd_out, + const struct string_list *server_options, + struct oid_array *oids, + struct string_list *object_info_options) { struct strbuf req_buf = STRBUF_INIT; - write_command_and_capabilities(&req_buf, "object-info", args->server_options); + write_command_and_capabilities(&req_buf, "object-info", server_options); - if (unsorted_string_list_has_string(args->object_info_options, "size")) + if (unsorted_string_list_has_string(object_info_options, "size")) packet_buf_write(&req_buf, "size"); - else if (args->object_info_options->nr) + else if (object_info_options->nr) BUG("only size should be in object_info_options"); - if (args->oids) - for (size_t i = 0; i < args->oids->nr; i++) - packet_buf_write(&req_buf, "oid %s", oid_to_hex(&args->oids->oid[i])); + if (oids) + for (size_t i = 0; i < oids->nr; i++) + packet_buf_write(&req_buf, "oid %s", + oid_to_hex(&oids->oid[i])); packet_buf_flush(&req_buf); if (write_in_full(fd_out, req_buf.buf, req_buf.len) < 0) @@ -45,8 +49,12 @@ static int parse_object_size(const char *s, size_t *res) return 0; } -int fetch_object_info(const enum protocol_version version, struct object_info_args *args, - struct packet_reader *reader, struct object_info *object_info_data, +int fetch_object_info(const enum protocol_version version, + const struct string_list *server_options, + struct oid_array *oids, + struct string_list *object_info_options, + struct packet_reader *reader, + struct object_info *object_info_data, const int stateless_rpc, const int fd_out) { int size_index = -1; @@ -64,16 +72,17 @@ int fetch_object_info(const enum protocol_version version, struct object_info_ar * because the number of options is a small known number (the * supported placeholders which currently are size and type). */ - for (int i = (int)args->object_info_options->nr - 1; i >= 0; i--) + for (int i = (int)object_info_options->nr - 1; i >= 0; i--) if (!server_supports_feature("object-info", - args->object_info_options->items[i].string, 0)) - unsorted_string_list_delete_item(args->object_info_options, i, 0); + object_info_options->items[i].string, 0)) + unsorted_string_list_delete_item(object_info_options, i, 0); /* * Even if no options are left, we still send the oid so we get * at least an existence check. */ - send_object_info_request(fd_out, args); + send_object_info_request(fd_out, server_options, oids, + object_info_options); break; case protocol_v1: case protocol_v0: @@ -82,14 +91,14 @@ int fetch_object_info(const enum protocol_version version, struct object_info_ar BUG("unknown protocol version"); } - for (size_t i = 0; i < args->object_info_options->nr; i++) { + for (size_t i = 0; i < object_info_options->nr; i++) { if (packet_reader_read(reader) != PACKET_READ_NORMAL) { check_stateless_delimiter(stateless_rpc, reader, "stateless delimiter expected"); return -1; } - if (!unsorted_string_list_has_string(args->object_info_options, reader->line)) + if (!unsorted_string_list_has_string(object_info_options, reader->line)) return -1; if (!strcmp(reader->line, "size")) { @@ -98,7 +107,7 @@ int fetch_object_info(const enum protocol_version version, struct object_info_ar * is only size. No risk of overflow. */ size_index = (int)i; - for (size_t j = 0; j < args->oids->nr; j++) + for (size_t j = 0; j < oids->nr; j++) object_info_data[j].sizep = xcalloc(1, sizeof(*object_info_data[j].sizep)); } else { @@ -106,19 +115,19 @@ int fetch_object_info(const enum protocol_version version, struct object_info_ar } } - for (size_t i = 0; i < args->oids->nr; i++) { + for (size_t i = 0; i < oids->nr; i++) { struct string_list object_info_values = STRING_LIST_INIT_DUP; if (packet_reader_read(reader) != PACKET_READ_NORMAL) die(_("object-info: expected %" PRIuMAX " objects, got %" PRIuMAX), - (uintmax_t)args->oids->nr, (uintmax_t)i); + (uintmax_t)oids->nr, (uintmax_t)i); string_list_split(&object_info_values, reader->line, " ", -1); if (strcmp(object_info_values.items[0].string, - oid_to_hex(&args->oids->oid[i]))) + oid_to_hex(&oids->oid[i]))) die(_("object-info: expected OID: %s, got %s"), - oid_to_hex(&args->oids->oid[i]), + oid_to_hex(&oids->oid[i]), object_info_values.items[0].string); /* @@ -138,7 +147,7 @@ int fetch_object_info(const enum protocol_version version, struct object_info_ar * the server we expect the server to answer with the same * number of attributes requested. */ - if (args->object_info_options->nr + 1 != object_info_values.nr) + if (object_info_options->nr + 1 != object_info_values.nr) die("object-info: unexpected number of attributes: %s", reader->line); @@ -154,7 +163,7 @@ int fetch_object_info(const enum protocol_version version, struct object_info_ar if (packet_reader_read(reader) != PACKET_READ_FLUSH) die(_("object-info: expected flush after %" PRIuMAX " objects"), - (uintmax_t)args->oids->nr); + (uintmax_t)oids->nr); check_stateless_delimiter(stateless_rpc, reader, "stateless delimiter expected"); diff --git a/fetch-object-info.h b/fetch-object-info.h index 269cebb3f7df48..316bf917ce2d5c 100644 --- a/fetch-object-info.h +++ b/fetch-object-info.h @@ -4,22 +4,21 @@ #include "pkt-line.h" #include "protocol.h" -struct object_info_args { - struct string_list *object_info_options; - const struct string_list *server_options; - struct oid_array *oids; -}; - struct object_info; +struct oid_array; /* * Sends git-cat-file object-info command into the request buf and read the * results from packets. * - * Modifies args->object_info_options, on return it contains only the supported + * Modifies object_info_options, on return it contains only the supported * options by the server. */ -int fetch_object_info(enum protocol_version version, struct object_info_args *args, - struct packet_reader *reader, struct object_info *object_info_data, +int fetch_object_info(enum protocol_version version, + const struct string_list *server_options, + struct oid_array *oids, + struct string_list *object_info_options, + struct packet_reader *reader, + struct object_info *object_info_data, int stateless_rpc, int fd_out); #endif /* FETCH_OBJECT_INFO_H */ diff --git a/transport.c b/transport.c index f0a6a455479800..c6df56129d7c8d 100644 --- a/transport.c +++ b/transport.c @@ -438,11 +438,6 @@ static int fetch_object_info_via_pack(struct transport *transport) int ret = 0; struct git_transport_data *data = transport->data; struct packet_reader reader; - struct object_info_args args = { 0 }; - - args.server_options = transport->server_options; - args.oids = transport->smart_options->object_info_oids; - args.object_info_options = transport->smart_options->object_info_options; connect_setup(transport, 0); packet_reader_init(&reader, data->fd[0], NULL, 0, @@ -453,7 +448,11 @@ static int fetch_object_info_via_pack(struct transport *transport) data->version = discover_version(&reader); transport->hash_algo = reader.hash_algo; - ret = fetch_object_info(data->version, &args, &reader, + ret = fetch_object_info(data->version, + transport->server_options, + transport->smart_options->object_info_oids, + transport->smart_options->object_info_options, + &reader, data->options.object_info_data, transport->stateless_rpc, data->fd[1]); From c5c971d967617592d27c1b7db72455f9277fad47 Mon Sep 17 00:00:00 2001 From: Pablo Sabater Date: Sat, 8 Aug 2026 02:02:19 +0200 Subject: [PATCH 12/43] fetch-object-info: use dedicated struct for the results fetch_object_info() collects information about N objects, but it stores the results in an array of object_info. That struct holds the extended parameters of read_object_info() (The optional outputs the caller wants filled). Its pointers tell that function where to write the answers for a single object. object_info is not meant to be the final storage, and since fetch_object_info() does not call read_object_info(), there is no reason to use it. Using it means allocating one scalar per object per attribute just to have those pointers somewhere to point at. Add struct fetch_object_info_results. The caller sets the wants_* flags to say what it is interested in, and fetch_object_info() allocates one array per attribute. A set wants_* flag means "asked for", while a non-NULL array means "available". The caller releases the arrays with free_fetch_object_info_results(). The object_info_options string list is no longer needed. Filtering against the server's advertisement now sets local ask_* flags, and send_object_info_request() turns those into the v2 protocol option strings. remote_atom_map[] existed only to map those strings back into atom names, so drop it and build remote_allowed_atoms from the result arrays. Currently for wants_* and ask_* there is only the 'size' variant but a subsequent commit will add '*_type'. free_object_info_contents() loses its only caller and is dropped. Dropping the allow-list check makes the final else reachable from the wire, so die() instead of BUG(): an unknown attribute is the server's error, not ours. Helped-by: Jeff King Helped-by: Junio C Hamano Mentored-by: Karthik Nayak Mentored-by: Chandra Pratap Signed-off-by: Pablo Sabater Signed-off-by: Junio C Hamano --- builtin/cat-file.c | 59 ++++++++------------------------- fetch-object-info.c | 81 ++++++++++++++++++++++----------------------- fetch-object-info.h | 27 +++++++++++---- object-file.c | 10 ------ odb.h | 3 -- transport.c | 3 +- transport.h | 5 +-- 7 files changed, 77 insertions(+), 111 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 884b6d5ad348b5..e1650b2921ffcf 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -31,6 +31,7 @@ #include "alias.h" #include "remote.h" #include "transport.h" +#include "fetch-object-info.h" /* * Maximum length for a remote URL. While no universal standard exists, @@ -681,9 +682,8 @@ static void batch_one_object(const char *obj_name, static int get_remote_info(int argc, const char **argv, - struct object_info **remote_object_info, - struct oid_array *object_info_oids, - struct string_list *object_info_options) + struct fetch_object_info_results *results, + struct oid_array *object_info_oids) { int retval = 0; struct remote *remote = NULL; @@ -724,11 +724,9 @@ static int get_remote_info(int argc, goto cleanup; } - CALLOC_ARRAY(*remote_object_info, object_info_oids->nr); gtransport->smart_options->object_info_oids = object_info_oids; - gtransport->smart_options->object_info_options = object_info_options; - gtransport->smart_options->object_info_data = *remote_object_info; + gtransport->smart_options->object_info_results = results; retval = transport_fetch_object_info(gtransport); cleanup: transport_disconnect(gtransport); @@ -816,21 +814,6 @@ static void parse_cmd_mailmap(struct batch_options *opt UNUSED, load_mailmap(); } -struct protocol_placeholder_entry { - const char *option; - const char *atom; -}; - -static const struct protocol_placeholder_entry remote_atom_map[] = { - {"size", "objectsize"}, - {"type", "objecttype"}, - /* - * Add new protocol options here. Even if the server doesn't support - * them the allow_list will drop them if the server doesn't advertise - * them. - */ -}; - static void parse_cmd_remote_object_info(struct batch_options *opt, const char *line, struct strbuf *output, struct expand_data *data) @@ -838,9 +821,8 @@ static void parse_cmd_remote_object_info(struct batch_options *opt, int count; const char **argv; char *line_to_split; - struct object_info *remote_object_info = NULL; + struct fetch_object_info_results results = FETCH_OBJECT_INFO_RESULTS_INIT; struct oid_array object_info_oids = OID_ARRAY_INIT; - struct string_list object_info_options = STRING_LIST_INIT_NODUP; const char *saved_format = opt->format; if (strlen(line) >= MAX_REMOTE_OBJ_INFO_LINE) @@ -861,26 +843,21 @@ static void parse_cmd_remote_object_info(struct batch_options *opt, MAX_ALLOWED_OBJ_LIMIT); if (data->info.sizep) - string_list_append(&object_info_options, "size"); - if (data->info.typep) - string_list_append(&object_info_options, "type"); + results.wants_size = 1; - if (get_remote_info(count, argv, &remote_object_info, - &object_info_oids, &object_info_options)) + if (get_remote_info(count, argv, &results, &object_info_oids)) die(_("failed to get object info from the remote: %s"), argv[0]); string_list_clear(&data->remote_allowed_atoms, 0); string_list_append(&data->remote_allowed_atoms, "objectname"); - for (size_t i = 0; i < ARRAY_SIZE(remote_atom_map); i++) - if (unsorted_string_list_has_string(&object_info_options, remote_atom_map[i].option)) - string_list_append(&data->remote_allowed_atoms, - remote_atom_map[i].atom); + if (results.sizes) + string_list_append(&data->remote_allowed_atoms, "objectsize"); data->skip_object_info = 1; - for (size_t i = 0; i < object_info_oids.nr; i++) { + for (size_t i = 0; i < results.nr; i++) { data->oid = object_info_oids.oid[i]; - if (remote_object_info[i].unrecognized) { + if (results.unrecognized[i]) { report_object_status(opt, oid_to_hex(&data->oid), &data->oid, "missing"); continue; @@ -890,13 +867,8 @@ static void parse_cmd_remote_object_info(struct batch_options *opt, * When reaching here, it means remote-object-info can retrieve * information from server without downloading them. */ - if (remote_object_info[i].sizep) { - data->size = *remote_object_info[i].sizep; - } - - if (remote_object_info[i].typep) { - data->type = *remote_object_info[i].typep; - } + if (results.sizes) + data->size = results.sizes[i]; opt->batch_mode = BATCH_MODE_INFO; data->is_remote = 1; @@ -906,12 +878,9 @@ static void parse_cmd_remote_object_info(struct batch_options *opt, data->skip_object_info = 0; opt->format = saved_format; - for (size_t i = 0; i < object_info_oids.nr; i++) - free_object_info_contents(&remote_object_info[i]); - string_list_clear(&object_info_options, 0); + free_fetch_object_info_results(&results); free(line_to_split); free(argv); - free(remote_object_info); oid_array_clear(&object_info_oids); } diff --git a/fetch-object-info.c b/fetch-object-info.c index 53eec88cf0de10..5f53dbd6b90109 100644 --- a/fetch-object-info.c +++ b/fetch-object-info.c @@ -12,16 +12,14 @@ static void send_object_info_request(const int fd_out, const struct string_list *server_options, struct oid_array *oids, - struct string_list *object_info_options) + unsigned ask_size) { struct strbuf req_buf = STRBUF_INIT; write_command_and_capabilities(&req_buf, "object-info", server_options); - if (unsorted_string_list_has_string(object_info_options, "size")) + if (ask_size) packet_buf_write(&req_buf, "size"); - else if (object_info_options->nr) - BUG("only size should be in object_info_options"); if (oids) for (size_t i = 0; i < oids->nr; i++) @@ -52,37 +50,32 @@ static int parse_object_size(const char *s, size_t *res) int fetch_object_info(const enum protocol_version version, const struct string_list *server_options, struct oid_array *oids, - struct string_list *object_info_options, struct packet_reader *reader, - struct object_info *object_info_data, - const int stateless_rpc, const int fd_out) + struct fetch_object_info_results *results, + const int stateless_rpc, + const int fd_out) { + unsigned ask_size = 0; int size_index = -1; + size_t wanted; + + results->nr = oids->nr; + CALLOC_ARRAY(results->unrecognized, results->nr); switch (version) { case protocol_v2: if (!server_supports_v2("object-info")) die(_("object-info capability is not enabled on the server")); - /* - * When removing an element from the list it gets swapped by the - * last element, iterate backwards to prevent elements skipping - * evaluation. - * - * object_info_options->nr can be safely casted without overflow - * because the number of options is a small known number (the - * supported placeholders which currently are size and type). - */ - for (int i = (int)object_info_options->nr - 1; i >= 0; i--) - if (!server_supports_feature("object-info", - object_info_options->items[i].string, 0)) - unsorted_string_list_delete_item(object_info_options, i, 0); + + if (results->wants_size && + server_supports_feature("object-info", "size", 0)) + ask_size = 1; /* * Even if no options are left, we still send the oid so we get * at least an existence check. */ - send_object_info_request(fd_out, server_options, oids, - object_info_options); + send_object_info_request(fd_out, server_options, oids, ask_size); break; case protocol_v1: case protocol_v0: @@ -90,28 +83,25 @@ int fetch_object_info(const enum protocol_version version, case protocol_unknown_version: BUG("unknown protocol version"); } + wanted = ask_size; - for (size_t i = 0; i < object_info_options->nr; i++) { + for (size_t i = 0; i < wanted; i++) { if (packet_reader_read(reader) != PACKET_READ_NORMAL) { check_stateless_delimiter(stateless_rpc, reader, "stateless delimiter expected"); return -1; } - if (!unsorted_string_list_has_string(object_info_options, reader->line)) - return -1; - if (!strcmp(reader->line, "size")) { - /* - * i is the number of supported options which currently - * is only size. No risk of overflow. - */ + if (!ask_size) + die(_("object-info: unrequested 'size' attribute")); + if (results->sizes) + die(_("object-info: duplicate 'size' attribute")); size_index = (int)i; - for (size_t j = 0; j < oids->nr; j++) - object_info_data[j].sizep = - xcalloc(1, sizeof(*object_info_data[j].sizep)); + CALLOC_ARRAY(results->sizes, results->nr); } else { - BUG("only size is supported"); + die(_("object-info: unknown attribute '%s'"), + reader->line); } } @@ -137,24 +127,24 @@ int fetch_object_info(const enum protocol_version version, */ if (object_info_values.nr >= 2 && !strcmp(object_info_values.items[1].string, "")) { - object_info_data[i].unrecognized = 1; + results->unrecognized[i] = 1; string_list_clear(&object_info_values, 0); continue; } /* - * Because we filter the options to be only the supported by - * the server we expect the server to answer with the same - * number of attributes requested. + * Because we only ask for attributes the server said it + * supports, we expect the answer to have one value per + * requested attribute, plus the OID. */ - if (object_info_options->nr + 1 != object_info_values.nr) + if (wanted + 1 != object_info_values.nr) die("object-info: unexpected number of attributes: %s", reader->line); - if (size_index >= 0 && + if (results->sizes && parse_object_size(object_info_values.items[size_index + 1].string, - object_info_data[i].sizep)) - die("object-info: ref %s has invalid size %s", + &results->sizes[i])) + die("object-info: object %s has invalid size %s", object_info_values.items[0].string, object_info_values.items[size_index + 1].string); @@ -169,3 +159,10 @@ int fetch_object_info(const enum protocol_version version, return 0; } + +void free_fetch_object_info_results(struct fetch_object_info_results *results) +{ + free(results->sizes); + free(results->unrecognized); + memset(results, 0, sizeof(*results)); +} diff --git a/fetch-object-info.h b/fetch-object-info.h index 316bf917ce2d5c..9f72e91155336f 100644 --- a/fetch-object-info.h +++ b/fetch-object-info.h @@ -4,21 +4,34 @@ #include "pkt-line.h" #include "protocol.h" -struct object_info; +struct fetch_object_info_results { + size_t *sizes; + uint8_t *unrecognized; + size_t nr; + unsigned wants_size:1; +}; + +#define FETCH_OBJECT_INFO_RESULTS_INIT { 0 } + struct oid_array; /* - * Sends git-cat-file object-info command into the request buf and read the + * Sends git-cat-file object-info command into the request buf and reads the * results from packets. * - * Modifies object_info_options, on return it contains only the supported - * options by the server. + * The caller sets the wants_* flags in "results" to indicate which attributes + * it is interested in. On return, "results" holds one array per attribute that + * the server both advertised and answered with. An array left NULL means the + * attribute is not available. + * Release them with free_fetch_object_info_results(). */ int fetch_object_info(enum protocol_version version, const struct string_list *server_options, struct oid_array *oids, - struct string_list *object_info_options, struct packet_reader *reader, - struct object_info *object_info_data, - int stateless_rpc, int fd_out); + struct fetch_object_info_results *results, + int stateless_rpc, + int fd_out); + +void free_fetch_object_info_results(struct fetch_object_info_results *results); #endif /* FETCH_OBJECT_INFO_H */ diff --git a/object-file.c b/object-file.c index c5809db598703d..7ff2b730ac0ead 100644 --- a/object-file.c +++ b/object-file.c @@ -1740,13 +1740,3 @@ int odb_transaction_files_begin(struct odb_source *source, return 0; } - -void free_object_info_contents(struct object_info *object_info) -{ - if (!object_info) - return; - free(object_info->typep); - free(object_info->sizep); - free(object_info->disk_sizep); - free(object_info->delta_base_oid); -} diff --git a/odb.h b/odb.h index 3f7c48365675d1..b7bc0ee8443e9e 100644 --- a/odb.h +++ b/odb.h @@ -635,7 +635,4 @@ void parse_alternates(const char *string, const char *relative_base, struct strvec *out); -/* Free pointers inside of object_info, but not object_info itself */ -void free_object_info_contents(struct object_info *object_info); - #endif /* ODB_H */ diff --git a/transport.c b/transport.c index c6df56129d7c8d..35d3e98d9739a7 100644 --- a/transport.c +++ b/transport.c @@ -451,9 +451,8 @@ static int fetch_object_info_via_pack(struct transport *transport) ret = fetch_object_info(data->version, transport->server_options, transport->smart_options->object_info_oids, - transport->smart_options->object_info_options, &reader, - data->options.object_info_data, + data->options.object_info_results, transport->stateless_rpc, data->fd[1]); close(data->fd[0]); diff --git a/transport.h b/transport.h index a7869d18e020fb..6948b65db984be 100644 --- a/transport.h +++ b/transport.h @@ -7,6 +7,8 @@ #include "string-list.h" #include "connect.h" +struct fetch_object_info_results; + struct git_transport_options { unsigned thin : 1; unsigned keep : 1; @@ -57,8 +59,7 @@ struct git_transport_options { struct oidset *acked_commits; struct oid_array *object_info_oids; - struct object_info *object_info_data; - struct string_list *object_info_options; + struct fetch_object_info_results *object_info_results; }; enum transport_family { From 50dd6d370cd6421b523347ffa94bdd35bd264833 Mon Sep 17 00:00:00 2001 From: Pablo Sabater Date: Sat, 8 Aug 2026 02:02:20 +0200 Subject: [PATCH 13/43] fetch-object-info: die() on the remaining error path Every failure in fetch_object_info() dies except one: a short read while parsing the attribute lines returns -1. That -1 is then passed through fetch_object_info_via_pack() and get_remote_info() up to cat-file, only to die() with a generic message. Die in fetch_object_info() instead, consistently with the rest of its error paths, and make fetch_object_info() void. Mentored-by: Karthik Nayak Mentored-by: Chandra Pratap Signed-off-by: Pablo Sabater Signed-off-by: Junio C Hamano --- fetch-object-info.c | 19 +++++++++---------- fetch-object-info.h | 14 +++++++------- transport.c | 12 ++++++------ 3 files changed, 22 insertions(+), 23 deletions(-) diff --git a/fetch-object-info.c b/fetch-object-info.c index 5f53dbd6b90109..4db879c2dc7cc5 100644 --- a/fetch-object-info.c +++ b/fetch-object-info.c @@ -47,13 +47,13 @@ static int parse_object_size(const char *s, size_t *res) return 0; } -int fetch_object_info(const enum protocol_version version, - const struct string_list *server_options, - struct oid_array *oids, - struct packet_reader *reader, - struct fetch_object_info_results *results, - const int stateless_rpc, - const int fd_out) +void fetch_object_info(const enum protocol_version version, + const struct string_list *server_options, + struct oid_array *oids, + struct packet_reader *reader, + struct fetch_object_info_results *results, + const int stateless_rpc, + const int fd_out) { unsigned ask_size = 0; int size_index = -1; @@ -89,7 +89,8 @@ int fetch_object_info(const enum protocol_version version, if (packet_reader_read(reader) != PACKET_READ_NORMAL) { check_stateless_delimiter(stateless_rpc, reader, "stateless delimiter expected"); - return -1; + die(_("object-info: expected %" PRIuMAX " attributes, got %" PRIuMAX), + (uintmax_t)wanted, (uintmax_t)i); } if (!strcmp(reader->line, "size")) { @@ -156,8 +157,6 @@ int fetch_object_info(const enum protocol_version version, (uintmax_t)oids->nr); check_stateless_delimiter(stateless_rpc, reader, "stateless delimiter expected"); - - return 0; } void free_fetch_object_info_results(struct fetch_object_info_results *results) diff --git a/fetch-object-info.h b/fetch-object-info.h index 9f72e91155336f..97ee5314c99b00 100644 --- a/fetch-object-info.h +++ b/fetch-object-info.h @@ -24,13 +24,13 @@ struct oid_array; * attribute is not available. * Release them with free_fetch_object_info_results(). */ -int fetch_object_info(enum protocol_version version, - const struct string_list *server_options, - struct oid_array *oids, - struct packet_reader *reader, - struct fetch_object_info_results *results, - int stateless_rpc, - int fd_out); +void fetch_object_info(enum protocol_version version, + const struct string_list *server_options, + struct oid_array *oids, + struct packet_reader *reader, + struct fetch_object_info_results *results, + int stateless_rpc, + int fd_out); void free_fetch_object_info_results(struct fetch_object_info_results *results); diff --git a/transport.c b/transport.c index 35d3e98d9739a7..242fdea95b5fa4 100644 --- a/transport.c +++ b/transport.c @@ -448,12 +448,12 @@ static int fetch_object_info_via_pack(struct transport *transport) data->version = discover_version(&reader); transport->hash_algo = reader.hash_algo; - ret = fetch_object_info(data->version, - transport->server_options, - transport->smart_options->object_info_oids, - &reader, - data->options.object_info_results, - transport->stateless_rpc, data->fd[1]); + fetch_object_info(data->version, + transport->server_options, + transport->smart_options->object_info_oids, + &reader, + data->options.object_info_results, + transport->stateless_rpc, data->fd[1]); close(data->fd[0]); if (data->fd[1] >= 0) From 567e62b1b946407224dedfb85f8c9220983a5e13 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 8 Aug 2026 02:02:21 +0200 Subject: [PATCH 14/43] transport: drop remote object-info fields from transport struct A remote object-info request needs three things: the transport for contacting the remote, the list of oids to request, and a place to store the output. Rather than take these as function parameters, we take only the transport object, and expect the caller to have placed the other two into special fields in the transport struct. But this doesn't make much sense. The set of oids and results are really only valid for one request. There is no reason the transport would need to hang on to them outside of the single function call. Even though we save a few lines passing the parameters around through the various vtable functions, the result is harder to understand (for example, who is responsible for cleaning up results, and when should it happen?). It also opens up the possibility of a subtle bug. A caller is likely to point those fields to stack variables which could go out of scope, and the transport struct would be left holding invalid pointers. This is mostly harmless now, as we disconnect the transport immediately after the sole caller of transport_fetch_object_info(). But conceptually we could keep the transport open and make multiple fetch calls (and reuse the same connection to the helper, to a remote HTTP server, and so on). So let's pull these out of the struct and pass them as function parameters. It's a little more verbose, but I think more clearly illustrates the intent. I've also tweaked a few function signatures to mark the input oid array as const, since it is purely an input to the function. Signed-off-by: Jeff King Signed-off-by: Pablo Sabater Signed-off-by: Junio C Hamano --- builtin/cat-file.c | 6 ++---- fetch-object-info.c | 4 ++-- fetch-object-info.h | 2 +- transport-helper.c | 7 +++++-- transport-internal.h | 6 +++++- transport.c | 14 +++++++++----- transport.h | 7 +++---- 7 files changed, 27 insertions(+), 19 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index e1650b2921ffcf..8dcad2f5ebf9f4 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -724,10 +724,8 @@ static int get_remote_info(int argc, goto cleanup; } - gtransport->smart_options->object_info_oids = object_info_oids; - - gtransport->smart_options->object_info_results = results; - retval = transport_fetch_object_info(gtransport); + retval = transport_fetch_object_info(gtransport, object_info_oids, + results); cleanup: transport_disconnect(gtransport); return retval; diff --git a/fetch-object-info.c b/fetch-object-info.c index 4db879c2dc7cc5..fe26bf4bbc9dc2 100644 --- a/fetch-object-info.c +++ b/fetch-object-info.c @@ -11,7 +11,7 @@ /* Sends object-info command and its arguments into the request buffer. */ static void send_object_info_request(const int fd_out, const struct string_list *server_options, - struct oid_array *oids, + const struct oid_array *oids, unsigned ask_size) { struct strbuf req_buf = STRBUF_INIT; @@ -49,7 +49,7 @@ static int parse_object_size(const char *s, size_t *res) void fetch_object_info(const enum protocol_version version, const struct string_list *server_options, - struct oid_array *oids, + const struct oid_array *oids, struct packet_reader *reader, struct fetch_object_info_results *results, const int stateless_rpc, diff --git a/fetch-object-info.h b/fetch-object-info.h index 97ee5314c99b00..10cf9f5f63a455 100644 --- a/fetch-object-info.h +++ b/fetch-object-info.h @@ -26,7 +26,7 @@ struct oid_array; */ void fetch_object_info(enum protocol_version version, const struct string_list *server_options, - struct oid_array *oids, + const struct oid_array *oids, struct packet_reader *reader, struct fetch_object_info_results *results, int stateless_rpc, diff --git a/transport-helper.c b/transport-helper.c index 623463dcea891a..b69cb733d85108 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -784,11 +784,14 @@ static int fetch_refs(struct transport *transport, return -1; } -static int fetch_object_info_helper(struct transport *transport) +static int fetch_object_info_helper(struct transport *transport, + const struct oid_array *oids, + struct fetch_object_info_results *results) { get_helper(transport); if (process_connect(transport, 0)) - return transport->vtable->fetch_object_info(transport); + return transport->vtable->fetch_object_info(transport, oids, + results); die(_("object-info requires protocol v2")); } diff --git a/transport-internal.h b/transport-internal.h index 60db0bedcdb9ae..a10b27cc81f511 100644 --- a/transport-internal.h +++ b/transport-internal.h @@ -7,6 +7,8 @@ struct ref; struct transport; struct strvec; struct transport_ls_refs_options; +struct oid_array; +struct fetch_object_info_results; struct transport_vtable { /** @@ -51,7 +53,9 @@ struct transport_vtable { * * Uses object-info capability of v2 protocol. */ - int (*fetch_object_info)(struct transport *transport); + int (*fetch_object_info)(struct transport *transport, + const struct oid_array *oids, + struct fetch_object_info_results *results); /** * Push the objects and refs. Send the necessary objects, and diff --git a/transport.c b/transport.c index 242fdea95b5fa4..abca9ac29aa9d7 100644 --- a/transport.c +++ b/transport.c @@ -433,7 +433,9 @@ static int get_bundle_uri(struct transport *transport) transport->bundles, stateless_rpc); } -static int fetch_object_info_via_pack(struct transport *transport) +static int fetch_object_info_via_pack(struct transport *transport, + const struct oid_array *oids, + struct fetch_object_info_results *results) { int ret = 0; struct git_transport_data *data = transport->data; @@ -450,9 +452,9 @@ static int fetch_object_info_via_pack(struct transport *transport) fetch_object_info(data->version, transport->server_options, - transport->smart_options->object_info_oids, + oids, &reader, - data->options.object_info_results, + results, transport->stateless_rpc, data->fd[1]); close(data->fd[0]); @@ -465,11 +467,13 @@ static int fetch_object_info_via_pack(struct transport *transport) return ret; } -int transport_fetch_object_info(struct transport *transport) +int transport_fetch_object_info(struct transport *transport, + const struct oid_array *oids, + struct fetch_object_info_results *results) { if (!transport->vtable->fetch_object_info) die(_("remote does not support object-info")); - return transport->vtable->fetch_object_info(transport); + return transport->vtable->fetch_object_info(transport, oids, results); } static int fetch_refs_via_pack(struct transport *transport, diff --git a/transport.h b/transport.h index 6948b65db984be..39193d0077a312 100644 --- a/transport.h +++ b/transport.h @@ -57,9 +57,6 @@ struct git_transport_options { * common commits to this oidset instead of fetching any packfiles. */ struct oidset *acked_commits; - - struct oid_array *object_info_oids; - struct fetch_object_info_results *object_info_results; }; enum transport_family { @@ -317,7 +314,9 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs); /* * Fetch the object info from remote */ -int transport_fetch_object_info(struct transport *transport); +int transport_fetch_object_info(struct transport *transport, + const struct oid_array *oids, + struct fetch_object_info_results *results); /* * If this flag is set, unlocking will avoid to call non-async-signal-safe From 7692fa90199c623629f07e8883f5df8cfa859e03 Mon Sep 17 00:00:00 2001 From: Pablo Sabater Date: Sat, 8 Aug 2026 02:02:22 +0200 Subject: [PATCH 15/43] protocol-caps: add type support to object-info Teach the server-side object-info handler to accept type as a requested field. When the client includes type in its object-info request, the server returns the requested object type. While touching send_info(), wrap an over-long line and fix the bit field style of requested_info.size. Mentored-by: Karthik Nayak Mentored-by: Chandra Pratap Signed-off-by: Pablo Sabater Signed-off-by: Junio C Hamano --- protocol-caps.c | 21 ++++++++++++++++++--- t/t5701-git-serve.sh | 30 ++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/protocol-caps.c b/protocol-caps.c index 02261be14d817a..27e0f85b100cb9 100644 --- a/protocol-caps.c +++ b/protocol-caps.c @@ -11,7 +11,8 @@ #include "strbuf.h" struct requested_info { - unsigned size : 1; + unsigned size:1; + unsigned type:1; }; /* @@ -73,15 +74,20 @@ static void send_info(struct repository *r, struct packet_writer *writer, if (info->size) packet_writer_write(writer, "size"); + if (info->type) + packet_writer_write(writer, "type"); + for_each_string_list_item (item, oid_str_list) { const char *oid_str = item->string; + enum object_type object_type; struct object_id oid; size_t object_size; if (get_oid_hex_algop(oid_str, &oid, r->hash_algo) < 0) { packet_writer_error( writer, - "object-info: protocol error, expected to get oid, not '%s'", + "object-info: protocol error, expected to get " + "oid, not '%s'", oid_str); continue; } @@ -93,7 +99,8 @@ static void send_info(struct repository *r, struct packet_writer *writer, * If an object is not recognized by the server append SP to * the response. */ - if (get_object_info(r->objects, &oid, &object_size) <= OBJ_NONE) { + object_type = get_object_info(r->objects, &oid, &object_size); + if (object_type <= OBJ_NONE) { strbuf_addstr(&send_buffer, " "); goto write; } @@ -103,6 +110,9 @@ static void send_info(struct repository *r, struct packet_writer *writer, (uintmax_t)object_size); } + if (info->type) + strbuf_addf(&send_buffer, " %s", type_name(object_type)); + write: packet_writer_write(writer, "%s", send_buffer.buf); strbuf_reset(&send_buffer); @@ -124,6 +134,11 @@ int cap_object_info(struct repository *r, struct packet_reader *request) continue; } + if (!strcmp("type", request->line)) { + info.type = 1; + continue; + } + if (parse_oid(request->line, &oid_str_list)) continue; diff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh index 51d5dd1ae6f389..f57e36a88d3cfb 100755 --- a/t/t5701-git-serve.sh +++ b/t/t5701-git-serve.sh @@ -369,6 +369,36 @@ test_expect_success 'basics of object-info' ' test_cmp expect actual ' +test_expect_success 'object-info supports type' ' + test_config transfer.advertiseObjectInfo true && + + two_oid=$(git rev-parse two:two.t) && + two_size=$(test_file_size two.t) && + + test-tool pkt-line pack >in <<-EOF && + command=object-info + object-format=$(test_oid algo) + 0001 + size + type + oid $two_oid + oid $two_oid + 0000 + EOF + + cat >expect <<-EOF && + size + type + $two_oid $two_size blob + $two_oid $two_size blob + 0000 + EOF + + test-tool serve-v2 --stateless-rpc out && + test-tool pkt-line unpack actual && + test_cmp expect actual +' + test_expect_success 'bare OID request' ' test_config transfer.advertiseObjectInfo true && From afa4ea56fedc3ca04d3b00a3ef930833e7ad68ea Mon Sep 17 00:00:00 2001 From: Pablo Sabater Date: Sat, 8 Aug 2026 02:02:23 +0200 Subject: [PATCH 16/43] fetch-object-info: parse type from server response The server can handle type requests but does not advertise the capability yet. Prepare the client to know how to parse the server response once the server advertises the type capability. Mentored-by: Karthik Nayak Mentored-by: Chandra Pratap Signed-off-by: Pablo Sabater Signed-off-by: Junio C Hamano --- builtin/cat-file.c | 7 +++++++ fetch-object-info.c | 38 +++++++++++++++++++++++++++++++++++--- fetch-object-info.h | 3 +++ 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 8dcad2f5ebf9f4..85020200835337 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -842,6 +842,8 @@ static void parse_cmd_remote_object_info(struct batch_options *opt, if (data->info.sizep) results.wants_size = 1; + if (data->info.typep) + results.wants_type = 1; if (get_remote_info(count, argv, &results, &object_info_oids)) die(_("failed to get object info from the remote: %s"), argv[0]); @@ -850,6 +852,8 @@ static void parse_cmd_remote_object_info(struct batch_options *opt, string_list_append(&data->remote_allowed_atoms, "objectname"); if (results.sizes) string_list_append(&data->remote_allowed_atoms, "objectsize"); + if (results.types) + string_list_append(&data->remote_allowed_atoms, "objecttype"); data->skip_object_info = 1; for (size_t i = 0; i < results.nr; i++) { @@ -868,6 +872,9 @@ static void parse_cmd_remote_object_info(struct batch_options *opt, if (results.sizes) data->size = results.sizes[i]; + if (results.types) + data->type = results.types[i]; + opt->batch_mode = BATCH_MODE_INFO; data->is_remote = 1; batch_object_write(argv[i + 1], output, opt, data, NULL, 0); diff --git a/fetch-object-info.c b/fetch-object-info.c index fe26bf4bbc9dc2..0a58308f9b2559 100644 --- a/fetch-object-info.c +++ b/fetch-object-info.c @@ -1,6 +1,7 @@ #include "git-compat-util.h" #include "gettext.h" #include "hex.h" +#include "object.h" #include "pkt-line.h" #include "connect.h" #include "oid-array.h" @@ -12,7 +13,8 @@ static void send_object_info_request(const int fd_out, const struct string_list *server_options, const struct oid_array *oids, - unsigned ask_size) + unsigned ask_size, + unsigned ask_type) { struct strbuf req_buf = STRBUF_INIT; @@ -21,6 +23,9 @@ static void send_object_info_request(const int fd_out, if (ask_size) packet_buf_write(&req_buf, "size"); + if (ask_type) + packet_buf_write(&req_buf, "type"); + if (oids) for (size_t i = 0; i < oids->nr; i++) packet_buf_write(&req_buf, "oid %s", @@ -56,7 +61,9 @@ void fetch_object_info(const enum protocol_version version, const int fd_out) { unsigned ask_size = 0; + unsigned ask_type = 0; int size_index = -1; + int type_index = -1; size_t wanted; results->nr = oids->nr; @@ -71,11 +78,16 @@ void fetch_object_info(const enum protocol_version version, server_supports_feature("object-info", "size", 0)) ask_size = 1; + if (results->wants_type && + server_supports_feature("object-info", "type", 0)) + ask_type = 1; + /* * Even if no options are left, we still send the oid so we get * at least an existence check. */ - send_object_info_request(fd_out, server_options, oids, ask_size); + send_object_info_request(fd_out, server_options, oids, ask_size, + ask_type); break; case protocol_v1: case protocol_v0: @@ -83,7 +95,7 @@ void fetch_object_info(const enum protocol_version version, case protocol_unknown_version: BUG("unknown protocol version"); } - wanted = ask_size; + wanted = ask_size + ask_type; for (size_t i = 0; i < wanted; i++) { if (packet_reader_read(reader) != PACKET_READ_NORMAL) { @@ -100,6 +112,13 @@ void fetch_object_info(const enum protocol_version version, die(_("object-info: duplicate 'size' attribute")); size_index = (int)i; CALLOC_ARRAY(results->sizes, results->nr); + } else if (!strcmp(reader->line, "type")) { + if (!ask_type) + die(_("object-info: unrequested 'type' attribute")); + if (results->types) + die(_("object-info: duplicate 'type' attribute")); + type_index = (int)i; + CALLOC_ARRAY(results->types, results->nr); } else { die(_("object-info: unknown attribute '%s'"), reader->line); @@ -149,6 +168,18 @@ void fetch_object_info(const enum protocol_version version, object_info_values.items[0].string, object_info_values.items[size_index + 1].string); + if (results->types) { + const char *type_str = + object_info_values.items[type_index + 1].string; + int type = type_from_string_gently(type_str, -1, 1); + + if (type < 0) + die(_("object-info: object %s has invalid type '%s'"), + object_info_values.items[0].string, type_str); + + results->types[i] = type; + } + string_list_clear(&object_info_values, 0); } @@ -162,6 +193,7 @@ void fetch_object_info(const enum protocol_version version, void free_fetch_object_info_results(struct fetch_object_info_results *results) { free(results->sizes); + free(results->types); free(results->unrecognized); memset(results, 0, sizeof(*results)); } diff --git a/fetch-object-info.h b/fetch-object-info.h index 10cf9f5f63a455..2fba96c6f7de52 100644 --- a/fetch-object-info.h +++ b/fetch-object-info.h @@ -1,14 +1,17 @@ #ifndef FETCH_OBJECT_INFO_H #define FETCH_OBJECT_INFO_H +#include "object.h" #include "pkt-line.h" #include "protocol.h" struct fetch_object_info_results { size_t *sizes; + enum object_type *types; uint8_t *unrecognized; size_t nr; unsigned wants_size:1; + unsigned wants_type:1; }; #define FETCH_OBJECT_INFO_RESULTS_INIT { 0 } From 4c842de0e4580dc217d9ba5519e8348915f9c816 Mon Sep 17 00:00:00 2001 From: Pablo Sabater Date: Sat, 8 Aug 2026 02:02:24 +0200 Subject: [PATCH 17/43] serve: advertise type capability The server and the client can handle type requests but the client won't ask for it until the server advertises it. Add type to the advertised capabilities so the client knows that it can request it. Mentored-by: Karthik Nayak Mentored-by: Chandra Pratap Signed-off-by: Pablo Sabater Signed-off-by: Junio C Hamano --- serve.c | 4 ++-- t/t1017-cat-file-remote-object-info.sh | 26 ++++++++++++++++++++++---- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/serve.c b/serve.c index 2b07d922b3dde1..2ce513cf2d5892 100644 --- a/serve.c +++ b/serve.c @@ -97,9 +97,9 @@ static int object_info_advertise(struct repository *r, struct strbuf *value) /* disabled by default */ advertise_object_info = 0; } - /* Currently only size is supported */ + /* Currently only size and type are supported */ if (value && advertise_object_info) - strbuf_addstr(value, "size"); + strbuf_addstr(value, "size type"); return advertise_object_info; } diff --git a/t/t1017-cat-file-remote-object-info.sh b/t/t1017-cat-file-remote-object-info.sh index 116862f9d0b447..190c45eefc21bd 100755 --- a/t/t1017-cat-file-remote-object-info.sh +++ b/t/t1017-cat-file-remote-object-info.sh @@ -7,6 +7,7 @@ test_description='git cat-file --batch-command with remote-object-info command' hello_content="Hello World" hello_size=$(strlen "$hello_content") +hello_type="blob" hello_oid=$(echo_without_newline "$hello_content" | git hash-object --stdin) hello_short_oid=$(git rev-parse --short "$hello_oid") @@ -19,6 +20,7 @@ unstored_oid=$(echo_without_newline "$unstored_content" | git hash-object --stdi # file name is hello, which is 5 characters # a space is 1 character and a null is 1 character tree_size=$(($(test_oid rawsz) + 13)) +tree_type="tree" commit_message="Initial commit" @@ -31,6 +33,7 @@ commit_message="Initial commit" # An easier way to calculate is: 1. use `git cat-file commit | wc -c`, # to get 177, 2. then deduct 40 hex characters to get 137 commit_size=$(($(test_oid hexsz) + 137)) +commit_type="commit" tag_header_without_oid="type blob tag hellotag @@ -44,6 +47,7 @@ $tag_description" tag_oid=$(echo_without_newline "$tag_content" | git hash-object -t tag --stdin -w) tag_size=$(strlen "$tag_content") +tag_type="tag" set_transport_variables () { hello_oid=$(echo_without_newline "$hello_content" | git hash-object --stdin) @@ -256,14 +260,12 @@ test_expect_success 'remote-object-info does not die on missing oid like info' ' ) ' -# This tests depends on %(objecttype) not being supported yet, once supported -# it needs to be updated. -test_expect_success 'unsupported placeholder on remote returns empty string' ' +test_expect_success 'objecttype is supported by remote-object-info' ' ( set_transport_variables "$daemon_parent" && cd "$daemon_parent/daemon_client_empty" && - echo "" >expect && + echo "$hello_type" >expect && git cat-file --batch-command="%(objecttype)" >actual <<-EOF && remote-object-info "$GIT_DAEMON_URL/parent" $hello_oid EOF @@ -271,6 +273,22 @@ test_expect_success 'unsupported placeholder on remote returns empty string' ' ) ' +test_expect_success 'unsupported placeholders on remote return empty string' ' + ( + set_transport_variables "$daemon_parent" && + cd "$daemon_parent/daemon_client_empty" && + + fmt="%(objectmode) %(objectsize:disk) %(rest) %(deltabase)" && + + # The hardcoded SPs between the atoms are respected. + echo " " >expect && + git cat-file --batch-command="$fmt" >actual <<-EOF && + remote-object-info "$GIT_DAEMON_URL/parent" $hello_oid + EOF + test_cmp expect actual + ) +' + test_expect_success 'requesting only objectname echoes back' ' ( set_transport_variables "$daemon_parent" && From 245f2a8b2efb1cf93358cd4c143d0a91c23e0e1d Mon Sep 17 00:00:00 2001 From: Pablo Sabater Date: Sat, 8 Aug 2026 02:02:25 +0200 Subject: [PATCH 18/43] cat-file: unify default format %(objecttype) is supported both by the client and by the server. Change the temporary default format to the unified version that the other commands use. Update documentation to remove %(objecttype) from the caveats of remote-object-info and show %(objecttype) support. Now that type is supported and the default format unified, update the tests to expect the new default format. Mentored-by: Karthik Nayak Mentored-by: Chandra Pratap Signed-off-by: Pablo Sabater Signed-off-by: Junio C Hamano --- Documentation/git-cat-file.adoc | 17 ++++----- Documentation/gitprotocol-v2.adoc | 18 +++++++-- builtin/cat-file.c | 7 ---- t/t1017-cat-file-remote-object-info.sh | 52 +++++++++++++------------- 4 files changed, 47 insertions(+), 47 deletions(-) diff --git a/Documentation/git-cat-file.adoc b/Documentation/git-cat-file.adoc index ac3b528c6f00f6..514bfc00328caf 100644 --- a/Documentation/git-cat-file.adoc +++ b/Documentation/git-cat-file.adoc @@ -348,15 +348,12 @@ newline. The available atoms are: after that first run of whitespace (i.e., the "rest" of the line) are output in place of the `%(rest)` atom. -The command `remote-object-info` only supports the `%(objectname)` and -`%(objectsize)` placeholders. See `CAVEATS` below for more information. +The command `remote-object-info` only supports the `%(objectname)`, +`%(objectsize)` and `%(objecttype)` placeholders. See `CAVEATS` below for more +information. If no format is specified, the default format is `%(objectname) -%(objecttype) %(objectsize)`, except for `remote-object-info` commands which -use `%(objectname) %(objectsize)` because `%(objecttype)` is not supported yet. - -WARNING: When "%(objecttype)" is supported, the default format WILL be unified, -so DO NOT RELY on the current default format to stay the same!!! +%(objecttype) %(objectsize)`. If `--batch` is specified, or if `--batch-command` is used with the `contents` command, the object information is followed by the object contents (consisting @@ -453,9 +450,9 @@ scripting purposes. CAVEATS ------- -Note that only `%(objectname)` and `%(objectsize)` are currently -supported by the `remote-object-info` command. Using any other placeholder in -the format string will return an empty string in its position. +Note that only `%(objectname)`, `%(objectsize)` and `%(objecttype)` are +currently supported by the `remote-object-info` command. Using any other +placeholder in the format string will return an empty string in its position. Note that the sizes of objects on disk are reported accurately, but care should be taken in drawing conclusions about which refs or objects are diff --git a/Documentation/gitprotocol-v2.adoc b/Documentation/gitprotocol-v2.adoc index 7bf62014c3917d..dd52fd8110dcf1 100644 --- a/Documentation/gitprotocol-v2.adoc +++ b/Documentation/gitprotocol-v2.adoc @@ -558,14 +558,17 @@ object-info `object-info` is the command to retrieve information about one or more objects. Its main purpose is to allow a client to make decisions based on this -information without having to fully fetch objects. Object size is the only -information that is currently supported. +information without having to fully fetch objects. Currently only object size +and type are supported. An `object-info` request takes the following arguments: size Requests size information to be returned for each listed object id. + type + Requests type information to be returned for each listed object id. + oid Indicates to the server an object which the client wants to obtain information for. They must be full OIDs. @@ -580,11 +583,18 @@ space. info = *PKT-LINE(attr LF) *PKT-LINE(obj-info LF) - attr = "size" + attr = "size" | "type" obj-size = 1*DIGIT - obj-info = obj-id [SP [obj-size]] + obj-type = "blob" | "tree" | "commit" | "tag" + + obj-val = obj-size | obj-type + + obj-info = obj-id [SP [obj-val *(SP obj-val)]] + +The values in `obj-info` appear in the same order as the corresponding `attr` +lines, with exactly one value per requested attribute. If the server does not recognize the OID, the response will be ` SP` regardless of the number of attributes requested. diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 85020200835337..011acdec09ef61 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -821,15 +821,9 @@ static void parse_cmd_remote_object_info(struct batch_options *opt, char *line_to_split; struct fetch_object_info_results results = FETCH_OBJECT_INFO_RESULTS_INIT; struct oid_array object_info_oids = OID_ARRAY_INIT; - const char *saved_format = opt->format; if (strlen(line) >= MAX_REMOTE_OBJ_INFO_LINE) die(_("remote-object-info command too long")); - /* - * TODO: Use the default format once %(objecttype) is supported. - */ - if (!opt->format) - opt->format = "%(objectname) %(objectsize)"; line_to_split = xstrdup(line); count = split_cmdline(line_to_split, &argv); @@ -881,7 +875,6 @@ static void parse_cmd_remote_object_info(struct batch_options *opt, data->is_remote = 0; } data->skip_object_info = 0; - opt->format = saved_format; free_fetch_object_info_results(&results); free(line_to_split); diff --git a/t/t1017-cat-file-remote-object-info.sh b/t/t1017-cat-file-remote-object-info.sh index 190c45eefc21bd..e2919aa061830a 100755 --- a/t/t1017-cat-file-remote-object-info.sh +++ b/t/t1017-cat-file-remote-object-info.sh @@ -139,10 +139,10 @@ test_expect_success 'batch-command remote-object-info git:// default filter' ' set_transport_variables "$daemon_parent" && cd "$daemon_parent/daemon_client_empty" && - echo "$hello_oid $hello_size" >expect && - echo "$tree_oid $tree_size" >>expect && - echo "$commit_oid $commit_size" >>expect && - echo "$tag_oid $tag_size" >>expect && + echo "$hello_oid $hello_type $hello_size" >expect && + echo "$tree_oid $tree_type $tree_size" >>expect && + echo "$commit_oid $commit_type $commit_size" >>expect && + echo "$tag_oid $tag_type $tag_size" >>expect && git cat-file --batch-command >actual <<-EOF && remote-object-info "$GIT_DAEMON_URL/parent" $hello_oid $tree_oid @@ -152,7 +152,7 @@ test_expect_success 'batch-command remote-object-info git:// default filter' ' ) ' -test_expect_success 'remote-object-info does not change the default format of info' ' +test_expect_success 'remote-object-info and info can be mixed using the unified default format' ' ( set_transport_variables "$daemon_parent" && cd "$daemon_parent/daemon_client_empty" && @@ -162,7 +162,7 @@ test_expect_success 'remote-object-info does not change the default format of in local_size=$(strlen "$local_content") && echo "$local_oid blob $local_size" >expect && - echo "$hello_oid $hello_size" >>expect && + echo "$hello_oid blob $hello_size" >>expect && echo "$local_oid blob $local_size" >>expect && git cat-file --batch-command >actual <<-EOF && @@ -209,10 +209,10 @@ test_expect_success 'batch-command -Z remote-object-info git:// default filter' set_transport_variables "$daemon_parent" && cd "$daemon_parent/daemon_client_empty" && - printf "%s\0" "$hello_oid $hello_size" >expect && - printf "%s\0" "$tree_oid $tree_size" >>expect && - printf "%s\0" "$commit_oid $commit_size" >>expect && - printf "%s\0" "$tag_oid $tag_size" >>expect && + printf "%s\0" "$hello_oid $hello_type $hello_size" >expect && + printf "%s\0" "$tree_oid $tree_type $tree_size" >>expect && + printf "%s\0" "$commit_oid $commit_type $commit_size" >>expect && + printf "%s\0" "$tag_oid $tag_type $tag_size" >>expect && printf "%s\0" "$hello_oid missing" >>expect && printf "%s\0" "$tree_oid missing" >>expect && @@ -448,10 +448,10 @@ test_expect_success 'batch-command remote-object-info file:// default filter' ' server_path="$(pwd)/server" && cd file_client_empty && - echo "$hello_oid $hello_size" >expect && - echo "$tree_oid $tree_size" >>expect && - echo "$commit_oid $commit_size" >>expect && - echo "$tag_oid $tag_size" >>expect && + echo "$hello_oid $hello_type $hello_size" >expect && + echo "$tree_oid $tree_type $tree_size" >>expect && + echo "$commit_oid $commit_type $commit_size" >>expect && + echo "$tag_oid $tag_type $tag_size" >>expect && git cat-file --batch-command >actual <<-EOF && remote-object-info "file://${server_path}" $hello_oid $tree_oid @@ -467,10 +467,10 @@ test_expect_success 'batch-command -Z remote-object-info file:// default filter' server_path="$(pwd)/server" && cd file_client_empty && - printf "%s\0" "$hello_oid $hello_size" >expect && - printf "%s\0" "$tree_oid $tree_size" >>expect && - printf "%s\0" "$commit_oid $commit_size" >>expect && - printf "%s\0" "$tag_oid $tag_size" >>expect && + printf "%s\0" "$hello_oid $hello_type $hello_size" >expect && + printf "%s\0" "$tree_oid $tree_type $tree_size" >>expect && + printf "%s\0" "$commit_oid $commit_type $commit_size" >>expect && + printf "%s\0" "$tag_oid $tag_type $tag_size" >>expect && printf "%s\0" "$hello_oid missing" >>expect && printf "%s\0" "$tree_oid missing" >>expect && @@ -618,10 +618,10 @@ test_expect_success 'batch-command remote-object-info http:// default filter' ' set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && cd "$HTTPD_DOCUMENT_ROOT_PATH/http_client_empty" && - echo "$hello_oid $hello_size" >expect && - echo "$tree_oid $tree_size" >>expect && - echo "$commit_oid $commit_size" >>expect && - echo "$tag_oid $tag_size" >>expect && + echo "$hello_oid $hello_type $hello_size" >expect && + echo "$tree_oid $tree_type $tree_size" >>expect && + echo "$commit_oid $commit_type $commit_size" >>expect && + echo "$tag_oid $tag_type $tag_size" >>expect && git cat-file --batch-command >actual <<-EOF && remote-object-info "$HTTPD_URL/smart/http_parent" $hello_oid $tree_oid @@ -636,10 +636,10 @@ test_expect_success 'batch-command -Z remote-object-info http:// default filter' set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && cd "$HTTPD_DOCUMENT_ROOT_PATH/http_client_empty" && - printf "%s\0" "$hello_oid $hello_size" >expect && - printf "%s\0" "$tree_oid $tree_size" >>expect && - printf "%s\0" "$commit_oid $commit_size" >>expect && - printf "%s\0" "$tag_oid $tag_size" >>expect && + printf "%s\0" "$hello_oid $hello_type $hello_size" >expect && + printf "%s\0" "$tree_oid $tree_type $tree_size" >>expect && + printf "%s\0" "$commit_oid $commit_type $commit_size" >>expect && + printf "%s\0" "$tag_oid $tag_type $tag_size" >>expect && batch_input="remote-object-info $HTTPD_URL/smart/http_parent $hello_oid $tree_oid remote-object-info $HTTPD_URL/smart/http_parent $commit_oid $tag_oid From b0b304c10a65882a8e36abbb694603648a78edfd Mon Sep 17 00:00:00 2001 From: Harald Nordgren Date: Mon, 10 Aug 2026 17:53:32 +0000 Subject: [PATCH 19/43] send-email: clarify missing subject error Clarify that a message file is missing a 'Subject:' line. Terminate the error with a newline so Perl does not append its internal source location. Signed-off-by: Harald Nordgren Signed-off-by: Junio C Hamano --- git-send-email.perl | 2 +- t/t9001-send-email.sh | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/git-send-email.perl b/git-send-email.perl index bb8ddd1eef2c25..2071cff6ae3f0a 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -863,7 +863,7 @@ sub get_patch_subject { return "GIT: $1\n"; } close $fh; - die sprintf(__("No subject line in %s?"), $fn); + die sprintf(__("No 'Subject:' line in '%s'\n"), $fn); } if ($compose) { diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index e9d814a34aa86c..d1393ef1978761 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -1422,6 +1422,21 @@ test_expect_success $PREREQ 'detects ambiguous reference/file conflict' ' test_grep disambiguate errors ' +test_expect_success $PREREQ 'missing subject omits Perl location' ' + cat >no-subject.patch <<-\EOF && + This is the body. + EOF + test_must_fail git send-email \ + --dry-run \ + --from="Example " \ + --to=nobody@example.com \ + no-subject.patch 2>actual && + cat >expect <<-\EOF && + No '\''Subject:'\'' line in '\''no-subject.patch'\'' + EOF + test_cmp expect actual +' + test_expect_success $PREREQ 'feed two files' ' rm -fr outdir && git format-patch -2 -o outdir && From 5ab204e7dfe4e5affb779c406a24b3b6e46ae54e Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 11 Aug 2026 10:33:03 +0200 Subject: [PATCH 20/43] parse-options: introduce OPT_HIDDEN_GROUP Hidden options are not shown by `git -h`, but are still shown by `git --help-all`. If there are a lot of hidden options or if they don't belong to the same categories as other options, there is currently no way to properly group them. Using `OPT_GROUP("Foo")` means that "Foo" will always be shown which we don't want if that group contains only hidden options. To provide a way to have groups shown only when hidden options are shown, let's implement an OPT_HIDDEN_GROUP macro. To test this new macro, let's also improve `test-tool parse-options` and test its output with `--help-all`. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- parse-options.c | 4 ++-- parse-options.h | 5 +++++ t/helper/test-parse-options.c | 4 ++++ t/t0040-parse-options.sh | 25 ++++++++++++++++++++++++- 4 files changed, 35 insertions(+), 3 deletions(-) diff --git a/parse-options.c b/parse-options.c index 08c21d9fc0a585..4519ead9dc77b8 100644 --- a/parse-options.c +++ b/parse-options.c @@ -1414,6 +1414,8 @@ static enum parse_opt_result usage_with_options_internal(struct parse_opt_ctx_t if (opts->type == OPTION_SUBCOMMAND) continue; + if (!full && (opts->flags & PARSE_OPT_HIDDEN)) + continue; if (opts->type == OPTION_GROUP) { fputc('\n', outfile); need_newline = 0; @@ -1421,8 +1423,6 @@ static enum parse_opt_result usage_with_options_internal(struct parse_opt_ctx_t fprintf(outfile, "%s\n", _(opts->help)); continue; } - if (!full && (opts->flags & PARSE_OPT_HIDDEN)) - continue; if (need_newline) { fputc('\n', outfile); diff --git a/parse-options.h b/parse-options.h index 3ec8ba5cc83c60..d7f896a9337024 100644 --- a/parse-options.h +++ b/parse-options.h @@ -237,6 +237,11 @@ struct option { .type = OPTION_GROUP, \ .help = (h), \ } +#define OPT_HIDDEN_GROUP(h) { \ + .type = OPTION_GROUP, \ + .help = (h), \ + .flags = PARSE_OPT_HIDDEN, \ +} #define OPT_BIT(s, l, v, h, b) OPT_BIT_F(s, l, v, h, b, 0) #define OPT_BITOP(s, l, v, h, set, clear) { \ .type = OPTION_BITOP, \ diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c index 68579d83f3939e..f181f0c02d355a 100644 --- a/t/helper/test-parse-options.c +++ b/t/helper/test-parse-options.c @@ -209,6 +209,10 @@ int cmd__parse_options(int argc, const char **argv) OPT_GROUP("Alias"), OPT_STRING('A', "alias-source", &string, "string", "get a string"), OPT_ALIAS('Z', "alias-target", "alias-source"), + OPT_HIDDEN_GROUP("Hidden options"), + OPT_HIDDEN_BOOL(0, "hidden-bool", &boolean, "get a boolean"), + OPT_INTEGER_F('k', "hidden-integer", &integer, "get a integer", + PARSE_OPT_HIDDEN), OPT_END(), }; int ret = 0; diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh index a22533f9ed6d16..449fff4d34b172 100755 --- a/t/t0040-parse-options.sh +++ b/t/t0040-parse-options.sh @@ -7,7 +7,7 @@ test_description='our own option parser' . ./test-lib.sh -cat >expect <<\EOF +cat >expect-part1 <<\EOF usage: test-tool parse-options A helper function for the parse-options API. @@ -41,6 +41,9 @@ String options --[no-]string2 get another string --[no-]st get another string (pervert ordering) -o get another string +EOF + +cat >expect-part2 <<\EOF --longhelp help text of this entry spans multiple lines --[no-]list add str to list @@ -67,12 +70,32 @@ Alias EOF +cat >expect-noop <<\EOF + --[no-]obsolete no-op (backward compatibility) +EOF + +cat >expect-hidden <<\EOF +Hidden options + --[no-]hidden-bool get a boolean + -k, --[no-]hidden-integer + get a integer + +EOF + test_expect_success 'test help' ' + cat expect-part1 expect-part2 >expect && test-tool parse-options -h >output 2>output.err && test_must_be_empty output.err && test_cmp expect output ' +test_expect_success 'test --help-all shows hidden group and options' ' + cat expect-part1 expect-noop expect-part2 expect-hidden >expect-help-all && + test-tool parse-options --help-all >output 2>output.err && + test_must_be_empty output.err && + test_cmp expect-help-all output +' + mv expect expect.err check () { From e455bade3a5167c36b42b3b8aa3c690722597c5b Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 11 Aug 2026 10:33:04 +0200 Subject: [PATCH 21/43] api-parse-options.adoc: document per-option flags The "Flags" section in "Documentation/technical/api-parse-options.adoc" documents the flags that can be passed to parse_options() itself. It does not, however, document the flags that can be set on individual options through the `flags` member of `struct option` (and through the `OPT_*_F()` macro variants). These per-option flags are used throughout the codebase (for example `PARSE_OPT_HIDDEN` is used to hide an option from `-h` while still showing it with `--help-all`), but a reader currently has to dig into "parse-options.h" to find them. To remediate that, let's add an "Option flags" subsection to the "Data Structure" section, just before the list of option macros. Let's also make it explicit that these are distinct from the parse_options() flags described earlier, and let's describe the `-h` versus `--help-all` behavior for `PARSE_OPT_HIDDEN`. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- .../technical/api-parse-options.adoc | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/Documentation/technical/api-parse-options.adoc b/Documentation/technical/api-parse-options.adoc index 880eb94642587a..5602cd44b23783 100644 --- a/Documentation/technical/api-parse-options.adoc +++ b/Documentation/technical/api-parse-options.adoc @@ -150,6 +150,68 @@ Data Structure The main data structure is an array of the `option` struct, say `static struct option builtin_add_options[]`. + +Option flags +~~~~~~~~~~~~ + +Each option can carry flags in the `flags` field of its `option` +struct. These are per-option flags and are distinct from the +`parse_options()` flags described above; they are usually set through +the `OPT_*_F()` macro variants (see below) rather than by hand. They +are the bitwise-or of: + +`PARSE_OPT_OPTARG`:: + The option's argument is optional, i.e. both `--option` and + `--option=` are accepted. + +`PARSE_OPT_NOARG`:: + The option takes no argument at all. Using `--option=` + is rejected. + +`PARSE_OPT_NONEG`:: + Disable the automatically generated negated `--no-option` + form. + +`PARSE_OPT_HIDDEN`:: + Hide the option: it is omitted from the usage shown by + `git -h`, but is still shown by `git --help-all`. + The option is parsed as usual either way. This is meant for + deprecated, advanced or otherwise uncommon options. + +`PARSE_OPT_LASTARG_DEFAULT`:: + The no-argument form is only accepted when the option is the + last token on the command line; used earlier, it still + requires an argument. Should not be combined with + `PARSE_OPT_OPTARG`. + +`PARSE_OPT_NODASH`:: + The option is a single character without a leading dash, such + as the `+` used by some commands. + +`PARSE_OPT_LITERAL_ARGHELP`:: + Use the argument help string (`argh`) verbatim in the usage + output instead of surrounding it with `<>` or `[]`. Useful when + `argh` already contains a hand-formatted description. + +`PARSE_OPT_FROM_ALIAS`:: + Internal flag, set on options that were expanded from a + configured alias. It should not be set by callers. + +`PARSE_OPT_NOCOMPLETE`:: + Do not offer this option for completion. + +`PARSE_OPT_COMP_ARG`:: + The option's argument, rather than the option itself, is what + should be completed. + +`PARSE_OPT_CMDMODE`:: + The option is one of several mutually exclusive "command mode" + options that share the same variable. Using more than one of + them at once is rejected. + +Macros +~~~~~~ + There are some macros to easily define options: `OPT__ABBREV(&int_var)`:: From 08b7e358e377ec2d7cd3cfe0679261b1ffffc8a7 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 11 Aug 2026 10:33:05 +0200 Subject: [PATCH 22/43] api-parse-options.adoc: document hidden and OPT_*_F option macros In "Documentation/technical/api-parse-options.adoc", the list of option macros does not mention the `OPT_*_F()` macro variants that take a trailing `flags` argument, nor the `OPT_HIDDEN_GROUP()` and `OPT_HIDDEN_BOOL()` convenience macros. Now that a previous commit documents the per-option flags, let's document these macros too: - Add a paragraph explaining the `OPT_*_F` convention and how it relates to the per-option flags. - Document `OPT_HIDDEN_GROUP()`, introduced in a previous commit, right after `OPT_GROUP()`. - Document `OPT_HIDDEN_BOOL()` right after `OPT_BOOL()`. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- Documentation/technical/api-parse-options.adoc | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Documentation/technical/api-parse-options.adoc b/Documentation/technical/api-parse-options.adoc index 5602cd44b23783..95b7924e84e2c8 100644 --- a/Documentation/technical/api-parse-options.adoc +++ b/Documentation/technical/api-parse-options.adoc @@ -214,6 +214,13 @@ Macros There are some macros to easily define options: +Many of the macros below have an `_F` variant (for example `OPT_BOOL_F`, +`OPT_STRING_F`, `OPT_INTEGER_F`, `OPT_SET_INT_F`, `OPT_BIT_F` and +`OPT_CALLBACK_F`) that takes an additional trailing `flags` argument. +That argument is the bitwise-or of the per-option flags described in the +"Option flags" section above; the non-`_F` macros are simply defined +with `flags` set to `0`. + `OPT__ABBREV(&int_var)`:: Add `--abbrev[=]`. @@ -237,10 +244,21 @@ There are some macros to easily define options: describes the group or an empty string. Start the description with an upper-case letter. +`OPT_HIDDEN_GROUP(description)`:: + Like `OPT_GROUP()`, but the group header carries + `PARSE_OPT_HIDDEN`, so it is only shown by `--help-all` and not + by `-h`. Use it to label a group that contains only hidden + options, which would otherwise show an empty header under `-h`. + `OPT_BOOL(short, long, &int_var, description)`:: Introduce a boolean option. `int_var` is set to one with `--option` and set to zero with `--no-option`. +`OPT_HIDDEN_BOOL(short, long, &int_var, description)`:: + Like `OPT_BOOL()`, but the option carries `PARSE_OPT_HIDDEN`, + so it is hidden from `-h` while still being shown by + `--help-all`. + `OPT_COUNTUP(short, long, &int_var, description)`:: Introduce a count-up option. Each use of `--option` increments `int_var`, starting from zero From 62c1a58eebe81708f9c2408918f76713285e62a0 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 11 Aug 2026 10:33:06 +0200 Subject: [PATCH 23/43] fast-import: localize 'i' into the 'for' loops using it In cmd_fast_import(), a local variable 'i' is defined as an `unsigned int` and then used as a loop counter in four different `for (i = ...; i < ...; i++) { ... }` loops. But in three out of the four cases, `unsigned int` isn't the best type to use. To give each loop counter the type matching its bound (int/unsigned/size_t), let's localize 'i' into each loop that uses it. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- builtin/fast-import.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/builtin/fast-import.c b/builtin/fast-import.c index 6692f7cd812d0e..9fc9ebe65a6072 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -3937,8 +3937,6 @@ int cmd_fast_import(int argc, const char *prefix, struct repository *repo) { - unsigned int i; - show_usage_if_asked(argc, argv, fast_import_usage); reset_pack_idx_option(&pack_idx_opts); @@ -3959,7 +3957,7 @@ int cmd_fast_import(int argc, * line to override stream data). But we must do an early parse of any * command-line options that impact how we interpret the feature lines. */ - for (i = 1; i < argc; i++) { + for (int i = 1; i < argc; i++) { const char *arg = argv[i]; if (*arg != '-' || !strcmp(arg, "--")) break; @@ -3972,7 +3970,7 @@ int cmd_fast_import(int argc, global_prefix = prefix; rc_free = mem_pool_alloc(&fi_mem_pool, cmd_save * sizeof(*rc_free)); - for (i = 0; i < (cmd_save - 1); i++) + for (unsigned int i = 0; i < (cmd_save - 1); i++) rc_free[i].next = &rc_free[i + 1]; rc_free[cmd_save - 1].next = NULL; @@ -4035,9 +4033,9 @@ int cmd_fast_import(int argc, if (show_stats) { uintmax_t total_count = 0, duplicate_count = 0; - for (i = 0; i < ARRAY_SIZE(object_count_by_type); i++) + for (size_t i = 0; i < ARRAY_SIZE(object_count_by_type); i++) total_count += object_count_by_type[i]; - for (i = 0; i < ARRAY_SIZE(duplicate_count_by_type); i++) + for (size_t i = 0; i < ARRAY_SIZE(duplicate_count_by_type); i++) duplicate_count += duplicate_count_by_type[i]; fprintf(stderr, "%s statistics:\n", argv[0]); From a895a37c1bccbfc19892df14ea83c56ea4f50e87 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 11 Aug 2026 10:33:07 +0200 Subject: [PATCH 24/43] fast-import: use int for some bool flags The `show_stats` and `quiet` flags are meant to be parsed and used as boolean flags. To easily parse them using OPT_BOOL in a following commit, let's change their type from 'unsigned int' to just 'int'. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- builtin/fast-import.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/fast-import.c b/builtin/fast-import.c index 9fc9ebe65a6072..9c8edd7c8936e6 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -182,8 +182,8 @@ static unsigned long branch_count; static unsigned long branch_load_count; static int failure; static FILE *pack_edges; -static unsigned int show_stats = 1; -static unsigned int quiet; +static int show_stats = 1; +static int quiet; static int global_argc; static const char **global_argv; static const char *global_prefix; From 63112e71a3f62cf49a7ee8f13d1250d1f02970ac Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 11 Aug 2026 10:33:08 +0200 Subject: [PATCH 25/43] fast-import: factor out option_*() functions In a following commit we are going to use the parse-options API to start parsing options. Some options will have to be parsed using OPT_CALLBACK as they process their arguments in special ways. When the processing code is already factored out in an option_*() function, like for `--date-format`, we can reuse that function. Unfortunately for other options the processing code has not been factored out yet. Let's do it now and factor out the code that handles the following options: - `--max-pack-size=` - `--big-file-threshold=` - `--signed-commits=` - `--signed-tags=` - `--quiet` into new option_*() functions: - option_max_pack_size() - option_big_file_threshold() - option_signed_commits() - option_signed_tags() - option_quiet() so that we can reuse these functions in following commits when the parse-option API will be used. Note that there are some behavior changes as we now die() with a proper error message when git_parse_ulong() cannot parse the argument from --max-pack-size or from --big-file-threshold. Previously we would end up calling die("unknown option") instead. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- builtin/fast-import.c | 69 ++++++++++++++++++++++++++++++------------- 1 file changed, 48 insertions(+), 21 deletions(-) diff --git a/builtin/fast-import.c b/builtin/fast-import.c index 9c8edd7c8936e6..a6e3cc00332c84 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -3751,25 +3751,55 @@ static void option_rewrite_submodules(const char *arg, struct string_list *list) free(s); } +static void option_max_pack_size(const char *arg) +{ + unsigned long v; + + if (!git_parse_ulong(arg, &v)) + die(_("--max-pack-size: argument must be a non-negative integer")); + if (v < 8192) { + warning(_("max-pack-size is now in bytes, assuming --max-pack-size=%lum"), v); + v *= 1024 * 1024; + } else if (v < 1024 * 1024) { + warning(_("minimum max-pack-size is 1 MiB")); + v = 1024 * 1024; + } + max_packsize = v; +} + +static void option_big_file_threshold(const char *arg) +{ + unsigned long v; + + if (!git_parse_ulong(arg, &v)) + die(_("--big-file-threshold: argument must be a non-negative integer")); + repo_settings_set_big_file_threshold(the_repository, v); +} + +static void option_signed_commits(const char *arg) +{ + if (parse_sign_mode(arg, &signed_commit_mode, &signed_commit_keyid)) + usagef(_("unknown --signed-commits mode '%s'"), arg); +} + +static void option_signed_tags(const char *arg) +{ + if (parse_sign_mode(arg, &signed_tag_mode, &signed_tag_keyid)) + usagef(_("unknown --signed-tags mode '%s'"), arg); +} + +static void option_quiet(void) +{ + show_stats = 0; + quiet = 1; +} + static int parse_one_option(const char *option) { if (skip_prefix(option, "max-pack-size=", &option)) { - unsigned long v; - if (!git_parse_ulong(option, &v)) - return 0; - if (v < 8192) { - warning(_("max-pack-size is now in bytes, assuming --max-pack-size=%lum"), v); - v *= 1024 * 1024; - } else if (v < 1024 * 1024) { - warning(_("minimum max-pack-size is 1 MiB")); - v = 1024 * 1024; - } - max_packsize = v; + option_max_pack_size(option); } else if (skip_prefix(option, "big-file-threshold=", &option)) { - unsigned long v; - if (!git_parse_ulong(option, &v)) - return 0; - repo_settings_set_big_file_threshold(the_repository, v); + option_big_file_threshold(option); } else if (skip_prefix(option, "depth=", &option)) { option_depth(option); } else if (skip_prefix(option, "active-branches=", &option)) { @@ -3777,14 +3807,11 @@ static int parse_one_option(const char *option) } else if (skip_prefix(option, "export-pack-edges=", &option)) { option_export_pack_edges(option); } else if (skip_prefix(option, "signed-commits=", &option)) { - if (parse_sign_mode(option, &signed_commit_mode, &signed_commit_keyid)) - usagef(_("unknown --signed-commits mode '%s'"), option); + option_signed_commits(option); } else if (skip_prefix(option, "signed-tags=", &option)) { - if (parse_sign_mode(option, &signed_tag_mode, &signed_tag_keyid)) - usagef(_("unknown --signed-tags mode '%s'"), option); + option_signed_tags(option); } else if (!strcmp(option, "quiet")) { - show_stats = 0; - quiet = 1; + option_quiet(); } else if (!strcmp(option, "stats")) { show_stats = 1; } else if (!strcmp(option, "allow-unsafe-features")) { From 417afb2b4d2d2c02121300d64e874cb97e23531c Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 11 Aug 2026 10:33:09 +0200 Subject: [PATCH 26/43] fast-import: introduce 'struct fast_import_state' "builtin/fast-import.c" uses a large number of global variables. This makes it harder than necessary to reason about and improve. Especially adding new features requires adding more global variables, while modernizing and eventually libifying the code becomes more and more difficult. To start reverting the sad trend to more and more globals and to start cleaning things up, let's introduce a 'struct fast_import_state' and pass an instance of it as the first argument to many functions. This is similar to what was done for "builtin/apply.c" by introducing a 'struct apply_state', see 07d7e290ff (apply: move 'struct apply_state' to a header file, 2016-08-11) and related commits. As a first step only the 'global_argc', 'global_argv' and 'global_prefix' variables are moved into the new struct. More variables will be moved into it in the following commits. Some functions receive the new 'state' parameter only to pass it along or for future use, so they are marked with UNUSED for now to satisfy '-Werror=unused-parameter'. This is a mostly mechanical refactoring with no intended behavior change. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- builtin/fast-import.c | 294 ++++++++++++++++++++++++------------------ 1 file changed, 169 insertions(+), 125 deletions(-) diff --git a/builtin/fast-import.c b/builtin/fast-import.c index a6e3cc00332c84..0f838d8488158c 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -184,10 +184,6 @@ static int failure; static FILE *pack_edges; static int show_stats = 1; static int quiet; -static int global_argc; -static const char **global_argv; -static const char *global_prefix; - static enum sign_mode signed_tag_mode = SIGN_VERBATIM; static enum sign_mode signed_commit_mode = SIGN_VERBATIM; static const char *signed_commit_keyid; @@ -276,10 +272,29 @@ static kh_oid_map_t *sub_oid_map; /* Where to write output of cat-blob commands */ static int cat_blob_fd = STDOUT_FILENO; -static void parse_argv(void); -static void parse_get_mark(const char *p); -static void parse_cat_blob(const char *p); -static void parse_ls(const char *p, struct branch *b); +/* Command state */ +struct fast_import_state { + int argc; + const char **argv; + const char *prefix; +}; + +static void fast_import_state_init(struct fast_import_state *state, + int argc, const char **argv, + const char *prefix) +{ + memset(state, 0, sizeof(*state)); + state->argc = argc; + state->argv = argv; + state->prefix = prefix; +} + +static void parse_argv(struct fast_import_state *state); +static void parse_get_mark(struct fast_import_state *state, const char *p); +static void parse_cat_blob(struct fast_import_state *state, const char *p); +static void parse_ls(struct fast_import_state *state, + const char *p, + struct branch *b); static void for_each_mark(struct mark_set *m, uintmax_t base, each_mark_fn_t callback, void *p) { @@ -1845,7 +1860,7 @@ static void read_marks(void) } -static int read_next_command(void) +static int read_next_command(struct fast_import_state *state) { static int stdin_eof = 0; @@ -1867,7 +1882,7 @@ static int read_next_command(void) if (!seen_data_command && !starts_with(command_buf.buf, "feature ") && !starts_with(command_buf.buf, "option ")) { - parse_argv(); + parse_argv(state); } rc = rc_free; @@ -1899,22 +1914,22 @@ static void skip_optional_lf(void) ungetc(term_char, stdin); } -static void parse_mark(void) +static void parse_mark(struct fast_import_state *state) { const char *v; if (skip_prefix(command_buf.buf, "mark :", &v)) { next_mark = strtoumax(v, NULL, 10); - read_next_command(); + read_next_command(state); } else next_mark = 0; } -static void parse_original_identifier(void) +static void parse_original_identifier(struct fast_import_state *state) { const char *v; if (skip_prefix(command_buf.buf, "original-oid ", &v)) - read_next_command(); + read_next_command(state); } static int parse_data(struct strbuf *sb, uintmax_t limit, uintmax_t *len_res) @@ -2068,11 +2083,11 @@ static void parse_and_store_blob( } } -static void parse_new_blob(void) +static void parse_new_blob(struct fast_import_state *state) { - read_next_command(); - parse_mark(); - parse_original_identifier(); + read_next_command(state); + parse_mark(state); + parse_original_identifier(state); parse_and_store_blob(&last_blob, NULL, next_mark); } @@ -2368,7 +2383,9 @@ static void parse_path_space(struct strbuf *sb, const char *p, (*endp)++; } -static void file_change_m(const char *p, struct branch *b) +static void file_change_m(struct fast_import_state *state, + const char *p, + struct branch *b) { static struct strbuf path = STRBUF_INIT; struct object_entry *oe; @@ -2435,10 +2452,10 @@ static void file_change_m(const char *p, struct branch *b) if (S_ISDIR(mode)) die(_("directories cannot be specified 'inline': %s"), command_buf.buf); - while (read_next_command() != EOF) { + while (read_next_command(state) != EOF) { const char *v; if (skip_prefix(command_buf.buf, "cat-blob ", &v)) - parse_cat_blob(v); + parse_cat_blob(state, v); else { parse_and_store_blob(&last_blob, &oid, 0); break; @@ -2512,7 +2529,10 @@ static void file_change_cr(const char *p, struct branch *b, int rename) leaf.tree); } -static void note_change_n(const char *p, struct branch *b, unsigned char *old_fanout) +static void note_change_n(struct fast_import_state *state, + const char *p, + struct branch *b, + unsigned char *old_fanout) { struct object_entry *oe; struct branch *s; @@ -2577,7 +2597,7 @@ static void note_change_n(const char *p, struct branch *b, unsigned char *old_fa die(_("invalid ref name or SHA1 expression: %s"), p); if (inline_data) { - read_next_command(); + read_next_command(state); parse_and_store_blob(&last_blob, &oid, 0); } else if (oe) { if (oe->type != OBJ_BLOB) @@ -2644,7 +2664,9 @@ static void parse_from_existing(struct branch *b) } } -static int parse_objectish(struct branch *b, const char *objectish) +static int parse_objectish(struct fast_import_state *state, + struct branch *b, + const char *objectish) { struct branch *s; struct object_id oid; @@ -2687,31 +2709,34 @@ static int parse_objectish(struct branch *b, const char *objectish) b->branch_tree.tree = NULL; } - read_next_command(); + read_next_command(state); return 1; } -static int parse_from(struct branch *b) +static int parse_from(struct fast_import_state *state, struct branch *b) { const char *from; if (!skip_prefix(command_buf.buf, "from ", &from)) return 0; - return parse_objectish(b, from); + return parse_objectish(state, b, from); } -static int parse_objectish_with_prefix(struct branch *b, const char *prefix) +static int parse_objectish_with_prefix(struct fast_import_state *state, + struct branch *b, + const char *prefix) { const char *base; if (!skip_prefix(command_buf.buf, prefix, &base)) return 0; - return parse_objectish(b, base); + return parse_objectish(state, b, base); } -static struct hash_list *parse_merge(unsigned int *count) +static struct hash_list *parse_merge(struct fast_import_state *state, + unsigned int *count) { struct hash_list *list = NULL, **tail = &list, *n; const char *from; @@ -2745,7 +2770,7 @@ static struct hash_list *parse_merge(unsigned int *count) tail = &n->next; (*count)++; - read_next_command(); + read_next_command(state); } return list; } @@ -2756,7 +2781,9 @@ struct signature_data { struct strbuf data; /* The actual signature data */ }; -static void parse_one_signature(struct signature_data *sig, const char *v) +static void parse_one_signature(struct fast_import_state *state, + struct signature_data *sig, + const char *v) { char *args = xstrdup(v); /* Will be freed when sig->hash_algo is freed */ char *space = strchr(args, ' '); @@ -2781,15 +2808,15 @@ static void parse_one_signature(struct signature_data *sig, const char *v) warning(_("'unknown' signature format in gpgsig")); /* Read signature data */ - read_next_command(); + read_next_command(state); parse_data(&sig->data, 0, NULL); } -static void discard_one_signature(void) +static void discard_one_signature(struct fast_import_state *state) { struct strbuf data = STRBUF_INIT; - read_next_command(); + read_next_command(state); parse_data(&data, 0, NULL); strbuf_release(&data); } @@ -2827,13 +2854,14 @@ static void store_signature(struct signature_data *stored_sig, } } -static void import_one_signature(struct signature_data *sig_sha1, +static void import_one_signature(struct fast_import_state *state, + struct signature_data *sig_sha1, struct signature_data *sig_sha256, const char *v) { struct signature_data sig = { NULL, NULL, STRBUF_INIT }; - parse_one_signature(&sig, v); + parse_one_signature(state, &sig, v); if (!strcmp(sig.hash_algo, "sha1")) store_signature(sig_sha1, &sig, "SHA-1"); @@ -2947,7 +2975,7 @@ static void handle_signature_if_invalid(struct strbuf *new_data, strbuf_release(&tmp_buf); } -static void parse_new_commit(const char *arg) +static void parse_new_commit(struct fast_import_state *state, const char *arg) { static struct strbuf msg = STRBUF_INIT; struct signature_data sig_sha1 = { NULL, NULL, STRBUF_INIT }; @@ -2965,16 +2993,16 @@ static void parse_new_commit(const char *arg) if (!b) b = new_branch(arg); - read_next_command(); - parse_mark(); - parse_original_identifier(); + read_next_command(state); + parse_mark(state); + parse_original_identifier(state); if (skip_prefix(command_buf.buf, "author ", &v)) { author = parse_ident(v); - read_next_command(); + read_next_command(state); } if (skip_prefix(command_buf.buf, "committer ", &v)) { committer = parse_ident(v); - read_next_command(); + read_next_command(state); } if (!committer) die(_("expected committer but didn't get one")); @@ -2990,7 +3018,7 @@ static void parse_new_commit(const char *arg) warning(_("stripping a commit signature")); /* fallthru */ case SIGN_STRIP: - discard_one_signature(); + discard_one_signature(state); break; /* Second, modes that parse the signature */ @@ -3001,24 +3029,24 @@ static void parse_new_commit(const char *arg) case SIGN_STRIP_IF_INVALID: case SIGN_SIGN_IF_INVALID: case SIGN_ABORT_IF_INVALID: - import_one_signature(&sig_sha1, &sig_sha256, v); + import_one_signature(state, &sig_sha1, &sig_sha256, v); break; /* Third, BUG */ default: BUG("invalid signed_commit_mode value %d", signed_commit_mode); } - read_next_command(); + read_next_command(state); } if (skip_prefix(command_buf.buf, "encoding ", &v)) { encoding = xstrdup(v); - read_next_command(); + read_next_command(state); } parse_data(&msg, 0, NULL); - read_next_command(); - parse_from(b); - merge_list = parse_merge(&merge_count); + read_next_command(state); + parse_from(state, b); + merge_list = parse_merge(state, &merge_count); /* ensure the branch is active/loaded */ if (!b->branch_tree.tree || !max_active_branches) { @@ -3031,7 +3059,7 @@ static void parse_new_commit(const char *arg) /* file_change* */ while (command_buf.len > 0) { if (skip_prefix(command_buf.buf, "M ", &v)) - file_change_m(v, b); + file_change_m(state, v, b); else if (skip_prefix(command_buf.buf, "D ", &v)) file_change_d(v, b); else if (skip_prefix(command_buf.buf, "R ", &v)) @@ -3039,18 +3067,18 @@ static void parse_new_commit(const char *arg) else if (skip_prefix(command_buf.buf, "C ", &v)) file_change_cr(v, b, 0); else if (skip_prefix(command_buf.buf, "N ", &v)) - note_change_n(v, b, &prev_fanout); + note_change_n(state, v, b, &prev_fanout); else if (!strcmp("deleteall", command_buf.buf)) file_change_deleteall(b); else if (skip_prefix(command_buf.buf, "ls ", &v)) - parse_ls(v, b); + parse_ls(state, v, b); else if (skip_prefix(command_buf.buf, "cat-blob ", &v)) - parse_cat_blob(v); + parse_cat_blob(state, v); else { unread_command_buf = 1; break; } - if (read_next_command() == EOF) + if (read_next_command(state) == EOF) break; } @@ -3188,7 +3216,7 @@ static void handle_tag_signature(struct strbuf *buf, struct strbuf *msg, const c } } -static void parse_new_tag(const char *arg) +static void parse_new_tag(struct fast_import_state *state, const char *arg) { static struct strbuf msg = STRBUF_INIT; const char *from; @@ -3207,8 +3235,8 @@ static void parse_new_tag(const char *arg) else first_tag = t; last_tag = t; - read_next_command(); - parse_mark(); + read_next_command(state); + parse_mark(state); /* from ... */ if (!skip_prefix(command_buf.buf, "from ", &from)) @@ -3236,15 +3264,15 @@ static void parse_new_tag(const char *arg) type = oe->type; } else die(_("invalid ref name or SHA1 expression: %s"), from); - read_next_command(); + read_next_command(state); /* original-oid ... */ - parse_original_identifier(); + parse_original_identifier(state); /* tagger ... */ if (skip_prefix(command_buf.buf, "tagger ", &v)) { tagger = parse_ident(v); - read_next_command(); + read_next_command(state); } else tagger = NULL; @@ -3275,7 +3303,7 @@ static void parse_new_tag(const char *arg) t->pack_id = pack_id; } -static void parse_reset_branch(const char *arg) +static void parse_reset_branch(struct fast_import_state *state, const char *arg) { struct branch *b; const char *tag_name; @@ -3292,8 +3320,8 @@ static void parse_reset_branch(const char *arg) } else b = new_branch(arg); - read_next_command(); - parse_from(b); + read_next_command(state); + parse_from(state, b); if (b->delete && skip_prefix(b->name, "refs/tags/", &tag_name)) { /* * Elsewhere, we call dump_branches() before dump_tags(), @@ -3378,7 +3406,8 @@ static void cat_blob(struct object_entry *oe, struct object_id *oid) free(buf); } -static void parse_get_mark(const char *p) +static void parse_get_mark(struct fast_import_state *state UNUSED, + const char *p) { struct object_entry *oe; char output[GIT_MAX_HEXSZ + 2]; @@ -3395,7 +3424,8 @@ static void parse_get_mark(const char *p) cat_blob_write(output, the_hash_algo->hexsz + 1); } -static void parse_cat_blob(const char *p) +static void parse_cat_blob(struct fast_import_state *state UNUSED, + const char *p) { struct object_entry *oe; struct object_id oid; @@ -3560,7 +3590,9 @@ static void print_ls(int mode, const unsigned char *hash, const char *path) cat_blob_write(line.buf, line.len); } -static void parse_ls(const char *p, struct branch *b) +static void parse_ls(struct fast_import_state *state UNUSED, + const char *p, + struct branch *b) { static struct strbuf path = STRBUF_INIT; struct tree_entry *root = NULL; @@ -3607,13 +3639,13 @@ static void checkpoint(void) dump_marks(); } -static void parse_checkpoint(void) +static void parse_checkpoint(struct fast_import_state *state UNUSED) { checkpoint_requested = 1; skip_optional_lf(); } -static void parse_progress(void) +static void parse_progress(struct fast_import_state *state UNUSED) { fwrite(command_buf.buf, 1, command_buf.len, stdout); fputc('\n', stdout); @@ -3621,37 +3653,40 @@ static void parse_progress(void) skip_optional_lf(); } -static void parse_alias(void) +static void parse_alias(struct fast_import_state *state) { struct object_entry *e; struct branch b; skip_optional_lf(); - read_next_command(); + read_next_command(state); /* mark ... */ - parse_mark(); + parse_mark(state); if (!next_mark) die(_("expected 'mark' command, got %s"), command_buf.buf); /* to ... */ memset(&b, 0, sizeof(b)); - if (!parse_objectish_with_prefix(&b, "to ")) + if (!parse_objectish_with_prefix(state, &b, "to ")) die(_("expected 'to' command, got %s"), command_buf.buf); e = find_object(&b.oid); assert(e); insert_mark(&marks, next_mark, e); } -static char* make_fast_import_path(const char *path) +static char* make_fast_import_path(struct fast_import_state *state, + const char *path) { if (!relative_marks_paths || is_absolute_path(path)) - return prefix_filename(global_prefix, path); + return prefix_filename(state->prefix, path); return repo_git_path(the_repository, "info/fast-import/%s", path); } -static void option_import_marks(const char *marks, - int from_stream, int ignore_missing) +static void option_import_marks(struct fast_import_state *state, + const char *marks, + int from_stream, + int ignore_missing) { if (import_marks_file) { if (from_stream) @@ -3663,7 +3698,7 @@ static void option_import_marks(const char *marks, } free(import_marks_file); - import_marks_file = make_fast_import_path(marks); + import_marks_file = make_fast_import_path(state, marks); import_marks_file_from_stream = from_stream; import_marks_file_ignore_missing = ignore_missing; } @@ -3703,13 +3738,15 @@ static void option_active_branches(const char *branches) max_active_branches = ulong_arg("--active-branches", branches); } -static void option_export_marks(const char *marks) +static void option_export_marks(struct fast_import_state *state, + const char *marks) { free(export_marks_file); - export_marks_file = make_fast_import_path(marks); + export_marks_file = make_fast_import_path(state, marks); } -static void option_cat_blob_fd(const char *fd) +static void option_cat_blob_fd(struct fast_import_state *state UNUSED, + const char *fd) { unsigned long n = ulong_arg("--cat-blob-fd", fd); if (n > (unsigned long) INT_MAX) @@ -3717,16 +3754,19 @@ static void option_cat_blob_fd(const char *fd) cat_blob_fd = (int) n; } -static void option_export_pack_edges(const char *edges) +static void option_export_pack_edges(struct fast_import_state *state, + const char *edges) { - char *fn = prefix_filename(global_prefix, edges); + char *fn = prefix_filename(state->prefix, edges); if (pack_edges) fclose(pack_edges); pack_edges = xfopen(fn, "a"); free(fn); } -static void option_rewrite_submodules(const char *arg, struct string_list *list) +static void option_rewrite_submodules(struct fast_import_state *state, + const char *arg, + struct string_list *list) { struct mark_set *ms; FILE *fp; @@ -3738,7 +3778,7 @@ static void option_rewrite_submodules(const char *arg, struct string_list *list) f++; CALLOC_ARRAY(ms, 1); - f = prefix_filename(global_prefix, f); + f = prefix_filename(state->prefix, f); fp = fopen(f, "r"); if (!fp) die_errno(_("cannot read '%s'"), f); @@ -3794,7 +3834,7 @@ static void option_quiet(void) quiet = 1; } -static int parse_one_option(const char *option) +static int parse_one_option(struct fast_import_state *state, const char *option) { if (skip_prefix(option, "max-pack-size=", &option)) { option_max_pack_size(option); @@ -3805,7 +3845,7 @@ static int parse_one_option(const char *option) } else if (skip_prefix(option, "active-branches=", &option)) { option_active_branches(option); } else if (skip_prefix(option, "export-pack-edges=", &option)) { - option_export_pack_edges(option); + option_export_pack_edges(state, option); } else if (skip_prefix(option, "signed-commits=", &option)) { option_signed_commits(option); } else if (skip_prefix(option, "signed-tags=", &option)) { @@ -3823,34 +3863,38 @@ static int parse_one_option(const char *option) return 1; } -static void check_unsafe_feature(const char *feature, int from_stream) +static void check_unsafe_feature(struct fast_import_state *state UNUSED, + const char *feature, + int from_stream) { if (from_stream && !allow_unsafe_features) die(_("feature '%s' forbidden in input without --allow-unsafe-features"), feature); } -static int parse_one_feature(const char *feature, int from_stream) +static int parse_one_feature(struct fast_import_state *state, + const char *feature, + int from_stream) { const char *arg; if (skip_prefix(feature, "date-format=", &arg)) { option_date_format(arg); } else if (skip_prefix(feature, "import-marks=", &arg)) { - check_unsafe_feature("import-marks", from_stream); - option_import_marks(arg, from_stream, 0); + check_unsafe_feature(state, "import-marks", from_stream); + option_import_marks(state, arg, from_stream, 0); } else if (skip_prefix(feature, "import-marks-if-exists=", &arg)) { - check_unsafe_feature("import-marks-if-exists", from_stream); - option_import_marks(arg, from_stream, 1); + check_unsafe_feature(state, "import-marks-if-exists", from_stream); + option_import_marks(state, arg, from_stream, 1); } else if (skip_prefix(feature, "export-marks=", &arg)) { - check_unsafe_feature(feature, from_stream); - option_export_marks(arg); + check_unsafe_feature(state, feature, from_stream); + option_export_marks(state, arg); } else if (!strcmp(feature, "alias")) { ; /* Don't die - this feature is supported */ } else if (skip_prefix(feature, "rewrite-submodules-to=", &arg)) { - option_rewrite_submodules(arg, &sub_marks_to); + option_rewrite_submodules(state, arg, &sub_marks_to); } else if (skip_prefix(feature, "rewrite-submodules-from=", &arg)) { - option_rewrite_submodules(arg, &sub_marks_from); + option_rewrite_submodules(state, arg, &sub_marks_from); } else if (!strcmp(feature, "get-mark")) { ; /* Don't die - this feature is supported */ } else if (!strcmp(feature, "cat-blob")) { @@ -3872,23 +3916,23 @@ static int parse_one_feature(const char *feature, int from_stream) return 1; } -static void parse_feature(const char *feature) +static void parse_feature(struct fast_import_state *state, const char *feature) { if (seen_data_command) die(_("got feature command '%s' after data command"), feature); - if (parse_one_feature(feature, 1)) + if (parse_one_feature(state, feature, 1)) return; die(_("this version of fast-import does not support feature %s."), feature); } -static void parse_option(const char *option) +static void parse_option(struct fast_import_state *state, const char *option) { if (seen_data_command) die(_("got option command '%s' after data command"), option); - if (parse_one_option(option)) + if (parse_one_option(state, option)) return; die(_("this version of fast-import does not support option: %s"), option); @@ -3924,12 +3968,12 @@ static void git_pack_config(void) static const char fast_import_usage[] = "git fast-import [--date-format=] [--max-pack-size=] [--big-file-threshold=] [--depth=] [--active-branches=] [--export-marks=]"; -static void parse_argv(void) +static void parse_argv(struct fast_import_state *state) { unsigned int i; - for (i = 1; i < global_argc; i++) { - const char *a = global_argv[i]; + for (i = 1; i < state->argc; i++) { + const char *a = state->argv[i]; if (*a != '-' || !strcmp(a, "--")) break; @@ -3937,20 +3981,20 @@ static void parse_argv(void) if (!skip_prefix(a, "--", &a)) die(_("unknown option %s"), a); - if (parse_one_option(a)) + if (parse_one_option(state, a)) continue; - if (parse_one_feature(a, 0)) + if (parse_one_feature(state, a, 0)) continue; if (skip_prefix(a, "cat-blob-fd=", &a)) { - option_cat_blob_fd(a); + option_cat_blob_fd(state, a); continue; } die(_("unknown option --%s"), a); } - if (i != global_argc) + if (i != state->argc) usage(fast_import_usage); seen_data_command = 1; @@ -3964,6 +4008,8 @@ int cmd_fast_import(int argc, const char *prefix, struct repository *repo) { + struct fast_import_state state; + show_usage_if_asked(argc, argv, fast_import_usage); reset_pack_idx_option(&pack_idx_opts); @@ -3992,9 +4038,7 @@ int cmd_fast_import(int argc, allow_unsafe_features = 1; } - global_argc = argc; - global_argv = argv; - global_prefix = prefix; + fast_import_state_init(&state, argc, argv, prefix); rc_free = mem_pool_alloc(&fi_mem_pool, cmd_save * sizeof(*rc_free)); for (unsigned int i = 0; i < (cmd_save - 1); i++) @@ -4004,34 +4048,34 @@ int cmd_fast_import(int argc, start_packfile(); set_die_routine(die_nicely); set_checkpoint_signal(); - while (read_next_command() != EOF) { + while (read_next_command(&state) != EOF) { const char *v; if (!strcmp("blob", command_buf.buf)) - parse_new_blob(); + parse_new_blob(&state); else if (skip_prefix(command_buf.buf, "commit ", &v)) - parse_new_commit(v); + parse_new_commit(&state, v); else if (skip_prefix(command_buf.buf, "tag ", &v)) - parse_new_tag(v); + parse_new_tag(&state, v); else if (skip_prefix(command_buf.buf, "reset ", &v)) - parse_reset_branch(v); + parse_reset_branch(&state, v); else if (skip_prefix(command_buf.buf, "ls ", &v)) - parse_ls(v, NULL); + parse_ls(&state, v, NULL); else if (skip_prefix(command_buf.buf, "cat-blob ", &v)) - parse_cat_blob(v); + parse_cat_blob(&state, v); else if (skip_prefix(command_buf.buf, "get-mark ", &v)) - parse_get_mark(v); + parse_get_mark(&state, v); else if (!strcmp("checkpoint", command_buf.buf)) - parse_checkpoint(); + parse_checkpoint(&state); else if (!strcmp("done", command_buf.buf)) break; else if (!strcmp("alias", command_buf.buf)) - parse_alias(); + parse_alias(&state); else if (starts_with(command_buf.buf, "progress ")) - parse_progress(); + parse_progress(&state); else if (skip_prefix(command_buf.buf, "feature ", &v)) - parse_feature(v); + parse_feature(&state, v); else if (skip_prefix(command_buf.buf, "option git ", &v)) - parse_option(v); + parse_option(&state, v); else if (starts_with(command_buf.buf, "option ")) /* ignore non-git options*/; else @@ -4043,7 +4087,7 @@ int cmd_fast_import(int argc, /* argv hasn't been parsed yet, do so */ if (!seen_data_command) - parse_argv(); + parse_argv(&state); if (require_explicit_termination && feof(stdin)) die(_("stream ends early")); From ee996af567434d3fc5204ab8f53a0ade5803303d Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 11 Aug 2026 10:33:10 +0200 Subject: [PATCH 27/43] fast-import: move command state globals into 'struct fast_import_state' A previous commit introduced 'struct fast_import_state' to hold some command state, and reduce the need for global variables. Let's continue in the same direction and move two more global variables that describe the command state into it: 'seen_data_command' and 'allow_unsafe_features'. All the sites accessing these variables are already in functions that receive the 'state' parameter (or in cmd_fast_import() which owns the struct), so no additional threading is needed. As 'state->allow_unsafe_features' is now dereferenced in check_unsafe_feature(), its 'state' parameter is no longer unused, so the UNUSED marker is removed. The fast_import_state_init() call is moved up before the early command-line scan for '--allow-unsafe-features', so that this option can be recorded directly into the struct without being clobbered by the memset() in fast_import_state_init(). This is a mechanical refactoring with no intended behavior change. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- builtin/fast-import.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/builtin/fast-import.c b/builtin/fast-import.c index 0f838d8488158c..52da29c1bde64e 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -257,9 +257,7 @@ static struct recent_command *rc_free; static unsigned int cmd_save = 100; static uintmax_t next_mark; static struct strbuf new_data = STRBUF_INIT; -static int seen_data_command; static int require_explicit_termination; -static int allow_unsafe_features; /* Signal handling */ static volatile sig_atomic_t checkpoint_requested; @@ -277,6 +275,8 @@ struct fast_import_state { int argc; const char **argv; const char *prefix; + int seen_data_command; + int allow_unsafe_features; }; static void fast_import_state_init(struct fast_import_state *state, @@ -1879,7 +1879,7 @@ static int read_next_command(struct fast_import_state *state) if (stdin_eof) return EOF; - if (!seen_data_command + if (!state->seen_data_command && !starts_with(command_buf.buf, "feature ") && !starts_with(command_buf.buf, "option ")) { parse_argv(state); @@ -3863,11 +3863,11 @@ static int parse_one_option(struct fast_import_state *state, const char *option) return 1; } -static void check_unsafe_feature(struct fast_import_state *state UNUSED, +static void check_unsafe_feature(struct fast_import_state *state, const char *feature, int from_stream) { - if (from_stream && !allow_unsafe_features) + if (from_stream && !state->allow_unsafe_features) die(_("feature '%s' forbidden in input without --allow-unsafe-features"), feature); } @@ -3918,7 +3918,7 @@ static int parse_one_feature(struct fast_import_state *state, static void parse_feature(struct fast_import_state *state, const char *feature) { - if (seen_data_command) + if (state->seen_data_command) die(_("got feature command '%s' after data command"), feature); if (parse_one_feature(state, feature, 1)) @@ -3929,7 +3929,7 @@ static void parse_feature(struct fast_import_state *state, const char *feature) static void parse_option(struct fast_import_state *state, const char *option) { - if (seen_data_command) + if (state->seen_data_command) die(_("got option command '%s' after data command"), option); if (parse_one_option(state, option)) @@ -3997,7 +3997,7 @@ static void parse_argv(struct fast_import_state *state) if (i != state->argc) usage(fast_import_usage); - seen_data_command = 1; + state->seen_data_command = 1; if (import_marks_file) read_marks(); build_mark_map(&sub_marks_from, &sub_marks_to); @@ -4012,6 +4012,8 @@ int cmd_fast_import(int argc, show_usage_if_asked(argc, argv, fast_import_usage); + fast_import_state_init(&state, argc, argv, prefix); + reset_pack_idx_option(&pack_idx_opts); git_pack_config(); @@ -4035,11 +4037,9 @@ int cmd_fast_import(int argc, if (*arg != '-' || !strcmp(arg, "--")) break; if (!strcmp(arg, "--allow-unsafe-features")) - allow_unsafe_features = 1; + state.allow_unsafe_features = 1; } - fast_import_state_init(&state, argc, argv, prefix); - rc_free = mem_pool_alloc(&fi_mem_pool, cmd_save * sizeof(*rc_free)); for (unsigned int i = 0; i < (cmd_save - 1); i++) rc_free[i].next = &rc_free[i + 1]; @@ -4086,7 +4086,7 @@ int cmd_fast_import(int argc, } /* argv hasn't been parsed yet, do so */ - if (!seen_data_command) + if (!state.seen_data_command) parse_argv(&state); if (require_explicit_termination && feof(stdin)) From 27ab5815f72848e9a25e11ee93d6b036a338933b Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 11 Aug 2026 10:33:11 +0200 Subject: [PATCH 28/43] fast-import: use struct option for usage string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently `git fast-import -h` shows the following on a single line: usage : git fast-import [--date-format=] [--max-pack-size=] \ [--big-file-threshold=] [--depth=] \ [--active-branches=] \ [--export-marks=] This output has a number of issues like: - It's missing a lot of options. - It's not consistent with the SYNOPSIS section of the doc. - With `--help-all` instead of `-h` additional hidden options should be shown, but that's not the case. - It's not standard style anymore. - Most other Git commands show additional lines for most of the options they support. Also while most commands use the parse-options API to handle their options, "builtin/fast-import.c" still doesn't use it. Let's improve on that by using the parse-options API to display the options when `-h` and `--help-all` are used. While at it, let's make the SYNOPSIS section of "Documentation/git-fast-import.adoc" consistent with the new usage string. This deliberately leaves it to future work to also use the parse-options API to actually parse the options. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- Documentation/git-fast-import.adoc | 2 +- builtin/fast-import.c | 83 +++++++++++++++++++++++++++--- t/t0450/adoc-help-mismatches | 1 - 3 files changed, 78 insertions(+), 8 deletions(-) diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc index d68bc52b7e9cd7..7c5900e048cefb 100644 --- a/Documentation/git-fast-import.adoc +++ b/Documentation/git-fast-import.adoc @@ -9,7 +9,7 @@ git-fast-import - Backend for fast Git data importers SYNOPSIS -------- [verse] -frontend | 'git fast-import' [] +'git fast-import' [] DESCRIPTION ----------- diff --git a/builtin/fast-import.c b/builtin/fast-import.c index 52da29c1bde64e..879c2860439ea3 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -30,6 +30,7 @@ #include "khash.h" #include "date.h" #include "gpg-interface.h" +#include "parse-options.h" #define PACK_ID_BITS 16 #define MAX_PACK_ID ((1<argc = argc; state->argv = argv; state->prefix = prefix; + state->option = option; } static void parse_argv(struct fast_import_state *state); @@ -3965,8 +3968,10 @@ static void git_pack_config(void) repo_config(the_repository, git_default_config, NULL); } -static const char fast_import_usage[] = -"git fast-import [--date-format=] [--max-pack-size=] [--big-file-threshold=] [--depth=] [--active-branches=] [--export-marks=]"; +static const char *const fast_import_usage[] = { + N_("git fast-import []"), + NULL +}; static void parse_argv(struct fast_import_state *state) { @@ -3995,7 +4000,7 @@ static void parse_argv(struct fast_import_state *state) die(_("unknown option --%s"), a); } if (i != state->argc) - usage(fast_import_usage); + usage_with_options(fast_import_usage, state->option); state->seen_data_command = 1; if (import_marks_file) @@ -4010,9 +4015,75 @@ int cmd_fast_import(int argc, { struct fast_import_state state; - show_usage_if_asked(argc, argv, fast_import_usage); + unsigned long pack_size_limit, big_file_threshold; + char *edges, *signed_commits, *signed_tags, *date_format; + char *import_marks_if_exists, *submodules_from, *submodules_to; - fast_import_state_init(&state, argc, argv, prefix); + /* + * NEEDSWORK: For now this is used only to render + * `-h`/`--help-all` usage messages. The actual parsing is + * done by parse_one_option()/parse_one_feature(). + */ + struct option fast_import_options[] = { + OPT_GROUP(N_("Common")), + OPT_STRING_F(0, "date-format", &date_format, N_("fmt"), + N_("format of the commit/tag dates"), PARSE_OPT_NONEG), + OPT_BOOL_F(0, "stats", &show_stats, + N_("display some basic statistics (objects, packfiles and memory)"), + PARSE_OPT_NONEG), + OPT_BOOL_F(0, "quiet", &quiet, + N_("disable the output shown by --stats"), PARSE_OPT_NONEG), + OPT_BOOL_F(0, "force", &force_update, + N_("force updating modified existing branches"), PARSE_OPT_NONEG), + OPT_BOOL_F(0, "done", &require_explicit_termination, + N_("require a terminating 'done' command"), PARSE_OPT_NONEG), + OPT_UNSIGNED(0, "max-pack-size", &pack_size_limit, + N_("maximum size of each output pack file")), + OPT_UNSIGNED(0, "big-file-threshold", &big_file_threshold, + N_("maximum size of a blob that will be deltified")), + OPT_UNSIGNED(0, "depth", &max_depth, + N_("maximum delta depth")), + OPT_UNSIGNED(0, "active-branches", &max_active_branches, + N_("maximum number of branches to maintain active")), + OPT_GROUP(N_("Marks")), + OPT_STRING_F(0, "import-marks", &import_marks_file, N_("file"), + N_("import marks from "), PARSE_OPT_NONEG), + OPT_STRING_F(0, "import-marks-if-exists", &import_marks_if_exists, N_("file"), + N_("import marks from if it exists"), PARSE_OPT_NONEG), + OPT_STRING_F(0, "export-marks", &export_marks_file, N_("file"), + N_("dump marks to "), PARSE_OPT_NONEG), + OPT_BOOL(0, "relative-marks", &relative_marks_paths, + N_("are --(import|export)-marks= paths relative to '.git/info/fast-import'?")), + OPT_GROUP(N_("Submodule rewrite")), + OPT_STRING_F(0, "rewrite-submodules-from", &submodules_from, N_("name:filename"), + N_("rewrite object IDs for submodule from "), + PARSE_OPT_NONEG), + OPT_STRING_F(0, "rewrite-submodules-to", &submodules_to, N_("name:filename"), + N_("rewrite object IDs for submodule to "), + PARSE_OPT_NONEG), + OPT_GROUP(N_("Signing")), + OPT_STRING_F(0, "signed-commits", &signed_commits, N_("mode"), + N_("how to handle signed commits"), + PARSE_OPT_NONEG), + OPT_STRING_F(0, "signed-tags", &signed_tags, N_("mode"), + N_("how to handle signed tags"), + PARSE_OPT_NONEG), + OPT_HIDDEN_GROUP(N_("Advanced")), + OPT_BOOL_F(0, "allow-unsafe-features", &state.allow_unsafe_features, + N_("allow unsafe mark commands from the stream"), + PARSE_OPT_HIDDEN | PARSE_OPT_NONEG), + OPT_STRING_F(0, "export-pack-edges", &edges, N_("file"), + N_("dump edge commits to "), + PARSE_OPT_HIDDEN | PARSE_OPT_NONEG), + OPT_INTEGER_F(0, "cat-blob-fd", &cat_blob_fd, + N_("write some responses to instead of stdout"), + PARSE_OPT_HIDDEN | PARSE_OPT_NONEG), + OPT_END() + }; + + show_usage_with_options_if_asked(argc, argv, fast_import_usage, fast_import_options); + + fast_import_state_init(&state, argc, argv, prefix, fast_import_options); reset_pack_idx_option(&pack_idx_opts); git_pack_config(); diff --git a/t/t0450/adoc-help-mismatches b/t/t0450/adoc-help-mismatches index c4a55ff4e35a4f..baf3b1d80927d4 100644 --- a/t/t0450/adoc-help-mismatches +++ b/t/t0450/adoc-help-mismatches @@ -12,7 +12,6 @@ column credential credential-cache credential-store -fast-import fetch-pack fmt-merge-msg format-patch From 87cf4d72999cdfdb343964e2027ce4a3c6cb8d04 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 11 Aug 2026 10:33:12 +0200 Subject: [PATCH 29/43] fast-import: use callbacks to parse some options A previous commit started using the parse-option API to generate proper `git fast-import -h` and `git fast-import --help-all` output. Let's prepare for when we can use that API to also parse the options by using OPT_CALLBACK for some options that require special processing of their arguments. A following commit will actually parse the options using these callbacks. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- builtin/fast-import.c | 208 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 168 insertions(+), 40 deletions(-) diff --git a/builtin/fast-import.c b/builtin/fast-import.c index 879c2860439ea3..40cc9c4a23fe7f 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -4008,6 +4008,126 @@ static void parse_argv(struct fast_import_state *state) build_mark_map(&sub_marks_from, &sub_marks_to); } +static int option_parse_date_format(const struct option *opt UNUSED, + const char *arg, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_date_format(arg); + return 0; +} + +static int option_parse_export_pack_edges(const struct option *opt, + const char *arg, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_export_pack_edges(opt->value, arg); + return 0; +} + +static int option_parse_max_pack_size(const struct option *opt UNUSED, + const char *arg, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_max_pack_size(arg); + return 0; +} + +static int option_parse_big_file_threshold(const struct option *opt UNUSED, + const char *arg, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_big_file_threshold(arg); + return 0; +} + +static int option_parse_signed_commits(const struct option *opt UNUSED, + const char *arg, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_signed_commits(arg); + return 0; +} + +static int option_parse_signed_tags(const struct option *opt UNUSED, + const char *arg, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_signed_tags(arg); + return 0; +} + +static int option_parse_rewrite_submodules_from(const struct option *opt, + const char *arg, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_rewrite_submodules(opt->value, arg, &sub_marks_from); + return 0; +} + +static int option_parse_rewrite_submodules_to(const struct option *opt, + const char *arg, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_rewrite_submodules(opt->value, arg, &sub_marks_to); + return 0; +} + +static int option_parse_cat_blob_fd(const struct option *opt, + const char *arg, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_cat_blob_fd(opt->value, arg); + return 0; +} + +static int option_parse_import_marks(const struct option *opt, + const char *arg, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_import_marks(opt->value, arg, 0, 0); + return 0; +} + +static int option_parse_import_marks_if_exists(const struct option *opt, + const char *arg, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_import_marks(opt->value, arg, 0, 1); + return 0; +} + +static int option_parse_export_marks(const struct option *opt, + const char *arg, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_export_marks(opt->value, arg); + return 0; +} + +static int option_parse_depth(const struct option *opt UNUSED, + const char *arg, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_depth(arg); + return 0; +} + +static int option_parse_active_branches(const struct option *opt UNUSED, + const char *arg, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_active_branches(arg); + return 0; +} + +static int option_parse_quiet(const struct option *opt UNUSED, + const char *arg UNUSED, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_quiet(); + return 0; +} + int cmd_fast_import(int argc, const char **argv, const char *prefix, @@ -4015,10 +4135,6 @@ int cmd_fast_import(int argc, { struct fast_import_state state; - unsigned long pack_size_limit, big_file_threshold; - char *edges, *signed_commits, *signed_tags, *date_format; - char *import_marks_if_exists, *submodules_from, *submodules_to; - /* * NEEDSWORK: For now this is used only to render * `-h`/`--help-all` usage messages. The actual parsing is @@ -4026,58 +4142,70 @@ int cmd_fast_import(int argc, */ struct option fast_import_options[] = { OPT_GROUP(N_("Common")), - OPT_STRING_F(0, "date-format", &date_format, N_("fmt"), - N_("format of the commit/tag dates"), PARSE_OPT_NONEG), + OPT_CALLBACK_F(0, "date-format", NULL, N_("fmt"), + N_("format of the commit/tag dates"), + PARSE_OPT_NONEG, option_parse_date_format), OPT_BOOL_F(0, "stats", &show_stats, N_("display some basic statistics (objects, packfiles and memory)"), PARSE_OPT_NONEG), - OPT_BOOL_F(0, "quiet", &quiet, - N_("disable the output shown by --stats"), PARSE_OPT_NONEG), + OPT_CALLBACK_F(0, "quiet", NULL, NULL, + N_("disable the output shown by --stats"), + PARSE_OPT_NOARG | PARSE_OPT_NONEG, + option_parse_quiet), OPT_BOOL_F(0, "force", &force_update, N_("force updating modified existing branches"), PARSE_OPT_NONEG), OPT_BOOL_F(0, "done", &require_explicit_termination, N_("require a terminating 'done' command"), PARSE_OPT_NONEG), - OPT_UNSIGNED(0, "max-pack-size", &pack_size_limit, - N_("maximum size of each output pack file")), - OPT_UNSIGNED(0, "big-file-threshold", &big_file_threshold, - N_("maximum size of a blob that will be deltified")), - OPT_UNSIGNED(0, "depth", &max_depth, - N_("maximum delta depth")), - OPT_UNSIGNED(0, "active-branches", &max_active_branches, - N_("maximum number of branches to maintain active")), + OPT_CALLBACK_F(0, "max-pack-size", NULL, N_("n"), + N_("maximum size of each output pack file"), + PARSE_OPT_NONEG, option_parse_max_pack_size), + OPT_CALLBACK_F(0, "big-file-threshold", NULL, N_("n"), + N_("maximum size of a blob that will be deltified"), + PARSE_OPT_NONEG, option_parse_big_file_threshold), + OPT_CALLBACK_F(0, "depth", NULL, N_("n"), + N_("maximum delta depth"), + PARSE_OPT_NONEG, option_parse_depth), + OPT_CALLBACK_F(0, "active-branches", NULL, N_("n"), + N_("maximum number of branches to maintain active"), + PARSE_OPT_NONEG, option_parse_active_branches), OPT_GROUP(N_("Marks")), - OPT_STRING_F(0, "import-marks", &import_marks_file, N_("file"), - N_("import marks from "), PARSE_OPT_NONEG), - OPT_STRING_F(0, "import-marks-if-exists", &import_marks_if_exists, N_("file"), - N_("import marks from if it exists"), PARSE_OPT_NONEG), - OPT_STRING_F(0, "export-marks", &export_marks_file, N_("file"), - N_("dump marks to "), PARSE_OPT_NONEG), + OPT_CALLBACK_F(0, "import-marks", &state, N_("file"), + N_("import marks from "), + PARSE_OPT_NONEG, option_parse_import_marks), + OPT_CALLBACK_F(0, "import-marks-if-exists", &state, N_("file"), + N_("import marks from if it exists"), + PARSE_OPT_NONEG, option_parse_import_marks_if_exists), + OPT_CALLBACK_F(0, "export-marks", &state, N_("file"), + N_("dump marks to "), + PARSE_OPT_NONEG, option_parse_export_marks), OPT_BOOL(0, "relative-marks", &relative_marks_paths, N_("are --(import|export)-marks= paths relative to '.git/info/fast-import'?")), OPT_GROUP(N_("Submodule rewrite")), - OPT_STRING_F(0, "rewrite-submodules-from", &submodules_from, N_("name:filename"), - N_("rewrite object IDs for submodule from "), - PARSE_OPT_NONEG), - OPT_STRING_F(0, "rewrite-submodules-to", &submodules_to, N_("name:filename"), - N_("rewrite object IDs for submodule to "), - PARSE_OPT_NONEG), + OPT_CALLBACK_F(0, "rewrite-submodules-from", &state, N_("name:filename"), + N_("rewrite object IDs for submodule from "), + PARSE_OPT_NONEG, option_parse_rewrite_submodules_from), + OPT_CALLBACK_F(0, "rewrite-submodules-to", &state, N_("name:filename"), + N_("rewrite object IDs for submodule to "), + PARSE_OPT_NONEG, option_parse_rewrite_submodules_to), OPT_GROUP(N_("Signing")), - OPT_STRING_F(0, "signed-commits", &signed_commits, N_("mode"), - N_("how to handle signed commits"), - PARSE_OPT_NONEG), - OPT_STRING_F(0, "signed-tags", &signed_tags, N_("mode"), - N_("how to handle signed tags"), - PARSE_OPT_NONEG), + OPT_CALLBACK_F(0, "signed-commits", NULL, N_("mode"), + N_("how to handle signed commits"), + PARSE_OPT_NONEG, option_parse_signed_commits), + OPT_CALLBACK_F(0, "signed-tags", NULL, N_("mode"), + N_("how to handle signed tags"), + PARSE_OPT_NONEG, option_parse_signed_tags), OPT_HIDDEN_GROUP(N_("Advanced")), OPT_BOOL_F(0, "allow-unsafe-features", &state.allow_unsafe_features, N_("allow unsafe mark commands from the stream"), PARSE_OPT_HIDDEN | PARSE_OPT_NONEG), - OPT_STRING_F(0, "export-pack-edges", &edges, N_("file"), - N_("dump edge commits to "), - PARSE_OPT_HIDDEN | PARSE_OPT_NONEG), - OPT_INTEGER_F(0, "cat-blob-fd", &cat_blob_fd, - N_("write some responses to instead of stdout"), - PARSE_OPT_HIDDEN | PARSE_OPT_NONEG), + OPT_CALLBACK_F(0, "export-pack-edges", &state, N_("file"), + N_("dump edge commits to "), + PARSE_OPT_HIDDEN | PARSE_OPT_NONEG, + option_parse_export_pack_edges), + OPT_CALLBACK_F(0, "cat-blob-fd", &state, N_("fd"), + N_("write some responses to instead of stdout"), + PARSE_OPT_HIDDEN | PARSE_OPT_NONEG, + option_parse_cat_blob_fd), OPT_END() }; From 863937696ef2bd8faaf4811aaea8cf5301bac78b Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 11 Aug 2026 10:33:13 +0200 Subject: [PATCH 30/43] fast-import: use parse_options() for command line options Previous commits have started to use the parse-options API to display output from `git fast-import -h` and `git fast-import --help-all` and to prepare for parsing the command line options using this API. Let's now actually use the API to parse command line options. This brings a number of changes that are mostly beneficial: - The `--alias`, `--get-mark`, `--cat-blob`, `--ls` and `--notes` options are no longer accepted on the command line. They were previously accepted as no-ops because parse_argv() fell through to parse_one_feature(). They are not documented in the OPTIONS section and are only meaningful as in-stream feature assertions, so accepting them on the command line was an accident of code sharing dating back to 9c8398f0c9 (fast-import: add option command, 2009-12-04). - Abbreviated options like `--dep=5` now work since parse_options() allows unambiguous prefixes. - As `--cat-blob` is an abbreviation of `--cat-blob-fd`, using the former on the command line will fail with "option `cat-blob-fd' requires a value" unlike the other four options that are not accepted anymore on the command line (see above). - Value-taking options now also accept the space-separated `--opt value` form, like `--depth 5`, in addition to the `--opt=value` form. - A bare or trailing `--` is now accepted and the stream is read normally, while it used to be a usage error. - The error messages for some options might differ a bit. - The code is shorter and more standard. Note that parse_one_feature() is now always called with its `from_stream` argument set to 1, but the code simplifications that can be made are left for a following clean-up commit. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- Documentation/git-fast-import.adoc | 7 +++++ builtin/fast-import.c | 43 ++++++++++-------------------- t/t9300-fast-import.sh | 7 +++++ 3 files changed, 28 insertions(+), 29 deletions(-) diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc index 7c5900e048cefb..fd165e11d2d259 100644 --- a/Documentation/git-fast-import.adoc +++ b/Documentation/git-fast-import.adoc @@ -65,6 +65,13 @@ Only enable this option if you trust the program generating the fast-import stream! This option is enabled automatically for remote-helpers that use the `import` capability, as they are already trusted to run their own code. ++ +Note that this option has to be spelled in full, and has to appear +before any option whose value is separated from it by a space, for +the unsafe `feature` commands in the stream to be allowed. So +`--allow-unsafe` or `--depth 5 --allow-unsafe-features` still refuse +them, while `--allow-unsafe-features --depth 5` and +`--depth=5 --allow-unsafe-features` allow them. `--signed-tags=`:: Specify how to handle signed tags. Behaves in the same way as diff --git a/builtin/fast-import.c b/builtin/fast-import.c index 40cc9c4a23fe7f..dd873ec4336b90 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -3975,31 +3975,11 @@ static const char *const fast_import_usage[] = { static void parse_argv(struct fast_import_state *state) { - unsigned int i; - - for (i = 1; i < state->argc; i++) { - const char *a = state->argv[i]; - - if (*a != '-' || !strcmp(a, "--")) - break; - - if (!skip_prefix(a, "--", &a)) - die(_("unknown option %s"), a); - - if (parse_one_option(state, a)) - continue; - - if (parse_one_feature(state, a, 0)) - continue; - - if (skip_prefix(a, "cat-blob-fd=", &a)) { - option_cat_blob_fd(state, a); - continue; - } + int argc = parse_options(state->argc, state->argv, state->prefix, + state->option, fast_import_usage, + PARSE_OPT_KEEP_ARGV0); - die(_("unknown option --%s"), a); - } - if (i != state->argc) + if (argc > 1) usage_with_options(fast_import_usage, state->option); state->seen_data_command = 1; @@ -4135,11 +4115,6 @@ int cmd_fast_import(int argc, { struct fast_import_state state; - /* - * NEEDSWORK: For now this is used only to render - * `-h`/`--help-all` usage messages. The actual parsing is - * done by parse_one_option()/parse_one_feature(). - */ struct option fast_import_options[] = { OPT_GROUP(N_("Common")), OPT_CALLBACK_F(0, "date-format", NULL, N_("fmt"), @@ -4230,6 +4205,16 @@ int cmd_fast_import(int argc, * "feature" lines at the start of the stream (which allows the command * line to override stream data). But we must do an early parse of any * command-line options that impact how we interpret the feature lines. + * + * NEEDSWORK: This scan only matches the exact "--allow-unsafe-features" + * spelling and stops at the first argument that doesn't start with a + * dash. As parse_options() below also accepts unambiguous abbreviations + * and values separated by a space from their option, the two disagree + * for command lines like "--allow-unsafe" or "--depth 5 + * --allow-unsafe-features": parse_options() accepts the option, but + * this scan doesn't see it, so unsafe features from the stream are + * still refused. This errs on the safe side, but should be fixed by + * teaching this scan about the options that take a value. */ for (int i = 1; i < argc; i++) { const char *arg = argv[i]; diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index fe6c2617acb2fe..d9de2ef0d88bc8 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -2827,6 +2827,13 @@ test_expect_success 'R: unknown commandline options are rejected' '\ test_must_fail git fast-import --non-existing-option < /dev/null ' +test_expect_success 'R: feature-only names are rejected on the command line' ' + for opt in --alias --get-mark --ls --notes + do + test_must_fail git fast-import "$opt" Date: Tue, 11 Aug 2026 10:33:14 +0200 Subject: [PATCH 31/43] fast-import: remove useless from_stream argument Now that a previous commit has removed a call to parse_one_feature() from parse_argv(), the former is always called with its `from_stream` argument set to 1. Let's take advantage of that to simplify and cleanup the code a bit. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- builtin/fast-import.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/builtin/fast-import.c b/builtin/fast-import.c index dd873ec4336b90..4e3c9601505da8 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -3867,30 +3867,28 @@ static int parse_one_option(struct fast_import_state *state, const char *option) } static void check_unsafe_feature(struct fast_import_state *state, - const char *feature, - int from_stream) + const char *feature) { - if (from_stream && !state->allow_unsafe_features) + if (!state->allow_unsafe_features) die(_("feature '%s' forbidden in input without --allow-unsafe-features"), feature); } static int parse_one_feature(struct fast_import_state *state, - const char *feature, - int from_stream) + const char *feature) { const char *arg; if (skip_prefix(feature, "date-format=", &arg)) { option_date_format(arg); } else if (skip_prefix(feature, "import-marks=", &arg)) { - check_unsafe_feature(state, "import-marks", from_stream); - option_import_marks(state, arg, from_stream, 0); + check_unsafe_feature(state, "import-marks"); + option_import_marks(state, arg, 1, 0); } else if (skip_prefix(feature, "import-marks-if-exists=", &arg)) { - check_unsafe_feature(state, "import-marks-if-exists", from_stream); - option_import_marks(state, arg, from_stream, 1); + check_unsafe_feature(state, "import-marks-if-exists"); + option_import_marks(state, arg, 1, 1); } else if (skip_prefix(feature, "export-marks=", &arg)) { - check_unsafe_feature(state, feature, from_stream); + check_unsafe_feature(state, feature); option_export_marks(state, arg); } else if (!strcmp(feature, "alias")) { ; /* Don't die - this feature is supported */ @@ -3924,7 +3922,7 @@ static void parse_feature(struct fast_import_state *state, const char *feature) if (state->seen_data_command) die(_("got feature command '%s' after data command"), feature); - if (parse_one_feature(state, feature, 1)) + if (parse_one_feature(state, feature)) return; die(_("this version of fast-import does not support feature %s."), feature); From cd2ab0e12896ef505f4be8822788912fe95394eb Mon Sep 17 00:00:00 2001 From: Kristofer Karlsson Date: Tue, 11 Aug 2026 09:28:43 +0000 Subject: [PATCH 32/43] Documentation/technical: add paint-down-to-common doc Add a technical document describing the paint_down_to_common() algorithm used for merge-base computation, covering the paint walk, generation number regions, and termination conditions. Signed-off-by: Kristofer Karlsson Signed-off-by: Junio C Hamano --- Documentation/Makefile | 1 + Documentation/technical/meson.build | 1 + .../technical/paint-down-to-common.adoc | 174 ++++++++++++++++++ commit-reach.c | 6 +- 4 files changed, 181 insertions(+), 1 deletion(-) create mode 100644 Documentation/technical/paint-down-to-common.adoc diff --git a/Documentation/Makefile b/Documentation/Makefile index 2699f0b24af192..f8dea4b3953250 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -129,6 +129,7 @@ TECH_DOCS += technical/long-running-process-protocol TECH_DOCS += technical/multi-pack-index TECH_DOCS += technical/packfile-uri TECH_DOCS += technical/pack-heuristics +TECH_DOCS += technical/paint-down-to-common TECH_DOCS += technical/parallel-checkout TECH_DOCS += technical/partial-clone TECH_DOCS += technical/platform-support diff --git a/Documentation/technical/meson.build b/Documentation/technical/meson.build index ec07088c57617f..9ce11d5e484d9c 100644 --- a/Documentation/technical/meson.build +++ b/Documentation/technical/meson.build @@ -18,6 +18,7 @@ articles = [ 'multi-pack-index.adoc', 'packfile-uri.adoc', 'pack-heuristics.adoc', + 'paint-down-to-common.adoc', 'parallel-checkout.adoc', 'partial-clone.adoc', 'platform-support.adoc', diff --git a/Documentation/technical/paint-down-to-common.adoc b/Documentation/technical/paint-down-to-common.adoc new file mode 100644 index 00000000000000..4bd3c2adb56a64 --- /dev/null +++ b/Documentation/technical/paint-down-to-common.adoc @@ -0,0 +1,174 @@ +Merge-Base Computation and paint_down_to_common() +================================================== + +The function `paint_down_to_common()` in `commit-reach.c` computes merge +bases by walking the commit graph backwards from two sets of tips and +finding where their ancestry meets. + +Use cases +--------- + +Computing merge bases is used in two different ways: + + 1. *Finding all merge bases* (`merge-base --all`, `merge-tree`, + `merge`, `rebase`). A merge base is a common ancestor that is + not itself an ancestor of another common ancestor. + + 2. *Ancestry checks* (`in_merge_bases`, used by `merge-base + --is-ancestor`, `branch -d`, `fetch`). These ask: "is commit A + an ancestor of commit B?" If a common ancestor equals one of the + inputs, that input is necessarily the only merge base -- no other + common ancestor can be both as recent and not an ancestor of it. + +Both use cases share the same algorithm and implementation. + +Algorithm +--------- + +Given a commit `one` and a set of commits `twos[]`, the walk paints +commits with two colors: + + - PARENT1: reachable from `one` + - PARENT2: reachable from any commit in `twos[]` + +The walk uses a priority queue ordered by generation number +(highest first), breaking ties by commit date. Each step dequeues +the highest-priority commit and propagates its paint flags to its +parents, enqueuing any parent that gained new flags. When a +commit receives both PARENT1 and PARENT2, it is a merge-base +candidate. A candidate gains the STALE flag so its ancestors +propagate staleness -- any deeper common ancestor is necessarily +redundant. + +[[generation-regions]] +Topologically ordered and unordered generation regions +------------------------------------------------------ + +Commits fall into two regions based on whether their generation +numbers provide a topological ordering guarantee: + +.... + +------------------------------------------+ + | Unordered region | + | generation = INFINITY or V1_MAX | + | queue order: heuristic (commit date) | + +------------------------------------------+ + | + v + +------------------------------------------+ + | Ordered region | + | generation = finite, unsaturated | + | queue order: topological | + +------------------------------------------+ +.... + +In the ordered region, a child's generation is strictly greater +than its parent's. Same-generation commits are necessarily +independent, so the queue always processes children before +their parents. + +In the unordered region, parent-child pairs can share the same +generation number, so topological order is not guaranteed. The +queue uses commit-date as a heuristic, which typically produces +a reasonable traversal order but may process a parent before +its child. + +Commits not in the commit-graph have generation INFINITY; v1 +commit-graphs saturate at V1_MAX. Both place commits in the +unordered region. Any optimization that depends on generation +ordering must account for this saturation boundary. + +With generation ordering, values in the unordered region exceed +those in the ordered region. The walk may therefore transition +from the unordered region into the ordered region, but never in +the reverse direction. Without a commit-graph, every commit has INFINITY +and the walk operates entirely in the unordered region. + +In the ordered region, paint on a dequeued commit is final -- +no future step can add flags to it. In the unordered region, +a dequeued commit may later gain additional paint. Paint flags +are only added, never removed, bounding the number of +re-enqueues per commit. + +Termination +----------- + +The walk uses a `nonstale_queue` wrapper around `prio_queue` that +tracks `max_nonstale`: the lowest-priority non-stale commit enqueued +so far. Once that commit is dequeued, every remaining entry is known +to be STALE and the loop terminates. Specifically, the main loop +ends when one of the following conditions holds: + + 1. The queue is empty. + 2. `max_nonstale` has been dequeued, meaning the queue only contains + STALE entries. + 3. Generation cutoff: the dequeued commit's generation is below + a caller-supplied `min_generation` threshold. + 4. Single result: the caller only needs one merge base, one has + been found, and the walk has entered the ordered region. + +Stale entry condition +~~~~~~~~~~~~~~~~~~~~~ +Once all queued entries are stale, no new merge-base candidates can +be discovered -- that requires at least one non-stale commit from +each side meeting. Continuing the walk could still invalidate +existing candidates by proving one is an ancestor of another, but +`remove_redundant()` handles that as a post-processing step, so it +is safe to exit early. + +Generation cutoff +~~~~~~~~~~~~~~~~~ +Some callers (notably `remove_redundant()`) supply a `min_generation` +threshold equal to the minimum generation of the input commits. +These callers only need to determine reachability among the inputs, +not find deep merge bases, so the walk can safely terminate when it +dequeues a commit below this threshold. + +Single result +~~~~~~~~~~~~~ +When only one merge base is needed and the walk is in the +ordered region with generation ordering, the first candidate +found is necessarily the highest-generation common ancestor. +No remaining commit in the queue can be a descendant of this +candidate (generation ordering guarantees children are visited +first), so it cannot be redundant and the walk can stop +immediately. + +This optimization is NOT safe when the date-ordering fallback is +active, because commit-date order can visit a deeper ancestor +before a shallower one -- see <>. + +[[date-ordering-fallback]] +Date-ordering fallback +---------------------- + +When the commit-graph has generation numbers v1 and no +generation floor is specified, topological ordering +(via generation numbers) is disabled. Topological levels are +correct but unbalanced -- ordering by such generation numbers +can sometimes cause the walk to detour too far before finding +merge bases. Commit-date ordering typically reaches them in +fewer steps -- see this change for more details: + + 091f4cf3 (commit: don't use generation numbers if not needed, + 2018-08-30) + +With generation number v2 (corrected commit dates) we have the best +of both worlds and do not need this fallback. + +For v1, `paint_down_to_common()` falls back to pure commit-date +ordering via `compare_commits_by_commit_date`. Because commit +dates are not monotonic (clock skew, rebases, etc.), the queue +may visit commits out of topological order. + +This disables the optimization that depends on generation ordering: + + - *Single result*: the first merge-base candidate found may not + be the shallowest, because a deeper ancestor with a higher + commit date can be dequeued first. + +Related documentation +--------------------- + + - `Documentation/technical/commit-graph.adoc` -- generation numbers + and the reachability closure property. diff --git a/commit-reach.c b/commit-reach.c index 708798a39b2d8e..bbf8c3eff068f6 100644 --- a/commit-reach.c +++ b/commit-reach.c @@ -96,7 +96,11 @@ static struct commit *nonstale_queue_get_dedup(struct nonstale_queue *queue) return commit; } -/* all input commits in one and twos[] must have been parsed! */ +/* + * See Documentation/technical/paint-down-to-common.adoc + * + * All input commits in one and twos[] must have been parsed! + */ static int paint_down_to_common(struct repository *r, struct commit *one, int n, struct commit **twos, From fe4877bc17b1dd5b25b9444eadee20ec5fa8bf83 Mon Sep 17 00:00:00 2001 From: Kristofer Karlsson Date: Tue, 11 Aug 2026 09:28:44 +0000 Subject: [PATCH 33/43] test-lib-functions: improve diagnostic output for trace2 data assertions test_trace2_data is a bare grep that silently exits on failure. Add a more informative variant that verifies the event appears exactly once and reports what went wrong: key not found, multiple entries, or value mismatch. Diagnostics go to FD 4 like test_grep. Before (value mismatch): $ test_trace2_data status count/changed 999 Signed-off-by: Junio C Hamano --- t/test-lib-functions.sh | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 809c6621241944..8c6d327b03cbe8 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -1996,6 +1996,41 @@ test_trace2_data () { grep -e '"category":"'"$1"'","key":"'"$2"'","value":"'"$3"'"' } +# Check that the given trace2 data event has the expected value and +# appears exactly once. Produces a diagnostic on failure. +# +# test_trace2_data_singular [