Re: [PATCH v4 4/5] sparse-checkout: error or warn when given individual files

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

"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

>  		int prefix_len = strlen(prefix);
>  
>  		if (!core_sparse_checkout_cone)
> @@ -703,20 +706,44 @@ static void sanitize_paths(int argc, const char **argv, const char *prefix)
>  		for (i = 0; i < argc; i++)
>  			argv[i] = prefix_path(prefix, prefix_len, argv[i]);
>  	}
> +
> +	if (skip_checks)
> +		return;

The placement of skip-checks here makes the earier die() between the
hunks in this block unskippable.

> +	for (i = 0; i < argc; i++) {
> +		struct cache_entry *ce;
> +		struct index_state *index = the_repository->index;
> +		int pos = index_name_pos(index, argv[i], strlen(argv[i]));
> +
> +		if (pos < 0)
> +			continue;

I _think_ the intent for this is to catch possbily common mistakes
rather than a hard rule, so while an unmerged path will evade this
check, I think it is an acceptable trade-off between code simplicity
and thoroughness.

> +		ce = index->cache[pos];
> +		if (S_ISSPARSEDIR(ce->ce_mode))
> +			continue;
> +
> +		if (core_sparse_checkout_cone)
> +			die(_("\"%s\" is not a directory; to treat it as a directory anyway, rerun with --skip-checks"), argv[i]);
> +		else
> +			warning(_("pass a leading slash before paths such as \"%s\" if you want a single file (see NON-CONE PROBLEMS in the git-sparse-checkout manual)."), argv[i]);
> +	}

Hmph. The skip_checks flag was introduced exactly so that what the
code catches as a possible problem is a false alarm, no?  I would
understand if both were die() to be conservative, of both were
warning() to be lenient, but one being die() and the other being
warning() makes this look somewhat uneven.

Besides, being leninent here somewhat defeats the point of having
skip_checks, no?  As long as we believe skip_checks option is useful,
shouldn't these be die() to be equally conservative?