Re: [PATCH] fetch: fix --depth= and --unshallow for multi-remote fetches

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

Rong Bao <rong.bao@csmantle.top> writes:

> When running `git fetch --all --depth=<SOME-DEPTH>` or `git fetch --all
> --unshallow` with Git 2.56.0, the fetch isn't really deepened:
> ...
>
> This is because the parent process doesn't pass the --depth= flag to the
> fetcher subprocess.
> ...
>
> Fix this by passing it when specified or implied by --unshallow. After
> fixing, both flags would work on shallow trees in conjunction with
> --all, as demonstrated below:

The above observes that "--depth=" given to the outer "fetch --all"
is not propagated down to subfetches.

> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index b2decc6cfdb8ae86b8b1e92caa85f4961ffc47ff..a110a2dcf09c7e6764eb232bff73286753b27009 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -2247,6 +2247,8 @@ static void add_options_to_argv(struct strvec *argv,

This function is used to prepare a command line to run a single
fetch in a subprocess for 'fetch --all', and also to fetch
submodules when '--recurse-submodules' is in effect.  Does the
latter case share the same problem that '--depth=<N>' is not
propagated down?  Should the depth of submodules be controlled
independently by not passing '--depth=<N>' down in the first place,
in which case, this patch may be a regression for users who depend
on the behavior?

>  		strvec_push(argv, "--tags");
>  	else if (tags == TAGS_UNSET)
>  		strvec_push(argv, "--no-tags");
> +	if (deepen)
> +		strvec_pushf(argv, "--depth=%s", depth);

The 'deepen' variable checked here is set to true when any of
the '--depth=<N>', '--deepen-since=<TIME>', or
'--shallow-exclude=<REF>' were used.  If the outer
'fetch --all' was told to deepen with '--deepen-since=<TIME>',
without using '--depth=<N>', what do we have in the variable
'depth' at this point?  NULL?

In other words, shouldn't this be written in a more obvious,
simple, and straightforward way as three separate additions,
e.g.,

	if (depth)
		strvec_pushf(argv, "--depth=%s", depth);
	if (deepen_since)
		strvec_pushf(argv, "--shallow-since=%s", deepen_since);
	if (deepen_not.nr) {
		struct string_list_item *it;
		for_each_string_list_item(it, &deepen_not) {
                	strvec_pushf(argv, "--shallow-since=%s", it->string);
		}
	}

Propagating 'shallow-exclude' would probably not be a good
idea if this helper function is called within the context of
recursing into submodules, as their history and refs have no
connection to the history of the top-level project.  Please do
not blindly copy and paste the above to an updated patch.