Re: [PATCH] pack-write: skip .rev work when not writing .rev

From: Taylor Blau <me@ttaylorr.com>
Date: Sep 8, 2021, 16:32 UTC
Message-ID: <YTjljc0kU2WhmTzU@nand.local>
In-Reply-To: <87r1dziczs.fsf@evledraar.gmail.com>

On Wed, Sep 08, 2021 at 12:18:38PM +0200, Ævar Arnfjörð Bjarmason wrote:

Show 18 quoted lines

>
> On Tue, Sep 07 2021, Taylor Blau wrote:
>
> > On Tue, Sep 07, 2021 at 10:50:58PM -0400, Taylor Blau wrote:
> >> Of the two, I think the former is more appealing (since no other
> >> functions called by finish_tmp_packfile() are guarded like that; they
> >> conditionally behave as noops depending on `flags`).
> >
> > Sorry; this is nonsensical. The only other function we call is
> > write_idx_file() which merely changes its behavior based on flags, but
> > it never behaves as a noop.
> >
> > That doesn't change my thinking about preferring the former of my two
> > suggestions, but just wanted to correct my error.
>
> I agree that this code is very confusing overall, but would prefer to
> wait on refactoring further until the two topics in flight (this and the
> other pack-write topic) settle.

I'm fine to wait on any further refactorings. And I agree that this code is confusing, since when I read it last night I thought that the check in write_rev_file_order() was a duplicate of the one you introduced, but it is not:

    if ((flags & WRITE_REV) && (flags & WRITE_REV_VERIFY))
      die(_("cannot both write and verify reverse index"));

and that check is different than the one you added, which I think is appropriate.

So this patch looks good to me, and sorry for the confusion.

Thanks, Taylor

Re: [PATCH] pack-write: skip *.rev work when not writing *.rev

Re: [PATCH] pack-write: skip .rev work when not writing .rev