git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com>
Cc: git@vger.kernel.org, vdye@github.com, derrickstolee@github.com
Subject: Re: [PATCH v2 0/4] rm: integrate with sparse-index
Date: Mon, 08 Aug 2022 10:24:25 -0700	[thread overview]
Message-ID: <xmqqmtcesl6e.fsf@gitster.g> (raw)
In-Reply-To: <20220807041335.1790658-1-shaoxuan.yuan02@gmail.com> (Shaoxuan Yuan's message of "Sun, 7 Aug 2022 12:13:31 +0800")

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.




  parent reply	other threads:[~2022-08-08 17:24 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-03  4:51 [PATCH v1 0/4] rm: integrate with sparse-index Shaoxuan Yuan
2022-08-03  4:51 ` [PATCH v1 1/4] t1092: add tests for `git-rm` Shaoxuan Yuan
2022-08-03 14:32   ` Derrick Stolee
2022-08-03  4:51 ` [PATCH v1 2/4] pathspec.h: move pathspec_needs_expanded_index() from reset.c to here Shaoxuan Yuan
2022-08-03 14:35   ` Derrick Stolee
2022-08-05  7:53     ` Shaoxuan Yuan
2022-08-03  4:51 ` [PATCH v1 3/4] rm: expand the index only when necessary Shaoxuan Yuan
2022-08-03 14:40   ` Derrick Stolee
2022-08-05  8:07     ` Shaoxuan Yuan
2022-08-03  4:51 ` [PATCH v1 4/4] rm: integrate with sparse-index Shaoxuan Yuan
2022-08-04 14:48   ` Derrick Stolee
2022-08-06  3:18     ` Shaoxuan Yuan
2022-08-07  4:13 ` [PATCH v2 0/4] " Shaoxuan Yuan
2022-08-07  4:13   ` [PATCH v2 1/4] t1092: add tests for `git-rm` Shaoxuan Yuan
2022-08-10 12:47     ` Derrick Stolee
2022-08-07  4:13   ` [PATCH v2 2/4] pathspec.h: move pathspec_needs_expanded_index() from reset.c to here Shaoxuan Yuan
2022-08-07  4:13   ` [PATCH v2 3/4] rm: expand the index only when necessary Shaoxuan Yuan
2022-08-10  0:24     ` Victoria Dye
2022-08-07  4:13   ` [PATCH v2 4/4] rm: integrate with sparse-index Shaoxuan Yuan
2022-08-08 17:24   ` Junio C Hamano [this message]
2022-08-08 17:51     ` [PATCH v2 0/4] " Victoria Dye
2022-08-08 19:01       ` Junio C Hamano
2022-08-10  0:27   ` Victoria Dye
2022-08-10  0:31     ` Shaoxuan Yuan
2022-08-12 18:36     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqmtcesl6e.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=shaoxuan.yuan02@gmail.com \
    --cc=vdye@github.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).