Re: [PATCH v4 2/2] builtin/cat-file: mark 'git cat-file' sparse-index compatible

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

"Kevin Lyles via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Kevin Lyles <klyles+github@epic.com>
>
> This change affects how 'git cat-file' works with the index when
> specifying an object with the ":<path>" syntax (which will give file
> contents from the index).

The above is not as suitable as the first paragraph as the one that
comes next, which describes the status quo and highlights what the
problem is.  With the few paragraphs below, that talk about the
interaction among ":<path>" syntax, get_oid_with_context(), and
the sparse-index, I think we can just remove it.

> 'git cat-file' expands a sparse index to a full index any time contents
> are requested from the index by specifying an object with the ":<path>"
> syntax. This is true even when the requested file is part of the sparse
> index, and results in much slower 'git cat-file' operations when working
> within the sparse index.
>
> Mark 'git cat-file' as not needing a full index, so that you only pay
> the cost of expanding the sparse index to a full index when you request
> a file outside of the sparse index.
>
> Add tests to ensure both that:
> - 'git cat-file' returns the correct file contents whether or not the
>   file is in the sparse index
> - 'git cat-file' expands to the full index any time you request
>   something outside of the sparse index
>
> Signed-off-by: Kevin Lyles <klyles+github@epic.com>
> ---

Nicely explained.

> @@ -1047,6 +1047,9 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
>  	if (batch.buffer_output < 0)
>  		batch.buffer_output = batch.all_objects;
>  
> +	prepare_repo_settings(the_repository);
> +	the_repository->settings.command_requires_full_index = 0;
> +

OK.  This command does not start parsing the command line arguments
before this point, and this is really a good place to toggle the bit
off.

> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
> index 4cbe9b1465d..eb32da2a7f2 100755
> --- a/t/t1092-sparse-checkout-compatibility.sh
> +++ b/t/t1092-sparse-checkout-compatibility.sh
> @@ -2358,4 +2358,40 @@ test_expect_success 'advice.sparseIndexExpanded' '
>  	grep "The sparse index is expanding to a full index" err
>  '
>  
> +test_expect_success 'cat-file -p' '
> +	init_repos &&
> +	echo "new content" >>full-checkout/deep/a &&
> +	echo "new content" >>sparse-checkout/deep/a &&
> +	echo "new content" >>sparse-index/deep/a &&
> +	run_on_all git add deep/a &&
> +
> +	test_all_match git cat-file -p :deep/a &&
> +	ensure_not_expanded cat-file -p :deep/a &&
> +	test_all_match git cat-file -p :folder1/a &&
> +	ensure_expanded cat-file -p :folder1/a
> +'

OK.  These are about the object names given from the command line.

> +test_expect_success 'cat-file --batch' '
> +	init_repos &&
> +	echo "new content" >>full-checkout/deep/a &&
> +	echo "new content" >>sparse-checkout/deep/a &&
> +	echo "new content" >>sparse-index/deep/a &&
> +	run_on_all git add deep/a &&
> +
> +	echo ":deep/a" >in &&
> +	test_all_match git cat-file --batch <in &&
> +	ensure_not_expanded cat-file --batch <in &&
> +
> +	echo ":folder1/a" >in &&
> +	test_all_match git cat-file --batch <in &&
> +	ensure_expanded cat-file --batch <in &&
> +
> +	cat >in <<-\EOF &&
> +	:deep/a
> +	:folder1/a
> +	EOF
> +	test_all_match git cat-file --batch <in &&
> +	ensure_expanded cat-file --batch <in
> +'

And these are about the object names fed via the --batch mechanism.

Looking good.

Will queue.  Thanks.