Upgrader for macOS MVP#1129
Conversation
jamill
force-pushed
the
upgrader_mac
branch
3 times, most recently
from
fdcb25b to
25cfb8e
Compare
wilbaker
left a comment
wilbaker
left a comment
There was a problem hiding this comment.
Just finished a first pass over the changes and left some comments/questions
|
|
||
| public abstract HashSet<string> UpgradeBlockingProcesses { get; } | ||
|
|
||
| public abstract bool SupportsInlineUpgrade { get; } |
There was a problem hiding this comment.
Minor style suggestion: If SupportsInlineUpgrade is true on Mac, is it because the file system allows replacing files that are currently in use?
If so, IPlatformFileSystem might be a better place for this (renamed to something like SupportsReplacmentOfInUseFiles).
(On the surface I think this property looks similar to SupportsFileMode which lives in IPlatformFileSystem)
| try | ||
| { | ||
| string platformKey = InstallManifest.WindowsPlatformKey; | ||
| string platformKey = "macOS"; |
There was a problem hiding this comment.
Move platformKey to GVFSPlatform?
(I am guessing the code is this way now just to get things working on the Mac)
| this.ReportInfoToConsole($"{Environment.NewLine}Installer launched in a new window. Do not run any git or gvfs commands until the installer has completed."); | ||
| if (supportsInlineUpgrade) | ||
| { | ||
| while (!this.processLauncher.HasExited) |
There was a problem hiding this comment.
Could we use Process.WaitForExit here instead of loop+sleep?
| Exception exception; | ||
| string args = string.Empty + (this.DryRun ? $" {DryRunOption}" : string.Empty) + (this.NoVerify ? $" {NoVerifyOption}" : string.Empty); | ||
| if (!this.processLauncher.TryStart(path, args, out exception)) | ||
| if (!this.processLauncher.TryStart(path, args, !runUpgradeInline, out exception)) |
There was a problem hiding this comment.
How is runUpgradeInline related to useShellExecute?
jamill
force-pushed
the
upgrader_mac
branch
3 times, most recently
from
24a4ada to
341d60b
Compare
jamill
force-pushed
the
upgrader_mac
branch
3 times, most recently
from
3c5e356 to
5a9cb30
Compare
Different platform have different structures for where the upgrade application and logs need to go. This change introduces the architecture for making this platform specific.
The config file location currently exists in the same directory as the GVFS service data directory. On Mac, the GVFS Service directory is per user, and not per machine. Even if the service is per user on Mac, we still would like the config to be per machine. This change enables the GVFS config file to exist at a location independent of the GVFS Service data directory. On Windows, the location will not change, but on macOS, it will now exist in the GVFS binary location.
…ks/remaining' into upgrader_mac
|
The individual PRs for an initial MVP have now all been merged in. I will close this PR as this work is complete. I will file a separate issue to track the remaining open issues. |
2 participants
This issue is to track the work for building the upgrade functionality for macOS. It will also contain a proof of concept for the end-to-end functionality. As the PRs for individual tasks are ready, I will link them here.
Tasks
Deferred / Followup Tasks
CopyDirectoryRecursive