On Tue, Jul 20, 2021 at 1:14 PM Derrick Stolee via GitGitGadget <gitgitgadget@xxxxxxxxx> 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@xxxxxxxxx> > > 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