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

From: Junio C Hamano <[email-masked]>
Date: Fri, 05 Mar 2021 11:46:39 -0800

Junio C Hamano <[email-masked]> 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 <[email-masked]>
> 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.

>> This is on jt/transfer-fsck-across-packs.
>
> Ouch.  This definitely is an -rc material.

Thanks.