Re: [PATCH v9 0/9] Implement a batched fsync option for core.fsyncObjectFiles

On Mon, Nov 15 2021, Neeraj K. Singh via GitGitGadget wrote:

 * Per [2], I'm leaving the fsyncObjectFiles configuration as is with
   'true', 'false', and 'batch'. This makes using old and new versions of
   git with 'batch' mode a little trickier, but hopefully people will
   generally be moving forward in versions.

[1] See
https://lore.kernel.org/git/pull.1067.git.1635287730.gitgitgadget@gmail.com/ (local)
[2] https://lore.kernel.org/git/xmqqh7cimuxt.fsf@gitster.g/ (local)

I really think leaving that in-place is just being unnecessarily
cavalier. There's a lot of mixed-version environments where git is
deployed in, and we almost never break the configuration in this way (I
think in the past always by mistake).

In this case it's easy to avoid it, and coming up with a less narrow
config model[1] seems like a good idea in any case to unify the various
outstanding work in this area.

More generally on this series, per the thread ending in [2] I really
don't get why we have code like this:
	
	@@ -503,10 +504,12 @@ static void unpack_all(void)
	 	if (!quiet)
	 		progress = start_progress(_("Unpacking objects"), nr_objects);
	 	CALLOC_ARRAY(obj_list, nr_objects);
	+	plug_bulk_checkin();
	 	for (i = 0; i < nr_objects; i++) {
	 		unpack_one(i);
	 		display_progress(progress, i + 1);
	 	}
	+	unplug_bulk_checkin();
	 	stop_progress(&progress);
	 
	 	if (delta_list)

As opposed to doing an fsync on the last object we're
processing. I.e. why do we need the step of intentionally making the
objects unavailable in the tmp-objdir, and creating a "cookie" file to
sync at the start/end, as opposed to fsyncing on the last file (which
we're writing out anyway).

1. https://lore.kernel.org/git/211110.86r1bogg27.gmgdl@evledraar.gmail.com/ (local)
2. https://lore.kernel.org/git/20211111000349.GA703@neerajsi-x1.localdomain/ (local)