Calvin Wan <calvinwan@xxxxxxxxxx> writes:
> When the state machine in do_fetch_pack_v2() is in FETCH_CHECK_LOCAL, we
> set a few v2-specific defaults. It will be helpful for a future patch to
> have these defaults set if the initial state is not FETCH_CHECK_LOCAL.
> This is a safe change since the initial state is currently always
> FETCH_CHECK_LOCAL, so we're guaranteed to hit that code whether it's in
> the state machine or not.
What does "it" (which supposed to be able to be in the state machine
and also not to be in the state matchine) in the last sentence refer
to?
>
> ---
Missing sign-off.
The patch looks correct and I agree that this is a benign no-op
because the initial value of state is FETCH_CHECK_LOCAL (i.e. the
block of code moved here will execute pretty much as the first
thing, and the relative order between that block and sorting of ref
list should not matter). I just didn't understand the explanation
given by the patch why it is safe.
Thanks.
> fetch-pack.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/fetch-pack.c b/fetch-pack.c
> index e06125c90a..45473b4602 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -1610,18 +1610,18 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
> reader.me = "fetch-pack";
> }
>
> + /* v2 supports these by default */
> + allow_unadvertised_object_request |= ALLOW_REACHABLE_SHA1;
> + use_sideband = 2;
> + if (args->depth > 0 || args->deepen_since || args->deepen_not)
> + args->deepen = 1;
> +
> while (state != FETCH_DONE) {
> switch (state) {
> case FETCH_CHECK_LOCAL:
> sort_ref_list(&ref, ref_compare_name);
> QSORT(sought, nr_sought, cmp_ref_by_name);
>
> - /* v2 supports these by default */
> - allow_unadvertised_object_request |= ALLOW_REACHABLE_SHA1;
> - use_sideband = 2;
> - if (args->depth > 0 || args->deepen_since || args->deepen_not)
> - args->deepen = 1;
> -
> /* Filter 'ref' by 'sought' and those that aren't local */
> mark_complete_and_common_ref(negotiator, args, &ref);
> filter_refs(args, &ref, sought, nr_sought);
