-
Notifications
You must be signed in to change notification settings - Fork 869
utils: fix diff so subpaths work for sparse checkouts, fixes 1455 #1484
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
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
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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. 😂 |
|
@patricsss thank you for working on this. 👍 @onee-only thank you for the initial reviews. 🙇 |
utils: fix diff so subpaths work for sparse checkouts, fixes 1455
utils: fix diff so subpaths work for sparse checkouts, fixes 1455
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.