Subject: Re: [PATCH] sparse-checkout: disable advice in 'disable'

From: Junio C Hamano <[email-masked]>
Date: Mon, 23 Sep 2024 13:27:38 -0700

"Derrick Stolee via GitGitGadget" <[email-masked]> writes:

> From: Derrick Stolee <[email-masked]>
>
> When running 'git sparse-checkout disable' with the sparse index
> enabled, Git is expected to expand the index into a full index. However,
> it currently outputs the advice message saying that that is unexpected
> and likely due to an issue with the working directory.
> ...
> +	/*
> +	 * Disable the advice message for expanding a sparse index, as we
> +	 * are expecting to do that when disabling sparse-checkout.
> +	 */
> +	give_advice_on_expansion = 0;
>  	repo_read_index(the_repository);

Sounds sensible.

> +/*
> + * If performing an operation where the index is supposed to expand to a
> + * full index, then disable the advice message by setting this global to
> + * zero.
> + */
> +extern int give_advice_on_expansion;
> +
>  struct index_state;
>  #define SPARSE_INDEX_MEMORY_ONLY (1 << 0)
>  int is_sparse_index_allowed(struct index_state *istate, int flags);
> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
> index eb32da2a7f2..6e230b54876 100755
> --- a/t/t1092-sparse-checkout-compatibility.sh
> +++ b/t/t1092-sparse-checkout-compatibility.sh
> @@ -2355,7 +2355,10 @@ test_expect_success 'advice.sparseIndexExpanded' '
>  	mkdir -p sparse-index/deep/deeper2/deepest &&
>  	touch sparse-index/deep/deeper2/deepest/bogus &&
>  	git -C sparse-index status 2>err &&
> -	grep "The sparse index is expanding to a full index" err
> +	grep "The sparse index is expanding to a full index" err &&
> +
> +	git -C sparse-index sparse-checkout disable 2>err &&
> +	test_line_count = 0 err

I am not a huge fun of insisting that other code in the code path in
which I got rid of an unwanted error message must stay silent.  As
we are expanding to full, we are presumably rehydrating all the
directories that was sparsified, so it might be reasonable if we
want to see a progress output during this operation, for example [*].
Would it make more sense to look for the lack of specific advice
message instead?

[Footnote]

 * A mere example to illustrate the principle.  "We disable progress
   during test", or "this is so small that progress won't trigger"
   may both be a good reaction to the example, but the general point
   still stands.