From: Shaoxuan Yuan Date: Tue, 09 Aug 2022 02:55:31 GMT Subject: Re: [PATCH v2 1/9] t7002: add tests for moving from in-cone to out-of-cone Message-ID: <651d89e2-5282-2cf8-ffc3-8650a023c80a@gmail.com> In-Reply-To: On 8/9/2022 8:51 AM, Victoria Dye wrote: > Shaoxuan Yuan wrote: >> Add corresponding tests to test that user can move an in-cone >> to out-of-cone when --sparse is supplied. >> >> Such can be either clean or dirty, and moving it results in >> different behaviors: >> >> A clean move should move the to the , 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 to in the index (do *not* create > in the worktree) > 2. Delete from the working tree You are right. The description here seems to be my first in-mind implementation, which I wrote down for record. Later I changed the implementation to the one you mentioned here, but somehow forgot to change the description here. Thanks for the catch! >> A dirty move should move the to the , 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 exists in the index (existing >> check for if 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 >> Helped-by: Victoria Dye >> Signed-off-by: Shaoxuan Yuan >> --- >> 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'. OK. > > 2. Move a (clean or dirty) in-cone source directory to an out-of-cone > destination where one or more files in overwrite files in . > 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. OK. I think it will fail the whole `mv` operation. The program will report unspecified overwrite during the early checking phase. The actual "moving" phase won't be touched at all because Git complains early. -- Thanks, Shaoxuan