git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Victoria Dye <vdye@github.com>
To: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com>, git@vger.kernel.org
Cc: derrickstolee@github.com
Subject: Re: [PATCH v2 1/9] t7002: add tests for moving from in-cone to out-of-cone
Date: Mon, 8 Aug 2022 17:51:24 -0700	[thread overview]
Message-ID: <bd80881d-b2a3-c220-8f2d-a07a46e14207@github.com> (raw)
In-Reply-To: <20220805030528.1535376-2-shaoxuan.yuan02@gmail.com>

Shaoxuan Yuan wrote:
> Add corresponding tests to test that user can move an in-cone <source>
> to out-of-cone <destination> when --sparse is supplied.
> 
> Such <source> can be either clean or dirty, and moving it results in
> different behaviors:
> 
> A clean move should move the <source> to the <destination>, both in the
> working tree and the index, then remove the resulted path from the
> working tree, and turn on its CE_SKIP_WORKTREE bit.

Looking ahead to patch 6, I think the part about "move it in the working
tree, then delete from the working tree" doesn't quite match your
implementation. Instead, if I'm not mistaken, what happens is:

1. Move <source> to <destination> in the index (do *not* create
   <destination> in the worktree)
2. Delete <source> from the working tree

> 
> A dirty move should move the <source> to the <destination>, both in the
> working tree and the index, but should *not* remove the resulted path
> from the working tree and should *not* turn on its CE_SKIP_WORKTREE bit.

Makes sense.

> 
> Also make sure that if <destination> exists in the index (existing
> check for if <destination> is in the worktree is not enough in
> in-to-out moves), warn user against the overwrite. And Git should force
> the overwrite when supplied with -f or --force.

Also makes sense. 

> 
> Helped-by: Derrick Stolee <derrickstolee@github.com>
> Helped-by: Victoria Dye <vdye@github.com>
> Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com>
> ---
>  t/t7002-mv-sparse-checkout.sh | 122 ++++++++++++++++++++++++++++++++++
>  1 file changed, 122 insertions(+)
> 
> diff --git a/t/t7002-mv-sparse-checkout.sh b/t/t7002-mv-sparse-checkout.sh
> index 71fe29690f..9b3a9ab4c3 100755
> --- a/t/t7002-mv-sparse-checkout.sh
> +++ b/t/t7002-mv-sparse-checkout.sh
> @@ -290,4 +290,126 @@ test_expect_success 'move sparse file to existing destination with --force and -
>  	test_cmp expect sub/file1
>  '
>  
> +test_expect_failure 'move clean path from in-cone to out-of-cone' '
> +	test_when_finished "cleanup_sparse_checkout" &&
> +	setup_sparse_checkout &&
> +
> +	test_must_fail git mv sub/d folder1 2>stderr &&
> +	cat sparse_error_header >expect &&
> +	echo "folder1/d" >>expect &&
> +	cat sparse_hint >>expect &&
> +	test_cmp expect stderr &&
> +
> +	git mv --sparse sub/d folder1 2>stderr &&
> +	test_must_be_empty stderr &&
> +
> +	test_path_is_missing sub/d &&
> +	test_path_is_missing folder1/d &&
> +	git ls-files -t >actual &&
> +	! grep "^H sub/d\$" actual &&
> +	grep "S folder1/d" actual
> +'
> +
> +test_expect_failure 'move clean path from in-cone to out-of-cone overwrite' '
> +	test_when_finished "cleanup_sparse_checkout" &&
> +	setup_sparse_checkout &&
> +	echo "sub/file1 overwrite" >sub/file1 &&
> +	git add sub/file1 &&
> +
> +	test_must_fail git mv sub/file1 folder1 2>stderr &&
> +	cat sparse_error_header >expect &&
> +	echo "folder1/file1" >>expect &&
> +	cat sparse_hint >>expect &&
> +	test_cmp expect stderr &&
> +
> +	test_must_fail git mv --sparse sub/file1 folder1 2>stderr &&
> +	echo "fatal: destination exists in the index, source=sub/file1, destination=folder1/file1" \
> +	>expect &&
> +	test_cmp expect stderr &&
> +
> +	git mv --sparse -f sub/file1 folder1 2>stderr &&
> +	test_must_be_empty stderr &&
> +
> +	test_path_is_missing sub/file1 &&
> +	test_path_is_missing folder1/file1 &&
> +	git ls-files -t >actual &&
> +	! grep "H sub/file1" actual &&
> +	grep "S folder1/file1" actual &&
> +
> +	# compare file content before move and after move
> +	echo "sub/file1 overwrite" >expect &&
> +	git ls-files -s -- folder1/file1 | awk "{print \$2}" >oid &&
> +	git cat-file blob $(cat oid) >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_failure 'move dirty path from in-cone to out-of-cone' '
> +	test_when_finished "cleanup_sparse_checkout" &&
> +	setup_sparse_checkout &&
> +	echo "modified" >>sub/d &&
> +
> +	test_must_fail git mv sub/d folder1 2>stderr &&
> +	cat sparse_error_header >expect &&
> +	echo "folder1/d" >>expect &&
> +	cat sparse_hint >>expect &&
> +	test_cmp expect stderr &&
> +
> +	git mv --sparse sub/d folder1 2>stderr &&
> +
> +	test_path_is_missing sub/d &&
> +	test_path_is_file folder1/d &&
> +	git ls-files -t >actual &&
> +	! grep "^H sub/d\$" actual &&
> +	grep "H folder1/d" actual
> +'
> +
> +test_expect_failure 'move dir from in-cone to out-of-cone' '
> +	test_when_finished "cleanup_sparse_checkout" &&
> +	setup_sparse_checkout &&
> +
> +	test_must_fail git mv sub/dir folder1 2>stderr &&
> +	cat sparse_error_header >expect &&
> +	echo "folder1/dir/e" >>expect &&
> +	cat sparse_hint >>expect &&
> +	test_cmp expect stderr &&
> +
> +	git mv --sparse sub/dir folder1 2>stderr &&
> +	test_must_be_empty stderr &&
> +
> +	test_path_is_missing folder1 &&
> +	git ls-files -t >actual &&
> +	! grep "H sub/dir/e" actual &&
> +	grep "S folder1/dir/e" actual
> +'
> +
> +test_expect_failure 'move partially-dirty dir from in-cone to out-of-cone' '
> +	test_when_finished "cleanup_sparse_checkout" &&
> +	setup_sparse_checkout &&
> +	touch sub/dir/e2 sub/dir/e3 &&
> +	git add sub/dir/e2 sub/dir/e3 &&
> +	echo "modified" >>sub/dir/e2 &&
> +	echo "modified" >>sub/dir/e3 &&
> +
> +	test_must_fail git mv sub/dir folder1 2>stderr &&
> +	cat sparse_error_header >expect &&
> +	echo "folder1/dir/e" >>expect &&
> +	echo "folder1/dir/e2" >>expect &&
> +	echo "folder1/dir/e3" >>expect &&
> +	cat sparse_hint >>expect &&
> +	test_cmp expect stderr &&
> +
> +	git mv --sparse sub/dir folder1 2>stderr &&
> +
> +	test_path_is_missing folder1/dir/e &&
> +	test_path_is_file folder1/dir/e2 &&
> +	test_path_is_file folder1/dir/e3 &&
> +	git ls-files -t >actual &&
> +	! grep "H sub/dir/e" actual &&
> +	! grep "H sub/dir/e2" actual &&
> +	! grep "H sub/dir/e3" actual &&
> +	grep "S folder1/dir/e" actual &&
> +	grep "H folder1/dir/e2" actual &&
> +	grep "H folder1/dir/e3" actual
> +'
> +

There are two other test cases I'd be interested in seeing:

1. Move a (clean or dirty) in-cone source file to an out-of-cone destination
   *file*. For example:

	echo test >sub/dir/file1 && 
	git add sub/dir/file1 && 
	git mv --sparse sub/dir/file1 folder1/file1

   I'm assuming this should behave the same way as show in 'move clean path
   from in-cone to out-of-cone overwrite'. 

2. Move a (clean or dirty) in-cone source directory to an out-of-cone
   destination where one or more files in <src> overwrite files in <dst>.
   For example, something like:

	echo test >sub/dir/file1 &&
	git add sub/dir/file1 &&
	git mv --sparse sub/dir folder1

   I don't have a strong opinion on the behavior (does it fail the whole
   'mv' operation? move everything except the files that overwrite
   something?), but it would help to have it documented via test here.

>  test_done


  reply	other threads:[~2022-08-09  0:51 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-19 13:28 [PATCH v1 0/7] mv: from in-cone to out-of-cone Shaoxuan Yuan
2022-07-19 13:28 ` [PATCH v1 1/7] t7002: add tests for moving " Shaoxuan Yuan
2022-07-19 14:52   ` Ævar Arnfjörð Bjarmason
2022-07-19 17:36     ` Derrick Stolee
2022-07-19 18:30       ` Junio C Hamano
2022-07-19 13:28 ` [PATCH v1 2/7] mv: add documentation for check_dir_in_index() Shaoxuan Yuan
2022-07-19 17:43   ` Derrick Stolee
2022-07-21 13:58     ` Shaoxuan Yuan
2022-07-19 18:01   ` Victoria Dye
2022-07-19 18:10     ` Victoria Dye
2022-07-21 14:20     ` Shaoxuan Yuan
2022-07-19 13:28 ` [PATCH v1 3/7] mv: free the *with_slash in check_dir_in_index() Shaoxuan Yuan
2022-07-19 17:46   ` Derrick Stolee
2022-07-19 13:28 ` [PATCH v1 4/7] mv: check if <destination> is a SKIP_WORKTREE_DIR Shaoxuan Yuan
2022-07-19 17:59   ` Derrick Stolee
2022-07-21 14:13     ` Shaoxuan Yuan
2022-07-22 12:48       ` Derrick Stolee
2022-07-22 18:40         ` Junio C Hamano
2022-07-19 13:28 ` [PATCH v1 5/7] mv: remove BOTH from enum update_mode Shaoxuan Yuan
2022-07-19 18:00   ` Derrick Stolee
2022-07-19 13:28 ` [PATCH v1 6/7] mv: from in-cone to out-of-cone Shaoxuan Yuan
2022-07-19 18:14   ` Derrick Stolee
2022-08-03 11:50     ` Shaoxuan Yuan
2022-08-03 14:30       ` Derrick Stolee
2022-08-04  8:40     ` Shaoxuan Yuan
2022-07-19 13:28 ` [PATCH v1 7/7] mv: check overwrite for in-to-out move Shaoxuan Yuan
2022-07-19 18:15   ` Derrick Stolee
2022-07-19 18:16 ` [PATCH v1 0/7] mv: from in-cone to out-of-cone Derrick Stolee
2022-08-05  3:05 ` [PATCH v2 0/9] " Shaoxuan Yuan
2022-08-05  3:05   ` [PATCH v2 1/9] t7002: add tests for moving " Shaoxuan Yuan
2022-08-09  0:51     ` Victoria Dye [this message]
2022-08-09  2:55       ` Shaoxuan Yuan
2022-08-09 11:24         ` Shaoxuan Yuan
2022-08-09  7:53       ` Shaoxuan Yuan
2022-08-05  3:05   ` [PATCH v2 2/9] mv: rename check_dir_in_index() to empty_dir_has_sparse_contents() Shaoxuan Yuan
2022-08-05  3:05   ` [PATCH v2 3/9] mv: free the *with_slash in check_dir_in_index() Shaoxuan Yuan
2022-08-08 23:41     ` Victoria Dye
2022-08-09  2:33       ` Shaoxuan Yuan
2022-08-05  3:05   ` [PATCH v2 4/9] mv: check if <destination> is a SKIP_WORKTREE_DIR Shaoxuan Yuan
2022-08-08 23:41     ` Victoria Dye
2022-08-09  0:23       ` Victoria Dye
2022-08-09  2:31       ` Shaoxuan Yuan
2022-08-05  3:05   ` [PATCH v2 5/9] mv: remove BOTH from enum update_mode Shaoxuan Yuan
2022-08-05  3:05   ` [PATCH v2 6/9] mv: from in-cone to out-of-cone Shaoxuan Yuan
2022-08-09  0:53     ` Victoria Dye
2022-08-09  3:16       ` Shaoxuan Yuan
2022-08-05  3:05   ` [PATCH v2 7/9] mv: cleanup empty WORKING_DIRECTORY Shaoxuan Yuan
2022-08-05  3:05   ` [PATCH v2 8/9] advice.h: add advise_on_moving_dirty_path() Shaoxuan Yuan
2022-08-05  3:05   ` [PATCH v2 9/9] mv: check overwrite for in-to-out move Shaoxuan Yuan
2022-08-08 23:53     ` Victoria Dye
2022-08-09 12:09 ` [PATCH v3 0/9] mv: from in-cone to out-of-cone Shaoxuan Yuan
2022-08-09 12:09   ` [PATCH v3 1/9] t7002: add tests for moving " Shaoxuan Yuan
2022-08-09 12:09   ` [PATCH v3 2/9] mv: rename check_dir_in_index() to empty_dir_has_sparse_contents() Shaoxuan Yuan
2022-08-09 12:09   ` [PATCH v3 3/9] mv: free the with_slash in check_dir_in_index() Shaoxuan Yuan
2022-08-09 12:09   ` [PATCH v3 4/9] mv: check if <destination> is a SKIP_WORKTREE_DIR Shaoxuan Yuan
2022-08-09 12:09   ` [PATCH v3 5/9] mv: remove BOTH from enum update_mode Shaoxuan Yuan
2022-08-09 12:09   ` [PATCH v3 6/9] mv: from in-cone to out-of-cone Shaoxuan Yuan
2022-08-09 12:09   ` [PATCH v3 7/9] mv: cleanup empty WORKING_DIRECTORY Shaoxuan Yuan
2022-08-09 12:09   ` [PATCH v3 8/9] advice.h: add advise_on_moving_dirty_path() Shaoxuan Yuan
2022-08-09 12:09   ` [PATCH v3 9/9] mv: check overwrite for in-to-out move Shaoxuan Yuan
2022-08-16 15:48   ` [PATCH v3 0/9] mv: from in-cone to out-of-cone Victoria Dye

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=bd80881d-b2a3-c220-8f2d-a07a46e14207@github.com \
    --to=vdye@github.com \
    --cc=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=shaoxuan.yuan02@gmail.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).