Subject: Re: [PATCH v3 05/11] update-index: use the bulk-checkin infrastructure

From: Junio C Hamano <[email-masked]>
Date: Thu, 24 Mar 2022 11:18:15 -0700

"Neeraj Singh via GitGitGadget" <[email-masked]> writes:

> +static void end_odb_transaction_if_active(void)
> +{
> +	if (!odb_transaction_active)
> +		return;
> +
> +	end_odb_transaction();
> +	odb_transaction_active = 0;
> +}

>  __attribute__((format (printf, 1, 2)))
>  static void report(const char *fmt, ...)
>  {
> @@ -57,6 +68,16 @@ 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, so
> +	 * unplug bulk checkin to make sure that future objects
> +	 * are immediately visible.
> +	 */
> +
> +	end_odb_transaction_if_active();
> +
>  	va_start(vp, fmt);
>  	vprintf(fmt, vp);
>  	putchar('\n');
> @@ -1116,6 +1137,13 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
>  	 */
>  	parse_options_start(&ctx, argc, argv, prefix,
>  			    options, PARSE_OPT_STOP_AT_NON_OPTION);
> +
> +	/*
> +	 * Allow the object layer to optimize adding multiple objects in
> +	 * a batch.
> +	 */
> +	begin_odb_transaction();
> +	odb_transaction_active = 1;

This looks strange.  Shouldn't begin/end pair be responsible for
knowing if there is a transaction active already?  For that matter,
didn't the original unplug in plug/unplug pair automatically turned
into no-op when it is already unplugged?

IOW, I am not sure end_if_active() should exist in the first place.
Shouldn't end_transaction() do that instead?