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.

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

Remove dead GVFSLock and GitStatusCache code #29

Merged

Conversation

wilbaker
Copy link
Member

@wilbaker wilbaker commented Aug 8, 2019

This code is not needed for GSD

Copy link
Contributor

@derrickstolee derrickstolee left a comment

Choose a reason for hiding this comment

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

Maybe @jamill can chime in here, but the background status cache will not work at first for sure. We will see about enabling it later, but it may require a significant redesign and hence it makes sense to delete now.

@wilbaker
Copy link
Member Author

wilbaker commented Aug 8, 2019

Maybe @jamill can chime in here, but the background status cache will not work at first for sure. We will see about enabling it later, but it may require a significant redesign and hence it makes sense to delete now.

@derrickstolee @jamill yeah I was a bit on the fence with the status cache. In its current form it's refreshes are driven by events we don't have from the virtualization layer, and it answers requests from the pre-command hook which we don't have either.

My thought was that it would be easier to surgically lift whatever we end up wanting/needing from VFS4G at a later time than leaving the current code in place here.

My original plan was to leave the current code in, but it is so tightly coupled to the lock and virtualization code there was not much left after I remove those portions.

@derrickstolee
Copy link
Contributor

Ship it! I completely agree with your assessment.

@wilbaker wilbaker merged commit 6d26c81 into microsoft:master Aug 8, 2019
chrisd8088 added a commit to chrisd8088/scalar that referenced this pull request Aug 22, 2020
In PR microsoft#29 and commit cedeeaa the
VFSForGit GitStatusCache implementation was removed from Scalar.

However, the functional test suite still checked for and created
the EnableGitStatusCacheToken.dat file if it was missing (but only
on Windows).  Since this setup is no longer relevant, we can remove
it now.
chrisd8088 added a commit to chrisd8088/scalar that referenced this pull request Sep 4, 2020
In PR microsoft#29 and commit cedeeaa the
VFSForGit GitStatusCache implementation was removed from Scalar.

However, the functional test suite still checked for and created
the EnableGitStatusCacheToken.dat file if it was missing (but only
on Windows).  Since this setup is no longer relevant, we can remove
it now.
chrisd8088 added a commit to chrisd8088/scalar that referenced this pull request Oct 11, 2020
The last caller of the IsProcessActive() methods in the
ScalarPlatform classes were removed in commit
cedeeaa in PR microsoft#29.

Therefore we can remove this method as well as the
native Windows ProcessActiveFlags and several Win32
system call wrappers.
chrisd8088 added a commit to chrisd8088/scalar that referenced this pull request Oct 11, 2020
The last caller of the IsProcessActive() methods in the
ScalarPlatform classes was removed in commit
cedeeaa in PR microsoft#29.

Therefore we can remove this method as well as the native
Windows system call wrappers and enums which were used only
by the WindowPlatform implementation of this method.
chrisd8088 added a commit to chrisd8088/scalar that referenced this pull request Oct 11, 2020
The last caller of the IsProcessActive() methods in the
ScalarPlatform classes was removed in commit
cedeeaa in PR microsoft#29.

Therefore we can remove this method as well as the native
Windows system call wrappers and enums which were used only
by the WindowPlatform implementation of this method.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants