[Release] Milestone M155#1385
Merged
Merged
Conversation
Initial implementation of additional fields on the FileSystemCallbacks heartbeat to cover folder placeholder creation and file hydration. Includes plumbing changes to route the image path name through for folder placeholders and file hydration.
Instead of querying interop services at runtime (with a dll that is not included with the windows install), determine this at compile time using the under construction flags.
Add new helper to 'swap' the ConcurrentDictionary, move away from managing EventLevel by passing a ref to passing an out bool. Rename methods for clarity that were brought up in code review
Upgrader - platform specific directory creation
This change moves the vnode eligibility check (file system type, vnode type) from ShouldHandleVnodeOpEvent to HandleVnodeOperation to avoid repeated calls. A unit test has been added to provide test coverage for the new code branch.
This adds an enum scoped in a namespace to be used instead of magic numbers in version checks. The reasons for choosing a classic enum in a namespace are: * The namespace gives us explicit scoping rather than polluting the root namespace. * The using a classic enum vs a modern C++ enum class means the values are implicitly convertible to integers, which is how we actually use them.
Replaces a use of the old mock vnode creation function with the new tree and mountpoint-based one.
This change utilises the KAUTH_FILEOP_WILL_RENAME event which was introduced with Mojave/10.14 to determine if a KAUTH_VNODE_DELETE authorisation check is due to a rename() operation. In this case, the file or directory genuinely needs hydrating, otherwise hydration can be skipped. This change also adds unit tests for the new rename operation tracking code, and adapts existing unit tests so hydration is not expected when a KAUTH_VNODE_DELETE check occurs outside of a rename() operation. Note that this change means the behaviour on macOS 10.13 and earlier versus 10.14 and later subtly diverges.
Includes a change to the post-indexchanged hook (now the post-index-change hook). Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
…nd file hydration Update heartbeats to contain folder placeholder and file hydration metadata
Display about box with VFSForGit and Git version info. - Fetch Version info by running gvfs & git commands - Added UT (updated tests cases added required mocks) - Renamed GVFSTask to ProductInfoFetcher - New ProcessHelper wrapper class around NSTask - Separate xcodebuild test & build steps for native app. This helps prevent XCTest frameworks from getting injected into deployment binaries - Use nullability decorators - VFSForGit classes are prefixed with "VFS". - Use default Xcode code indentations
- Quote the directory being deleted
Mac: Bash Functional Test Hang
Added component-plist file for "VFS For Git" package. This is an xml file that contains specifications for how installer should handle bundles in VFSForGit. BundleIsRelocatable is set to true, for "VFS For Git.app". This stops the installer from searching the disc for a copy of the app, which it then upgrades. Now the installer will always fresh-install (or upgrade) "/Library/Application\ Supp ort/VFS For Git/VFS For Git.app" only. Also updated CreateInstaller script to use the component-plist file.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Add logging to clone/mount paths
Currently, this is hardcoded to run the "where" program, which is not appropriate on non-Windows platforms. Add a platform abstraction so different platforms can run different programs.
Different platforms have different requirements for the list of processes that block installation. The POSIX platforms also remove "gvfs" from the list of blocked processes, as they can run upgrade while the 'gvfs upgrade' command is running. We might want to come back and restrict this to only the `gvfs upgrade` command (or only if the running command is a parent process) in a future change.
The script would fail to properly parse Git Version strings that included multiple hyphens, which broke Git Version strings that were generated by "draft" pipelines, which had the form: 1.2.3-DRAFT-pr as it would not include any text after the second hyphen.
Remove all TODO(POSIX) and TODO(Mac) comments.
Mac build scripts: consume git installer pkg directly
As specified in the documentation [1], the PriorityClass setter will throw an InvalidOperationException if the process ID is not available. This could happen due to a race condition where the process completes before we can set its priority. The other exceptions it can throw are either platform-specific or signal a bug, so do not be overly aggressive in the 'catch'. [1] https://docs.microsoft.com/en-us/dotnet/api/system.diagnostics.process.priorityclass?view=netframework-4.8 Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
If we fail to communicate with the mount process, then 'gvfs status' gives us the information we want, not 'git status'. Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Use a new environment variable, GVFS_UPGRADE_DETERMINISTIC, to always print the "upgrade available" message, if it is present. This should fix test flakiness and issue #852. Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Includes updates from microsoft/git#164. Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
… when setting priority class As specified in the documentation [1], the PriorityClass setter will throw an InvalidOperationException if the process ID is not available. This could happen due to a race condition where the process completes before we can set its priority. The other exceptions it can throw are either platform-specific or signal a bug, so do not be overly aggressive in the 'catch'. [1] https://docs.microsoft.com/en-us/dotnet/api/system.diagnostics.process.priorityclass?view=netframework-4.8
…reminders Should fix #852. The fix is to use an environment variable to always show the upgrade message, if present. While looking at the post-command hook code, I discovered some simple cleanups that are isolated to different commits: * Removed unreachable code. * Changed a message to refer to `gvfs status` instead of `git status`.
Includes updates from microsoft/git#164.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Member
|
/azp run GitHub VFSForGit Large Repo Build |
|
No pipelines are associated with this pull request. |
Contributor
Author
|
Large Repo Perf run succeeded with expected results. All difference was within error bounds. |
Initial cut of GVFS Health Feature for the M155 milestone
derrickstolee
commented
Jul 30, 2019
Contributor
Author
derrickstolee
left a comment
derrickstolee
left a comment
There was a problem hiding this comment.
I looked through the changes and couldn't find anything alarming. Can't approve my own PR, though. 🤷♂
jamill
approved these changes
Jul 30, 2019
Member
jamill
left a comment
jamill
left a comment
There was a problem hiding this comment.
I did a quick spot check of a couple of things, and it looks as expected to me :) Thanks!
derrickstolee
requested review from
Halterer,
jeschu1,
kewillford,
mjcheetham and
wilbaker
kewillford
approved these changes
Jul 31, 2019
Member
kewillford
left a comment
kewillford
left a comment
There was a problem hiding this comment.
This looks good from a quick run through.
wilbaker
reviewed
Jul 31, 2019
| return false; | ||
| } | ||
|
|
||
| this.FileSystem.CreateDirectory(upgradeApplicationDirectory); |
Member
There was a problem hiding this comment.
@alameenshah, @jamill should we be calling TryCreateDirectory here instead (or catching exceptions that CreateDirectory might throw)?
As best I can tell the caller of TryPrepareApplicationDirectory does not expect it to throw.
wilbaker
reviewed
Jul 31, 2019
| return false; | ||
| } | ||
|
|
||
| this.FileSystem.CreateDirectory(directory); |
Member
There was a problem hiding this comment.
@alameenshah @jamill same comment here.
I think we don't have to hold up the release for this, but if you agree can you file a work item or create a PR into master to update this code?
wilbaker
approved these changes
Jul 31, 2019
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
12 participants
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.
Mac
Cross-Platform
Git
Upgrade
FastFetch
Polish
Backports from M153