git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <gitster@pobox.com>
Date
Mar 18, 2022, 01:00 UTC
Message-ID
<xmqqo824cbxl.fsf@gitster.g>
In-Reply-To
<97a665fe-07c9-c4f6-4ab6-b6c0e1397c31@github.com>
Victoria Dye <vdye@github.com> writes:
Show 37 quoted lines
>> 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".

Previous: Victoria DyeNext: Derrick Stolee
Message 12 of 22 in “mv: integrate with sparse-index”
  1. 0/1 mv: integrate with sparse-indexShaoxuan Yuan, Mar 15, 2022
  2. 1/1 mv: integrate with sparse-indexShaoxuan Yuan, Mar 15, 2022
  3. Victoria DyeMar 15, 2022
  4. Derrick StoleeMar 15, 2022
  5. Shaoxuan YuanMar 16, 2022
  6. Shaoxuan YuanMar 17, 2022
  7. Shaoxuan YuanMar 16, 2022
  8. Shaoxuan YuanMar 16, 2022
  9. Derrick StoleeMar 16, 2022
  10. Shaoxuan YuanMar 16, 2022
  11. Victoria DyeMar 17, 2022
  12. Junio C HamanoMar 18, 2022
  13. Derrick StoleeMar 21, 2022
  14. Junio C HamanoMar 21, 2022
  15. Derrick StoleeMar 21, 2022
  16. Shaoxuan YuanMar 22, 2022
  17. Derrick StoleeMar 23, 2022
  18. Junio C HamanoMar 23, 2022
  19. Shaoxuan YuanMar 27, 2022
  20. Derrick StoleeMar 28, 2022
  21. Junio C HamanoMar 15, 2022
  22. Derrick StoleeMar 15, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.