Subject: Re: [PATCH v2 3/7] update-index: use the bulk-checkin infrastructure

From: Junio C Hamano <[email-masked]>
Date: Mon, 21 Mar 2022 10:50:23 -0700

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

> diff --git a/builtin/update-index.c b/builtin/update-index.c
> index 75d646377cc..38e9d7e88cb 100644
> --- a/builtin/update-index.c
> +++ b/builtin/update-index.c
> @@ -5,6 +5,7 @@
>   */
>  #define USE_THE_INDEX_COMPATIBILITY_MACROS
>  #include "cache.h"
> +#include "bulk-checkin.h"
>  #include "config.h"
>  #include "lockfile.h"
>  #include "quote.h"
> @@ -1110,6 +1111,9 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
>  
>  	the_index.updated_skipworktree = 1;
>  
> +	/* we might be adding many objects to the object database */
> +	plug_bulk_checkin();
> +
>  	/*
>  	 * Custom copy of parse_options() because we want to handle
>  	 * filename arguments as they come.
> @@ -1190,6 +1194,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
>  		strbuf_release(&buf);
>  	}
>  
> +	/* by now we must have added all of the new objects */
> +	unplug_bulk_checkin();

I understand read-from-stdin code path would be worth plugging, but
the list of paths on the command line?  How many of them would one
fit?

Of course, the feeder may be expecting for the objects to appear in
the object store as it feeds the paths and will be utterly broken by
this change, as you mentioned in the proposed log message.  The
existing plug/unplug will change the behaviour by making the objects
sent to the packfile available only after getting unplugged.  This
series makes it even worse by making loose objects also unavailable
until unplug is called.

So, it probably is safer and more sensible approach to introduce a
new command line option to allow the bulk checkin, and those who do
not care about the intermediate state to opt into the new feature.