On Tue, Apr 13, 2021 at 7:01 AM Derrick Stolee via GitGitGadget <gitgitgadget@xxxxxxxxx> wrote: > > From: Derrick Stolee <dstolee@xxxxxxxxxxxxx> > > Before moving to update 'git status' and 'git add' to work with sparse > indexes, add an explicit test that ensures the sparse-index works the > same as a normal sparse-checkout when the worktree contains directories > and files outside of the sparse cone. > > Specifically, 'folder1/a' is a file in our test repo, but 'folder1' is > not in the sparse cone. When 'folder1/a' is modified, the file > 'folder1/a' is shown as modified, but adding it fails. This is new > behavior as of a20f704 (add: warn when asked to update SKIP_WORKTREE > entries, 2021-04-08). Before that change, these adds would be silently > ignored. > > Untracked files are fine: adding new files both with 'git add .' and > 'git add folder1/' works just as in a full checkout. This may not be > entirely desirable, but we are not intending to change behavior at the > moment, only document it. Personally, I'd say not desirable and we should throw an error just like we do with skip-worktree entries that the user happens to try to git add. I've had reports from users that got confused by what happens after this. I've been meaning to create some patches to fix it up, but wanted to avoid getting in the way of the sparse-index work and have been a bit tied up on other projects to boot. I'll note in particular that it's easy for users after running "git add" to run other things such as "git sparse-checkout reapply" or "git switch $otherbranch" and suddenly the file disappears from the working tree. From the sparse-checkout machinery that makes sense; this path doesn't match the .git/info/sparse-checkout list of paths, so it should be removed from the working tree. But it's very disorienting to users. Especially if some of those commands are side-effects of other commands (e.g. our build system invokes "git sparse-checkout reapply" in various cases, most common of which is that even a simple "git pull" can bring down code with dependency changes and thus a need for new sparsity rules and whatnot), but it definitely can just happen in ways users don't expect with their own commands (e.g. the git switch/checkout example). The patch looks good, but it'd be nice if while documenting it we also add a comment that we believe we want to change the behavior (for sparse-checkout both with and without sparse-index). It's one of those many paper-cuts we still have. > Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx> > --- > t/t1092-sparse-checkout-compatibility.sh | 36 ++++++++++++++++++++++++ > 1 file changed, 36 insertions(+) > > diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh > index 12e6c453024f..6598c12a2069 100755 > --- a/t/t1092-sparse-checkout-compatibility.sh > +++ b/t/t1092-sparse-checkout-compatibility.sh > @@ -232,6 +232,42 @@ test_expect_success 'add, commit, checkout' ' > test_all_match git checkout - > ' > > +test_expect_success 'status/add: outside sparse cone' ' > + init_repos && > + > + # folder1 is at HEAD, but outside the sparse cone > + run_on_sparse mkdir folder1 && > + cp initial-repo/folder1/a sparse-checkout/folder1/a && > + cp initial-repo/folder1/a sparse-index/folder1/a && > + > + test_sparse_match git status && > + > + write_script edit-contents <<-\EOF && > + echo text >>$1 > + EOF > + run_on_all ../edit-contents folder1/a && > + run_on_all ../edit-contents folder1/new && > + > + test_sparse_match git status --porcelain=v2 && > + > + # This "git add folder1/a" is completely ignored > + # by the sparse-checkout repos. It causes the > + # full repo to have a different staged environment. > + test_must_fail git -C sparse-checkout add folder1/a && > + test_must_fail git -C sparse-index add folder1/a && > + git -C full-checkout checkout HEAD -- folder1/a && > + test_sparse_match git status --porcelain=v2 && > + > + test_all_match git add . && > + test_all_match git status --porcelain=v2 && > + test_all_match git commit -m folder1/new && > + > + run_on_all ../edit-contents folder1/newer && > + test_all_match git add folder1/ && > + test_all_match git status --porcelain=v2 && > + test_all_match git commit -m folder1/newer > +' > + > test_expect_success 'checkout and reset --hard' ' > init_repos && > > -- > gitgitgadget >
