| Message ID | 20220805030528.1535376-2-shaoxuan.yuan02@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | mv: from in-cone to out-of-cone | expand |
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
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 <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 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 <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'. OK. > > 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. 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
On 8/9/2022 8:51 AM, Victoria Dye wrote: > 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'. It's interesting that this test covers an aspect that was not properly considered. When designing in-to-out, I only thought <destination> to be a sparse directory, rather than a possible sparse file, which is a totally valid argument. Hence, all the logics and mechanisms about in-to-out are skipped because Git is completely blind to this file <destination>. It seems like an easy fix, but I was completely unaware. Thanks for catching this!
On 8/9/2022 10:55 AM, Shaoxuan Yuan wrote: >> 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. > 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. No, actually this example does not fail anything. Because "sub/dir/file1" after the move will be "folder1/dir/file1", and this move does not involve overwrite at all. I'm still adding a test case that precisely reflects the main idea here, though. Thanks for the suggestion :) -- Thanks, Shaoxuan
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 +' + test_done
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. 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. 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. 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(+)