Re: [PATCH v2 4/7] object-file: remove flags from transaction packfile writes

[Patrick Steinhardt <ps@pks.im>]

On Tue, Mar 31, 2026 at 10:03:12PM -0500, Justin Tobler wrote:
> diff --git a/object-file.c b/object-file.c
> index f3038756fc..f317a24ccf 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -1412,6 +1411,38 @@ 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, size_t size)
> +{
> +	unsigned char buf[16384];
> +	struct git_hash_ctx ctx;
> +	unsigned header_len;
> +	size_t total = 0;

One nit: I think `total` and `size` don't really give a good sense of
which variable tracks what. If this was instead `bytes_hashed` and
`size` it would become a lot more obvious.

> @@ -1666,18 +1683,28 @@ 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 object_database *odb = the_repository->objects;
> -		struct odb_transaction_files *files_transaction;
> -		struct odb_transaction *transaction;
> -
> -		transaction = odb_transaction_begin(odb);
> -		files_transaction = container_of(odb->transaction,
> -						 struct odb_transaction_files,
> -						 base);
> -		ret = index_blob_packfile_transaction(files_transaction, oid, fd,
> -						      xsize_t(st->st_size),
> -						      path, flags);
> -		odb_transaction_commit(transaction);
> +		struct odb_write_stream stream = { 0 };
> +		odb_write_stream_from_fd(&stream, fd, xsize_t(st->st_size));

I would assume that `odb_write_stream_from_fd()` knows to fully
initialize the stream, so zero-initializing shouldn't be necessary,
right?

> diff --git a/odb/streaming.h b/odb/streaming.h
> index c7861f7e13..e5232cd4d1 100644
> --- a/odb/streaming.h
> +++ b/odb/streaming.h
> @@ -5,6 +5,7 @@
>  #define STREAMING_H 1
>  
>  #include "object.h"
> +#include "odb.h"
>  
>  struct object_database;
>  struct odb_read_stream;
> @@ -64,4 +65,11 @@ 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. The caller is expected to
> + * free the underlying stream data.
> + */

Hm. Shouldn't we provide an interface that let's the caller do this
without having to know about the stream's internals, like
`odb_write_stream_release()`?

Patrick