Re: [PATCH] object-file: avoid ODB transaction when not writing objects

[Jeff King <peff@peff.net>]

On Tue, Apr 07, 2026 at 02:18:06PM -0700, Junio C Hamano wrote:

> From: Justin Tobler <jltobler@gmail.com>
> Date: Tue, 7 Apr 2026 15:17:30 -0500
> Subject: [PATCH] object-file: avoid ODB transaction when not writing objects
> [...]
> +static int hash_blob_stream(const struct git_hash_algo *hash_algo,
> +			    struct object_id *result_oid, int fd, size_t size)
> +{
> +	unsigned char buf[16384];
> +	struct git_hash_ctx ctx;
> +	unsigned header_len;
> +
> +	header_len = format_object_header((char *)buf, sizeof(buf),
> +					  OBJ_BLOB, size);
> +	hash_algo->init_fn(&ctx);
> +	git_hash_update(&ctx, buf, header_len);
> +
> +	while (size) {
> +		size_t rsize = size < sizeof(buf) ? size : sizeof(buf);
> +		ssize_t read_result = read_in_full(fd, buf, rsize);
> +
> +		if ((read_result < 0) || ((size_t)read_result != rsize))
> +			return -1;
> +
> +		git_hash_update(&ctx, buf, rsize);
> +		size -= read_result;
> +	}
> +
> +	git_hash_final_oid(result_oid, &ctx);

This looks correct to me. In the back of my mind I felt like we might
already have a function to check a streaming hash, but I was just
thinking of how parse_object() streams blobs for its hash-check. And
that is always coming from the object database, whereas here we are
taking data from elsewhere. So we do need this new function.

I probably would have used fewer parentheses in the conditional, but
that may be personal preference. ;)

> diff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh
> index c824c1a25c..c1dbc6359a 100755
> --- a/t/t1517-outside-repo.sh
> +++ b/t/t1517-outside-repo.sh
> @@ -93,6 +93,14 @@ test_expect_success 'diff outside repository' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'diff files exceeding bigFileThreshold outside repository' '
> +	cd non-repo &&
> +	echo foo >foo &&
> +	echo bar >bar &&
> +	test_must_fail git -c core.bigFileThreshold=1 diff -- foo bar >actual &&
> +	test_grep "diff --git a/foo b/bar" actual
> +'

This does a "cd" outside of a sub-shell, which affects all of the
subsequent tests.

We also are already using the "nongit" wrapper in this script, so it
could be used here.

Thought it was found originally with diff, the bug can also be
demonstrated with just hash-object, which does make the test a little
simpler.

The second and third are more style/taste questions, but I think the
first is a blocker.

-Peff