Re: [PATCH v2 2/6] builtin/update-index: end ODB transaction when --verbose is specified

> With 23a3a303 (update-index: use the bulk-checkin infrastructure,
> 2022-04-04), object database transactions were added to
> git-update-index(1) to facilitate writing objects in bulk. With
> transactions, newly added objects are instead written to a temporary
> object directory and migrated to the primary object database upon
> transaction commit.
>
> When the --verbose option is specified, each of the following objects is
> explicitly flushed via flush_odb_transaction() prior to reporting the
> update. Flushing the object database transaction migrates pending
> objects to the primary object database without marking the transaction
> as complete. This is done so objects are immediately visible to
> git-update-index(1) callers using the --verbose option and that rely on
> parsing verbose output to know when objects are written.
>
> Due to how git-update-index(1) parses options, each filename argument is
> evaluated with only the set of options that precede it. Therefore, it is
> possible for an initial set of objects to be written in a transaction
> before a --verbose option is encountered.
>
> As soon as the --verbose option is parsed in git-update-index(1), all
> subsequent object writes are flushed prior to being reported and thus no
> longer benefit from being transactional. Furthermore, the mechanism to
> flush a transaction without committing is rather awkward. Drop the call
> to flush_odb_transaction() in favor of ending the transaction early when
> the --verbose flag is encountered.
>
> Signed-off-by: Justin Tobler <jltobler@gmail.com>
> ---
>  builtin/update-index.c | 23 +++++++++++++++--------
>  1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/builtin/update-index.c b/builtin/update-index.c
> index 2ba2d29c95..d36bc55752 100644
> --- a/builtin/update-index.c
> +++ b/builtin/update-index.c
> @@ -70,14 +70,6 @@ static void report(const char *fmt, ...)
>  	if (!verbose)
>  		return;
>
> -	/*
> -	 * It is possible, though unlikely, that a caller could use the verbose
> -	 * output to synchronize with addition of objects to the object
> -	 * database. The current implementation of ODB transactions leaves
> -	 * objects invisible while a transaction is active, so flush the
> -	 * transaction here before reporting a change made by update-index.
> -	 */
> -	flush_odb_transaction(the_repository->objects->transaction);
>  	va_start(vp, fmt);
>  	vprintf(fmt, vp);
>  	putchar('\n');
> @@ -1150,6 +1142,21 @@ int cmd_update_index(int argc,
>  			const char *path = ctx.argv[0];
>  			char *p;
>
> +			/*
> +			 * It is possible, though unlikely, that a caller could
> +			 * use the verbose output to synchronize with addition
> +			 * of objects to the object database. The current
> +			 * implementation of ODB transactions leaves objects
> +			 * invisible while a transaction is active, so end the
> +			 * transaction here early before processing the next
> +			 * update. All further updates are performed outside of
> +			 * a transaction.
> +			 */
> +			if (transaction && verbose) {
> +				end_odb_transaction(transaction);
> +				transaction = NULL;
> +			}
> +

>  			setup_work_tree();
>  			p = prefix_path(prefix, prefix_length, path);
>  			update_one(p);
> --
> 2.51.0.193.g4975ec3473b