utils: fix diff so subpaths work for sparse checkouts, fixes 1455 #1484
Conversation
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
onee-only
reviewed
Apr 2, 2025
c183fb2 to
96914bf
Compare
|
@onee-only all tests pass. I re-based all the commits into a single one which I believe adheres to the comment format. Please LMK if you need anything else. Otherwise if you can r+ and merge it'd be greatly appreciated! :) |
onee-only
approved these changes
Apr 4, 2025
There was a problem hiding this comment.
@patricsss LGTM. But I don't have any permission to merge this PR. 😅
Let's wait for maintainers to review it.
My mistake haha. I assumed you were one. 😂 |
onee-only
added a commit
to onee-only/go-git
that referenced
this pull request
Apr 7, 2025
pjbgf
approved these changes
Apr 13, 2025
|
@patricsss thank you for working on this. 👍 @onee-only thank you for the initial reviews. 🙇 |
kane8n
pushed a commit
to kane8n/go-git
that referenced
this pull request
Jun 6, 2025
utils: fix diff so subpaths work for sparse checkouts, fixes 1455
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.
Summary
Fix for #1455
When attempting to check out anything that isn't a root directory the Sparse Checkout options do not work correctly. This is reproducible with the examples in the above issue.
Problem
"virtual" nodes are created in the tree that represent directories. The first time a file is added to the tree it's parts are split and any missing nodes (including any directories) are added to the tree.
For example, if the first added file is
foo/bar/file.yamland this file should be skipped, entries are created forfoo,foo/bar, andfoo/bar/file.yaml, with all three entries settingSkipWorktreetotrue. Iffoo/baz/file.yamlis subsequently added, but should not be skipped, this is the current bug.fooalready exists in the tree, and hadSkipWorktreeset totrue. Even thoughfoo/bazandfoo/baz/file.yamlwill have skip to false, they will never be included in the diff because their parentfoowas already marked as skipped.Fix
This change makes it so that any existing parents are re-checked and have their
SkipWorktreeboolean reset when any child of that subtree hasSkipWorktreeset to false.The final issue is in the
addRecursive()method. It doesn't take into consideration theSkipWorktreeflag via theSkip()interface. It now checks this condition and does not add any subtrees which were marked asSkip() == trueTesting
I tested the changes with the reproduction case provided as well as through a simple unit test that attempts to highlight the issue and solution.
Open to any suggestions.