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@xxxxxxxxxxxxx> 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@xxxxxxxxx> 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
