Re: [PATCH 2/6] object-file: propagate files transaction errors
From: Patrick Steinhardt <hidden>
Date: 2026-06-30 08:45:55
On Mon, Jun 29, 2026 at 02:04:08PM -0500, Justin Tobler wrote:
On 26/06/29 01:58PM, Justin Tobler wrote:quoted
On 26/06/24 01:26PM, Patrick Steinhardt wrote:quoted
On Tue, Jun 23, 2026 at 11:19:16PM -0500, Justin Tobler wrote:quoted
@@ -511,11 +511,15 @@ static void odb_transaction_files_prepare(struct odb_transaction *base) * added at the time they call odb_transaction_files_begin. */ if (!transaction || transaction->objdir) - return; + return 0; transaction->objdir = tmp_objdir_create(base->source->odb->repo, "bulk-fsync"); - if (transaction->objdir) - tmp_objdir_replace_primary_odb(transaction->objdir, 0); + if (!transaction->objdir) + return -1;Huh. So previously we just didn't handle this error at all and just continued to tag along? Did that result in anything sensible or was this just YOLOing it?Good question. Previously if there was an error, we wouldn't end up creating any tmpdir and would instead continue to use the primary ODB to write objects in. This change would make it a hard error if we fail to create the temp dir. This matches the behavior that git-receive-pack(1) expects, but I didn't consider that the existing callers could transparently handle there being no temp dir. I suspect we may want existing ODB transaction users to continue being resilient in the same manner. In the next version, I'll maintain the same behavior.I think I got a bit ahead of myself. The existing callers of odb_transaction_files_prepare() still continue to ignore this error. So the behavior already does remain the same here.
Oh, well, okay. I think this behaviour is plain bad -- if the caller wants to have a transaction, then we should bail in case we cannot create one. But this doesn't need to be fixed in this patch series. Patrick