Git development
 help / color / mirror / Atom feed
* [PATCH] object-file: avoid ODB transaction when not writing objects
@ 2026-04-07 20:17 Justin Tobler
  2026-04-07 21:18 ` Junio C Hamano
  0 siblings, 1 reply; 10+ messages in thread
From: Justin Tobler @ 2026-04-07 20:17 UTC (permalink / raw)
  To: git; +Cc: ps, gitster, peff, luca.stefani.ge1, Justin Tobler

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(-)

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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] object-file: avoid ODB transaction when not writing objects
  2026-04-07 20:17 [PATCH] object-file: avoid ODB transaction when not writing objects Justin Tobler
@ 2026-04-07 21:18 ` Junio C Hamano
  2026-04-07 21:29   ` Jeff King
                     ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Junio C Hamano @ 2026-04-07 21:18 UTC (permalink / raw)
  To: Justin Tobler; +Cc: git, ps, peff, luca.stefani.ge1

Justin Tobler <jltobler@gmail.com> writes:

> 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(-)

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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] object-file: avoid ODB transaction when not writing objects
  2026-04-07 21:18 ` Junio C Hamano
@ 2026-04-07 21:29   ` Jeff King
  2026-04-07 21:43     ` Junio C Hamano
  2026-04-07 21:43   ` Justin Tobler
  2026-04-07 21:53   ` Junio C Hamano
  2 siblings, 1 reply; 10+ messages in thread
From: Jeff King @ 2026-04-07 21:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Justin Tobler, git, ps, luca.stefani.ge1

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] object-file: avoid ODB transaction when not writing objects
  2026-04-07 21:29   ` Jeff King
@ 2026-04-07 21:43     ` Junio C Hamano
  0 siblings, 0 replies; 10+ messages in thread
From: Junio C Hamano @ 2026-04-07 21:43 UTC (permalink / raw)
  To: Jeff King; +Cc: Justin Tobler, git, ps, luca.stefani.ge1

Jeff King <peff@peff.net> writes:

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] object-file: avoid ODB transaction when not writing objects
  2026-04-07 21:18 ` Junio C Hamano
  2026-04-07 21:29   ` Jeff King
@ 2026-04-07 21:43   ` Justin Tobler
  2026-04-07 21:53   ` Junio C Hamano
  2 siblings, 0 replies; 10+ messages in thread
From: Justin Tobler @ 2026-04-07 21:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, ps, peff, luca.stefani.ge1

On 26/04/07 02:18PM, Junio C Hamano wrote:
> 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

> 
> --- >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
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] object-file: avoid ODB transaction when not writing objects
  2026-04-07 21:18 ` Junio C Hamano
  2026-04-07 21:29   ` Jeff King
  2026-04-07 21:43   ` Justin Tobler
@ 2026-04-07 21:53   ` Junio C Hamano
  2026-04-07 22:08     ` Justin Tobler
  2 siblings, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2026-04-07 21:53 UTC (permalink / raw)
  To: Justin Tobler; +Cc: git, ps, peff, luca.stefani.ge1

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.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] object-file: avoid ODB transaction when not writing objects
  2026-04-07 21:53   ` Junio C Hamano
@ 2026-04-07 22:08     ` Justin Tobler
  2026-04-07 22:24       ` Junio C Hamano
  0 siblings, 1 reply; 10+ messages in thread
From: Justin Tobler @ 2026-04-07 22:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, ps, peff, luca.stefani.ge1

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.

Thanks,
-Justin

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] object-file: avoid ODB transaction when not writing objects
  2026-04-07 22:08     ` Justin Tobler
@ 2026-04-07 22:24       ` Junio C Hamano
  2026-04-07 22:41         ` Justin Tobler
  0 siblings, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2026-04-07 22:24 UTC (permalink / raw)
  To: Justin Tobler; +Cc: git, ps, peff, luca.stefani.ge1

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.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] object-file: avoid ODB transaction when not writing objects
  2026-04-07 22:24       ` Junio C Hamano
@ 2026-04-07 22:41         ` Justin Tobler
  2026-04-08  0:42           ` Junio C Hamano
  0 siblings, 1 reply; 10+ messages in thread
From: Justin Tobler @ 2026-04-07 22:41 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, ps, peff, luca.stefani.ge1

On 26/04/07 03:24PM, Junio C Hamano wrote:
> 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 ---
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' '


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] object-file: avoid ODB transaction when not writing objects
  2026-04-07 22:41         ` Justin Tobler
@ 2026-04-08  0:42           ` Junio C Hamano
  0 siblings, 0 replies; 10+ messages in thread
From: Junio C Hamano @ 2026-04-08  0:42 UTC (permalink / raw)
  To: Justin Tobler; +Cc: git, ps, peff, luca.stefani.ge1

Justin Tobler <jltobler@gmail.com> writes:

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-04-08  0:42 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-07 20:17 [PATCH] object-file: avoid ODB transaction when not writing objects Justin Tobler
2026-04-07 21:18 ` Junio C Hamano
2026-04-07 21:29   ` Jeff King
2026-04-07 21:43     ` Junio C Hamano
2026-04-07 21:43   ` Justin Tobler
2026-04-07 21:53   ` Junio C Hamano
2026-04-07 22:08     ` Justin Tobler
2026-04-07 22:24       ` Junio C Hamano
2026-04-07 22:41         ` Justin Tobler
2026-04-08  0:42           ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox