diff --git a/AuthoringTests.md b/AuthoringTests.md index f67933a87d..ebe27cd201 100644 --- a/AuthoringTests.md +++ b/AuthoringTests.md @@ -92,8 +92,7 @@ Mac: ## How to Write a Functional Test -Each piece of functionality that we add to Scalar should have corresponding functional tests that clone a repo, mount the filesystem, and use existing tools and filesystem -APIs to interact with the virtual repo. +Each piece of functionality that we add to Scalar should have corresponding functional tests that clone a repo and use existing tools and filesystem APIs to interact with the virtual repo. Since these are functional tests that can potentially modify the state of files on disk, you need to be careful to make sure each test can run in a clean environment. There are three base classes that you can derive from when writing your tests. It's also important to put your new class into the same namespace @@ -101,14 +100,11 @@ as the base class, because NUnit treats namespaces like test suites, and we have 1. `TestsWithLongRunningEnlistment` - Before any test in this namespace is executed, we create a single enlistment and mount Scalar. We then run all tests in this namespace that derive - from this base class. Only put tests in here that are purely readonly and will leave the repo in a good state for future tests. + Before any test in this namespace is executed, we create a single enlistment. We then run all tests in this namespace that derive from this base class. Only put tests in here that are purely readonly and will leave the repo in a good state for future tests. 2. `TestsWithEnlistmentPerFixture` - For any test fixture (a fixture is the same as a class in NUnit) that derives from this class, we create an enlistment and mount Scalar before running - any of the tests in the fixture, and then we unmount and delete the enlistment after all tests are done (but before any other fixture runs). If you need - to write a sequence of tests that manipulate the same repo, this is the right base class. + For any test fixture (a fixture is the same as a class in NUnit) that derives from this class, we create an enlistment before running any of the tests in the fixture, and then we delete the enlistment after all tests are done (but before any other fixture runs). If you need to write a sequence of tests that manipulate the same repo, this is the right base class. 3. `TestsWithEnlistmentPerTestCase` diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 32f5c2b183..ed30687303 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -2,19 +2,18 @@ Thank you for taking the time to contribute! -## Guidelines - -* [Contributor License Agreement](#contributor-license-agreement) -* [Code of Conduct](#code-of-conduct) -* [Building Scalar on Windows](#building-scalar-on-windows) -* [Building Scalar on Mac](#building-scalar-on-mac) -* [Design Reviews](#design-reviews) -* [Platform Specific Code](#platform-specific-code) -* [Tracing and Logging](#tracing-and-logging) -* [Error Handling](#error-handling) -* [Background Threads](#background-threads) -* [Coding Conventions](#coding-conventions) -* [Testing](#testing) + - [Contributor License Agreement](#contributor-license-agreement) + - [Code of Conduct](#code-of-conduct) + - [Building Scalar on Windows](#building-scalar-on-windows) + - [Building Scalar on Mac](#building-scalar-on-mac) + - [Design Reviews](#design-reviews) + - [Platform Specific Code](#platform-specific-code) + - [Tracing and Logging](#tracing-and-logging) + - [Error Handling](#error-handling) + - [Background Threads](#background-threads) + - [Coding Conventions](#coding-conventions) + - [Testing](#testing) + - [C# Unit Tests](#c-unit-tests) ## Contributor License Agreement @@ -115,11 +114,11 @@ The design review process is as follows: catch (Exception e) { EventMetadata metadata = new EventMetadata(); - metadata.Add("Area", "Mount"); - metadata.Add(nameof(enlistmentHookPath), enlistmentHookPath); - metadata.Add(nameof(installedHookPath), installedHookPath); + metadata.Add("Area", "Upgrade"); + metadata.Add(nameof(packageVersion), packageVersion); + metadata.Add(nameof(packageName), packageName); metadata.Add("Exception", e.ToString()); - context.Tracer.RelatedError(metadata, $"Failed to compare {hook.Name} version"); + context.Tracer.RelatedError(metadata, $"Failed to compare {packageName} version"); } ``` @@ -127,7 +126,7 @@ The design review process is as follows: - *Fail fast: An error or exception that risks data loss or corruption should shut down Scalar immediately* - Preventing data loss and repository corruption is critical. If an error or exception occurs that could lead to data loss, it's better to shut down Scalar than keep the repository mounted and risk corruption. + Preventing data loss and repository corruption is critical. If an error or exception occurs that could lead to data loss, it's better to shut down Scalar than risk corruption. - *Do not catch exceptions that are indicative of a programmer error (e.g. `ArgumentNullException`)* @@ -222,7 +221,7 @@ The design review process is as follows: - *Catch all exceptions on long-running tasks and background threads* - Wrap all code that runs in the background thread in a top-level `try/catch(Exception)`. Any exceptions caught by this handler should be logged, and then Scalar should be forced to terminate with `Environment.Exit`. It's not safe to allow Scalar to continue to run after an unhandled exception stops a background thread or long-running task. Testing has shown that `Environment.Exit` consistently terminates the Scalar mount process regardless of how background threads are started (e.g. native thread, `new Thread()`, `Task.Factory.StartNew()`). + Wrap all code that runs in the background thread in a top-level `try/catch(Exception)`. Any exceptions caught by this handler should be logged, and then Scalar should be forced to terminate with `Environment.Exit`. It's not safe to allow Scalar to continue to run after an unhandled exception stops a background thread or long-running task. Testing has shown that `Environment.Exit` consistently terminates the process regardless of how background threads are started (e.g. native thread, `new Thread()`, `Task.Factory.StartNew()`). An example of this pattern can be seen in [`BackgroundFileSystemTaskRunner.ProcessBackgroundTasks`](https://github.com/Microsoft/Scalar/blob/4baa37df6bde2c9a9e1917fc7ce5debd653777c0/Scalar/Scalar.Virtualization/Background/BackgroundFileSystemTaskRunner.cs#L233). diff --git a/Scalar.Common/GitHubUpgrader.cs b/Scalar.Common/GitHubUpgrader.cs index f08400123f..5901e15cb0 100644 --- a/Scalar.Common/GitHubUpgrader.cs +++ b/Scalar.Common/GitHubUpgrader.cs @@ -19,7 +19,7 @@ public class GitHubUpgrader : ProductUpgrader private const string JSONMediaType = @"application/vnd.github.v3+json"; private const string UserAgent = @"Scalar_Auto_Upgrader"; private const string CommonInstallerArgs = "/VERYSILENT /CLOSEAPPLICATIONS /SUPPRESSMSGBOXES /NORESTART"; - private const string ScalarInstallerArgs = CommonInstallerArgs + " /REMOUNTREPOS=false"; + private const string ScalarInstallerArgs = CommonInstallerArgs; private const string GitInstallerArgs = CommonInstallerArgs + " /ALLOWDOWNGRADE=1"; private const string GitAssetId = "Git"; private const string ScalarAssetId = "Scalar"; diff --git a/Scalar.Common/InstallerPreRunChecker.cs b/Scalar.Common/InstallerPreRunChecker.cs index 293437b4e5..9f490980cb 100644 --- a/Scalar.Common/InstallerPreRunChecker.cs +++ b/Scalar.Common/InstallerPreRunChecker.cs @@ -44,32 +44,6 @@ public virtual bool TryRunPreUpgradeChecks(out string consoleError) return true; } - // TODO: Move repo mount calls to Scalar.Upgrader project. - // https://github.com/Microsoft/Scalar/issues/293 - public virtual bool TryMountAllScalarRepos(out string consoleError) - { - return this.TryRunScalarWithArgs("service --mount-all", out consoleError); - } - - public virtual bool TryUnmountAllScalarRepos(out string consoleError) - { - consoleError = null; - this.tracer.RelatedInfo("Unmounting any mounted Scalar repositories."); - - using (ITracer activity = this.tracer.StartActivity(nameof(this.TryUnmountAllScalarRepos), EventLevel.Informational)) - { - if (!this.TryRunScalarWithArgs("service --unmount-all", out consoleError)) - { - this.tracer.RelatedError($"{nameof(this.TryUnmountAllScalarRepos)}: {consoleError}"); - return false; - } - - activity.RelatedInfo("Successfully unmounted repositories."); - } - - return true; - } - public virtual bool IsInstallationBlockedByRunningProcess(out string consoleError) { consoleError = null; diff --git a/Scalar.Common/Maintenance/GitMaintenanceStep.cs b/Scalar.Common/Maintenance/GitMaintenanceStep.cs index e0a107345b..2c1c20706c 100644 --- a/Scalar.Common/Maintenance/GitMaintenanceStep.cs +++ b/Scalar.Common/Maintenance/GitMaintenanceStep.cs @@ -30,25 +30,25 @@ public GitMaintenanceStep(ScalarContext context, bool requireObjectCacheLock, Gi protected bool RequireObjectCacheLock { get; } protected GitProcessChecker GitProcessChecker { get; } - public static bool EnlistmentRootReady(ScalarContext context) - { - // If a user locks their drive or disconnects an external drive while the mount process - // is running, then it will appear as if the directories below do not exist or throw - // a "Device is not ready" error. - try - { - return context.FileSystem.DirectoryExists(context.Enlistment.EnlistmentRoot) - && context.FileSystem.DirectoryExists(context.Enlistment.GitObjectsRoot); - } - catch (IOException) - { - return false; - } + public static bool EnlistmentRootReady(ScalarContext context) + { + // If a user locks their drive or disconnects an external drive while the process + // is running, then it will appear as if the directories below do not exist or throw + // a "Device is not ready" error. + try + { + return context.FileSystem.DirectoryExists(context.Enlistment.EnlistmentRoot) + && context.FileSystem.DirectoryExists(context.Enlistment.GitObjectsRoot); + } + catch (IOException) + { + return false; + } } - public bool EnlistmentRootReady() - { - return EnlistmentRootReady(this.Context); + public bool EnlistmentRootReady() + { + return EnlistmentRootReady(this.Context); } public void Execute() @@ -85,20 +85,20 @@ public void Execute() } catch (Exception e) { - if (this.EnlistmentRootReady()) - { - this.Context.Tracer.RelatedError( - metadata: this.CreateEventMetadata(e), - message: "Exception while running action: " + e.Message, - keywords: Keywords.Telemetry); - } - else - { - this.Context.Tracer.RelatedWarning( - metadata: this.CreateEventMetadata(e), - message: "Exception while running action inside a repo that's not ready: " + e.Message); - } - + if (this.EnlistmentRootReady()) + { + this.Context.Tracer.RelatedError( + metadata: this.CreateEventMetadata(e), + message: "Exception while running action: " + e.Message, + keywords: Keywords.Telemetry); + } + else + { + this.Context.Tracer.RelatedWarning( + metadata: this.CreateEventMetadata(e), + message: "Exception while running action inside a repo that's not ready: " + e.Message); + } + Environment.Exit((int)ReturnCode.GenericError); } } @@ -188,7 +188,7 @@ protected GitProcess.Result RunGitCommand(Func wo { this.Context.Tracer.RelatedWarning( metadata: null, - message: $"{this.Area}: Not launching Git process {gitCommand} because the mount is stopping", + message: $"{this.Area}: Not launching Git process {gitCommand} because the process is stopping", keywords: Keywords.Telemetry); throw new StoppingException(); } diff --git a/Scalar.Common/Platforms/Windows/WindowsPlatform.Shared.cs b/Scalar.Common/Platforms/Windows/WindowsPlatform.Shared.cs index f1fa3b3c50..e10bc789a5 100644 --- a/Scalar.Common/Platforms/Windows/WindowsPlatform.Shared.cs +++ b/Scalar.Common/Platforms/Windows/WindowsPlatform.Shared.cs @@ -50,7 +50,7 @@ public static bool IsProcessActiveImplementation(int processId, bool tryGetProce } else if (tryGetProcessById) { - // The process.IsInvalid may be true when the mount process doesn't have access to call + // The process.IsInvalid may be true when the process doesn't have access to call // OpenProcess for the specified processId. Fallback to slow way of finding process. try { diff --git a/Scalar.Common/ScalarConstants.cs b/Scalar.Common/ScalarConstants.cs index 1e9872452f..f6b336ec64 100644 --- a/Scalar.Common/ScalarConstants.cs +++ b/Scalar.Common/ScalarConstants.cs @@ -83,14 +83,10 @@ public static class SpecialGitFiles public static class LogFileTypes { - public const string MountPrefix = "mount"; public const string UpgradePrefix = "productupgrade"; public const string Clone = "clone"; public const string Maintenance = "maintenance"; - public const string MountVerb = MountPrefix + "_verb"; - public const string MountProcess = MountPrefix + "_process"; - public const string MountUpgrade = MountPrefix + "_repoupgrade"; public const string Repair = "repair"; public const string Service = "service"; public const string ServiceUI = "service_ui"; @@ -195,23 +191,6 @@ public static class VerbParameters { public const string InternalUseOnly = "internal_use_only"; - public static class Mount - { - public const string StartedByService = "StartedByService"; - public const string StartedByVerb = "StartedByVerb"; - public const string Verbosity = "verbosity"; - public const string Keywords = "keywords"; - public const string DebugWindow = "debug-window"; - - public const string DefaultVerbosity = "Informational"; - public const string DefaultKeywords = "Any"; - } - - public static class Unmount - { - public const string SkipLock = "skip-wait-for-lock"; - } - public static class Maintenance { public const string AllTasksName = "all"; diff --git a/Scalar.Common/ScalarEnlistment.cs b/Scalar.Common/ScalarEnlistment.cs index cf83da70b7..4cda07d14e 100644 --- a/Scalar.Common/ScalarEnlistment.cs +++ b/Scalar.Common/ScalarEnlistment.cs @@ -45,7 +45,7 @@ private ScalarEnlistment(string enlistmentRoot, string workingDirectory, string public bool UsesGvfsProtocol { get; protected set; } - // These version properties are only used in logging during clone and mount to track version numbers + // These version properties are only used in logging during clone to track version numbers public string GitVersion { get { return this.gitVersion; } @@ -85,7 +85,7 @@ public static ScalarEnlistment CreateFromDirectory( public static string GetNewScalarLogFileName( string logsRoot, - string logFileType, + string logFileType, string logId = null, PhysicalFileSystem fileSystem = null) { @@ -100,7 +100,7 @@ public static bool TryGetScalarEnlistmentRoot( string directory, out string enlistmentRoot, out string workingDirectoryRoot, - Func exists = null) + Func exists = null) { if (exists == null) { diff --git a/Scalar.FunctionalTests/Tests/MultiEnlistmentTests/SharedCacheTests.cs b/Scalar.FunctionalTests/Tests/MultiEnlistmentTests/SharedCacheTests.cs index c928f3b240..9eee4f0956 100644 --- a/Scalar.FunctionalTests/Tests/MultiEnlistmentTests/SharedCacheTests.cs +++ b/Scalar.FunctionalTests/Tests/MultiEnlistmentTests/SharedCacheTests.cs @@ -111,8 +111,6 @@ public void SecondCloneSucceedsWithMissingTrees() result.ExitCode.ShouldEqual(0, $"git {command} failed on {nameof(enlistment2)} with error: {result.Errors}"); } - // Override OnTearDownEnlistmentsDeleted rathern than using [TearDown] as the enlistments need to be unmounted before - // localCacheParentPath can be deleted (as the SQLite blob sizes database cannot be deleted while Scalar is mounted) protected override void OnTearDownEnlistmentsDeleted() { RepositoryHelpers.DeleteTestDirectory(this.localCacheParentPath); diff --git a/Scalar.FunctionalTests/Tests/MultiEnlistmentTests/TestsWithMultiEnlistment.cs b/Scalar.FunctionalTests/Tests/MultiEnlistmentTests/TestsWithMultiEnlistment.cs index d283f428f0..d8d0835bb3 100644 --- a/Scalar.FunctionalTests/Tests/MultiEnlistmentTests/TestsWithMultiEnlistment.cs +++ b/Scalar.FunctionalTests/Tests/MultiEnlistmentTests/TestsWithMultiEnlistment.cs @@ -22,7 +22,7 @@ public void DeleteEnlistments() } /// - /// Can be overridden for custom [TearDown] steps that occur after the test enlistements have been unmounted and deleted + /// Can be overridden for custom [TearDown] steps that occur after the test enlistments have been deleted /// protected virtual void OnTearDownEnlistmentsDeleted() { diff --git a/Scalar.FunctionalTests/Tests/TestResultsHelper.cs b/Scalar.FunctionalTests/Tests/TestResultsHelper.cs index 665093fc87..0bf87cb2b2 100644 --- a/Scalar.FunctionalTests/Tests/TestResultsHelper.cs +++ b/Scalar.FunctionalTests/Tests/TestResultsHelper.cs @@ -20,18 +20,7 @@ public static void OutputScalarLogs(ScalarFunctionalTestEnlistment enlistment) foreach (string filename in GetAllFilesInDirectory(enlistment.ScalarLogsRoot)) { - if (filename.Contains("mount_process")) - { - // Validate that all mount processes started by the functional tests were started - // by verbs, and that "StartedByVerb" was set to true when the mount process was launched - OutputFileContents( - filename, - contents => contents.ShouldContain("\"StartedByVerb\":true")); - } - else - { - OutputFileContents(filename); - } + OutputFileContents(filename); } } diff --git a/Scalar.FunctionalTests/Tools/ScalarProcess.cs b/Scalar.FunctionalTests/Tools/ScalarProcess.cs index 4687246495..20ce4ae25a 100644 --- a/Scalar.FunctionalTests/Tools/ScalarProcess.cs +++ b/Scalar.FunctionalTests/Tools/ScalarProcess.cs @@ -40,22 +40,6 @@ public void Clone(string repositorySource, string branchToCheckout, bool skipFet this.CallScalar(args, expectedExitCode: SuccessExitCode); } - public void Mount() - { - string output; - this.TryMount(out output).ShouldEqual(true, "Scalar did not mount: " + output); - - // TODO: Re-add this warning after we work out the version detail information - // output.ShouldNotContain(ignoreCase: true, unexpectedSubstrings: "warning"); - } - - public bool TryMount(out string output) - { - this.IsEnlistmentMounted().ShouldEqual(false, "Scalar is already mounted"); - output = this.CallScalar("mount \"" + this.enlistmentRoot + "\""); - return this.IsEnlistmentMounted(); - } - public string RunVerb(string task, long? batchSize = null, bool failOnError = true) { string batchArg = batchSize == null @@ -106,21 +90,6 @@ public string CacheServer(string args) return this.CallScalar("cache-server " + args + " \"" + this.enlistmentRoot + "\""); } - public void Unmount() - { - if (this.IsEnlistmentMounted()) - { - string result = this.CallScalar("unmount \"" + this.enlistmentRoot + "\"", expectedExitCode: SuccessExitCode); - this.IsEnlistmentMounted().ShouldEqual(false, "Scalar did not unmount: " + result); - } - } - - public bool IsEnlistmentMounted() - { - string statusResult = this.CallScalar("status \"" + this.enlistmentRoot + "\""); - return statusResult.Contains("Mount status: Ready"); - } - public string RunServiceVerb(string argument) { return this.CallScalar("service " + argument, expectedExitCode: SuccessExitCode); diff --git a/Scalar.Installer.Mac/scripts/preinstall b/Scalar.Installer.Mac/scripts/preinstall deleted file mode 100755 index 205e27c904..0000000000 --- a/Scalar.Installer.Mac/scripts/preinstall +++ /dev/null @@ -1,8 +0,0 @@ -#!/bin/bash - -SCALARBINPATH="/usr/local/scalar/scalar" -if [ -f "${SCALARBINPATH}" ]; then - unmountCmd="${SCALARBINPATH} service --help" - echo $unmountCmd - eval $unmountCmd || exit 1 -fi diff --git a/Scalar.Service.UI/ScalarToastRequestHandler.cs b/Scalar.Service.UI/ScalarToastRequestHandler.cs index 8b71766c6f..5d4813172c 100644 --- a/Scalar.Service.UI/ScalarToastRequestHandler.cs +++ b/Scalar.Service.UI/ScalarToastRequestHandler.cs @@ -1,166 +1,137 @@ -using Scalar.Common.NamedPipes; -using Scalar.Common.Tracing; -using System; -using System.Diagnostics; -using System.IO; - -namespace Scalar.Service.UI -{ - public class ScalarToastRequestHandler - { - private const string ScalarAutomountStartTitle= "Scalar Automount"; - private const string ScalarAutomountStartMessageFormat = "Attempting to mount {0} Scalar {1}"; - private const string ScalarMultipleRepos = "repos"; - private const string ScalarSingleRepo = "repo"; - - private const string ScalarAutomountSuccessTitle = "Scalar Automount"; - private const string ScalarAutomountSuccessMessageFormat = "The following Scalar repo is now mounted: {0}{1}"; - - private const string ScalarAutomountErrorTitle = "Scalar Automount"; - private const string ScalarAutomountErrorMessageFormat = "The following Scalar repo failed to mount: {0}{1}"; - private const string ScalarAutomountButtonTitle = "Retry"; - - private const string ScalarUpgradeTitleFormat = "New version {0} is available"; - private const string ScalarUpgradeMessage = "When ready, click Upgrade button to run upgrade."; - private const string ScalarUpgradeButtonTitle = "Upgrade"; - - private const string ScalarRemountActionPrefix = "scalar mount"; - private const string ScalarUpgradeActionPrefix = "scalar upgrade --confirm"; - - private readonly ITracer tracer; - private readonly IToastNotifier toastNotifier; - - public ScalarToastRequestHandler(IToastNotifier toastNotifier, ITracer tracer) - { - this.toastNotifier = toastNotifier; - this.toastNotifier.UserResponseCallback = this.UserResponseCallback; - this.tracer = tracer; - } - - public void HandleToastRequest(ITracer tracer, NamedPipeMessages.Notification.Request request) - { - string title = null; - string message = null; - string buttonTitle = null; - string args = null; - - switch (request.Id) - { - case NamedPipeMessages.Notification.Request.Identifier.UpgradeAvailable: - title = string.Format(ScalarUpgradeTitleFormat, request.NewVersion); - message = string.Format(ScalarUpgradeMessage); - buttonTitle = ScalarUpgradeButtonTitle; - args = $"{ScalarUpgradeActionPrefix}"; - break; - } - - if (title != null && message != null) - { - this.toastNotifier.Notify(title, message, buttonTitle, args); - } - } - - public void UserResponseCallback(string args) - { - if (string.IsNullOrEmpty(args)) - { - this.tracer.RelatedError($"{nameof(this.UserResponseCallback)}: Received null arguments in Toaster callback."); - return; - } - - using (ITracer activity = this.tracer.StartActivity("GVFSToastCallback", EventLevel.Informational)) - { - string gvfsCmd = null; - bool elevate = false; - - if (args.StartsWith(ScalarUpgradeActionPrefix)) - { - this.tracer.RelatedInfo($"scalar upgrade action."); - gvfsCmd = "scalar upgrade --confirm"; - elevate = true; - } - else if (args.StartsWith(ScalarRemountActionPrefix)) - { - string path = args.Substring(ScalarRemountActionPrefix.Length, args.Length - ScalarRemountActionPrefix.Length); - if (this.TryValidatePath(path, out string enlistment, activity)) - { - this.tracer.RelatedInfo($"scalar mount action {enlistment}."); - gvfsCmd = $"scalar mount \"{enlistment}\""; - } - else - { - EventMetadata metadata = new EventMetadata(); - metadata.Add(nameof(args), args); - metadata.Add(nameof(path), path); - this.tracer.RelatedError(metadata, $"{nameof(this.UserResponseCallback)}- Invalid enlistment path specified in Toaster callback."); - } - } - else - { - this.tracer.RelatedError($"{nameof(this.UserResponseCallback)}- Unknown action({args}) specified in Toaster callback."); - } - - if (!string.IsNullOrEmpty(gvfsCmd)) - { - this.launchGVFSInCommandPrompt(gvfsCmd, elevate, activity); - } - } - } - - private bool TryValidatePath(string path, out string validatedPath, ITracer tracer) - { - try - { - validatedPath = Path.GetFullPath(path); - return true; - } - catch (Exception ex) - { - EventMetadata metadata = new EventMetadata(); - metadata.Add("Exception", ex.ToString()); - metadata.Add("Path", path); - - tracer.RelatedError(metadata, $"{nameof(this.TryValidatePath)}: {path}. {ex.ToString()}"); - } - - validatedPath = null; - return false; - } - - private void launchGVFSInCommandPrompt(string fullGvfsCmd, bool elevate, ITracer tracer) - { - const string cmdPath = "CMD.exe"; - ProcessStartInfo processInfo = new ProcessStartInfo(cmdPath); - processInfo.UseShellExecute = true; - processInfo.RedirectStandardInput = false; - processInfo.RedirectStandardOutput = false; - processInfo.RedirectStandardError = false; - processInfo.WindowStyle = ProcessWindowStyle.Normal; - processInfo.CreateNoWindow = false; - - // /K option is so the user gets the time to read the output of the command and - // manually close the cmd window after that. - processInfo.Arguments = "/K " + fullGvfsCmd; - if (elevate) - { - processInfo.Verb = "runas"; - } - - tracer.RelatedInfo($"{nameof(this.UserResponseCallback)}- Running {cmdPath} /K {fullGvfsCmd}"); - - try - { - Process.Start(processInfo); - } - catch (Exception ex) - { - EventMetadata metadata = new EventMetadata(); - metadata.Add("Exception", ex.ToString()); - metadata.Add(nameof(fullGvfsCmd), fullGvfsCmd); - metadata.Add(nameof(elevate), elevate); - - tracer.RelatedError(metadata, $"{nameof(this.launchGVFSInCommandPrompt)}: Error launching {fullGvfsCmd}. {ex.ToString()}"); - } - } - } -} +using Scalar.Common.NamedPipes; +using Scalar.Common.Tracing; +using System; +using System.Diagnostics; +using System.IO; + +namespace Scalar.Service.UI +{ + public class ScalarToastRequestHandler + { + private const string ScalarUpgradeTitleFormat = "New version {0} is available"; + private const string ScalarUpgradeMessage = "When ready, click Upgrade button to run upgrade."; + private const string ScalarUpgradeButtonTitle = "Upgrade"; + + private const string ScalarUpgradeActionPrefix = "scalar upgrade --confirm"; + + private readonly ITracer tracer; + private readonly IToastNotifier toastNotifier; + + public ScalarToastRequestHandler(IToastNotifier toastNotifier, ITracer tracer) + { + this.toastNotifier = toastNotifier; + this.toastNotifier.UserResponseCallback = this.UserResponseCallback; + this.tracer = tracer; + } + + public void HandleToastRequest(ITracer tracer, NamedPipeMessages.Notification.Request request) + { + string title = null; + string message = null; + string buttonTitle = null; + string args = null; + + switch (request.Id) + { + case NamedPipeMessages.Notification.Request.Identifier.UpgradeAvailable: + title = string.Format(ScalarUpgradeTitleFormat, request.NewVersion); + message = string.Format(ScalarUpgradeMessage); + buttonTitle = ScalarUpgradeButtonTitle; + args = $"{ScalarUpgradeActionPrefix}"; + break; + } + + if (title != null && message != null) + { + this.toastNotifier.Notify(title, message, buttonTitle, args); + } + } + + public void UserResponseCallback(string args) + { + if (string.IsNullOrEmpty(args)) + { + this.tracer.RelatedError($"{nameof(this.UserResponseCallback)}: Received null arguments in Toaster callback."); + return; + } + + using (ITracer activity = this.tracer.StartActivity("ScalarToastCallback", EventLevel.Informational)) + { + string command = null; + bool elevate = false; + + if (args.StartsWith(ScalarUpgradeActionPrefix)) + { + this.tracer.RelatedInfo($"scalar upgrade action."); + command = "scalar upgrade --confirm"; + elevate = true; + } + else + { + this.tracer.RelatedError($"{nameof(this.UserResponseCallback)}- Unknown action({args}) specified in Toaster callback."); + } + + if (!string.IsNullOrEmpty(command)) + { + this.LaunchCommandInCommandPrompt(command, elevate, activity); + } + } + } + + private bool TryValidatePath(string path, out string validatedPath, ITracer tracer) + { + try + { + validatedPath = Path.GetFullPath(path); + return true; + } + catch (Exception ex) + { + EventMetadata metadata = new EventMetadata(); + metadata.Add("Exception", ex.ToString()); + metadata.Add("Path", path); + + tracer.RelatedError(metadata, $"{nameof(this.TryValidatePath)}: {path}. {ex.ToString()}"); + } + + validatedPath = null; + return false; + } + + private void LaunchCommandInCommandPrompt(string fullCommand, bool elevate, ITracer tracer) + { + const string cmdPath = "CMD.exe"; + ProcessStartInfo processInfo = new ProcessStartInfo(cmdPath); + processInfo.UseShellExecute = true; + processInfo.RedirectStandardInput = false; + processInfo.RedirectStandardOutput = false; + processInfo.RedirectStandardError = false; + processInfo.WindowStyle = ProcessWindowStyle.Normal; + processInfo.CreateNoWindow = false; + + // /K option is so the user gets the time to read the output of the command and + // manually close the cmd window after that. + processInfo.Arguments = "/K " + fullCommand; + if (elevate) + { + processInfo.Verb = "runas"; + } + + tracer.RelatedInfo($"{nameof(this.UserResponseCallback)}- Running {cmdPath} /K {fullCommand}"); + + try + { + Process.Start(processInfo); + } + catch (Exception ex) + { + EventMetadata metadata = new EventMetadata(); + metadata.Add("Exception", ex.ToString()); + metadata.Add(nameof(fullCommand), fullCommand); + metadata.Add(nameof(elevate), elevate); + + tracer.RelatedError(metadata, $"{nameof(this.LaunchCommandInCommandPrompt)}: Error launching {fullCommand}. {ex.ToString()}"); + } + } + } +} diff --git a/Scalar.UnitTests/Mock/MockInstallerPreRunChecker.cs b/Scalar.UnitTests/Mock/MockInstallerPreRunChecker.cs index 62b067cb6c..4d37907ab9 100644 --- a/Scalar.UnitTests/Mock/MockInstallerPreRunChecker.cs +++ b/Scalar.UnitTests/Mock/MockInstallerPreRunChecker.cs @@ -22,8 +22,6 @@ public enum FailOnCheckType IsElevated = 0x2, BlockingProcessesRunning = 0x4, UnattendedMode = 0x8, - UnMountRepos = 0x10, - RemountRepos = 0x20, IsServiceInstalledAndNotRunning = 0x40, } @@ -86,20 +84,6 @@ protected override bool IsBlockingProcessRunning(out HashSet processes) protected override bool TryRunScalarWithArgs(string args, out string error) { - if (string.CompareOrdinal(args, "service --unmount-all") == 0) - { - bool result = this.FakedResultOfCheck(FailOnCheckType.UnMountRepos); - error = result == false ? "Unmount of some of the repositories failed." : null; - return result; - } - - if (string.CompareOrdinal(args, "service --mount-all") == 0) - { - bool result = this.FakedResultOfCheck(FailOnCheckType.RemountRepos); - error = result == false ? "Auto remount failed." : null; - return result; - } - error = "Unknown Scalar command"; return false; } diff --git a/Scalar.UnitTests/Service/Mac/MacScalarVerbRunnerTests.cs b/Scalar.UnitTests/Service/Mac/MacScalarVerbRunnerTests.cs index d4a3c024e2..997d4e1837 100644 --- a/Scalar.UnitTests/Service/Mac/MacScalarVerbRunnerTests.cs +++ b/Scalar.UnitTests/Service/Mac/MacScalarVerbRunnerTests.cs @@ -35,17 +35,17 @@ public void CallMaintenance_LaunchesVerbUsingCorrectArgs() string expectedArgs = $"run {taskVerbName} \"{ExpectedActiveRepoPath}\" --{ScalarConstants.VerbParameters.InternalUseOnly} {new InternalVerbParameters(startedByService: true).ToJson()}"; - Mock mountLauncherMock = new Mock(MockBehavior.Strict, this.tracer); - mountLauncherMock.Setup(mp => mp.LaunchProcess( + Mock procLauncherMock = new Mock(MockBehavior.Strict, this.tracer); + procLauncherMock.Setup(mp => mp.LaunchProcess( scalarBinPath, expectedArgs, ExpectedActiveRepoPath)) .Returns(new ProcessResult(output: string.Empty, errors: string.Empty, exitCode: 0)); - MacScalarVerbRunner verbProcess = new MacScalarVerbRunner(this.tracer, mountLauncherMock.Object); + MacScalarVerbRunner verbProcess = new MacScalarVerbRunner(this.tracer, procLauncherMock.Object); verbProcess.CallMaintenance(task, ExpectedActiveRepoPath, ExpectedActiveUserId); - mountLauncherMock.VerifyAll(); + procLauncherMock.VerifyAll(); } } } diff --git a/Scalar.UnitTests/Upgrader/UpgradeOrchestratorWithGitHubUpgraderTests.cs b/Scalar.UnitTests/Upgrader/UpgradeOrchestratorWithGitHubUpgraderTests.cs index 28ca12e21e..d6be79da15 100644 --- a/Scalar.UnitTests/Upgrader/UpgradeOrchestratorWithGitHubUpgraderTests.cs +++ b/Scalar.UnitTests/Upgrader/UpgradeOrchestratorWithGitHubUpgraderTests.cs @@ -34,24 +34,6 @@ public void UpgradeNoError() this.Tracer.RelatedErrorEvents.ShouldBeEmpty(); } - [TestCase] - public void AutoUnmountError() - { - this.ConfigureRunAndVerify( - configure: () => - { - this.PrerunChecker.SetReturnFalseOnCheck(MockInstallerPrerunChecker.FailOnCheckType.UnMountRepos); - }, - expectedReturn: ReturnCode.GenericError, - expectedOutput: new List - { - "Unmount of some of the repositories failed." - }, - expectedErrors: new List - { - "Unmount of some of the repositories failed." - }); - } [TestCase] public void AbortOnBlockingProcess() @@ -194,7 +176,7 @@ public void ScalarInstallationArgs() string args; gitInstallerInfo.TryGetValue("Args", out args).ShouldBeTrue(); - args.ShouldContain(new string[] { "/VERYSILENT", "/CLOSEAPPLICATIONS", "/SUPPRESSMSGBOXES", "/NORESTART", "/Log", "/REMOUNTREPOS=false" }); + args.ShouldContain(new string[] { "/VERYSILENT", "/CLOSEAPPLICATIONS", "/SUPPRESSMSGBOXES", "/NORESTART", "/Log" }); } [TestCase] @@ -271,25 +253,6 @@ public void GitCleanupError() }); } - [TestCase] - public void RemountReposError() - { - this.ConfigureRunAndVerify( - configure: () => - { - this.PrerunChecker.SetReturnFalseOnCheck(MockInstallerPrerunChecker.FailOnCheckType.RemountRepos); - }, - expectedReturn: ReturnCode.Success, - expectedOutput: new List - { - "Auto remount failed." - }, - expectedErrors: new List - { - "Auto remount failed." - }); - } - [TestCase] public void DryRunDoesNotRunInstallerExes() { diff --git a/Scalar.Upgrader/MacUpgradeOrchestrator.cs b/Scalar.Upgrader/MacUpgradeOrchestrator.cs index 2951771e36..4c272e76f9 100644 --- a/Scalar.Upgrader/MacUpgradeOrchestrator.cs +++ b/Scalar.Upgrader/MacUpgradeOrchestrator.cs @@ -6,12 +6,5 @@ public MacUpgradeOrchestrator(UpgradeOptions options) : base(options) { } - - protected override bool TryMountRepositories(out string consoleError) - { - // Mac upgrader does not mount repositories - consoleError = null; - return true; - } } } diff --git a/Scalar.Upgrader/UpgradeOrchestrator.cs b/Scalar.Upgrader/UpgradeOrchestrator.cs index 0a653eb44c..3afdc3d23c 100644 --- a/Scalar.Upgrader/UpgradeOrchestrator.cs +++ b/Scalar.Upgrader/UpgradeOrchestrator.cs @@ -11,7 +11,6 @@ namespace Scalar.Upgrader public abstract class UpgradeOrchestrator { protected InstallerPreRunChecker preRunChecker; - protected bool mount; protected ITracer tracer; private const EventLevel DefaultEventLevel = EventLevel.Informational; @@ -37,7 +36,6 @@ public UpgradeOrchestrator( this.preRunChecker = preRunChecker; this.output = output; this.input = input; - this.mount = false; this.ExitCode = ReturnCode.Success; this.installationId = DateTime.Now.ToString("yyyyMMdd_HHmmss"); } @@ -59,7 +57,6 @@ public UpgradeOrchestrator() this.fileSystem = new PhysicalFileSystem(); this.output = Console.Out; this.input = Console.In; - this.mount = false; this.ExitCode = ReturnCode.Success; this.installationId = DateTime.Now.ToString("yyyyMMdd_HHmmss"); } @@ -73,7 +70,6 @@ public UpgradeOrchestrator() public void Execute() { string error = null; - string mountError = null; Version newVersion = null; if (this.tracer == null) @@ -99,12 +95,6 @@ public void Execute() } finally { - if (!this.TryMountRepositories(out mountError)) - { - mountError = Environment.NewLine + "WARNING: " + mountError; - this.output.WriteLine(mountError); - } - this.DeletedDownloadedAssets(); } } @@ -130,7 +120,7 @@ public void Execute() { if (newVersion != null) { - this.output.WriteLine($"{Environment.NewLine}Upgrade completed successfully{(string.IsNullOrEmpty(mountError) ? "." : ", but one or more repositories will need to be mounted manually.")}"); + this.output.WriteLine($"{Environment.NewLine}Upgrade completed successfully."); } } } @@ -157,8 +147,6 @@ protected bool LaunchInsideSpinner(Func method, string message) this.output == Console.Out && !ScalarPlatform.Instance.IsConsoleOutputRedirectedToFile()); } - protected abstract bool TryMountRepositories(out string consoleError); - private JsonTracer CreateTracer() { string logFilePath = ScalarEnlistment.GetNewScalarLogFileName( @@ -259,25 +247,6 @@ private bool TryRunUpgrade(out Version newVersion, out string consoleError) return true; } - if (!this.LaunchInsideSpinner( - () => - { - if (!this.preRunChecker.TryUnmountAllScalarRepos(out error)) - { - return false; - } - - this.mount = true; - - return true; - }, - "Unmounting repositories")) - { - newVersion = null; - consoleError = error; - return false; - } - if (!this.LaunchInsideSpinner( () => { diff --git a/Scalar.Upgrader/WindowsUpgradeOrchestrator.cs b/Scalar.Upgrader/WindowsUpgradeOrchestrator.cs index 184c723691..e43da2a813 100644 --- a/Scalar.Upgrader/WindowsUpgradeOrchestrator.cs +++ b/Scalar.Upgrader/WindowsUpgradeOrchestrator.cs @@ -22,34 +22,5 @@ public WindowsUpgradeOrchestrator(UpgradeOptions options) : base(options) { } - - protected override bool TryMountRepositories(out string consoleError) - { - string errorMessage = string.Empty; - if (this.mount && !this.LaunchInsideSpinner( - () => - { - string mountError; - if (!this.preRunChecker.TryMountAllScalarRepos(out mountError)) - { - EventMetadata metadata = new EventMetadata(); - metadata.Add("Upgrade Step", nameof(this.TryMountRepositories)); - metadata.Add("Mount Error", mountError); - this.tracer.RelatedError(metadata, $"{nameof(this.preRunChecker.TryMountAllScalarRepos)} failed."); - errorMessage += mountError; - return false; - } - - return true; - }, - "Mounting repositories")) - { - consoleError = errorMessage; - return false; - } - - consoleError = null; - return true; - } } } diff --git a/Scalar/CommandLine/CacheServerVerb.cs b/Scalar/CommandLine/CacheServerVerb.cs index f21d7a946e..eccc13c1b3 100644 --- a/Scalar/CommandLine/CacheServerVerb.cs +++ b/Scalar/CommandLine/CacheServerVerb.cs @@ -62,8 +62,6 @@ protected override void Execute(ScalarEnlistment enlistment) { this.ReportErrorAndExit("Failed to save cache to config: " + error); } - - this.Output.WriteLine("You must remount Scalar for this to take effect."); } else if (this.ListCacheServers) { diff --git a/Scalar/CommandLine/ScalarVerb.cs b/Scalar/CommandLine/ScalarVerb.cs index 068366e610..979b3af57b 100644 --- a/Scalar/CommandLine/ScalarVerb.cs +++ b/Scalar/CommandLine/ScalarVerb.cs @@ -46,13 +46,13 @@ public string InternalParameters { try { - InternalVerbParameters mountInternal = InternalVerbParameters.FromJson(value); - if (!string.IsNullOrEmpty(mountInternal.ServiceName)) + InternalVerbParameters internalParams = InternalVerbParameters.FromJson(value); + if (!string.IsNullOrEmpty(internalParams.ServiceName)) { - this.ServiceName = mountInternal.ServiceName; + this.ServiceName = internalParams.ServiceName; } - this.StartedByService = mountInternal.StartedByService; + this.StartedByService = internalParams.StartedByService; } catch (JsonReaderException e) {