restore: better integrate with sparse index#2121
Draft
derrickstolee wants to merge 2 commits into
Draft
Conversation
A user reported that 'git restore --staged .' causes the sparse index to expand. This is somewhat natural because the '.' pathspec means 'check every path'. However, the restore will not update paths marked with the SKIP_WORKTREE bit, so we shouldn't need to process such entries. For now, establish the current behavior, including the sparse index expansion, in the t1092 test case as a baseline. Signed-off-by: Derrick Stolee <stolee@gmail.com>
Teach update_some() to handle sparse directory entries at the tree level rather than expanding the entire sparse index. When iterating a source tree during checkout/restore operations: - If a directory matches a sparse directory entry with the same OID, skip it entirely (no change needed). - If the OID differs and we are in non-overlay mode (e.g., restore --staged), update the sparse directory entry's OID in place. This is semantically correct because non-overlay mode removes paths not in the source tree anyway. - In overlay mode (e.g., checkout <tree> -- .), fall through to recursive descent so individual file entries are preserved correctly. Also switch from index_name_pos() to index_name_pos_sparse() for individual file lookups to avoid triggering ensure_full_index() when the file is already individually tracked in the index. Update the test expectation in t1092 to assert that 'restore --staged' no longer expands the sparse index. Signed-off-by: Derrick Stolee <stolee@gmail.com>
2e5932a to
47542cb
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There's still a long tail of situations where Git expands a sparse index in-memory in order to operate on blob path entries instead of intelligently handling trees. I was recently alerted to one such case with
git restore --staged -- ..The basic idea here is that the pathspec
.signals that all paths matter, but what we want to do across those pathspecs will ignore the expanded blob paths with the SKIP_WORKTREE bit, so we should avoid expanding the tree when we can.This series has two patches: first a test to demonstrate the baseline behavior of
git restoreacross different sparsity cases as well as demonstrate that the index is currently expanded. The second patch includes the fix and maintains the same end-to-end behavior with the only change being the performance improvement from not expanding the sparse index.Thanks,
-Stolee
cc: gitster@pobox.com