Re: [PATCH] fetch: add a config option to always use the depth argument

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 1, 2014, 20:58 UTC
Message-ID: <xmqqvblvnl3r.fsf@gitster.dls.corp.google.com>
In-Reply-To: <xmqqzjb7nlyx.fsf@gitster.dls.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 12 quoted lines

> Stefan Beller <sbeller@google.com> writes:
>
>> +fetch.depth::
>> +	If set, fetch will automatically behave as if the `--depth`
>> +	option was given on the command line.  This allows users to keep
>> +	the git directory at low space requirements, and thus comes in handy
>> +	for users with large binary files in the repository.
>> +
>
> Hmm, is this something a user would typically want repository-wide?
> I am wondering if "remote.$nick.fetchDepth" or something scoped to
> remote is more appropriate.

Regardless of what configuration variable is used, I think setting depth directly from the config will mean the user cannot defeat a configured value by passing --unshallow because of this code sequence in builtin/fetch.c; am I mistaken?

	...
	git_config(git_fetch_config, NULL);
	argc = parse_options(argc, argv, prefix,
			     builtin_fetch_options, builtin_fetch_usage, 0);
	if (unshallow) {
		if (depth)
			die(_("--depth and --unshallow cannot be used together"));
	...

I think depth variable should be left alone.

The right solution may be to leave the above "unshallow and depth are incompatible" check done only for the command line options as the original code, and much later in the code path after you figure out which remote we are talking to, only when neither --depth nor --unshallow is given, consult the configuration system to see if there is a default, perhaps in prepare_transport(). That approach will let you later support "remote.$nick.fetchDepth", even if you start with a repository-wide "fetch.depth" option, I would think.