| Message ID | pull.973.v2.git.1626812081.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Sparse index: integrate with commit and checkout | expand |
On Tue, Jul 20, 2021 at 1:14 PM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote: > > This series extends our integration of sparse-index to 'git commit' and 'git > checkout'. > > This is based on ds/status-with-sparse-index (v7) and v2.32.0. The hard work > was already done in that topic, so these changes are simple. > > Recall that we have delayed our integration with 'git add' until we can work > out the concerns about how to deal with pathspecs outside of the > sparse-checkout definition. Those concerns might have some overlap with how > 'git commit' takes a pathspec, but this seems like a rare enough case to > handle here and we can be more careful with the behavior change in the next > series which will integrate with git add. > > In addition to the tests that already exist in t1092, I have integrated > these changes in microsoft/git and tested them against the Scalar functional > tests, which go through quite a few complicated scenarios, verifying that > things work the same across the full index and sparse-index cases. > > > Update in V2 > ============ > > * There is no change to the code, but it is presented in a slightly > different order. > * We've been discussing some complicated directory/file conflict cases, in > particular with a staged change inside the directory. These tests are > added and described as documenting incorrect behavior that should be > changed. > * After those tests are in place, we can motivate the change to > twoway_merge() as necessary for a more-common situation (still rare) but > still incorrect in an already-broken situation. Hopefully that balance is > sufficient for now, until we can do the bigger work of fixing the bad > behavior. I read the first five patches previously. The tiny changes there in the range-diff still look good to me. I very much appreciate the new patch 6. As noted in 7/7, I'm a little unhappy with the patch to twoway_merge(), BUT you've clearly documented the shortcomings in very good detail and pointed out how git has (likely for decades) messed up in related ways for non-sparse checkouts with D/F conflicts. You've documented it well enough and argued well enough about the relative merits, that I have to agree with you that this is a good step forward. I do hope we circle back and tie up the loose ends at some point. So, the whole series is: Reviewed-by: Elijah Newren <newren@gmail.com> > > Thanks, -Stolee > > Derrick Stolee (7): > p2000: add 'git checkout -' test and decrease depth > p2000: compress repo names > commit: integrate with sparse-index > sparse-index: recompute cache-tree > checkout: stop expanding sparse indexes > t1092: document bad 'git checkout' behavior > unpack-trees: resolve sparse-directory/file conflicts > > builtin/checkout.c | 8 +- > builtin/commit.c | 3 + > cache-tree.c | 2 - > sparse-index.c | 2 + > t/perf/p2000-sparse-operations.sh | 47 ++++-- > t/t1092-sparse-checkout-compatibility.sh | 197 ++++++++++++++++++++++- > unpack-trees.c | 11 ++ > 7 files changed, 240 insertions(+), 30 deletions(-) > > > base-commit: e5ca291076a8a936283bb2c57433c4393d3f80c2 > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-973%2Fderrickstolee%2Fsparse-index%2Fcommit-and-checkout-v2 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-973/derrickstolee/sparse-index/commit-and-checkout-v2 > Pull-Request: https://github.com/gitgitgadget/git/pull/973 > > Range-diff vs v1: > > 1: bb3dd1fdd48 = 1: 6e74958f590 p2000: add 'git checkout -' test and decrease depth > 2: eb15bf37685 = 2: 3e1d03c41be p2000: compress repo names > 3: 413babe6e77 ! 3: cd94f820052 commit: integrate with sparse-index > @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'sparse-index is e > + ensure_not_expanded commit --include deep/deeper1/a -m deeper > ' > > - test_expect_success 'reset mixed and checkout orphan' ' > + # NEEDSWORK: a sparse-checkout behaves differently from a full checkout > 4: ffe8473caab = 4: 65e79b8037c sparse-index: recompute cache-tree > 5: 8710fee36b7 ! 5: e9a9981477e checkout: stop expanding sparse indexes > @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'sparse-index is n > + ensure_not_expanded restore -s rename-out-to-out -- deep/deeper1 > ' > > - test_expect_success 'reset mixed and checkout orphan' ' > + # NEEDSWORK: a sparse-checkout behaves differently from a full checkout > -: ----------- > 6: 4b801c854fb t1092: document bad 'git checkout' behavior > -: ----------- > 7: 71e301501c8 unpack-trees: resolve sparse-directory/file conflicts > > -- > gitgitgadget
This series extends our integration of sparse-index to 'git commit' and 'git checkout'. This is based on ds/status-with-sparse-index (v7) and v2.32.0. The hard work was already done in that topic, so these changes are simple. Recall that we have delayed our integration with 'git add' until we can work out the concerns about how to deal with pathspecs outside of the sparse-checkout definition. Those concerns might have some overlap with how 'git commit' takes a pathspec, but this seems like a rare enough case to handle here and we can be more careful with the behavior change in the next series which will integrate with git add. In addition to the tests that already exist in t1092, I have integrated these changes in microsoft/git and tested them against the Scalar functional tests, which go through quite a few complicated scenarios, verifying that things work the same across the full index and sparse-index cases. Update in V2 ============ * There is no change to the code, but it is presented in a slightly different order. * We've been discussing some complicated directory/file conflict cases, in particular with a staged change inside the directory. These tests are added and described as documenting incorrect behavior that should be changed. * After those tests are in place, we can motivate the change to twoway_merge() as necessary for a more-common situation (still rare) but still incorrect in an already-broken situation. Hopefully that balance is sufficient for now, until we can do the bigger work of fixing the bad behavior. Thanks, -Stolee Derrick Stolee (7): p2000: add 'git checkout -' test and decrease depth p2000: compress repo names commit: integrate with sparse-index sparse-index: recompute cache-tree checkout: stop expanding sparse indexes t1092: document bad 'git checkout' behavior unpack-trees: resolve sparse-directory/file conflicts builtin/checkout.c | 8 +- builtin/commit.c | 3 + cache-tree.c | 2 - sparse-index.c | 2 + t/perf/p2000-sparse-operations.sh | 47 ++++-- t/t1092-sparse-checkout-compatibility.sh | 197 ++++++++++++++++++++++- unpack-trees.c | 11 ++ 7 files changed, 240 insertions(+), 30 deletions(-) base-commit: e5ca291076a8a936283bb2c57433c4393d3f80c2 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-973%2Fderrickstolee%2Fsparse-index%2Fcommit-and-checkout-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-973/derrickstolee/sparse-index/commit-and-checkout-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/973 Range-diff vs v1: 1: bb3dd1fdd48 = 1: 6e74958f590 p2000: add 'git checkout -' test and decrease depth 2: eb15bf37685 = 2: 3e1d03c41be p2000: compress repo names 3: 413babe6e77 ! 3: cd94f820052 commit: integrate with sparse-index @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'sparse-index is e + ensure_not_expanded commit --include deep/deeper1/a -m deeper ' - test_expect_success 'reset mixed and checkout orphan' ' + # NEEDSWORK: a sparse-checkout behaves differently from a full checkout 4: ffe8473caab = 4: 65e79b8037c sparse-index: recompute cache-tree 5: 8710fee36b7 ! 5: e9a9981477e checkout: stop expanding sparse indexes @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'sparse-index is n + ensure_not_expanded restore -s rename-out-to-out -- deep/deeper1 ' - test_expect_success 'reset mixed and checkout orphan' ' + # NEEDSWORK: a sparse-checkout behaves differently from a full checkout -: ----------- > 6: 4b801c854fb t1092: document bad 'git checkout' behavior -: ----------- > 7: 71e301501c8 unpack-trees: resolve sparse-directory/file conflicts