From: Neeraj Singh <nksingh85@gmail.com>
Date: Mon, 21 Mar 2022 18:28:31 GMT
Subject: Re: [PATCH v2 2/7] core.fsyncmethod: batched disk flushes for loose-objects
Message-ID: <CANQDOdcPAb31JY6LdMiCn26192_1wvKkHodpNEBkSh3WbD+e4g@mail.gmail.com>
In-Reply-To: <220321.86a6dj9xja.gmgdl@evledraar.gmail.com>

On Mon, Mar 21, 2022 at 7:43 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
>
> On Sun, Mar 20 2022, Neeraj Singh via GitGitGadget wrote:
>
> > From: Neeraj Singh <neerajsi@microsoft.com>
> > [...]
> > +     if (batch_fsync_enabled(FSYNC_COMPONENT_LOOSE_OBJECT)) {
> > +             bulk_fsync_objdir = tmp_objdir_create("bulk-fsync");
> > +             if (!bulk_fsync_objdir)
> > +                     die(_("Could not create temporary object directory for core.fsyncobjectfiles=batch"));
>
> Should camel-case the config var, and we should have a die_errno() here
> which tell us why we couldn't create it (possibly needing to ferry it up
> from the tempfile API...)

Thanks for noticing the camelCasing.  The config var name was also
wrong. Now it will read:
> > +                     die(_("Could not create temporary object directory for core.fsyncMethod=batch"));

Do you have any recommendations on how to easily ferry the correct
errno out of tmp_objdir_create?
It looks like the remerge-diff usage has the same die behavior w/o
errno, and the builtin/receive-pack.c usage
doesn't die, but also loses the errno.  I'm concerned about preserving
the errno across the or tmp_objdir_destroy
calls.  I could introduce a temp errno var to preserve it across
those. Is that what you had in mind?

Thanks,
Neeraj