Subject: Re: [PATCH v2 1/9] t7002: add tests for moving from in-cone to out-of-cone

From: Victoria Dye <[email-masked]>
Date: Mon, 8 Aug 2022 17:51:24 -0700

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 <[email-masked]>
> Helped-by: Victoria Dye <[email-masked]>
> Signed-off-by: Shaoxuan Yuan <[email-masked]>
> ---
>  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