Re: [RFC PATCH 1/1] mv: integrate with sparse-index

[Derrick Stolee <derrickstolee@github.com>]

On 3/17/2022 9:00 PM, Junio C Hamano wrote:
> Victoria Dye <vdye@github.com> writes:
> 
>>> I tested this and it fails as expected with:
>>> "fatal: bad source, source=folder2/a, destination=deep/new"
>>
>> Great! This should then probably be turned into a "test_expect_fail" test in
>> 't1092' - that'll make sure we get both the right behavior and right error
>> message with sparse index after it's enabled.
>>
>> However, I also get the same result when I add the '--sparse' option. I
>> would expect the behavior to be "move 'folder2/a' to 'deep/new' and check it
>> out in the worktree" - this may be a good candidate for improving the
>> existing integration with sparse *checkout* before enabling sparse *index*
>> (e.g., like when 'git add' was updated to not add sparse files by default
>> [1]).
>> ...
>> I think you're right that this is a bug. This appears to come from the fact
>> that 'mv' decides whether a directory is sparse only *after* it sees that it
>> doesn't exist on-disk. 
>> ...
>> So I think there are three potential things to fix here: 
>>
>> 1. When empty folder2/ is on-disk, 'git mv' (without '--sparse') doesn't
>>    fail with "bad source", even though it should.
>> 2. When you try to move a sparse file with 'git mv --sparse', it still
>>    fails.
>> 3. SKIP_WORKTREE is not removed from out-of-cone files moved into the sparse
>>    cone.
>>
>> On a related note, there is precedent for needing to make fixes like this
>> before integrating with sparse index. For example: in addition to the
>> earlier examples in 'add' and 'reset', 'checkout-index' was changed to no
>> longer checkout SKIP_WORKTREE files by default [3]. It's a somewhat expected
>> part of this process ...
>> ...
>> Another tool that may help you here is 'git ls-files --sparse -t'. It lists
>> the files in the index and their "tags" ('H' is "normal" tracked files, 'S'
>> is SKIP_WORKTREE, etc. [4]), which can help identify when a file you'd
>> expect to be SKIP_WORKTREE is not and vice versa.
> 
> Wonderful.
> 
> Quite honestly, because the code will most likely compile correctly
> if you just remove the unconditional "we first expand the in-core
> index fully" code, and because the "sparse index" makes the existing
> index walking code fail in unexpected and surprising ways, I
> consider it unsuitably harder for people who are not yet familiar
> with the system.  Without a good test coverage (which is hard to
> give unless you are familiar with the code being tested X-<), one
> can easily get confused and lost.

Certainly, 'git mv' is looking to be harder than expected, but there
is a lot of interesting exploration happening in the process.

Thanks for persisting on this one, Shaoxuan!

Thanks,
-Stolee