Re: [PATCH] fetch: when deepening, check connectivity fully

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

Jonathan Tan <jonathantanmy@google.com> writes:

> Do not stop at ancestors of our refs when deepening during fetching.
> This is because when performing such a fetch, shallow entries may have
> been updated; the client can reasonably assume that the existing refs
> are connected up to the old shallow points, but not the new.

OK.

> diff --git a/connected.c b/connected.c
> index 91feb78815..1bba888eff 100644
> --- a/connected.c
> +++ b/connected.c
> @@ -58,8 +58,10 @@ int check_connected(oid_iterate_fn fn, void *cb_data,
>  	argv_array_push(&rev_list.args, "--stdin");
>  	if (repository_format_partial_clone)
>  		argv_array_push(&rev_list.args, "--exclude-promisor-objects");
> -	argv_array_push(&rev_list.args, "--not");
> -	argv_array_push(&rev_list.args, "--all");
> +	if (!opt->is_deepening_fetch) {
> +		argv_array_push(&rev_list.args, "--not");
> +		argv_array_push(&rev_list.args, "--all");
> +	}
>  	argv_array_push(&rev_list.args, "--quiet");
>  	if (opt->progress)
>  		argv_array_pushf(&rev_list.args, "--progress=%s",

Hmph, remind me how old and/or new shallow cut-off points affect
this traversal?  In order to verify that everything above the new
cut-off points, opt->shallow_file should be pointing at the new
cut-off points, then we do the full sweep (without --not --all) to
ensure we won't find missing or broken objects but still stop at the
new cut-off points, and then only after check_connected() passes,
update the shallow file to make new shallow cut-off points active
(and if the traversal fails, then we do nto install the new shallow
cut-off points)?

If so, that sounds sensible.  By not having "--not --all", we
potentially would do a full sweep, but if we are really deepening to
the root commits, then we do need one full sweep anyway (as these
deepest parts of the history were previously hidden by the shallow
cutoff points), and if we are only deepening the history by a bit,
even if we do not have "--not --all", we would hit the updated
shallow cutoff points (which may be at older parts of the history)
and stop, and for correctness we need to visit there anyway, so I
think we are not being overly pessimistic with this change, either.