Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix sparse-checkout set crashes #607

Merged
merged 3 commits into from
Sep 20, 2023
Merged

Fix sparse-checkout set crashes #607

merged 3 commits into from
Sep 20, 2023

Conversation

vdye
Copy link

@vdye vdye commented Sep 20, 2023 •
edited
Loading

Thanks for taking the time to contribute to Git!

This fork contains changes specific to monorepo scenarios. If you are an
external contributor, then please detail your reason for submitting to
this fork:

  • This is an early version of work already under review upstream.
  • This change only applies to interactions with Azure DevOps and the
    GVFS Protocol.
  • This change only applies to the virtualization hook and VFS for Git.

This PR corrects a couple of issues in order to fix #606:

  • Commit 1 ensures the dtype arg to path_matches_pattern_list() is always initialized, since the code added in 2cff5e9 requires an initialized value.
  • Commit 2 fixes copying & modification of the index name_hash, dir_hash, and name_hash_initialized in expand_index(). This problem originates upstream (and should eventually be fixed there as well), but it doesn't actually trigger any bugs there; it's the interaction with 2cff5e9 that causes the segfault in our fork.
  • Commit 3 adds a test around the treatment of untracked files & directories when sparse-checkout patterns are updated, moving them from "in cone" to "outside of cone." Again, while this is a good thing to have upstream (I plan to submit it eventually), its real value here is showing that this segfault no longer occurs.

vdye added 3 commits September 20, 2023 12:52
@vdye
fixup! Add virtual file system settings and hook proc
fc03c1b 
The 'dtype' arg to 'path_matches_pattern_list()' is not assumed to be
initialized, and in a few callers it is not. However, the changes in
2cff5e9 (Add virtual file system settings and hook proc, 2018-01-11)
_do_ assume 'dtype' is initialized.

This can cause indeterminate behavior. For example, seemingly unrelated
changes from 33b1b4c (sparse-checkout: avoid using internal API of
unpack-trees, take 2, 2023-02-27) changed the initial value of 'dtype' in
'expand_index()' from non-zero to zero. That caused
'path_matches_pattern_list()' to call 'resolve_dtype()', which initialized
the 'name_hash' and 'dir_hash' of 'istate', which triggered another bug that
ultimately led to a segfault.

Update the callers with uninitialized 'dtype' values to intialize with a
value appropriate to their respective use cases.
@vdye
sparse-index.c: fix use of index hashes in expand_index
433c5e9 
In ac8acb4 (sparse-index: complete partial expansion, 2022-05-23),
'expand_index()' was updated to expand the index to a given pathspec.
However, the 'path_matches_pattern_list()' method used to facilitate this
has the side effect of initializing or updating the index hash variables
('name_hash', 'dir_hash', and 'name_hash_initialized'). This operation is
performed on 'istate', though, not 'full'; as a result, the initialized
hashes are later overwritten when copied from 'full'. To ensure the correct
hashes are in 'istate' after the index expansion, change the arg used in
'path_matches_pattern_list()' from 'istate' to 'full'.

Note that this does not fully solve the problem. If 'istate' does not have
an initialized 'name_hash' when its contents are copied to 'full',
initialized hashes will be copied back into 'istate' but
'name_hash_initialized' will be 0. Therefore, we also need to copy
'full->name_hash_initialized' back to 'istate' after the index expansion is
complete.

Signed-off-by: Victoria Dye <[email protected]>
@vdye
t1092: add test for untracked files and directories
c9f342c 
Add a test verifying that sparse-checkout (with and without sparse index
enabled) treat untracked files & directories correctly when changing sparse
patterns. Specifically, it ensures that 'git sparse-checkout set'

* deletes empty directories outside the sparse cone
* does _not_ delete untracked files outside the sparse cone

Signed-off-by: Victoria Dye <[email protected]>
@vdye vdye requested a review from jeffhostetler September 20, 2023 20:25
@vdye vdye self-assigned this Sep 20, 2023
@vdye vdye marked this pull request as ready for review September 20, 2023 20:30
jeffhostetler
jeffhostetler approved these changes Sep 20, 2023
View reviewed changes
Copy link

@jeffhostetler jeffhostetler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice!

Good find on the very obscure dtype init.

mjcheetham
mjcheetham approved these changes Sep 20, 2023
View reviewed changes
Copy link
Member

@mjcheetham mjcheetham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

@vdye vdye merged commit 00315c6 into microsoft:vfs-2.42.0 Sep 20, 2023
43 of 44 checks passed
@vdye vdye deleted the vdye/sparse-checkout-crash-fix branch September 20, 2023 21:21
@vdye vdye mentioned this pull request Sep 20, 2023
Closed
6 tasks
@aaronberger-msft
Copy link

Thank you guys so much! Awesome work and super fast response! Kudos!

dscho pushed a commit that referenced this pull request Nov 3, 2023
@vdye @dscho
Fix sparse-checkout set crashes (#607)
7cce1ce
dscho pushed a commit that referenced this pull request Nov 3, 2023
@vdye @dscho
Fix sparse-checkout set crashes (#607)
cebeb92
dscho pushed a commit that referenced this pull request Nov 3, 2023
@vdye @dscho
Fix sparse-checkout set crashes (#607)
c078dd3
dscho pushed a commit that referenced this pull request Nov 3, 2023
@vdye @dscho
Fix sparse-checkout set crashes (#607)
52ccfa1
dscho pushed a commit that referenced this pull request Nov 8, 2023
@vdye @dscho
Fix sparse-checkout set crashes (#607)
318c149
dscho pushed a commit that referenced this pull request Nov 14, 2023
@vdye @dscho
Fix sparse-checkout set crashes (#607)
bda1816
dscho pushed a commit that referenced this pull request Nov 20, 2023
@vdye @dscho
Fix sparse-checkout set crashes (#607)
7a3b441
vdye added a commit that referenced this pull request Feb 27, 2024
@vdye
Fix sparse-checkout set crashes (#607)
25699f0
dscho pushed a commit that referenced this pull request Apr 23, 2024
@vdye @dscho
Fix sparse-checkout set crashes (#607)
058abbb
dscho pushed a commit that referenced this pull request Apr 23, 2024
@vdye @dscho
Fix sparse-checkout set crashes (#607)
57e7d5b
dscho pushed a commit that referenced this pull request Apr 23, 2024
@vdye @dscho
Fix sparse-checkout set crashes (#607)
7f4bb8f
dscho pushed a commit that referenced this pull request Apr 23, 2024
@vdye @dscho
Fix sparse-checkout set crashes (#607)
6afd40a
dscho pushed a commit that referenced this pull request Apr 24, 2024
@vdye @dscho
Fix sparse-checkout set crashes (#607)
4824feb
dscho pushed a commit that referenced this pull request Apr 29, 2024
@vdye @dscho
Fix sparse-checkout set crashes (#607)
b68812e
dscho pushed a commit that referenced this pull request May 14, 2024
@vdye @dscho
Fix sparse-checkout set crashes (#607)
ba5346d
dscho pushed a commit that referenced this pull request May 14, 2024
@vdye @dscho
Fix sparse-checkout set crashes (#607)
a7e44ae
dscho pushed a commit that referenced this pull request May 14, 2024
@vdye @dscho
Fix sparse-checkout set crashes (#607)
6f018f2
dscho pushed a commit that referenced this pull request Jun 3, 2024
@vdye @dscho
Fix sparse-checkout set crashes (#607)
3e40279
dscho pushed a commit that referenced this pull request Jun 3, 2024
@vdye @dscho
Fix sparse-checkout set crashes (#607)
4b84f3c
dscho pushed a commit that referenced this pull request Jun 3, 2024
@vdye @dscho
Fix sparse-checkout set crashes (#607)
bd8c507
dscho pushed a commit that referenced this pull request Jul 17, 2024
@vdye @dscho
Fix sparse-checkout set crashes (#607)
215c831
dscho pushed a commit that referenced this pull request Jul 17, 2024
@vdye @dscho
Fix sparse-checkout set crashes (#607)
d43829c
dscho pushed a commit that referenced this pull request Jul 17, 2024
@vdye @dscho
Fix sparse-checkout set crashes (#607)
f02d261
dscho pushed a commit that referenced this pull request Jul 18, 2024
@vdye @dscho
Fix sparse-checkout set crashes (#607)
349314f
mjcheetham pushed a commit that referenced this pull request Jul 23, 2024
@vdye @mjcheetham
Fix sparse-checkout set crashes (#607)
f17b6ca
dscho pushed a commit that referenced this pull request Jul 25, 2024
@vdye @dscho
Fix sparse-checkout set crashes (#607)
16d1ba7
mjcheetham pushed a commit that referenced this pull request Jul 29, 2024
@vdye @mjcheetham
Fix sparse-checkout set crashes (#607)
bf80997
dscho pushed a commit that referenced this pull request Sep 18, 2024
@vdye @dscho
Fix sparse-checkout set crashes (#607)
2319618
dscho pushed a commit that referenced this pull request Sep 24, 2024
@vdye @dscho
Fix sparse-checkout set crashes (#607)
b01b9c0
dscho pushed a commit that referenced this pull request Oct 8, 2024
@vdye @dscho
Fix sparse-checkout set crashes (#607)
ce904bd
mjcheetham pushed a commit that referenced this pull request Dec 3, 2024
@vdye @mjcheetham
Fix sparse-checkout set crashes (#607)
c933f74
dscho pushed a commit that referenced this pull request Dec 17, 2024
@vdye @dscho
Fix sparse-checkout set crashes (#607)
5b78a90
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeffhostetler jeffhostetler jeffhostetler approved these changes

@mjcheetham mjcheetham mjcheetham approved these changes

Assignees

@vdye vdye

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Crashing during sparse-checkout operations
4 participants
@vdye @aaronberger-msft @mjcheetham @jeffhostetler