RE: [PATCH v2 0/6] Implement a batched fsync option for core.fsyncObjectFiles

On September 7, 2021 9:23 PM, Ævar Arnfjörð Bjarmason wrote:

Subject: Re: [PATCH v2 0/6] Implement a batched fsync option for core.fsyncObjectFiles


On Tue, Sep 07 2021, Neeraj Singh wrote:

quoted

On Tue, Sep 7, 2021 at 12:54 PM Randall S. Becker
[off-list ref] wrote:

quoted

On September 7, 2021 3:44 PM, Neeraj Singh wrote:

quoted

On Fri, Aug 27, 2021 at 4:49 PM Neeraj K. Singh via GitGitGadget [off-list ref] wrote:

quoted

Thanks to everyone for review so far! I've responded to the
previous feedback and changed the patch series a bit.

Changes since v1:

 * Switch from futimes(2) to futimens(2), which is in POSIX.1-2008. Contrary
   to dscho's suggestion, I'm still implementing the Windows version in the
   same patch and I'm not doing autoconf detection since this is a POSIX
   function.

While POSIX.1-2008, this function is not available on every single
POSIX-compliant platform. Please make sure that the code will not
cause a breakage on some platforms - the ones I maintain, in
particular. Neither futimes nor futimens is available on either
NonStop ia64 or x86. The platform only has utime, so this needs to be
wrapped with an option in config.mak.uname.

Thanks,
Randall

Ugh. Fair enough.  How do other contributors feel about me moving back
to utime, but instead just doing the utime over in
builtins/pack-objects.c?  The idea would be to eliminate the mtime
logic entirely from write_loose_object and just do it at the top-level
in loosen_unused_packed_objects.

Aside from where it lives, can't we just have a wrapper that takes both the filename & fd, and then on some platforms will need to
dispatch to a slower filename-only version, but can hopefully use the new fd-accepting function?

I'm not really enamoured with this direction at all. It means that any platform would have to potentially skip a version of git (resulting from the broken build from a wrapper that is not compilable) after the patches were applied, unless the patches for all of those platforms are included. Even adding a Makefile option would be similar. This should be an "Enable if supported" feature, not a default-or-broken, feature. At best, I'd have to monitor for the time where the patch is applied and hope I can figure out the wrapper changes (around my $DAYJOB) in time to make the same release. This seems a bit counter to a "keeping things compatible" philosophy. Maybe there's something I'm missing here.
-Randall