Re: [PATCH v5 00/21] Sparse Index: Design, Format, Tests

[Derrick Stolee <stolee@gmail.com>]

On 3/30/2021 4:11 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>>      @@ repo-settings.c: void prepare_repo_settings(struct repository *r)
>>       +	 * Initialize this as off.
>>       +	 */
>>       +	r->settings.sparse_index = 0;
>>      -+	if (!repo_config_get_bool(r, "extensions.sparseindex", &value) && value)
>>      ++	if (!repo_config_get_bool(r, "index.sparse", &value) && value)
>>       +		r->settings.sparse_index = 1;
>>        }
> 
> It would be helpful to have a way for the repository owner to say
> "Even if the version of Git may be capable of handling 'sdir'
> extension, and my checkout uses sparse-cone settings, I do not want
> to use it", and the other way around, i.e. "Even if my checkout
> currently does not use sparse-cone settings, do use 'sdir'
> extension".  But for that, .sparse_index member may need to be
> tristate (i.e. forbidden, enable-if-needed, use-even-unneeded)?

I believe as presented, index.sparse=false will prevent the sdir
extension from being used. If index.sparse=true, then it will only
be used if sparse-checkout is enabled in cone mode.

I don't see the value in using the 'sdir' extension when not using
sparse-checkout in cone mode (and hence there are no sparse directory
entries in the index). What am I missing?

> We have a similar setting in index.version; I believe we always
> auto-demote 3 down to 2 when extended flags are not used, and
> I think "always auto-demote" would be sufficient (iow,
> "use-even-unneeded" may not be necessary, even though that might
> help debugging).

Yes, the same is happening here: we auto-demote to not use 'sdir'
if it the other settings are not configured as well.

There is the rare scenario where these things all occur:

1. index.sparse = true
2. core.sparseCheckout = true
3. core.sparseCheckoutCone = true
4. Every path in the index matches the cone-mode patterns.

In this case, convert_to_sparse() is called and the istate->sparse
bit is set, telling do_write_index() to add the 'sdir' extension.

This seems like a rare occurrence. Is it still worth adding logic
to avoid 'sdir' when these are all true?

Thanks,
-Stolee