Re: [PATCH] fetch-pack: do not mix --pack_header and packfile uri

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> I already said that I think 2aec3bc4 (fetch-pack: do not mix
> --pack_header and packfile uri, 2021-03-04) is OK as a short-term
> fix for the upcoming release, but it does not change the fact that
> it is piling technical debt on top of existing technical debt.
>
> And that is why I am reacting against your earlier mention of
> "filering out" rather strongly.  The approach continues the "keep
> the single args array in the belief that two must be mostly the
> same", which I view as a misguided starting point that must be
> rethought.

Another way to think about the codepath is this.

Can the bulk of get_pack() that deals with a single incoming
packfile (from the part that makes the decision to use either
index-pack or unpack-objects and chooses what options to pass to the
command, to the part that actually calls run_command() and feeds the
packdata to the command) be made into a helper function that handles
one packdata stream and nothing else?  Such a helper would most
likely take as its parameters

 - a stream to read the packdata from (for in-stream packfile that
   is handled by get_pack(), we already have it available)

 - fetch_pack_args and other options that are meant to affect the
   operation of fetch-pack, among which are two bits that are of
   interest in this topic: if we want to run fsck-objects and if the
   entire fetch-pack is dealing with more than one packfile
   (currently, the only source of need to process multiple packfiles
   is packfile URI mechanism, but that does not have to stay that
   way).

Then get_pack() can move a lot of code out of it to this helper and
just call it.  The processing the other packfile obtained by the
packfile URI mechanism out of band can open the packstream and call
the helper the same way.  When packfile URI mechanism is in use, both
invocations of the helper would get "you are not alone so fsck may
hit missing objects" bit, if fsck-objects are asked for.

That would avoid the "duplicated logic" and still allow the code to
choose the best disposition of the incoming packdata per packfile.

In an extreme case, it is not hard to imagine that somebody prepares
a very small base packfile and feed it via packfile URI mechanism,
but have accumulated so many objects that are not yet rolled into an
updated base packfile---cloning from such a repository may result in
running unpack-objects for the packfile that came out of band, while
processing the in-stream packfile with index-pack.

Hmm?