diff mbox series

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

Message ID ac913257309960d86a9c11e825c76621c6ac405c.1725401207.git.gitgitgadget@gmail.com (mailing list archive)
State Accepted
Commit e65b0c7c36683a8634b345af1cc3dc7676b3904a
Headers show
Series builtin/cat-file: mark 'git cat-file' sparse-index compatible | expand

Commit Message

Kevin Lyles Sept. 3, 2024, 10:06 p.m. UTC
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).

'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>
---
 builtin/cat-file.c                       |  3 ++
 t/t1092-sparse-checkout-compatibility.sh | 36 ++++++++++++++++++++++++
 2 files changed, 39 insertions(+)

Comments

Junio C Hamano Sept. 4, 2024, 4:35 p.m. UTC | #1
"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.
diff mbox series

Patch

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 18fe58d6b8b..1afdfb5cbae 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -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;
+
 	/* Return early if we're in batch mode? */
 	if (batch.enabled) {
 		if (opt_cw)
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
+'
+
+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
+'
+
 test_done