Re: [PATCH v5 07/14] update-index: use the bulk-checkin infrastructure

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 30, 2022, 17:52 UTC
Message-ID: <xmqqpmm38h01.fsf@gitster.g>
In-Reply-To: <8cac94598a58704d9b625a9d8a593779f7adc30f.1648616734.git.gitgitgadget@gmail.com>

"Neeraj Singh via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 35 quoted lines

> +	/*
> +	 * Allow the object layer to optimize adding multiple objects in
> +	 * a batch.
> +	 */
> +	begin_odb_transaction();
>  	while (ctx.argc) {
>  		if (parseopt_state != PARSE_OPT_DONE)
>  			parseopt_state = parse_options_step(&ctx, options,
> @@ -1167,6 +1174,17 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
>  		the_index.version = preferred_index_format;
>  	}
>  
> +	/*
> +	 * 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 if verbose output is enabled.
> +	 */
> +
> +	if (verbose)
> +		end_odb_transaction();
> +
>  	if (read_from_stdin) {
>  		struct strbuf buf = STRBUF_INIT;
>  		struct strbuf unquoted = STRBUF_INIT;
> @@ -1190,6 +1208,12 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
>  		strbuf_release(&buf);
>  	}
>  
> +	/*
> +	 * By now we have added all of the new objects
> +	 */
> +	if (!verbose)
> +		end_odb_transaction();

If we had "flush" in addition to "begin" and "end", then we could, instead of the above

    begin_transaction
	do things
    if condition:
	end_transaction
    loop:
	do thing
    if !condition:
	end_transaction

which is somewhat hard to follow and maintain, consider using a different flow, which is

    begin_transaction
	do things
    loop:
	do thing
	if condition:
	    flush
    end_transaction

and that might make it easier to follow and maintain. I am not 100% sure if it is worth it, but I am leaning to say it would be.

Thanks.