Re: [PATCH v5 0/4] sparse-index: expand/collapse based on 'index.sparse'

[Elijah Newren <newren@gmail.com>]

On Mon, Nov 22, 2021 at 4:20 PM Victoria Dye via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> This series updates do_read_index to use the index.sparse config setting
> when determining whether the index should be expanded or collapsed. If the
> command & repo allow use of a sparse index, index.sparse is enabled, and a
> full index is read from disk, the index is collapsed before returning to the
> caller. Conversely, if index.sparse is disabled but the index read from disk
> is sparse, the index is expanded before returning. This allows index.sparse
> to control use of the sparse index in addition to its existing control over
> how the index is written to disk. It also introduces the ability to
> enable/disable the sparse index on a command-by-command basis (e.g.,
> allowing a user to troubleshoot a sparse-aware command with '-c
> index.sparse=false' [1]).
>
> While testing this change, a bug was found in 'test-tool read-cache' in
> which config settings for the repository were not initialized before
> preparing the repo settings. This caused index.sparse to always be 'false'
> when using the test helper in a cone-mode sparse checkout, breaking tests in
> t1091 and t1092. The issue is fixed by moving prepare_repo_settings after
> config setup.
>
>
> Changes since V1
> ================
>
>  * Add ensure_correct_sparsity function that ensures the index is sparse if
>    the repository settings (including index.sparse) allow it, otherwise
>    ensuring the index is expanded to full.
>  * Restructure condition in do_read_index to, rather than check specifically
>    for the index.sparse config setting, call ensure_correct_sparsity
>    unconditionally when command_requires_full_index is false.
>
>
> Changes since V2
> ================
>
>  * Rename can_convert_to_sparse to is_sparse_index_allowed to more
>    accurately reflect what the function returns.
>  * Remove index-iterating checks from is_sparse_index_allowed, leaving only
>    inexpensive checks on config settings & sparse checkout patterns. Checks
>    are still part of convert_to_sparse to ensure it behaves exactly as it
>    did before this series.
>  * Restructure ensure_correct_sparsity for better readability.
>  * Fix test_env variable scope.
>
>
> Changes since V3
> ================
>
>  * Add a new patch to avoid unnecessary cache tree free/recreation when
>    possible in convert_to_sparse.
>
>
> Changes since V4
> ================
>
>  * Updated patch 4/4 commit message to better explain practical reasons for
>    making this change.

Thanks.  This version looks good to me:

Reviewed-by: Elijah Newren <newren@gmail.com>