Subject: Re: [PATCH v3 2/2] blame: enable and test the sparse index

From: Junio C Hamano <[email-masked]>
Date: Wed, 03 Nov 2021 09:47:23 -0700

"Lessley Dennington via GitGitGadget" <[email-masked]>
writes:

> We do not include paths outside the sparse checkout cone because blame
> currently does not support blaming files outside of the sparse definition.
> Attempting to do so fails with the following error:
>
>   fatal: no such path '<path outside sparse definition>' in HEAD

Does this indicate that we need to update how the command line
safety in verify_working_tree_path() works in a sparsely checked out
working tree?  If foo/bar is outside the sparse definition,

    git blame HEAD foo/bar

may get such a message, but shouldn't

    git blame HEAD -- foo/bar

make it work?

> -# TODO: blame currently does not support blaming files outside of the
> -# sparse definition. It complains that the file doesn't exist locally.
> -test_expect_failure 'blame with pathspec outside sparse definition' '
> +# Blame does not support blaming files outside of the sparse
> +# definition, so we verify this scenario.

IOW, why is it a good idea to drop the "TODO" and "currently" and pretend
as if the current behaviour is the desirable one?

> +test_expect_success 'blame with pathspec outside sparse definition' '
>  	init_repos &&
> +	test_sparse_match git sparse-checkout set &&
>  
> -	test_all_match git blame folder1/a &&
> -	test_all_match git blame folder2/a &&
> -	test_all_match git blame deep/deeper2/a &&
> -	test_all_match git blame deep/deeper2/deepest/a
> +	for file in a \
> +			deep/a \
> +			deep/deeper1/a \
> +			deep/deeper1/deepest/a
> +	do
> +		test_sparse_match test_must_fail git blame $file &&
> +		cat >expect <<-EOF &&
> +		fatal: Cannot lstat '"'"'$file'"'"': No such file or directory
> +		EOF
> +		# We compare sparse-checkout-err and sparse-index-err in
> +		# `test_sparse_match`. Given we know they are the same, we
> +		# only check the content of sparse-index-err here.
> +		test_cmp expect sparse-index-err
> +	done
>  '
>  
>  test_expect_success 'checkout and reset (mixed)' '
> @@ -878,6 +892,18 @@ test_expect_success 'sparse index is not expanded: diff' '
>  	ensure_not_expanded diff --staged
>  '
>  
> +test_expect_success 'sparse index is not expanded: blame' '
> +	init_repos &&
> +
> +	for file in a \
> +			deep/a \
> +			deep/deeper1/a \
> +			deep/deeper1/deepest/a
> +	do
> +		ensure_not_expanded blame $file
> +	done
> +'
> +
>  # NEEDSWORK: a sparse-checkout behaves differently from a full checkout
>  # in this scenario, but it shouldn't.
>  test_expect_success 'reset mixed and checkout orphan' '