Re: [PATCH v3 8/9] bundle-uri: fetch a list of bundles

[Derrick Stolee <derrickstolee@github.com>]

On 10/4/22 5:44 PM, Jonathan Tan wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
>> +static int unbundle_all_bundles(struct repository *r,
>> +				struct bundle_list *list)
>> +{
>> +	/*
>> +	 * Iterate through all bundles looking for ones that can
>> +	 * successfully unbundle. If any succeed, then perhaps another
>> +	 * will succeed in the next attempt.
>> +	 *
>> +	 * Keep in mind that a non-zero result for the loop here means
>> +	 * the loop terminated early on a successful unbundling, which
>> +	 * signals that we can try again.
>> +	 */
>> +	while (for_all_bundles_in_list(list, attempt_unbundle, r)) ;
>> +
>> +	return 0;
>> +}
> 
> This function always returns 0 regardless of how many successful 
> iterations there were: we would need the number to be equal to the 
> number of bundles in the list if ALL, and 1 if ANY. 

The ALL mode is a bit more permissive than requiring literally
every bundle: if some fail to download or apply, then we continue
with whatever we were able to unbundle. The ALL mode indicates that
the bundles build on each other, so the client should download as
many as possible. By contrast, ANY indicates that they are independent
so the client should stop after the first successful download.

We could still find a way to indicate how many bundles were downloaded
in the return of this method, but we don't want to have additional
warnings based on that return value.

> Which brings up the question...we probably need a test for when the 
> unbundling is unsuccessful. 

I will add more failure scenarios, including no successful downloads
or only a partial success in ALL mode.
 
> Other than that, everything looks good, including the removal of one 
> patch and the addition of the "bundle-uri: suppress stderr from 
> remote-https" patch.

Thanks!

-Stolee