Subject: Re: [PATCH 0/7] Sparse index: integrate with 'read-tree'

From: Derrick Stolee <[email-masked]>
Date: Thu, 24 Feb 2022 11:59:37 -0500

On 2/23/2022 1:25 PM, Victoria Dye via GitGitGadget wrote:
> Like previous integrations [1] [2], this series allows 'git read-tree' to
> operate using a sparse index.

(Fixed the CCs on this reply, too.)
 
> The first two patches are bugfixes for issues found while implementing the
> 'read-tree' integration:
> 
>  * The first (patch 1/7) fixes an edge case in which a repo with no in-cone
>    files or directories would have its root collapsed into a sparse
>    directory; the fix ensures the root directory is never collapsed into a
>    sparse directory.
>  * The second (patch 2/7) corrects the 'git status' reporting of changes
>    nested inside the subdirectory of a sparse directory, ensuring that the
>    modified file (not the subdirectory) is correctly reported as having
>    changes.

Thanks for these! I just found one tiny optimization in the first patch.

> The remainder of the series focuses on utilizing the sparse index in 'git
> read-tree'. After some baseline behavior-establishing tests (patch 3/7),
> sparse index usage is trivially enabled (patch 4/7) for 'read-tree' except:
> 
>  * usage with '--prefix'
>  * two- and three-way merge
> 
> These cases require additional changes in order to work as expected (i.e.,
> outwardly matching non-sparse index sparse-checkout). For the former, the
> sparse index can be enabled as long as the index is expanded when the prefix
> is a directory outside the sparse cone (patch 5/7). For the latter, sparse
> directories that cannot be trivially merged must have their contents merged
> file-by-file, done by recursively traversing the trees represented by the
> sparse directories (patches 6/7 & 7/7).

I enjoyed reading these remaining patches. I'm impressed with how you
constructed these tests and patches to do the smallest amount of change
per patch.

I couldn't find any fault in these patches, but perhaps Elijah's deep
experience with merge machinery could help add confidence, especially
for patches 6 & 7.

Thanks,
-Stolee