| Message ID | 3bac9edae7d82ef9fdabbe2f3959e574f79f1dd0.1619213665.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Sparse-index: integrate with status | expand |
On Fri, Apr 23, 2021 at 6:34 PM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote: > > From: Derrick Stolee <dstolee@microsoft.com> > > 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. Hmm, I might be doing something wrong, but I think `folder1/a` is not shown as modified. $ git init test $ mkdir test/folder1 $ echo original >test/folder1/a $ echo original >test/b $ git -C test add . && git -C test commit -m files $ git -C test sparse-checkout init --cone --sparse-index $ ls test b $ mkdir test/folder1 && echo modified >test/folder1/a $ git -C test status On branch master You are in a sparse checkout with 50% of tracked files present. nothing to commit, working tree clean > 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. A future change could alter the behavior to > be more sensible, and this test could be modified to satisfy the new > expected behavior. > > Signed-off-by: Derrick Stolee <dstolee@microsoft.com> > --- > t/t1092-sparse-checkout-compatibility.sh | 40 ++++++++++++++++++++++++ > 1 file changed, 40 insertions(+) > > diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh > index 12e6c453024f..0ec487acd283 100755 > --- a/t/t1092-sparse-checkout-compatibility.sh > +++ b/t/t1092-sparse-checkout-compatibility.sh > @@ -232,6 +232,46 @@ test_expect_success 'add, commit, checkout' ' > test_all_match git checkout - > ' > > +test_expect_success 'status/add: outside sparse cone' ' > + init_repos && A minor suggestion: before recreating folder1/a, we could also test that `git add folder1/a` will not remove the sparse entry from the index and will properly warn about it on both sparse repos. I.e. adding a: test_sparse_match test_must_fail git add folder1/a > + # 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 && Hmm, we modify `folder1/a` in all repos, but we only try adding it on the sparse repos, and then we immediately restore it on the full repo. As we won't use the modified version on the full repo, could this perhaps be `run_on_sparse` instead? If so, we could also save the later `git -C full-checkout checkout HEAD -- 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. > + # > + # This is not a desirable behavior, but this test > + # ensures that the sparse-index is not the cause > + # of a behavior change. I'm not sure I understand what the undesirable behavior is in this sentence. Is it "git add folder1/a" erroring out and not updating `folder1/a`? Or the full repo having a different staged environment? > + test_sparse_match test_must_fail git add folder1/a && > + test_sparse_match test_must_fail git add --refresh folder1/a && > + git -C full-checkout checkout HEAD -- folder1/a && > + test_all_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 >
On 5/13/2021 8:40 AM, Matheus Tavares Bernardino wrote: > On Fri, Apr 23, 2021 at 6:34 PM Derrick Stolee via GitGitGadget > <gitgitgadget@gmail.com> wrote: >> >> From: Derrick Stolee <dstolee@microsoft.com> >> >> 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. > > Hmm, I might be doing something wrong, but I think `folder1/a` is not > shown as modified. > > $ git init test > $ mkdir test/folder1 > $ echo original >test/folder1/a > $ echo original >test/b > $ git -C test add . && git -C test commit -m files > $ git -C test sparse-checkout init --cone --sparse-index > $ ls test > b > $ mkdir test/folder1 && echo modified >test/folder1/a > $ git -C test status > On branch master > You are in a sparse checkout with 50% of tracked files present. > nothing to commit, working tree clean You are correct. This happens in both the sparse-index case and the regular full-index case. The modifications outside of the sparse-checkout definition are ignored, as long as they matched a tracked file. I checked my latest code against this example and see that the sparse index is not expanded to a full one. It _will_ be if we add an untracked file outside of the sparse cone. >> 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. A future change could alter the behavior to >> be more sensible, and this test could be modified to satisfy the new >> expected behavior. >> >> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> >> --- >> t/t1092-sparse-checkout-compatibility.sh | 40 ++++++++++++++++++++++++ >> 1 file changed, 40 insertions(+) >> >> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh >> index 12e6c453024f..0ec487acd283 100755 >> --- a/t/t1092-sparse-checkout-compatibility.sh >> +++ b/t/t1092-sparse-checkout-compatibility.sh >> @@ -232,6 +232,46 @@ test_expect_success 'add, commit, checkout' ' >> test_all_match git checkout - >> ' >> >> +test_expect_success 'status/add: outside sparse cone' ' >> + init_repos && > > A minor suggestion: before recreating folder1/a, we could also test > that `git add folder1/a` will not remove the sparse entry from the > index and will properly warn about it on both sparse repos. I.e. > adding a: > > test_sparse_match test_must_fail git add folder1/a Will do. >> + # 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 && > > Hmm, we modify `folder1/a` in all repos, but we only try adding it on > the sparse repos, and then we immediately restore it on the full repo. > As we won't use the modified version on the full repo, could this > perhaps be `run_on_sparse` instead? If so, we could also save the > later `git -C full-checkout checkout HEAD -- folder1/a`. Good idea. >> + 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. >> + # >> + # This is not a desirable behavior, but this test >> + # ensures that the sparse-index is not the cause >> + # of a behavior change. > > I'm not sure I understand what the undesirable behavior is in this > sentence. Is it "git add folder1/a" erroring out and not updating > `folder1/a`? Or the full repo having a different staged environment? Perhaps this isn't actually undesirable, now that we are actually returning an error. It's no longer silent, so maybe my comment is stale from an earlier version. Thanks, -Stolee
diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index 12e6c453024f..0ec487acd283 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -232,6 +232,46 @@ 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. + # + # This is not a desirable behavior, but this test + # ensures that the sparse-index is not the cause + # of a behavior change. + test_sparse_match test_must_fail git add folder1/a && + test_sparse_match test_must_fail git add --refresh folder1/a && + git -C full-checkout checkout HEAD -- folder1/a && + test_all_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 &&