Skip to content

Commit 70569fa

Browse files
derrickstoleegitster
authored andcommitted
t1092: document bad 'git checkout' behavior
Add new branches to the test repo that demonstrate directory/file conflicts in different ways. Since the directory 'folder1/' has adjacent files 'folder1-', 'folder1.txt', and 'folder10' it causes searches for 'folder1/' to land in a different place in the index than a search for 'folder1'. This causes a change in behavior when working with the df-conflict-1 and df-conflict-2 branches, whose only difference is that the first uses 'folder1' as the conflict and the other uses 'folder2' which does not have these adjacent files. We can extend two tests that compare the behavior across different 'git checkout' commands, and we see already that the behavior will be different in some cases and not in others. The difference between the two test loops is that one uses 'git reset --hard' between iterations. Further, we isolate the behavior of creating a staged change within a directory and then checking out a branch where that directory is replaced with a file. A full checkout behaves differently across these two cases, while a sparse-checkout cone behaves consistently. In both cases, the behavior is wrong. In one case, the staged change is dropped entirely. The other case the staged change is kept, replacing the file at that location, but none of the other files in the directory are kept. Likely, the correct behavior in this case is to reject the checkout and report the conflict, leaving HEAD in its previous location. None of the cases behave this way currently. Use comments to demonstrate that the tested behavior is only a documentation of the current, incorrect behavior to ensure we do not _accidentally_ change it. Instead, we would prefer to change it on purpose with a future change. At this point, the sparse-index does not handle these 'git checkout' commands correctly. Or rather, it _does_ reject the 'git checkout' when we have the staged change, but for the wrong reason. It also rejects the 'git checkout' commands when there is no staged change and we want to replace a directory with a file. A fix for that unstaged case will follow in the next change, but that will make the sparse-index agree with the full checkout case in these documented incorrect behaviors. Helped-by: Elijah Newren <[email protected]> Signed-off-by: Derrick Stolee <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 1ba5f45 commit 70569fa

File tree

1 file changed

+140
-2
lines changed

1 file changed

+140
-2
lines changed

t/t1092-sparse-checkout-compatibility.sh

Lines changed: 140 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,25 @@ test_expect_success 'setup' '
9595
git add . &&
9696
git commit -m "rename deep/deeper1/... to folder1/..." &&
9797
98+
git checkout -b df-conflict-1 base &&
99+
rm -rf folder1 &&
100+
echo content >folder1 &&
101+
git add . &&
102+
git commit -m "dir to file" &&
103+
104+
git checkout -b df-conflict-2 base &&
105+
rm -rf folder2 &&
106+
echo content >folder2 &&
107+
git add . &&
108+
git commit -m "dir to file" &&
109+
110+
git checkout -b fd-conflict base &&
111+
rm a &&
112+
mkdir a &&
113+
echo content >a/a &&
114+
git add . &&
115+
git commit -m "file to dir" &&
116+
98117
git checkout -b deepest base &&
99118
echo "updated deepest" >deep/deeper1/deepest/a &&
100119
git commit -a -m "update deepest" &&
@@ -358,10 +377,16 @@ test_expect_success 'diff --staged' '
358377
test_all_match git diff --staged
359378
'
360379

380+
# NEEDSWORK: sparse-checkout behaves differently from full-checkout when
381+
# running this test with 'df-conflict-2' after 'df-conflict-1'.
361382
test_expect_success 'diff with renames and conflicts' '
362383
init_repos &&
363384
364-
for branch in rename-out-to-out rename-out-to-in rename-in-to-out
385+
for branch in rename-out-to-out \
386+
rename-out-to-in \
387+
rename-in-to-out \
388+
df-conflict-1 \
389+
fd-conflict
365390
do
366391
test_all_match git checkout rename-base &&
367392
test_all_match git checkout $branch -- . &&
@@ -371,10 +396,15 @@ test_expect_success 'diff with renames and conflicts' '
371396
done
372397
'
373398

399+
# NEEDSWORK: the sparse-index fails to move HEAD across a directory/file
400+
# conflict such as when checking out df-conflict-1 and df-conflict2.
374401
test_expect_success 'diff with directory/file conflicts' '
375402
init_repos &&
376403
377-
for branch in rename-out-to-out rename-out-to-in rename-in-to-out
404+
for branch in rename-out-to-out \
405+
rename-out-to-in \
406+
rename-in-to-out \
407+
fd-conflict
378408
do
379409
git -C full-checkout reset --hard &&
380410
test_sparse_match git reset --hard &&
@@ -606,4 +636,112 @@ test_expect_success 'add everything with deep new file' '
606636
test_all_match git status --porcelain=v2
607637
'
608638

639+
# NEEDSWORK: 'git checkout' behaves incorrectly in the case of
640+
# directory/file conflicts, even without sparse-checkout. Use this
641+
# test only as a documentation of the incorrect behavior, not a
642+
# measure of how it _should_ behave.
643+
test_expect_success 'checkout behaves oddly with df-conflict-1' '
644+
init_repos &&
645+
646+
test_sparse_match git sparse-checkout disable &&
647+
648+
write_script edit-content <<-\EOF &&
649+
echo content >>folder1/larger-content
650+
git add folder1
651+
EOF
652+
653+
run_on_all ../edit-content &&
654+
test_all_match git status --porcelain=v2 &&
655+
656+
git -C sparse-checkout sparse-checkout init --cone &&
657+
git -C sparse-index sparse-checkout init --cone --sparse-index &&
658+
659+
test_all_match git status --porcelain=v2 &&
660+
661+
# This checkout command should fail, because we have a staged
662+
# change to folder1/larger-content, but the destination changes
663+
# folder1 to a file.
664+
git -C full-checkout checkout df-conflict-1 \
665+
1>full-checkout-out \
666+
2>full-checkout-err &&
667+
git -C sparse-checkout checkout df-conflict-1 \
668+
1>sparse-checkout-out \
669+
2>sparse-checkout-err &&
670+
671+
# NEEDSWORK: the sparse-index case refuses to change HEAD here,
672+
# but for the wrong reason.
673+
test_must_fail git -C sparse-index checkout df-conflict-1 \
674+
1>sparse-index-out \
675+
2>sparse-index-err &&
676+
677+
# Instead, the checkout deletes the folder1 file and adds the
678+
# folder1/larger-content file, leaving all other paths that were
679+
# in folder1/ as deleted (without any warning).
680+
cat >expect <<-EOF &&
681+
D folder1
682+
A folder1/larger-content
683+
EOF
684+
test_cmp expect full-checkout-out &&
685+
test_cmp expect sparse-checkout-out &&
686+
687+
# stderr: Switched to branch df-conflict-1
688+
test_cmp full-checkout-err sparse-checkout-err
689+
'
690+
691+
# NEEDSWORK: 'git checkout' behaves incorrectly in the case of
692+
# directory/file conflicts, even without sparse-checkout. Use this
693+
# test only as a documentation of the incorrect behavior, not a
694+
# measure of how it _should_ behave.
695+
test_expect_success 'checkout behaves oddly with df-conflict-2' '
696+
init_repos &&
697+
698+
test_sparse_match git sparse-checkout disable &&
699+
700+
write_script edit-content <<-\EOF &&
701+
echo content >>folder2/larger-content
702+
git add folder2
703+
EOF
704+
705+
run_on_all ../edit-content &&
706+
test_all_match git status --porcelain=v2 &&
707+
708+
git -C sparse-checkout sparse-checkout init --cone &&
709+
git -C sparse-index sparse-checkout init --cone --sparse-index &&
710+
711+
test_all_match git status --porcelain=v2 &&
712+
713+
# This checkout command should fail, because we have a staged
714+
# change to folder1/larger-content, but the destination changes
715+
# folder1 to a file.
716+
git -C full-checkout checkout df-conflict-2 \
717+
1>full-checkout-out \
718+
2>full-checkout-err &&
719+
git -C sparse-checkout checkout df-conflict-2 \
720+
1>sparse-checkout-out \
721+
2>sparse-checkout-err &&
722+
723+
# NEEDSWORK: the sparse-index case refuses to change HEAD
724+
# here, but for the wrong reason.
725+
test_must_fail git -C sparse-index checkout df-conflict-2 \
726+
1>sparse-index-out \
727+
2>sparse-index-err &&
728+
729+
# The full checkout deviates from the df-conflict-1 case here!
730+
# It drops the change to folder1/larger-content and leaves the
731+
# folder1 path as-is on disk.
732+
test_must_be_empty full-checkout-out &&
733+
734+
# In the sparse-checkout case, the checkout deletes the folder1
735+
# file and adds the folder1/larger-content file, leaving all other
736+
# paths that were in folder1/ as deleted (without any warning).
737+
cat >expect <<-EOF &&
738+
D folder2
739+
A folder2/larger-content
740+
EOF
741+
test_cmp expect sparse-checkout-out &&
742+
743+
# Switched to branch df-conflict-1
744+
test_cmp full-checkout-err sparse-checkout-err
745+
'
746+
609747
test_done

0 commit comments

Comments
 (0)