Re: [PATCH 1/6] odb/transaction: add transaction release interface

[Patrick Steinhardt <ps@pks.im>]

On Thu, Aug 06, 2026 at 04:38:54PM -0500, Justin Tobler wrote:
> When committing an ODB transaction via `odb_transaction_commit()`, the
> staged objects are made visible and the underlying transaction is freed
> at the same time. Coupling these two steps does not leave room for any
> post-commit transaction operations to be introduced though. Such a
> capability is useful if an ODB transaction backend needs to hold on to
> lockfiles after transaction commit until references are updated, as is
> the case with the existing "files" backend in git-receive-pack(1).

Right. We don't want to remove ".keep" files until references have been
updated so that the potentially still unreachable objects won't get
pruned. And consequently we have to introduce an additional phase after
the transaction has been committed but before the refs were updated.

> Stop freeing the transaction in `odb_transaction_commit()` and introduce
> `odb_transaction_release()` to explicitly clean up the transaction
> accordingly. Note that the release interface also provides an optional
> callback for any backend-specific deferred cleanup. In a subsequent
> commit, the "files" transaction backend will use this to remove ".keep"
> files generated for packfiles received via git-receive-pack(1) after
> references have been updated.

I'm not a 100% sure whether I like "release" as a name, as it typically
indicates that we release memory and other resources hold on by Git. On
the other hand we also kind of release state in this case here, but it
feels like the consequence of that is broader than it usually is.

How about we call this "finalize" instead?

> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index 86933d8d7e..420de9aa7f 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -2714,6 +2714,7 @@ int cmd_receive_pack(int argc,
>  		use_keepalive = KEEPALIVE_ALWAYS;
>  		execute_commands(commands, unpack_status, &si, transaction,
>  				 &push_options);
> +		odb_transaction_release(transaction);
>  		delete_tempfile(&pack_lockfile);
>  		sigchain_push(SIGPIPE, SIG_IGN);
>  		if (report_status_v2)

I think this here is the only caller that we care about where we release
the transaction not immediately after committing it. This is because
`execute_commands()` is the function that's responsible for updating the
references, and thus we don't want to delete the ".keep" files before
it.

It would make sense to single out this caller in the commit message.

> diff --git a/odb/transaction.h b/odb/transaction.h
> index 4cb2eafcbf..ec0b27c449 100644
> --- a/odb/transaction.h
> +++ b/odb/transaction.h
> @@ -75,6 +82,13 @@ static inline void odb_transaction_begin_or_die(struct object_database *odb,
>   */
>  int odb_transaction_commit(struct odb_transaction *transaction);
>  
> +/*
> + * Releases an ODB transaction, performing any deferred cleanup and freeing it.
> + * Must be called for every successfully started transaction. Note that, if the
> + * specified transaction is NULL, the function is a no-op.
> + */
> +void odb_transaction_release(struct odb_transaction *transaction);

Should this function be able to report errors? Cleaning up ".keep" files
can fail, and I'm not sure whether we should simply ignore those.

Patrick