Re: [PATCH v6 4/8] core.fsyncobjectfiles: add windows support for batch mode

[Neeraj Singh <nksingh85@gmail.com>]

On Mon, Sep 27, 2021 at 1:55 PM Neeraj Singh <nksingh85@gmail.com> wrote:
>
> On Mon, Sep 27, 2021 at 1:07 PM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > "Neeraj Singh via GitGitGadget" <gitgitgadget@gmail.com> writes:
> >
> > > diff --git a/compat/mingw.h b/compat/mingw.h
> > > index c9a52ad64a6..6074a3d3ced 100644
> > > --- a/compat/mingw.h
> > > +++ b/compat/mingw.h
> > > @@ -329,6 +329,9 @@ int mingw_getpagesize(void);
> > >  #define getpagesize mingw_getpagesize
> > >  #endif
> > >
> > > +int win32_fsync_no_flush(int fd);
> > > +#define fsync_no_flush win32_fsync_no_flush
> >
> > ...
> >
> > > diff --git a/wrapper.c b/wrapper.c
> > > index bb4f9f043ce..1a1e2fba9c9 100644
> > > --- a/wrapper.c
> > > +++ b/wrapper.c
> > > @@ -567,6 +567,10 @@ int git_fsync(int fd, enum fsync_action action)
> > >                                                SYNC_FILE_RANGE_WAIT_AFTER);
> > >  #endif
> > >
> > > +#ifdef fsync_no_flush
> > > +             return fsync_no_flush(fd);
> > > +#endif
> > > +
> > >               errno = ENOSYS;
> > >               return -1;
> >
> > This almost makes me wonder if we want to have a fallback
> > implementation of fsync_no_flush() that does
> >
> >    int fsync_no_flush(int unused)
> >    {
> >         errno = ENOSYS;
> >         return -1;
> >    }
> >
> > when nobody (like Windows) define their own fsync_no_flush().  That
> > way, this codepath does not have to have #ifdef/#endif here.
> >
> > This function is already #ifdef ridden anyway, so reducing just one
> > instance may not make much difference, but since I noticed it ...
> >
> > Thanks.
>
> I'll make your suggested change on Github so that it will be available if
> we do another re-roll.
>
> Thanks,
> Nereaj

Actually, while trying your suggestion, my conclusion is that we'd
either have the
inverse ifdef around the fsync_no_flush fallback or an #undef, or some
other confusing
state.  The current ifdeffery is unpleasant to read but not too long
and also pretty direct.
Win32 has an extra level of indirection, but the unix platforms
syscalls are directly written
in one place.

Thanks,
Neeraj