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

[Junio C Hamano <gitster@pobox.com>]

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.

Thanks for guiding a new contributor with the usual process of
loosening "require-full-index".