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

["Randall S. Becker" <rsbecker@nexbridge.com>]

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:
>
>> On Tue, Sep 7, 2021 at 12:54 PM Randall S. Becker
>> <rsbecker@nexbridge.com> wrote:
>>>
>>> On September 7, 2021 3:44 PM, Neeraj Singh wrote:
>>> >On Fri, Aug 27, 2021 at 4:49 PM Neeraj K. Singh via GitGitGadget <gitgitgadget@gmail.com> wrote:
>>> >>
>>> >> 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