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

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 4, 2024, 16:35 UTC
Message-ID: <xmqqcyljbclu.fsf@gitster.g>
In-Reply-To: <ac913257309960d86a9c11e825c76621c6ac405c.1725401207.git.gitgitgadget@gmail.com>

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

Show 5 quoted lines

> 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.

Show 18 quoted lines

> '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.

Show 7 quoted lines

> @@ -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.

Show 20 quoted lines

> 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.

Show 22 quoted lines

> +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.