Re: [PATCH 2/5] add: allow operating on a sparse-only index

[Derrick Stolee <stolee@gmail.com>]

On 7/21/21 6:19 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Derrick Stolee <dstolee@microsoft.com>
>>
>> Disable command_requires_full_index for 'git add'. This does not require
>> any additional removals of ensure_full_index(). The main reason is that
>> 'git add' discovers changes based on the pathspec and the worktree
>> itself. These are then inserted into the index directly, and calls to
>> index_name_pos() or index_file_exists() already call expand_to_path() at
>> the appropriate time to support a sparse-index.
> 
> OK.  With that explained, it still is quite surprising that we only
> need this change (eh, rather, doing this change is safe without
> doing anything else).

Yes, all of the hard work was done by the earlier work to expand
a sparse index when we search for a specific path that lands
within a sparse directory. See 95e0321 (read-cache: expand on query
into sparse-directory entry, 2021-04-01) for the specifics.

>> -	# This "git add folder1/a" fails with a warning
>> -	# in the sparse repos, differing from the full
>> -	# repo. This is intentional.
>> -	test_sparse_match test_must_fail git add folder1/a &&
>> -	test_sparse_match test_must_fail git add --refresh folder1/a &&
>> -	test_all_match git status --porcelain=v2 &&
> 
> And nice to see a known limitation lifted.

Thank you for pointing this out. This actually starts to _fail_ now
that we allow sparse indexes in 'git add', but it's because the error
messages don't match, not that the 'test_must_fail' is violated.

Patch 4 adds a similar test that is then set to work in patch 5. That
allows us a clear way to describe the behavior change and to motivate
the fix in patch 5. This could be explained better, perhaps by merging
Patch 4 into this one. That helps describe how this specific case
changes behavior (for the worse) in this patch, but is handled in a
careful way later, once the behavior change is documented.

If there is a better way to reorganize these patches, then I could
try another approach.

Thanks,
-Stolee