Volume XXII, number 280Wednesday, October 7, 2026Latest message 1 hour ago

The Git List

News and archive of git@vger.kernel.org, since April 2005

patchobject-file: avoid ODB transaction when not writing objects

10 messages between Apr 7, 2026 and Apr 8, 2026, from Justin Tobler, Junio C Hamano, Jeff King.

Plain Markdown or JSON for tools and agents. Diffs are folded; open one to read it.

Justin ToblerApr 7, 2026, 20:17 UTC on lore

In ce1661f9da (odb: add transaction interface, 2025-09-16), existing ODB transaction logic is adapted to create a transaction interface at the ODB layer. The intent here is for the ODB transaction interface to eventually provide an object source agnostic means to manage transactions.

An unintended consequence of this change though is that `object-file.c:index_fd()` may enter the ODB transaction path even when no object write is requested. In non-repository contexts, this can result in a NULL dereference and segfault. One such case occurs when running git-diff(1) outside of a repository with "core.bigFileThreshold" forcing the streaming path in `index_fd()`:

        $ echo foo >foo
        $ echo bar >bar
        $ git -c core.bigFileThreshold=1 diff -- foo bar

In this scenario, the caller only needs to compute the object ID. Object hashing does not require an ODB, so starting a transaction is both unnecessary and invalid.

Fix the bug by avoiding the use of ODB transactions in `index_fd()` when callers are only interested in computing the object hash.

Reported-by: Luca Stefani <luca.stefani.ge1@gmail.com>
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
Greetings,

This patch addresses a bug report[1] where performing git-diff(1) on files that exceed "core.bigFileThreshold" outside of a repository causes a segfault. Originally this patch was included in another series sent to the mailing list[2] as a preparatory refactor. Since it happens to fix the reported bug though, I've extracted it from that series with the hope of upstreaming more quickly.

I wasn't entirely sure if this patch should be based on master or maint. I went with master, but am happy to resend if this is incorrect.

Thanks, -Justin

[1]: <CAO0HQ0X_pQmew5tJReOL=u+CMxCjAQynx8JfjykoYAUE59YNzw@mail.gmail.com> [2]: <20260331033835.2863514-1-jltobler@gmail.com>

---
 object-file.c           | 57 ++++++++++++++++++++++++++++++++---------
 t/t1517-outside-repo.sh |  8 ++++++
 2 files changed, 53 insertions(+), 12 deletions(-)
Show changes to 2 files +53 −12

object-file.c, t/t1517-outside-repo.sh

diff --git a/object-file.c b/object-file.c
index 4f77ce0982..63408fc290 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1640,6 +1640,34 @@ static int index_blob_packfile_transaction(struct odb_transaction_files *transac
 	return 0;
 }
 
+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);
+
+	return 0;
+}
+
 int index_fd(struct index_state *istate, struct object_id *oid,
 	     int fd, struct stat *st,
 	     enum object_type type, const char *path, unsigned flags)
@@ -1661,18 +1689,23 @@ 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);
+		if (flags & INDEX_WRITE_OBJECT) {
+			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);
+		} else {
+			ret = hash_blob_stream(the_repository->hash_algo, oid,
+					       fd, xsize_t(st->st_size));
+		}
 	}
 
 	close(fd);
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
+'
+
 test_expect_success 'stripspace outside repository' '
 	nongit git stripspace -s </dev/null
 '

base-commit: 1adf5bca8c3cf778103548b9355777cf2d12efdd
-- 
2.53.0.381.g628a66ccf6
Junio C HamanoApr 7, 2026, 21:18 UTC in reply to Justin Tobler on lore

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

Justin Tobler <jltobler@gmail.com> writes:
Show 6 quoted lines
> This patch addresses a bug report[1] where performing git-diff(1) on
> files that exceed "core.bigFileThreshold" outside of a repository causes
> a segfault. Originally this patch was included in another series sent to
> the mailing list[2] as a preparatory refactor. Since it happens to fix
> the reported bug though, I've extracted it from that series with the
> hope of upstreaming more quickly.

OK, so the bug was introduced in fd13909e (Merge branch 'jt/odb-transaction', 2025-10-02) aka v2.52.0-rc0~91 and applying the test part of your patch alone on v2.52 indeed fails t1517. The code change does not directly apply to the old codebase, so I wiggled your patch to make it apply to v2.52 codebase.

> I wasn't entirely sure if this patch should be based on master or maint.
> I went with master, but am happy to resend if this is incorrect.

So if we wanted to fix the past releases, the attached may be where we want to start, then adjusting backwards to the shape of the patch you posted as we merge it up to v2.53 and v2.54 track.

I've applied this to 'maint-2.52' (done and tested), 'maint-2.53' (done and tested), and to 'master', and made sure that the last one matches the result of applying your patch directly on top of 'master'.

I'd appreciate it if you can give your eyeballs to the attached to see if that is how you would fixed the bug in the original context of v2.52 track. If everything looks OK, then there is no need to spend time backporting on your side. We have everything necessary.

Thanks.
--- >8 ---
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

In ce1661f9da (odb: add transaction interface, 2025-09-16), existing ODB transaction logic is adapted to create a transaction interface at the ODB layer. The intent here is for the ODB transaction interface to eventually provide an object source agnostic means to manage transactions.

An unintended consequence of this change though is that `object-file.c:index_fd()` may enter the ODB transaction path even when no object write is requested. In non-repository contexts, this can result in a NULL dereference and segfault. One such case occurs when running git-diff(1) outside of a repository with "core.bigFileThreshold" forcing the streaming path in `index_fd()`:

        $ echo foo >foo
        $ echo bar >bar
        $ git -c core.bigFileThreshold=1 diff -- foo bar

In this scenario, the caller only needs to compute the object ID. Object hashing does not require an ODB, so starting a transaction is both unnecessary and invalid.

Fix the bug by avoiding the use of ODB transactions in `index_fd()` when callers are only interested in computing the object hash.

Reported-by: Luca Stefani <luca.stefani.ge1@gmail.com>
Signed-off-by: Justin Tobler <jltobler@gmail.com>
[jc: adjusted to fd13909e (Merge branch 'jt/odb-transaction', 2025-10-02)]
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 object-file.c           | 49 ++++++++++++++++++++++++++++++++++-------
 t/t1517-outside-repo.sh |  8 +++++++
 2 files changed, 49 insertions(+), 8 deletions(-)
Show changes to 2 files +49 −8

object-file.c, t/t1517-outside-repo.sh

diff --git a/object-file.c b/object-file.c
index 17a236d2fe..0969e27f3d 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1599,6 +1599,34 @@ static int index_blob_packfile_transaction(struct odb_transaction *transaction,
 	return 0;
 }
 
+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);
+
+	return 0;
+}
+
 int index_fd(struct index_state *istate, struct object_id *oid,
 	     int fd, struct stat *st,
 	     enum object_type type, const char *path, unsigned flags)
@@ -1620,14 +1648,19 @@ 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_transaction *transaction;
-
-		transaction = odb_transaction_begin(the_repository->objects);
-		ret = index_blob_packfile_transaction(the_repository->objects->transaction,
-						      oid, fd,
-						      xsize_t(st->st_size),
-						      path, flags);
-		odb_transaction_commit(transaction);
+		if (flags & INDEX_WRITE_OBJECT) {
+			struct odb_transaction *transaction;
+
+			transaction = odb_transaction_begin(the_repository->objects);
+			ret = index_blob_packfile_transaction(the_repository->objects->transaction,
+							      oid, fd,
+							      xsize_t(st->st_size),
+							      path, flags);
+			odb_transaction_commit(transaction);
+		} else {
+			ret = hash_blob_stream(the_repository->hash_algo, oid,
+					       fd, xsize_t(st->st_size));
+		}
 	}
 
 	close(fd);
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
+'
+
 test_expect_success 'stripspace outside repository' '
 	nongit git stripspace -s </dev/null
 '
-- 
2.54.0-rc0-282-gaf2821fd49
Jeff KingApr 7, 2026, 21:29 UTC in reply to Junio C Hamano on lore

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

On Tue, Apr 07, 2026 at 02:18:06PM -0700, Junio C Hamano wrote:
Show 28 quoted lines
> 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. ;)

Show 15 quoted lines
> 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
Junio C HamanoApr 7, 2026, 21:43 UTC in reply to Jeff King on lore

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

Jeff King <peff@peff.net> writes:
Show 21 quoted lines
>> 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.

Yup, the non-repo being somehow outside any repository is also used in the test immediately above this one, so I am inclined to say that we can just enclose the whole thing inside a subshell.

> 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.
Yeah, that is true, too.
Justin ToblerApr 7, 2026, 21:43 UTC in reply to Junio C Hamano on lore

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

On 26/04/07 02:18PM, Junio C Hamano wrote:
Show 6 quoted lines
> Justin Tobler <jltobler@gmail.com> writes:
> 
> I'd appreciate it if you can give your eyeballs to the attached to
> see if that is how you would fixed the bug in the original context
> of v2.52 track.  If everything looks OK, then there is no need to
> spend time backporting on your side.  We have everything necessary.

The amended patch below looks correct to me. Peff had some comments regarding the test that I will work on addressing.

Thanks, -Justin

Show 128 quoted lines
> 
> --- >8 ---
> 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
> 
> In ce1661f9da (odb: add transaction interface, 2025-09-16), existing
> ODB transaction logic is adapted to create a transaction interface
> at the ODB layer. The intent here is for the ODB transaction
> interface to eventually provide an object source agnostic means to
> manage transactions.
> 
> An unintended consequence of this change though is that
> `object-file.c:index_fd()` may enter the ODB transaction path even
> when no object write is requested. In non-repository contexts, this
> can result in a NULL dereference and segfault. One such case occurs
> when running git-diff(1) outside of a repository with
> "core.bigFileThreshold" forcing the streaming path in `index_fd()`:
> 
>         $ echo foo >foo
>         $ echo bar >bar
>         $ git -c core.bigFileThreshold=1 diff -- foo bar
> 
> In this scenario, the caller only needs to compute the object ID. Object
> hashing does not require an ODB, so starting a transaction is both
> unnecessary and invalid.
> 
> Fix the bug by avoiding the use of ODB transactions in `index_fd()` when
> callers are only interested in computing the object hash.
> 
> Reported-by: Luca Stefani <luca.stefani.ge1@gmail.com>
> Signed-off-by: Justin Tobler <jltobler@gmail.com>
> [jc: adjusted to fd13909e (Merge branch 'jt/odb-transaction', 2025-10-02)]
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  object-file.c           | 49 ++++++++++++++++++++++++++++++++++-------
>  t/t1517-outside-repo.sh |  8 +++++++
>  2 files changed, 49 insertions(+), 8 deletions(-)
> 
> diff --git a/object-file.c b/object-file.c
> index 17a236d2fe..0969e27f3d 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -1599,6 +1599,34 @@ static int index_blob_packfile_transaction(struct odb_transaction *transaction,
>  	return 0;
>  }
>  
> +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);
> +
> +	return 0;
> +}
> +
>  int index_fd(struct index_state *istate, struct object_id *oid,
>  	     int fd, struct stat *st,
>  	     enum object_type type, const char *path, unsigned flags)
> @@ -1620,14 +1648,19 @@ 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_transaction *transaction;
> -
> -		transaction = odb_transaction_begin(the_repository->objects);
> -		ret = index_blob_packfile_transaction(the_repository->objects->transaction,
> -						      oid, fd,
> -						      xsize_t(st->st_size),
> -						      path, flags);
> -		odb_transaction_commit(transaction);
> +		if (flags & INDEX_WRITE_OBJECT) {
> +			struct odb_transaction *transaction;
> +
> +			transaction = odb_transaction_begin(the_repository->objects);
> +			ret = index_blob_packfile_transaction(the_repository->objects->transaction,
> +							      oid, fd,
> +							      xsize_t(st->st_size),
> +							      path, flags);
> +			odb_transaction_commit(transaction);
> +		} else {
> +			ret = hash_blob_stream(the_repository->hash_algo, oid,
> +					       fd, xsize_t(st->st_size));
> +		}
>  	}
>  
>  	close(fd);
> 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
> +'
> +
>  test_expect_success 'stripspace outside repository' '
>  	nongit git stripspace -s </dev/null
>  '
> -- 
> 2.54.0-rc0-282-gaf2821fd49
> 
Junio C HamanoApr 7, 2026, 21:53 UTC in reply to Junio C Hamano on lore

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

Junio C Hamano <gitster@pobox.com> writes:
> I'd appreciate it if you can give your eyeballs to the attached to
> see if that is how you would fixed the bug in the original context
> of v2.52 track.  If everything looks OK, then there is no need to
> spend time backporting on your side.  We have everything necessary.

Another thing. Your jt/odb-transaction-write topic (in 'seen') already addresses this issue, so we may merge this single patch down to 'next' and 'master' first, but the merge that brings in the topic can just supersede this patch, perhaps keeping the test added to t1517.

Do I understand correctly? The t1517 part of the patch, when applied to 'seen', does indicate that the problem is not there.

Justin ToblerApr 7, 2026, 22:08 UTC in reply to Junio C Hamano on lore

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

On 26/04/07 02:53PM, Junio C Hamano wrote:
Show 12 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > I'd appreciate it if you can give your eyeballs to the attached to
> > see if that is how you would fixed the bug in the original context
> > of v2.52 track.  If everything looks OK, then there is no need to
> > spend time backporting on your side.  We have everything necessary.
> 
> Another thing.  Your jt/odb-transaction-write topic (in 'seen')
> already addresses this issue, so we may merge this single patch down
> to 'next' and 'master' first, but the merge that brings in the topic
> can just supersede this patch, perhaps keeping the test added to
> t1517.
That would work :)
> Do I understand correctly?  The t1517 part of the patch, when
> applied to 'seen', does indicate that the problem is not there.

Yes, that is correct. The jt/odb-transaction-write topic indeed already fixes this issue. The patch here is just a slimmed down version of a patch from that series.

I can also send a follow up version for the topic built on top of this patch if that would be preferred. Otherwise, replacing the patch with the original topic and keeping the test as you suggested would work just fine.

Thanks, -Justin

Junio C HamanoApr 7, 2026, 22:24 UTC in reply to Justin Tobler on lore

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

Justin Tobler <jltobler@gmail.com> writes:
Show 27 quoted lines
> On 26/04/07 02:53PM, Junio C Hamano wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>> 
>> > I'd appreciate it if you can give your eyeballs to the attached to
>> > see if that is how you would fixed the bug in the original context
>> > of v2.52 track.  If everything looks OK, then there is no need to
>> > spend time backporting on your side.  We have everything necessary.
>> 
>> Another thing.  Your jt/odb-transaction-write topic (in 'seen')
>> already addresses this issue, so we may merge this single patch down
>> to 'next' and 'master' first, but the merge that brings in the topic
>> can just supersede this patch, perhaps keeping the test added to
>> t1517.
>
> That would work :)
>
>> Do I understand correctly?  The t1517 part of the patch, when
>> applied to 'seen', does indicate that the problem is not there.
>
> Yes, that is correct. The jt/odb-transaction-write topic indeed already
> fixes this issue. The patch here is just a slimmed down version of a
> patch from that series.
>
> I can also send a follow up version for the topic built on top of this
> patch if that would be preferred. Otherwise, replacing the patch with
> the original topic and keeping the test as you suggested would work just
> fine.

OK. I think I am almost done preparing for tonight's pushout, so please double check, and complain if you see something that is questionable, when it happens. It probably will happen in 2 hours or so.

Thanks.
Justin ToblerApr 7, 2026, 22:41 UTC in reply to Junio C Hamano on lore

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

On 26/04/07 03:24PM, Junio C Hamano wrote:
Show 34 quoted lines
> Justin Tobler <jltobler@gmail.com> writes:
> 
> > On 26/04/07 02:53PM, Junio C Hamano wrote:
> >> Junio C Hamano <gitster@pobox.com> writes:
> >> 
> >> > I'd appreciate it if you can give your eyeballs to the attached to
> >> > see if that is how you would fixed the bug in the original context
> >> > of v2.52 track.  If everything looks OK, then there is no need to
> >> > spend time backporting on your side.  We have everything necessary.
> >> 
> >> Another thing.  Your jt/odb-transaction-write topic (in 'seen')
> >> already addresses this issue, so we may merge this single patch down
> >> to 'next' and 'master' first, but the merge that brings in the topic
> >> can just supersede this patch, perhaps keeping the test added to
> >> t1517.
> >
> > That would work :)
> >
> >> Do I understand correctly?  The t1517 part of the patch, when
> >> applied to 'seen', does indicate that the problem is not there.
> >
> > Yes, that is correct. The jt/odb-transaction-write topic indeed already
> > fixes this issue. The patch here is just a slimmed down version of a
> > patch from that series.
> >
> > I can also send a follow up version for the topic built on top of this
> > patch if that would be preferred. Otherwise, replacing the patch with
> > the original topic and keeping the test as you suggested would work just
> > fine.
> 
> OK.  I think I am almost done preparing for tonight's pushout, so
> please double check, and complain if you see something that is
> questionable, when it happens.  It probably will happen in 2 hours
> or so.
Thanks, I'll make sure to double check.

I'm not sure if you already included an update for the test per Peff's comments, but if not we can do something like below. If you would like I can send another version with it included too.

Thanks, -Justin

--- >8 ---
Show changes to t/t1517-outside-repo.sh +6 −6
diff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh
index c1dbc6359a..e1d35170de 100755
--- a/t/t1517-outside-repo.sh
+++ b/t/t1517-outside-repo.sh
@@ -93,12 +93,12 @@ 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
+test_expect_success 'hash object exceeding bigFileThreshold outside repository' '
+	(
+		cd non-repo &&
+		echo foo >foo &&
+		git -c core.bigFileThreshold=1 hash-object --stdin <foo
+	)
 '
 
 test_expect_success 'stripspace outside repository' '
Junio C HamanoApr 8, 2026, 00:42 UTC in reply to Justin Tobler on lore

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

Justin Tobler <jltobler@gmail.com> writes:
Show 31 quoted lines
> Thanks, I'll make sure to double check.
>
> I'm not sure if you already included an update for the test per Peff's
> comments, but if not we can do something like below. If you would like I
> can send another version with it included too.
>
> Thanks,
> -Justin
>
> --- >8 ---
> diff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh
> index c1dbc6359a..e1d35170de 100755
> --- a/t/t1517-outside-repo.sh
> +++ b/t/t1517-outside-repo.sh
> @@ -93,12 +93,12 @@ 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
> +test_expect_success 'hash object exceeding bigFileThreshold outside repository' '
> +	(
> +		cd non-repo &&
> +		echo foo >foo &&
> +		git -c core.bigFileThreshold=1 hash-object --stdin <foo
> +	)
>  '
I'll redo the material with the above.

Back to recent threads