{"thread":{"id":"65453","subject":"[PATCH] object-file: avoid ODB transaction when not writing objects","startedAt":"2026-04-07T20:17:35Z","lastAt":"2026-04-08T00:42:05Z","messageCount":10,"participants":["Justin Tobler","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"541092","messageId":"20260407201730.2526914-1-jltobler@gmail.com","threadId":"65453","inReplyTo":null,"subject":"[PATCH] object-file: avoid ODB transaction when not writing objects","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-04-07T20:17:30Z","receivedAt":"2026-04-07T20:17:35Z","isPatch":true,"body":"In ce1661f9da (odb: add transaction interface, 2025-09-16), existing ODB\ntransaction logic is adapted to create a transaction interface at the\nODB layer. The intent here is for the ODB transaction interface to\neventually provide an object source agnostic means to manage\ntransactions.\n\nAn unintended consequence of this change though is that\n`object-file.c:index_fd()` may enter the ODB transaction path even when\nno object write is requested. In non-repository contexts, this can\nresult in a NULL dereference and segfault. One such case occurs when\nrunning git-diff(1) outside of a repository with \"core.bigFileThreshold\"\nforcing the streaming path in `index_fd()`:\n\n $ echo foo >foo\n $ echo bar >bar\n $ git -c core.bigFileThreshold=1 diff -- foo bar\n\nIn this scenario, the caller only needs to compute the object ID. Object\nhashing does not require an ODB, so starting a transaction is both\nunnecessary and invalid.\n\nFix the bug by avoiding the use of ODB transactions in `index_fd()` when\ncallers are only interested in computing the object hash.\n\nReported-by: Luca Stefani \nSigned-off-by: Justin Tobler \n---\n\nGreetings,\n\nThis patch addresses a bug report[1] where performing git-diff(1) on\nfiles that exceed \"core.bigFileThreshold\" outside of a repository causes\na segfault. Originally this patch was included in another series sent to\nthe mailing list[2] as a preparatory refactor. Since it happens to fix\nthe reported bug though, I've extracted it from that series with the\nhope of upstreaming more quickly.\n\nI wasn't entirely sure if this patch should be based on master or maint.\nI went with master, but am happy to resend if this is incorrect.\n\nThanks,\n-Justin\n\n[1]: \n[2]: <20260331033835.2863514-1-jltobler@gmail.com>\n\n---\n object-file.c | 57 ++++++++++++++++++++++++++++++++---------\n t/t1517-outside-repo.sh | 8 ++++++\n 2 files changed, 53 insertions(+), 12 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex 4f77ce0982..63408fc290 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -1640,6 +1640,34 @@ static int index_blob_packfile_transaction(struct odb_transaction_files *transac\n \treturn 0;\n }\n \n+static int hash_blob_stream(const struct git_hash_algo *hash_algo,\n+\t\t\t struct object_id *result_oid, int fd, size_t size)\n+{\n+\tunsigned char buf[16384];\n+\tstruct git_hash_ctx ctx;\n+\tunsigned header_len;\n+\n+\theader_len = format_object_header((char *)buf, sizeof(buf),\n+\t\t\t\t\t OBJ_BLOB, size);\n+\thash_algo->init_fn(&ctx);\n+\tgit_hash_update(&ctx, buf, header_len);\n+\n+\twhile (size) {\n+\t\tsize_t rsize = size < sizeof(buf) ? size : sizeof(buf);\n+\t\tssize_t read_result = read_in_full(fd, buf, rsize);\n+\n+\t\tif ((read_result < 0) || ((size_t)read_result != rsize))\n+\t\t\treturn -1;\n+\n+\t\tgit_hash_update(&ctx, buf, rsize);\n+\t\tsize -= read_result;\n+\t}\n+\n+\tgit_hash_final_oid(result_oid, &ctx);\n+\n+\treturn 0;\n+}\n+\n int index_fd(struct index_state *istate, struct object_id *oid,\n \t int fd, struct stat *st,\n \t enum object_type type, const char *path, unsigned flags)\n@@ -1661,18 +1689,23 @@ int index_fd(struct index_state *istate, struct object_id *oid,\n \t\tret = index_core(istate, oid, fd, xsize_t(st->st_size),\n \t\t\t\t type, path, flags);\n \t} else {\n-\t\tstruct object_database *odb = the_repository->objects;\n-\t\tstruct odb_transaction_files *files_transaction;\n-\t\tstruct odb_transaction *transaction;\n-\n-\t\ttransaction = odb_transaction_begin(odb);\n-\t\tfiles_transaction = container_of(odb->transaction,\n-\t\t\t\t\t\t struct odb_transaction_files,\n-\t\t\t\t\t\t base);\n-\t\tret = index_blob_packfile_transaction(files_transaction, oid, fd,\n-\t\t\t\t\t\t xsize_t(st->st_size),\n-\t\t\t\t\t\t path, flags);\n-\t\todb_transaction_commit(transaction);\n+\t\tif (flags & INDEX_WRITE_OBJECT) {\n+\t\t\tstruct object_database *odb = the_repository->objects;\n+\t\t\tstruct odb_transaction_files *files_transaction;\n+\t\t\tstruct odb_transaction *transaction;\n+\n+\t\t\ttransaction = odb_transaction_begin(odb);\n+\t\t\tfiles_transaction = container_of(odb->transaction,\n+\t\t\t\t\t\t\t struct odb_transaction_files,\n+\t\t\t\t\t\t\t base);\n+\t\t\tret = index_blob_packfile_transaction(files_transaction, oid, fd,\n+\t\t\t\t\t\t\t xsize_t(st->st_size),\n+\t\t\t\t\t\t\t path, flags);\n+\t\t\todb_transaction_commit(transaction);\n+\t\t} else {\n+\t\t\tret = hash_blob_stream(the_repository->hash_algo, oid,\n+\t\t\t\t\t fd, xsize_t(st->st_size));\n+\t\t}\n \t}\n \n \tclose(fd);\ndiff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh\nindex c824c1a25c..c1dbc6359a 100755\n--- a/t/t1517-outside-repo.sh\n+++ b/t/t1517-outside-repo.sh\n@@ -93,6 +93,14 @@ test_expect_success 'diff outside repository' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'diff files exceeding bigFileThreshold outside repository' '\n+\tcd non-repo &&\n+\techo foo >foo &&\n+\techo bar >bar &&\n+\ttest_must_fail git -c core.bigFileThreshold=1 diff -- foo bar >actual &&\n+\ttest_grep \"diff --git a/foo b/bar\" actual\n+'\n+\n test_expect_success 'stripspace outside repository' '\n \tnongit git stripspace -s writes:\n\n> This patch addresses a bug report[1] where performing git-diff(1) on\n> files that exceed \"core.bigFileThreshold\" outside of a repository causes\n> a segfault. Originally this patch was included in another series sent to\n> the mailing list[2] as a preparatory refactor. Since it happens to fix\n> the reported bug though, I've extracted it from that series with the\n> hope of upstreaming more quickly.\n\nOK, so the bug was introduced in fd13909e (Merge branch\n'jt/odb-transaction', 2025-10-02) aka v2.52.0-rc0~91 and applying\nthe test part of your patch alone on v2.52 indeed fails t1517. The\ncode change does not directly apply to the old codebase, so I\nwiggled your patch to make it apply to v2.52 codebase.\n\n> I wasn't entirely sure if this patch should be based on master or maint.\n> I went with master, but am happy to resend if this is incorrect.\n\nSo if we wanted to fix the past releases, the attached may be where\nwe want to start, then adjusting backwards to the shape of the patch\nyou posted as we merge it up to v2.53 and v2.54 track.\n\nI've applied this to 'maint-2.52' (done and tested), 'maint-2.53'\n(done and tested), and to 'master', and made sure that the last one\nmatches the result of applying your patch directly on top of\n'master'.\n\nI'd appreciate it if you can give your eyeballs to the attached to\nsee if that is how you would fixed the bug in the original context\nof v2.52 track. If everything looks OK, then there is no need to\nspend time backporting on your side. We have everything necessary.\n\nThanks.\n\n--- >8 ---\nFrom: Justin Tobler \nDate: Tue, 7 Apr 2026 15:17:30 -0500\nSubject: [PATCH] object-file: avoid ODB transaction when not writing objects\n\nIn ce1661f9da (odb: add transaction interface, 2025-09-16), existing\nODB transaction logic is adapted to create a transaction interface\nat the ODB layer. The intent here is for the ODB transaction\ninterface to eventually provide an object source agnostic means to\nmanage transactions.\n\nAn unintended consequence of this change though is that\n`object-file.c:index_fd()` may enter the ODB transaction path even\nwhen no object write is requested. In non-repository contexts, this\ncan result in a NULL dereference and segfault. One such case occurs\nwhen running git-diff(1) outside of a repository with\n\"core.bigFileThreshold\" forcing the streaming path in `index_fd()`:\n\n $ echo foo >foo\n $ echo bar >bar\n $ git -c core.bigFileThreshold=1 diff -- foo bar\n\nIn this scenario, the caller only needs to compute the object ID. Object\nhashing does not require an ODB, so starting a transaction is both\nunnecessary and invalid.\n\nFix the bug by avoiding the use of ODB transactions in `index_fd()` when\ncallers are only interested in computing the object hash.\n\nReported-by: Luca Stefani \nSigned-off-by: Justin Tobler \n[jc: adjusted to fd13909e (Merge branch 'jt/odb-transaction', 2025-10-02)]\nSigned-off-by: Junio C Hamano \n---\n object-file.c | 49 ++++++++++++++++++++++++++++++++++-------\n t/t1517-outside-repo.sh | 8 +++++++\n 2 files changed, 49 insertions(+), 8 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex 17a236d2fe..0969e27f3d 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -1599,6 +1599,34 @@ static int index_blob_packfile_transaction(struct odb_transaction *transaction,\n \treturn 0;\n }\n \n+static int hash_blob_stream(const struct git_hash_algo *hash_algo,\n+\t\t\t struct object_id *result_oid, int fd, size_t size)\n+{\n+\tunsigned char buf[16384];\n+\tstruct git_hash_ctx ctx;\n+\tunsigned header_len;\n+\n+\theader_len = format_object_header((char *)buf, sizeof(buf),\n+\t\t\t\t\t OBJ_BLOB, size);\n+\thash_algo->init_fn(&ctx);\n+\tgit_hash_update(&ctx, buf, header_len);\n+\n+\twhile (size) {\n+\t\tsize_t rsize = size < sizeof(buf) ? size : sizeof(buf);\n+\t\tssize_t read_result = read_in_full(fd, buf, rsize);\n+\n+\t\tif ((read_result < 0) || ((size_t)read_result != rsize))\n+\t\t\treturn -1;\n+\n+\t\tgit_hash_update(&ctx, buf, rsize);\n+\t\tsize -= read_result;\n+\t}\n+\n+\tgit_hash_final_oid(result_oid, &ctx);\n+\n+\treturn 0;\n+}\n+\n int index_fd(struct index_state *istate, struct object_id *oid,\n \t int fd, struct stat *st,\n \t enum object_type type, const char *path, unsigned flags)\n@@ -1620,14 +1648,19 @@ int index_fd(struct index_state *istate, struct object_id *oid,\n \t\tret = index_core(istate, oid, fd, xsize_t(st->st_size),\n \t\t\t\t type, path, flags);\n \t} else {\n-\t\tstruct odb_transaction *transaction;\n-\n-\t\ttransaction = odb_transaction_begin(the_repository->objects);\n-\t\tret = index_blob_packfile_transaction(the_repository->objects->transaction,\n-\t\t\t\t\t\t oid, fd,\n-\t\t\t\t\t\t xsize_t(st->st_size),\n-\t\t\t\t\t\t path, flags);\n-\t\todb_transaction_commit(transaction);\n+\t\tif (flags & INDEX_WRITE_OBJECT) {\n+\t\t\tstruct odb_transaction *transaction;\n+\n+\t\t\ttransaction = odb_transaction_begin(the_repository->objects);\n+\t\t\tret = index_blob_packfile_transaction(the_repository->objects->transaction,\n+\t\t\t\t\t\t\t oid, fd,\n+\t\t\t\t\t\t\t xsize_t(st->st_size),\n+\t\t\t\t\t\t\t path, flags);\n+\t\t\todb_transaction_commit(transaction);\n+\t\t} else {\n+\t\t\tret = hash_blob_stream(the_repository->hash_algo, oid,\n+\t\t\t\t\t fd, xsize_t(st->st_size));\n+\t\t}\n \t}\n \n \tclose(fd);\ndiff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh\nindex c824c1a25c..c1dbc6359a 100755\n--- a/t/t1517-outside-repo.sh\n+++ b/t/t1517-outside-repo.sh\n@@ -93,6 +93,14 @@ test_expect_success 'diff outside repository' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'diff files exceeding bigFileThreshold outside repository' '\n+\tcd non-repo &&\n+\techo foo >foo &&\n+\techo bar >bar &&\n+\ttest_must_fail git -c core.bigFileThreshold=1 diff -- foo bar >actual &&\n+\ttest_grep \"diff --git a/foo b/bar\" actual\n+'\n+\n test_expect_success 'stripspace outside repository' '\n \tnongit git stripspace -s From: Justin Tobler \n> Date: Tue, 7 Apr 2026 15:17:30 -0500\n> Subject: [PATCH] object-file: avoid ODB transaction when not writing objects\n> [...]\n> +static int hash_blob_stream(const struct git_hash_algo *hash_algo,\n> +\t\t\t struct object_id *result_oid, int fd, size_t size)\n> +{\n> +\tunsigned char buf[16384];\n> +\tstruct git_hash_ctx ctx;\n> +\tunsigned header_len;\n> +\n> +\theader_len = format_object_header((char *)buf, sizeof(buf),\n> +\t\t\t\t\t OBJ_BLOB, size);\n> +\thash_algo->init_fn(&ctx);\n> +\tgit_hash_update(&ctx, buf, header_len);\n> +\n> +\twhile (size) {\n> +\t\tsize_t rsize = size < sizeof(buf) ? size : sizeof(buf);\n> +\t\tssize_t read_result = read_in_full(fd, buf, rsize);\n> +\n> +\t\tif ((read_result < 0) || ((size_t)read_result != rsize))\n> +\t\t\treturn -1;\n> +\n> +\t\tgit_hash_update(&ctx, buf, rsize);\n> +\t\tsize -= read_result;\n> +\t}\n> +\n> +\tgit_hash_final_oid(result_oid, &ctx);\n\nThis looks correct to me. In the back of my mind I felt like we might\nalready have a function to check a streaming hash, but I was just\nthinking of how parse_object() streams blobs for its hash-check. And\nthat is always coming from the object database, whereas here we are\ntaking data from elsewhere. So we do need this new function.\n\nI probably would have used fewer parentheses in the conditional, but\nthat may be personal preference. ;)\n\n> diff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh\n> index c824c1a25c..c1dbc6359a 100755\n> --- a/t/t1517-outside-repo.sh\n> +++ b/t/t1517-outside-repo.sh\n> @@ -93,6 +93,14 @@ test_expect_success 'diff outside repository' '\n> \ttest_cmp expect actual\n> '\n> \n> +test_expect_success 'diff files exceeding bigFileThreshold outside repository' '\n> +\tcd non-repo &&\n> +\techo foo >foo &&\n> +\techo bar >bar &&\n> +\ttest_must_fail git -c core.bigFileThreshold=1 diff -- foo bar >actual &&\n> +\ttest_grep \"diff --git a/foo b/bar\" actual\n> +'\n\nThis does a \"cd\" outside of a sub-shell, which affects all of the\nsubsequent tests.\n\nWe also are already using the \"nongit\" wrapper in this script, so it\ncould be used here.\n\nThought it was found originally with diff, the bug can also be\ndemonstrated with just hash-object, which does make the test a little\nsimpler.\n\nThe second and third are more style/taste questions, but I think the\nfirst is a blocker.\n\n-Peff\n"},{"id":"541096","messageId":"xmqqjyui30rf.fsf@gitster.g","threadId":"65453","inReplyTo":"20260407212930.GA1315247@coredump.intra.peff.net","subject":"Re: [PATCH] object-file: avoid ODB transaction when not writing objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-04-07T21:43:00Z","receivedAt":"2026-04-07T21:43:03Z","isPatch":true,"body":"Jeff King writes:\n\n>> diff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh\n>> index c824c1a25c..c1dbc6359a 100755\n>> --- a/t/t1517-outside-repo.sh\n>> +++ b/t/t1517-outside-repo.sh\n>> @@ -93,6 +93,14 @@ test_expect_success 'diff outside repository' '\n>> \ttest_cmp expect actual\n>> '\n>> \n>> +test_expect_success 'diff files exceeding bigFileThreshold outside repository' '\n>> +\tcd non-repo &&\n>> +\techo foo >foo &&\n>> +\techo bar >bar &&\n>> +\ttest_must_fail git -c core.bigFileThreshold=1 diff -- foo bar >actual &&\n>> +\ttest_grep \"diff --git a/foo b/bar\" actual\n>> +'\n>\n> This does a \"cd\" outside of a sub-shell, which affects all of the\n> subsequent tests.\n>\n> We also are already using the \"nongit\" wrapper in this script, so it\n> could be used here.\n\nYup, the non-repo being somehow outside any repository is also used\nin the test immediately above this one, so I am inclined to say that\nwe can just enclose the whole thing inside a subshell.\n\n> Thought it was found originally with diff, the bug can also be\n> demonstrated with just hash-object, which does make the test a little\n> simpler.\n\nYeah, that is true, too.\n"},{"id":"541097","messageId":"adV44IS5iPwdjKAe@denethor","threadId":"65453","inReplyTo":"xmqqo6ju31wx.fsf@gitster.g","subject":"Re: [PATCH] object-file: avoid ODB transaction when not writing objects","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-04-07T21:43:08Z","receivedAt":"2026-04-07T21:43:10Z","isPatch":true,"body":"On 26/04/07 02:18PM, Junio C Hamano wrote:\n> Justin Tobler writes:\n> \n> I'd appreciate it if you can give your eyeballs to the attached to\n> see if that is how you would fixed the bug in the original context\n> of v2.52 track. If everything looks OK, then there is no need to\n> spend time backporting on your side. We have everything necessary.\n\nThe amended patch below looks correct to me. Peff had some comments\nregarding the test that I will work on addressing.\n\nThanks,\n-Justin\n\n> \n> --- >8 ---\n> From: Justin Tobler \n> Date: Tue, 7 Apr 2026 15:17:30 -0500\n> Subject: [PATCH] object-file: avoid ODB transaction when not writing objects\n> \n> In ce1661f9da (odb: add transaction interface, 2025-09-16), existing\n> ODB transaction logic is adapted to create a transaction interface\n> at the ODB layer. The intent here is for the ODB transaction\n> interface to eventually provide an object source agnostic means to\n> manage transactions.\n> \n> An unintended consequence of this change though is that\n> `object-file.c:index_fd()` may enter the ODB transaction path even\n> when no object write is requested. In non-repository contexts, this\n> can result in a NULL dereference and segfault. One such case occurs\n> when running git-diff(1) outside of a repository with\n> \"core.bigFileThreshold\" forcing the streaming path in `index_fd()`:\n> \n> $ echo foo >foo\n> $ echo bar >bar\n> $ git -c core.bigFileThreshold=1 diff -- foo bar\n> \n> In this scenario, the caller only needs to compute the object ID. Object\n> hashing does not require an ODB, so starting a transaction is both\n> unnecessary and invalid.\n> \n> Fix the bug by avoiding the use of ODB transactions in `index_fd()` when\n> callers are only interested in computing the object hash.\n> \n> Reported-by: Luca Stefani \n> Signed-off-by: Justin Tobler \n> [jc: adjusted to fd13909e (Merge branch 'jt/odb-transaction', 2025-10-02)]\n> Signed-off-by: Junio C Hamano \n> ---\n> object-file.c | 49 ++++++++++++++++++++++++++++++++++-------\n> t/t1517-outside-repo.sh | 8 +++++++\n> 2 files changed, 49 insertions(+), 8 deletions(-)\n> \n> diff --git a/object-file.c b/object-file.c\n> index 17a236d2fe..0969e27f3d 100644\n> --- a/object-file.c\n> +++ b/object-file.c\n> @@ -1599,6 +1599,34 @@ static int index_blob_packfile_transaction(struct odb_transaction *transaction,\n> \treturn 0;\n> }\n> \n> +static int hash_blob_stream(const struct git_hash_algo *hash_algo,\n> +\t\t\t struct object_id *result_oid, int fd, size_t size)\n> +{\n> +\tunsigned char buf[16384];\n> +\tstruct git_hash_ctx ctx;\n> +\tunsigned header_len;\n> +\n> +\theader_len = format_object_header((char *)buf, sizeof(buf),\n> +\t\t\t\t\t OBJ_BLOB, size);\n> +\thash_algo->init_fn(&ctx);\n> +\tgit_hash_update(&ctx, buf, header_len);\n> +\n> +\twhile (size) {\n> +\t\tsize_t rsize = size < sizeof(buf) ? size : sizeof(buf);\n> +\t\tssize_t read_result = read_in_full(fd, buf, rsize);\n> +\n> +\t\tif ((read_result < 0) || ((size_t)read_result != rsize))\n> +\t\t\treturn -1;\n> +\n> +\t\tgit_hash_update(&ctx, buf, rsize);\n> +\t\tsize -= read_result;\n> +\t}\n> +\n> +\tgit_hash_final_oid(result_oid, &ctx);\n> +\n> +\treturn 0;\n> +}\n> +\n> int index_fd(struct index_state *istate, struct object_id *oid,\n> \t int fd, struct stat *st,\n> \t enum object_type type, const char *path, unsigned flags)\n> @@ -1620,14 +1648,19 @@ int index_fd(struct index_state *istate, struct object_id *oid,\n> \t\tret = index_core(istate, oid, fd, xsize_t(st->st_size),\n> \t\t\t\t type, path, flags);\n> \t} else {\n> -\t\tstruct odb_transaction *transaction;\n> -\n> -\t\ttransaction = odb_transaction_begin(the_repository->objects);\n> -\t\tret = index_blob_packfile_transaction(the_repository->objects->transaction,\n> -\t\t\t\t\t\t oid, fd,\n> -\t\t\t\t\t\t xsize_t(st->st_size),\n> -\t\t\t\t\t\t path, flags);\n> -\t\todb_transaction_commit(transaction);\n> +\t\tif (flags & INDEX_WRITE_OBJECT) {\n> +\t\t\tstruct odb_transaction *transaction;\n> +\n> +\t\t\ttransaction = odb_transaction_begin(the_repository->objects);\n> +\t\t\tret = index_blob_packfile_transaction(the_repository->objects->transaction,\n> +\t\t\t\t\t\t\t oid, fd,\n> +\t\t\t\t\t\t\t xsize_t(st->st_size),\n> +\t\t\t\t\t\t\t path, flags);\n> +\t\t\todb_transaction_commit(transaction);\n> +\t\t} else {\n> +\t\t\tret = hash_blob_stream(the_repository->hash_algo, oid,\n> +\t\t\t\t\t fd, xsize_t(st->st_size));\n> +\t\t}\n> \t}\n> \n> \tclose(fd);\n> diff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh\n> index c824c1a25c..c1dbc6359a 100755\n> --- a/t/t1517-outside-repo.sh\n> +++ b/t/t1517-outside-repo.sh\n> @@ -93,6 +93,14 @@ test_expect_success 'diff outside repository' '\n> \ttest_cmp expect actual\n> '\n> \n> +test_expect_success 'diff files exceeding bigFileThreshold outside repository' '\n> +\tcd non-repo &&\n> +\techo foo >foo &&\n> +\techo bar >bar &&\n> +\ttest_must_fail git -c core.bigFileThreshold=1 diff -- foo bar >actual &&\n> +\ttest_grep \"diff --git a/foo b/bar\" actual\n> +'\n> +\n> test_expect_success 'stripspace outside repository' '\n> \tnongit git stripspace -s '\n> -- \n> 2.54.0-rc0-282-gaf2821fd49\n> \n"},{"id":"541100","messageId":"xmqqfr563099.fsf@gitster.g","threadId":"65453","inReplyTo":"xmqqo6ju31wx.fsf@gitster.g","subject":"Re: [PATCH] object-file: avoid ODB transaction when not writing objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-04-07T21:53:54Z","receivedAt":"2026-04-07T21:53:56Z","isPatch":true,"body":"Junio C Hamano writes:\n\n> I'd appreciate it if you can give your eyeballs to the attached to\n> see if that is how you would fixed the bug in the original context\n> of v2.52 track. If everything looks OK, then there is no need to\n> spend time backporting on your side. We have everything necessary.\n\nAnother thing. Your jt/odb-transaction-write topic (in 'seen')\nalready addresses this issue, so we may merge this single patch down\nto 'next' and 'master' first, but the merge that brings in the topic\ncan just supersede this patch, perhaps keeping the test added to\nt1517.\n\nDo I understand correctly? The t1517 part of the patch, when\napplied to 'seen', does indicate that the problem is not there.\n\n\n"},{"id":"541103","messageId":"adV-NI-t6-jgJK7D@denethor","threadId":"65453","inReplyTo":"xmqqfr563099.fsf@gitster.g","subject":"Re: [PATCH] object-file: avoid ODB transaction when not writing objects","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-04-07T22:08:54Z","receivedAt":"2026-04-07T22:08:58Z","isPatch":true,"body":"On 26/04/07 02:53PM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n> \n> > I'd appreciate it if you can give your eyeballs to the attached to\n> > see if that is how you would fixed the bug in the original context\n> > of v2.52 track. If everything looks OK, then there is no need to\n> > spend time backporting on your side. We have everything necessary.\n> \n> Another thing. Your jt/odb-transaction-write topic (in 'seen')\n> already addresses this issue, so we may merge this single patch down\n> to 'next' and 'master' first, but the merge that brings in the topic\n> can just supersede this patch, perhaps keeping the test added to\n> t1517.\n\nThat would work :)\n\n> Do I understand correctly? The t1517 part of the patch, when\n> applied to 'seen', does indicate that the problem is not there.\n\nYes, that is correct. The jt/odb-transaction-write topic indeed already\nfixes this issue. The patch here is just a slimmed down version of a\npatch from that series.\n\nI can also send a follow up version for the topic built on top of this\npatch if that would be preferred. Otherwise, replacing the patch with\nthe original topic and keeping the test as you suggested would work just\nfine.\n\nThanks,\n-Justin\n"},{"id":"541104","messageId":"xmqq7bqi2yux.fsf@gitster.g","threadId":"65453","inReplyTo":"adV-NI-t6-jgJK7D@denethor","subject":"Re: [PATCH] object-file: avoid ODB transaction when not writing objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-04-07T22:24:06Z","receivedAt":"2026-04-07T22:24:08Z","isPatch":true,"body":"Justin Tobler writes:\n\n> On 26/04/07 02:53PM, Junio C Hamano wrote:\n>> Junio C Hamano writes:\n>> \n>> > I'd appreciate it if you can give your eyeballs to the attached to\n>> > see if that is how you would fixed the bug in the original context\n>> > of v2.52 track. If everything looks OK, then there is no need to\n>> > spend time backporting on your side. We have everything necessary.\n>> \n>> Another thing. Your jt/odb-transaction-write topic (in 'seen')\n>> already addresses this issue, so we may merge this single patch down\n>> to 'next' and 'master' first, but the merge that brings in the topic\n>> can just supersede this patch, perhaps keeping the test added to\n>> t1517.\n>\n> That would work :)\n>\n>> Do I understand correctly? The t1517 part of the patch, when\n>> applied to 'seen', does indicate that the problem is not there.\n>\n> Yes, that is correct. The jt/odb-transaction-write topic indeed already\n> fixes this issue. The patch here is just a slimmed down version of a\n> patch from that series.\n>\n> I can also send a follow up version for the topic built on top of this\n> patch if that would be preferred. Otherwise, replacing the patch with\n> the original topic and keeping the test as you suggested would work just\n> fine.\n\nOK. I think I am almost done preparing for tonight's pushout, so\nplease double check, and complain if you see something that is\nquestionable, when it happens. It probably will happen in 2 hours\nor so.\n\nThanks.\n\n"},{"id":"541106","messageId":"adWGjWNTXlR-glgt@denethor","threadId":"65453","inReplyTo":"xmqq7bqi2yux.fsf@gitster.g","subject":"Re: [PATCH] object-file: avoid ODB transaction when not writing objects","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-04-07T22:41:12Z","receivedAt":"2026-04-07T22:41:15Z","isPatch":true,"body":"On 26/04/07 03:24PM, Junio C Hamano wrote:\n> Justin Tobler writes:\n> \n> > On 26/04/07 02:53PM, Junio C Hamano wrote:\n> >> Junio C Hamano writes:\n> >> \n> >> > I'd appreciate it if you can give your eyeballs to the attached to\n> >> > see if that is how you would fixed the bug in the original context\n> >> > of v2.52 track. If everything looks OK, then there is no need to\n> >> > spend time backporting on your side. We have everything necessary.\n> >> \n> >> Another thing. Your jt/odb-transaction-write topic (in 'seen')\n> >> already addresses this issue, so we may merge this single patch down\n> >> to 'next' and 'master' first, but the merge that brings in the topic\n> >> can just supersede this patch, perhaps keeping the test added to\n> >> t1517.\n> >\n> > That would work :)\n> >\n> >> Do I understand correctly? The t1517 part of the patch, when\n> >> applied to 'seen', does indicate that the problem is not there.\n> >\n> > Yes, that is correct. The jt/odb-transaction-write topic indeed already\n> > fixes this issue. The patch here is just a slimmed down version of a\n> > patch from that series.\n> >\n> > I can also send a follow up version for the topic built on top of this\n> > patch if that would be preferred. Otherwise, replacing the patch with\n> > the original topic and keeping the test as you suggested would work just\n> > fine.\n> \n> OK. I think I am almost done preparing for tonight's pushout, so\n> please double check, and complain if you see something that is\n> questionable, when it happens. It probably will happen in 2 hours\n> or so.\n\nThanks, I'll make sure to double check.\n\nI'm not sure if you already included an update for the test per Peff's\ncomments, but if not we can do something like below. If you would like I\ncan send another version with it included too.\n\nThanks,\n-Justin\n\n--- >8 ---\ndiff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh\nindex c1dbc6359a..e1d35170de 100755\n--- a/t/t1517-outside-repo.sh\n+++ b/t/t1517-outside-repo.sh\n@@ -93,12 +93,12 @@ test_expect_success 'diff outside repository' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success 'diff files exceeding bigFileThreshold outside repository' '\n-\tcd non-repo &&\n-\techo foo >foo &&\n-\techo bar >bar &&\n-\ttest_must_fail git -c core.bigFileThreshold=1 diff -- foo bar >actual &&\n-\ttest_grep \"diff --git a/foo b/bar\" actual\n+test_expect_success 'hash object exceeding bigFileThreshold outside repository' '\n+\t(\n+\t\tcd non-repo &&\n+\t\techo foo >foo &&\n+\t\tgit -c core.bigFileThreshold=1 hash-object --stdin writes:\n\n> Thanks, I'll make sure to double check.\n>\n> I'm not sure if you already included an update for the test per Peff's\n> comments, but if not we can do something like below. If you would like I\n> can send another version with it included too.\n>\n> Thanks,\n> -Justin\n>\n> --- >8 ---\n> diff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh\n> index c1dbc6359a..e1d35170de 100755\n> --- a/t/t1517-outside-repo.sh\n> +++ b/t/t1517-outside-repo.sh\n> @@ -93,12 +93,12 @@ test_expect_success 'diff outside repository' '\n> \ttest_cmp expect actual\n> '\n> \n> -test_expect_success 'diff files exceeding bigFileThreshold outside repository' '\n> -\tcd non-repo &&\n> -\techo foo >foo &&\n> -\techo bar >bar &&\n> -\ttest_must_fail git -c core.bigFileThreshold=1 diff -- foo bar >actual &&\n> -\ttest_grep \"diff --git a/foo b/bar\" actual\n> +test_expect_success 'hash object exceeding bigFileThreshold outside repository' '\n> +\t(\n> +\t\tcd non-repo &&\n> +\t\techo foo >foo &&\n> +\t\tgit -c core.bigFileThreshold=1 hash-object --stdin +\t)\n> '\n\nI'll redo the material with the above.\n"}]}