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

Junio C Hamano [off-list ref] writes:

Avoiding the condition that exhibits the breakage is possible, and I
think it is what is done here, but I actually think that the only
right fix is to pass correct argument to commands we invoke in the
first place.  Why are we reusing the same argument array to begin
with?

    ... goes back and reads the offending commit ...

commit b664e9ffa153189dae9b88f32d1c5fedcf85056a
Author: Jonathan Tan [off-list ref]
Date:   Mon Feb 22 11:20:08 2021 -0800

    fetch-pack: with packfile URIs, use index-pack arg
    
    Unify the index-pack arguments used when processing the inline pack and
    when downloading packfiles referenced by URIs. This is done by teaching
    get_pack() to also store the index-pack arguments whenever at least one
    packfile URI is given, and then when processing the packfile URI(s),
    using the stored arguments.

THis makes it sound like the entire idea of this offending commit
was wrong, and before it, the codepath that processed the packfile
fetched from the packfile URI were using the index-pack correctly
by using index-pack arguments that are independent from the one that
is used to process the packfile given in-stream.  Why isn't the fix
just a straight revert of the commit???

By the way, the band-aid in this patch may be OK for the upcoming
release (purely because it is easy to see that is sufficient for
today's codebase), but I said the above because I worry about the
health of the codebase in the longer term.  The "pass_header" may
not stay to be the only difference between the URI packfile and
in-stream packfile in the way they make index-pack invocations.

quoted

This is on jt/transfer-fsck-across-packs.

Ouch.  This definitely is an -rc material.

Thanks.

By the way, the band-aid in this patch may be OK for the upcoming
release (purely because it is easy to see that is sufficient for
today's codebase), but I said the above because I worry about the
health of the codebase in the longer term.  The "pass_header" may
not stay to be the only difference between the URI packfile and
in-stream packfile in the way they make index-pack invocations.

That is true, but at the same time, I think it's better to have the
arguments be the same because there are options (e.g. --promisor and
--fsck-objects) that have to be duplicated, and I think that for the
most part, the URI packfiles and the inline packfile will be processed
identically.