Re: [PATCH v2 0/4] rm: integrate with sparse-index

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 8, 2022, 17:24 UTC
Message-ID: <xmqqmtcesl6e.fsf@gitster.g>
In-Reply-To: <20220807041335.1790658-1-shaoxuan.yuan02@gmail.com>

Shaoxuan Yuan <shaoxuan.yuan02@gmail.com> writes:

> Turn on sparse-index feature within `git-rm` command.

That is a clearly written single-line summary.

> Add necessary modifications and test them.

This states an obvious without adding any useful information. What modifications were necessary and why they were necessary, what old behaviour was undesirable and added tests prevent them to appear again? These details are better left to the proposed log message of individual patches.

This series, when queued on top of 'master' without anything else, seems to pass its own tests, but when combined with the "reset and checkout fixes" <pull.1312.v2.git.1659841030.gitgitgadget@gmail.com> by Victoria, the last one t1092 fails.

---- ---- ---- ---- ---- ---- ---- ---- ---- ---- 
expecting success of 1092.27 'reset hard with removed sparse dir':
        init_repos &&

        test_all_match git rm -r --sparse folder1 &&
        test_all_match git status --porcelain=v2 &&

        test_all_match git reset --hard &&
        test_all_match git status --porcelain=v2 &&

        cat >expect <<-\EOF &&
        folder1/
        EOF

        git -C sparse-index ls-files --sparse folder1 >out &&
        test_cmp expect out

HEAD is now at 703fd3e initial commit HEAD is now at 703fd3e initial commit HEAD is now at 703fd3e initial commit --- full-checkout-out 2022-08-08 17:19:19.820840016 +0000 +++ sparse-index-out 2022-08-08 17:19:19.836841239 +0000

@@ -1,3 +1 @@
-rm 'folder1/0/0/0'
-rm 'folder1/0/1'
-rm 'folder1/a'
+rm 'folder1/'
not ok 27 - reset hard with removed sparse dir
#
#               init_repos &&
#
#               test_all_match git rm -r --sparse folder1 &&
#               test_all_match git status --porcelain=v2 &&
#
#               test_all_match git reset --hard &&
#               test_all_match git status --porcelain=v2 &&
#
#               cat >expect <<-\EOF &&
#               folder1/
#               EOF
#
#               git -C sparse-index ls-files --sparse folder1 >out &&
#               test_cmp expect out
#
---- ---- ---- ---- ---- ---- ---- ---- ---- ---- 

When we have the index (incorrectly) fully expanded, and may have
(incorrectly) working tree files outside of our sparse-cone of
interest, we may have paths under the 'folder1/' that we may need to
remove (and report as removed), but after the bug that causes us to
"incorrectly check out" gets fixed, perhaps the 'folder1/' is the
only thing that needs removed if it is outside our sparse-cone of
interest?  IOW, is the test hardcoding the behaviour of a bug that
was fixed?  I dunno.