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

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Wed, 08 Sep 2021 12:18:38 +0200

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.

As shown in
https://lore.kernel.org/git/[email-masked]/ I think
the best thing to do is neither of the narrow fixes you suggest, but to
more deeply untangle the whole mess around how we choose to write these
files & with what options. A lot of it is bit-twiddling back and forth
for no real reason.

Once we do that it becomes impossible to land in a mode where these
functions need to in principle deal with writing a "real" file and the
"verify" mode, which as noted in the linked E-Mail is the case now, we
just need/want these "is more than one set?" checks & assertions because
we've made the interface overly confusing/general.