Skip to content

reset/checkout: fix miscellaneous sparse index bugs

Victoria Dye via GitGitGadget git 3 versions – In review

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v3 Latest4 patches 2 repliesIn review
  2. v24 patches 0 repliesSuperseded
  3. v14 patches 6 repliesSuperseded

Changes in v3 from the cover letter

================

* Adjusted 'reset hard with removed sparse dir' test in 't1092-sparse-checkout-compatibility.sh' to avoid 'git rm' log message conflicts with [1]

Patches in v3 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/4checkout: fix nested sparse directory diff in sparse indexbuiltin/checkout.c t/t1092-sparse-checkout-compatibility.sh+9 -00 replies
2/4oneway_diff: handle removed sparse directoriesdiff-lib.c+5 -00 replies
3/4cache.h: create 'index_name_pos_sparse()'cache.h read-cache.c+14 -00 replies
4/4unpack-trees: unpack new trees as sparse directoriest/t1092-sparse-checkout-compatibility.sh unpack-trees.c+113 -100 replies

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v3 yet.

Read the v3 thread

Cover letter [PATCH v3 0/4] reset/checkout: fix miscellaneous sparse index bugs

While working on sparse index integration for 'git rm' [1], Shaoxuan found that removed sparse directories, when reset, would no longer be sparse. This was due to how 'unpack_trees()' determined whether a traversed directory was a sparse directory or not; it would only unpack an entry as a sparse directory if it existed in the index. However, if the sparse directory was removed, it would be treated like a non-sparse directory and its contents would be individually unpacked.

To avoid this unnecessary traversal and keep the results of 'reset' as sparse as possible, the decision logic for whether a directory is sparse is changed to:

* If the directory is a sparse directory in the index, unpack it.
* If not, is the directory inside the sparse cone? If so, do not unpack it.
* If the directory is outside the sparse cone, does it have any child entries in the index? If so, do not unpack it.
* Otherwise, unpack the entry as a sparse directory.

In the process of updating 'reset', a separate issue was found in 'checkout' where collapsed sparse directories did not have modified contents reported file-by-file. A similar bug was found with 'status' in 2c521b0e49 (status:
fix nested sparse directory diff in sparse index, 2022-03-01), and 'checkout' was corrected the same way (setting the diff flag 'recursive' to 1).

Changes since V2
================

* Adjusted 'reset hard with removed sparse dir' test in 't1092-sparse-checkout-compatibility.sh' to avoid 'git rm' log message conflicts with [1]

Changes since V1
================

* Reverted the removal of 'index_entry_exists()' to avoid breaking other in-flight series.
* Renamed 'is_missing_sparse_dir()' to 'is_new_sparse_dir()'; revised comments and commit messages to clarify what that function is doing and why.
* Handled "unexpected" inputs to 'is_new_sparse_dir()' more gently, returning 0 if 'p' is not a directory or the directory already exists in the index (rather than exiting with 'BUG()'). This is intended to make 'is_new_sparse_dir()' less reliant on information about the index established by 'unpack_callback()' & 'unpack_single_entry()', resulting in easier-to-read and more reusable code.

Thanks!

* Victoria

[1]
https://lore.kernel.org/git/[email protected]/

Victoria Dye (4):
checkout: fix nested sparse directory diff in sparse index
oneway_diff: handle removed sparse directories cache.h: create 'index_name_pos_sparse()'
unpack-trees: unpack new trees as sparse directories

 builtin/checkout.c                       |   1 +
 cache.h                                  |   9 ++
 diff-lib.c                               |   5 ++
 read-cache.c                             |   5 ++
 t/t1092-sparse-checkout-compatibility.sh |  25 ++++++
 unpack-trees.c                           | 106 ++++++++++++++++++++---
 6 files changed, 141 insertions(+), 10 deletions(-)

base-commit: 4af7188bc97f70277d0f10d56d5373022b1fa385
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1312%2Fvdye%2Freset%2Fhandle-missing-dirs-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1312/vdye/reset/handle-missing-dirs-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1312

Range-diff vs v2:

 1:  255318f4dc6 = 1:  255318f4dc6 checkout: fix nested sparse directory diff in sparse index
 2:  55c77ba4b29 = 2:  55c77ba4b29 oneway_diff: handle removed sparse directories
 3:  d0bdec63286 = 3:  d0bdec63286 cache.h: create 'index_name_pos_sparse()'
 4:  97ca668102c ! 4:  010d5e51595 unpack-trees: unpack new trees as sparse directories
     @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'reset with wildca
      +test_expect_success 'reset hard with removed sparse dir' '
      +	init_repos &&
      +
     -+	test_all_match git rm -r --sparse folder1 &&
     ++	run_on_all git rm -r --sparse folder1 &&
      +	test_all_match git status --porcelain=v2 &&
      +
      +	test_all_match git reset --hard &&
Signature · 1 line
gitgitgadget