From: Derrick Stolee <dstolee@xxxxxxxxxxxxx> 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 <newren@xxxxxxxxx> Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx> --- t/t1092-sparse-checkout-compatibility.sh | 142 ++++++++++++++++++++++- 1 file changed, 140 insertions(+), 2 deletions(-) diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index fde3b41aba8..79b4a8ce199 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -95,6 +95,25 @@ test_expect_success 'setup' ' git add . && git commit -m "rename deep/deeper1/... to folder1/..." && + git checkout -b df-conflict-1 base && + rm -rf folder1 && + echo content >folder1 && + git add . && + git commit -m "dir to file" && + + git checkout -b df-conflict-2 base && + rm -rf folder2 && + echo content >folder2 && + git add . && + git commit -m "dir to file" && + + git checkout -b fd-conflict base && + rm a && + mkdir a && + echo content >a/a && + git add . && + git commit -m "file to dir" && + git checkout -b deepest base && echo "updated deepest" >deep/deeper1/deepest/a && git commit -a -m "update deepest" && @@ -358,10 +377,16 @@ test_expect_success 'diff --staged' ' test_all_match git diff --staged ' +# NEEDSWORK: sparse-checkout behaves differently from full-checkout when +# running this test with 'df-conflict-2' after 'df-conflict-1'. test_expect_success 'diff with renames and conflicts' ' init_repos && - for branch in rename-out-to-out rename-out-to-in rename-in-to-out + for branch in rename-out-to-out \ + rename-out-to-in \ + rename-in-to-out \ + df-conflict-1 \ + fd-conflict do test_all_match git checkout rename-base && test_all_match git checkout $branch -- . && @@ -371,10 +396,15 @@ test_expect_success 'diff with renames and conflicts' ' done ' +# NEEDSWORK: the sparse-index fails to move HEAD across a directory/file +# conflict such as when checking out df-conflict-1 and df-conflict2. test_expect_success 'diff with directory/file conflicts' ' init_repos && - for branch in rename-out-to-out rename-out-to-in rename-in-to-out + for branch in rename-out-to-out \ + rename-out-to-in \ + rename-in-to-out \ + fd-conflict do git -C full-checkout reset --hard && test_sparse_match git reset --hard && @@ -606,4 +636,112 @@ test_expect_success 'add everything with deep new file' ' test_all_match git status --porcelain=v2 ' +# NEEDSWORK: 'git checkout' behaves incorrectly in the case of +# directory/file conflicts, even without sparse-checkout. Use this +# test only as a documentation of the incorrect behavior, not a +# measure of how it _should_ behave. +test_expect_success 'checkout behaves oddly with df-conflict-1' ' + init_repos && + + test_sparse_match git sparse-checkout disable && + + write_script edit-content <<-\EOF && + echo content >>folder1/larger-content + git add folder1 + EOF + + run_on_all ../edit-content && + test_all_match git status --porcelain=v2 && + + git -C sparse-checkout sparse-checkout init --cone && + git -C sparse-index sparse-checkout init --cone --sparse-index && + + test_all_match git status --porcelain=v2 && + + # This checkout command should fail, because we have a staged + # change to folder1/larger-content, but the destination changes + # folder1 to a file. + git -C full-checkout checkout df-conflict-1 \ + 1>full-checkout-out \ + 2>full-checkout-err && + git -C sparse-checkout checkout df-conflict-1 \ + 1>sparse-checkout-out \ + 2>sparse-checkout-err && + + # NEEDSWORK: the sparse-index case refuses to change HEAD here, + # but for the wrong reason. + test_must_fail git -C sparse-index checkout df-conflict-1 \ + 1>sparse-index-out \ + 2>sparse-index-err && + + # Instead, the checkout deletes the folder1 file and adds the + # folder1/larger-content file, leaving all other paths that were + # in folder1/ as deleted (without any warning). + cat >expect <<-EOF && + D folder1 + A folder1/larger-content + EOF + test_cmp expect full-checkout-out && + test_cmp expect sparse-checkout-out && + + # stderr: Switched to branch df-conflict-1 + test_cmp full-checkout-err sparse-checkout-err +' + +# NEEDSWORK: 'git checkout' behaves incorrectly in the case of +# directory/file conflicts, even without sparse-checkout. Use this +# test only as a documentation of the incorrect behavior, not a +# measure of how it _should_ behave. +test_expect_success 'checkout behaves oddly with df-conflict-2' ' + init_repos && + + test_sparse_match git sparse-checkout disable && + + write_script edit-content <<-\EOF && + echo content >>folder2/larger-content + git add folder2 + EOF + + run_on_all ../edit-content && + test_all_match git status --porcelain=v2 && + + git -C sparse-checkout sparse-checkout init --cone && + git -C sparse-index sparse-checkout init --cone --sparse-index && + + test_all_match git status --porcelain=v2 && + + # This checkout command should fail, because we have a staged + # change to folder1/larger-content, but the destination changes + # folder1 to a file. + git -C full-checkout checkout df-conflict-2 \ + 1>full-checkout-out \ + 2>full-checkout-err && + git -C sparse-checkout checkout df-conflict-2 \ + 1>sparse-checkout-out \ + 2>sparse-checkout-err && + + # NEEDSWORK: the sparse-index case refuses to change HEAD + # here, but for the wrong reason. + test_must_fail git -C sparse-index checkout df-conflict-2 \ + 1>sparse-index-out \ + 2>sparse-index-err && + + # The full checkout deviates from the df-conflict-1 case here! + # It drops the change to folder1/larger-content and leaves the + # folder1 path as-is on disk. + test_must_be_empty full-checkout-out && + + # In the sparse-checkout case, the checkout deletes the folder1 + # file and adds the folder1/larger-content file, leaving all other + # paths that were in folder1/ as deleted (without any warning). + cat >expect <<-EOF && + D folder2 + A folder2/larger-content + EOF + test_cmp expect sparse-checkout-out && + + # Switched to branch df-conflict-1 + test_cmp full-checkout-err sparse-checkout-err +' + test_done -- gitgitgadget