Re: [PATCH v2 0/5] Sparse Index: Integrate with 'git add'

>
> This patch series re-submits the 'git add' integration with sparse-index.
> The performance gains are the same as before.
>
> It is based on ds/commit-and-checkout-with-sparse-index.
>
> This series was delayed from its initial submission for a couple reasons.
>
> The first was because it was colliding with some changes in
> mt/add-rm-in-sparse-checkout, so now we are far enough along that that
> branch is in our history and we can work forwards.
>
> The other concern was about how 'git add ' should respond when a path
> outside of the sparse-checkout cone exists. One recommendation (that I am
> failing to find a link to the message, sorry) was to disallow adding files
> that would become index entries with SKIP_WORKTREE on. However, as I worked
> towards that goal I found that change would cause problems for a realistic
> scenario: merge conflicts outside of the sparse-checkout cone.
>
> Update: Elijah points out that the SKIP_WORKTREE bit is removed from
> conflict files, which allows adding the conflicted files without warning.
> (However, we also need to be careful about untracked files, as documented in
> the test added here.)
>
> The first patch of this series adds tests that create merge conflicts
> outside of the sparse cone and then presents different ways a user could
> resolve the situation. We want all of them to be feasible, and this
> includes:
>
>  1. Reverting the file to a known version in history.
>  2. Adding the file with its contents on disk.
>  3. Moving the file to a new location in the sparse directory.
>
> The one place I did continue to update is 'git add --refresh ' to match the
> behavior added by mt/add-rm-in-sparse-checkout which outputs an error
> message. This happens even when the file exists in the working directory,
> but that seems appropriate enough.
>
>
> Updates in V2
> =============
>
>  * Test comments in patch 1 are improved.
>
>  * The test hunk that was removed in patch 2 and reintroduced in the old
>    patch 4 is modified to clarify how the behavior changes with that patch.
>    Then, the test is modified by future patches.
>
>  * Another instance of ensure_full_index() is removed from the --renormalize
>    option. This option already ignored files with the SKIP_WORKTREE bit, so
>    this should be an obviously-correct removal.
>
>  * a full proposal for what to do with "git (add|mv|rm)" and paths outside
>    the cone is delayed to another series (with an RFC round) because the
>    behavior of the sparse-index matches a full index with sparse-checkout.

> Thanks, -Stolee
>
> Derrick Stolee (5):
>   t1092: test merge conflicts outside cone
>   add: allow operating on a sparse-only index
>   pathspec: stop calling ensure_full_index
>   add: ignore outside the sparse-checkout in refresh()
>   add: remove ensure_full_index() with --renormalize
>
>  builtin/add.c                            | 15 ++++--
>  pathspec.c                               |  2 -
>  t/t1092-sparse-checkout-compatibility.sh | 62 ++++++++++++++++++++----
>  3 files changed, 65 insertions(+), 14 deletions(-)
>
>
> base-commit: 71e301501c88399711a1bf8515d1747e92cfbb9b
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-999%2Fderrickstolee%2Fsparse-index%2Fadd-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-999/derrickstolee/sparse-index/add-v2
> Pull-Request: https://github.com/gitgitgadget/git/pull/999
>
> Range-diff vs v1:
>
>  1:  a763a7d15b8 ! 1:  8f2fd9370fe t1092: test merge conflicts outside cone
>      @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'merge' '
>       + test_all_match test_must_fail git merge -m merge merge-right &&
>       + test_all_match git status --porcelain=v2 &&
>       +
>      -+ # resolve the conflict in different ways:
>      -+ # 1. revert to the base
>      ++ # Resolve the conflict in different ways:
>      ++ # 1. Revert to the base
>       + test_all_match git checkout base -- deep/deeper2/a &&
>       + test_all_match git status --porcelain=v2 &&
>       +
>      -+ # 2. add the file with conflict markers
>      ++ # 2. Add the file with conflict markers
>       + test_all_match git add folder1/a &&
>       + test_all_match git status --porcelain=v2 &&
>       +
>      -+ # 3. rename the file to another sparse filename
>      ++ # 3. Rename the file to another sparse filename and
>      ++ #    accept conflict markers as resolved content.
>       + run_on_all mv folder2/a folder2/z &&
>       + test_all_match git add folder2 &&
>       + test_all_match git status --porcelain=v2 &&
>  2:  791c6c2c9ad ! 2:  6e43f118fa0 add: allow operating on a sparse-only index
>      @@ Commit message
>           sparse-index. Comparing to the full index case, 'git add -A' goes from
>           0.37s to 0.05s, which is "only" an 86% improvement.
>
>      +    This modification to 'git add' creates some behavior change depending on
>      +    the use of a sparse index. We modify a test in t1092 to demonstrate
>      +    these changes which will be remedied in future changes.
>      +
>           Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
>
>        ## builtin/add.c ##
>      @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'status/add: outsi
>       - # This "git add folder1/a" fails with a warning
>       - # in the sparse repos, differing from the full
>       - # repo. This is intentional.
>      -- test_sparse_match test_must_fail git add folder1/a &&
>      ++ # Adding the path outside of the sparse-checkout cone should fail.
>      +  test_sparse_match test_must_fail git add folder1/a &&
>       - test_sparse_match test_must_fail git add --refresh folder1/a &&
>       - test_all_match git status --porcelain=v2 &&
>      --
>      ++
>      ++ test_must_fail git -C sparse-checkout add --refresh folder1/a 2>sparse-checkout-err &&
>      ++ test_must_fail git -C sparse-index add --refresh folder1/a 2>sparse-index-err &&
>      ++ # NEEDSWORK: A sparse index changes the error message.
>      ++ ! test_cmp sparse-checkout-err sparse-index-err &&
>      ++
>      ++ # NEEDSWORK: Adding a newly-tracked file outside the cone succeeds
>      ++ test_sparse_match git add folder1/new &&
>      +
>         test_all_match git add . &&
>         test_all_match git status --porcelain=v2 &&
>         test_all_match git commit -m folder1/new &&
>      ++ test_all_match git rev-parse HEAD^{tree} &&
>      +
>      +  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_all_match git commit -m folder1/newer &&
>      ++ test_all_match git rev-parse HEAD^{tree}
>      + '
>      +
>      + test_expect_success 'checkout and reset --hard' '
>       @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'sparse-index is not expanded' '
>         git -C sparse-index reset --hard &&
>         ensure_not_expanded checkout rename-out-to-out -- deep/deeper1 &&
>  3:  a577ea4c74d = 3:  2ae91e0af29 pathspec: stop calling ensure_full_index
>  4:  89ec6a7ce67 < -:  ----------- t1092: 'git add --refresh' difference with sparse-index
>  5:  76066a78ce0 ! 4:  a79728d4c64 add: ignore outside the sparse-checkout in refresh()
>      @@ builtin/add.c: static int refresh(int verbose, const struct pathspec *pathspec)
>
>        ## t/t1092-sparse-checkout-compatibility.sh ##
>       @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'status/add: outside sparse cone' '
>      -  test_all_match git commit -m folder1/newer
>      - '
>
>      --test_expect_failure 'add: pathspec within sparse directory' '
>      -+test_expect_success 'add: pathspec within sparse directory' '
>      -  init_repos &&
>      -
>      -  run_on_sparse mkdir folder1 &&
>      -@@ t/t1092-sparse-checkout-compatibility.sh: test_expect_failure 'add: pathspec within sparse directory' '
>      -  # This "git add folder1/a" fails with a warning
>      -  # in the sparse repos, differing from the full
>      -  # repo. This is intentional.
>      -- #
>      -- # However, in the sparse-index, folder1/a does not
>      -- # match any cache entry and fails with a different
>      -- # error message. This needs work.
>      +  # Adding the path outside of the sparse-checkout cone should fail.
>         test_sparse_match test_must_fail git add folder1/a &&
>      -  test_sparse_match test_must_fail git add --refresh folder1/a &&
>      -  test_all_match git status --porcelain=v2
>      +-
>      +- test_must_fail git -C sparse-checkout add --refresh folder1/a 2>sparse-checkout-err &&
>      +- test_must_fail git -C sparse-index add --refresh folder1/a 2>sparse-index-err &&
>      +- # NEEDSWORK: A sparse index changes the error message.
>      +- ! test_cmp sparse-checkout-err sparse-index-err &&
>      ++ test_sparse_match test_must_fail git add --refresh folder1/a &&
>      +
>      +  # NEEDSWORK: Adding a newly-tracked file outside the cone succeeds
>      +  test_sparse_match git add folder1/new &&
>  -:  ----------- > 5:  1543550a4e8 add: remove ensure_full_index() with --renormalize
>
> --
> gitgitgadget