diff --git a/.github/actions/spelling/expect.txt b/.github/actions/spelling/expect.txt index 60f9fd1b83..4aa912f131 100644 --- a/.github/actions/spelling/expect.txt +++ b/.github/actions/spelling/expect.txt @@ -210,6 +210,7 @@ SRL srs Standalone startswith +streambuf strtoull subdir subkey diff --git a/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj b/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj index d99b7b790f..96a1f29c0d 100644 --- a/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj +++ b/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj @@ -177,6 +177,7 @@ + @@ -221,6 +222,7 @@ + diff --git a/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj.filters b/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj.filters index 784c9210aa..a475b192b1 100644 --- a/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj.filters +++ b/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj.filters @@ -152,6 +152,9 @@ Header Files + + Public + @@ -271,6 +274,9 @@ Commands + + Source Files + diff --git a/src/AppInstallerCLICore/COMContext.cpp b/src/AppInstallerCLICore/COMContext.cpp new file mode 100644 index 0000000000..ee2cb11fea --- /dev/null +++ b/src/AppInstallerCLICore/COMContext.cpp @@ -0,0 +1,35 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#include "pch.h" +#include "COMContext.h" + +namespace AppInstaller +{ + + NullStream::NullStream() + { + m_nullOut.reset(new std::ostream(&m_nullStreamBuf)); + m_nullIn.reset(new std::istream(&m_nullStreamBuf)); + } + + void COMContext::BeginProgress() + { + m_comProgressCallback(ReportType::BeginProgress, 0, 0, ProgressType::None, m_executionStage); + }; + + void COMContext::OnProgress(uint64_t current, uint64_t maximum, ProgressType progressType) + { + m_comProgressCallback(ReportType::Progressing, current, maximum, progressType, m_executionStage); + } + + void COMContext::EndProgress(bool) + { + m_comProgressCallback(ReportType::EndProgress, 0, 0, ProgressType::None, m_executionStage); + }; + + void COMContext::SetExecutionStage(CLI::Workflow::ExecutionStage executionStage, bool) + { + m_executionStage = executionStage; + m_comProgressCallback(ReportType::ExecutionPhaseUpdate, 0, 0, ProgressType::None, m_executionStage); + } +} \ No newline at end of file diff --git a/src/AppInstallerCLICore/COMContext.h b/src/AppInstallerCLICore/COMContext.h new file mode 100644 index 0000000000..bcc4d1a7bb --- /dev/null +++ b/src/AppInstallerCLICore/COMContext.h @@ -0,0 +1,69 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#pragma once +#include "pch.h" +#include "..\AppInstallerCommonCore\Public\AppInstallerProgress.h" +#include "ExecutionContext.h" +#include "Workflows/WorkflowBase.h" + +namespace AppInstaller +{ + enum class ReportType: uint32_t + { + ExecutionPhaseUpdate, + BeginProgress, + Progressing, + EndProgress, + }; + + class NullStreamBuf : public std::streambuf {}; + + struct NullStream + { + NullStream(); + + ~NullStream() = default; + + protected: + NullStreamBuf m_nullStreamBuf; + std::unique_ptr m_nullOut; + std::unique_ptr m_nullIn; + }; + + typedef std::function ProgressCallBackFunction; + + // NullStream constructs the Stream parameters for Context constructor + // Hence, NullStream should always precede Context in base class order of COMContext's inheritance + struct COMContext : IProgressSink, NullStream, CLI::Execution::Context + { + // When no Console streams need involvement, construct NullStreams instead to pass to Context + COMContext() : NullStream(), CLI::Execution::Context(*m_nullOut, *m_nullIn) + { + Reporter.SetProgressSink(this); + } + + COMContext(std::ostream& out, std::istream& in) : CLI::Execution::Context(out, in) + { + Reporter.SetProgressSink(this); + } + + ~COMContext() = default; + + // IProgressSink + void BeginProgress() override; + void OnProgress(uint64_t current, uint64_t maximum, ProgressType type) override; + void EndProgress(bool) override; + + //Execution::Context + void SetExecutionStage(CLI::Workflow::ExecutionStage executionPhase, bool); + + void SetProgressCallbackFunction(ProgressCallBackFunction&& f) + { + m_comProgressCallback = std::move(f); + } + + private: + CLI::Workflow::ExecutionStage m_executionStage = CLI::Workflow::ExecutionStage::Initial; + ProgressCallBackFunction m_comProgressCallback; + }; +} \ No newline at end of file diff --git a/src/AppInstallerCLICore/Command.cpp b/src/AppInstallerCLICore/Command.cpp index abefea0bdb..efaaacd2bc 100644 --- a/src/AppInstallerCLICore/Command.cpp +++ b/src/AppInstallerCLICore/Command.cpp @@ -770,4 +770,59 @@ namespace AppInstaller::CLI return arguments; } + + int Execute(Execution::Context& context, std::unique_ptr& command) + { + try + { + if (!Settings::User().GetWarnings().empty()) + { + context.Reporter.Warn() << Resource::String::SettingsWarnings << std::endl; + } + + command->Execute(context); + } + // Exceptions that may occur in the process of executing an arbitrary command + catch (const wil::ResultException& re) + { + // Even though they are logged at their source, log again here for completeness. + Logging::Telemetry().LogException(command->FullName(), "wil::ResultException", re.what()); + context.Reporter.Error() << + Resource::String::UnexpectedErrorExecutingCommand << ' ' << std::endl << + GetUserPresentableMessage(re) << std::endl; + return re.GetErrorCode(); + } + catch (const winrt::hresult_error& hre) + { + std::string message = GetUserPresentableMessage(hre); + Logging::Telemetry().LogException(command->FullName(), "winrt::hresult_error", message); + context.Reporter.Error() << + Resource::String::UnexpectedErrorExecutingCommand << ' ' << std::endl << + message << std::endl; + return hre.code(); + } + catch (const std::exception& e) + { + Logging::Telemetry().LogException(command->FullName(), "std::exception", e.what()); + context.Reporter.Error() << + Resource::String::UnexpectedErrorExecutingCommand << ' ' << std::endl << + GetUserPresentableMessage(e) << std::endl; + return APPINSTALLER_CLI_ERROR_COMMAND_FAILED; + } + catch (...) + { + LOG_CAUGHT_EXCEPTION(); + Logging::Telemetry().LogException(command->FullName(), "unknown", {}); + context.Reporter.Error() << + Resource::String::UnexpectedErrorExecutingCommand << " ???"_liv << std::endl; + return APPINSTALLER_CLI_ERROR_COMMAND_FAILED; + } + + if (SUCCEEDED(context.GetTerminationHR())) + { + Logging::Telemetry().LogCommandSuccess(command->FullName()); + } + + return context.GetTerminationHR(); + } } diff --git a/src/AppInstallerCLICore/Command.h b/src/AppInstallerCLICore/Command.h index 478d5f539a..db1d6616b9 100644 --- a/src/AppInstallerCLICore/Command.h +++ b/src/AppInstallerCLICore/Command.h @@ -112,4 +112,6 @@ namespace AppInstaller::CLI return result; } + + int Execute(Execution::Context& context, std::unique_ptr& command); } diff --git a/src/AppInstallerCLICore/Core.cpp b/src/AppInstallerCLICore/Core.cpp index 1dfd95f947..01d68c77ae 100644 --- a/src/AppInstallerCLICore/Core.cpp +++ b/src/AppInstallerCLICore/Core.cpp @@ -6,6 +6,7 @@ #include "ExecutionContext.h" #include "Workflows/WorkflowBase.h" #include +#include "Commands/InstallCommand.h" using namespace winrt; using namespace winrt::Windows::Foundation; @@ -115,57 +116,7 @@ namespace AppInstaller::CLI return APPINSTALLER_CLI_ERROR_INVALID_CL_ARGUMENTS; } - try - { - if (!Settings::User().GetWarnings().empty()) - { - context.Reporter.Warn() << Resource::String::SettingsWarnings << std::endl; - } - - command->Execute(context); - } - // Exceptions that may occur in the process of executing an arbitrary command - catch (const wil::ResultException& re) - { - // Even though they are logged at their source, log again here for completeness. - Logging::Telemetry().LogException(command->FullName(), "wil::ResultException", re.what()); - context.Reporter.Error() << - Resource::String::UnexpectedErrorExecutingCommand << ' ' << std::endl << - GetUserPresentableMessage(re) << std::endl; - return re.GetErrorCode(); - } - catch (const winrt::hresult_error& hre) - { - std::string message = GetUserPresentableMessage(hre); - Logging::Telemetry().LogException(command->FullName(), "winrt::hresult_error", message); - context.Reporter.Error() << - Resource::String::UnexpectedErrorExecutingCommand << ' ' << std::endl << - message << std::endl; - return hre.code(); - } - catch (const std::exception& e) - { - Logging::Telemetry().LogException(command->FullName(), "std::exception", e.what()); - context.Reporter.Error() << - Resource::String::UnexpectedErrorExecutingCommand << ' ' << std::endl << - GetUserPresentableMessage(e) << std::endl; - return APPINSTALLER_CLI_ERROR_COMMAND_FAILED; - } - catch (...) - { - LOG_CAUGHT_EXCEPTION(); - Logging::Telemetry().LogException(command->FullName(), "unknown", {}); - context.Reporter.Error() << - Resource::String::UnexpectedErrorExecutingCommand << " ???"_liv << std::endl; - return APPINSTALLER_CLI_ERROR_COMMAND_FAILED; - } - - if (SUCCEEDED(context.GetTerminationHR())) - { - Logging::Telemetry().LogCommandSuccess(command->FullName()); - } - - return context.GetTerminationHR(); + return Execute(context, command); } // End of the line exceptions that are not ever expected. // Telemetry cannot be reliable beyond this point, so don't let these happen. @@ -173,5 +124,4 @@ namespace AppInstaller::CLI { return APPINSTALLER_CLI_ERROR_INTERNAL_ERROR; } - } diff --git a/src/AppInstallerCLICore/ExecutionContext.cpp b/src/AppInstallerCLICore/ExecutionContext.cpp index cba9155b47..47385895ce 100644 --- a/src/AppInstallerCLICore/ExecutionContext.cpp +++ b/src/AppInstallerCLICore/ExecutionContext.cpp @@ -126,4 +126,18 @@ namespace AppInstaller::CLI::Execution m_isTerminated = true; m_terminationHR = hr; } + + void Context::SetExecutionStage(Workflow::ExecutionStage stage, bool allowBackward) + { + if (m_executionStage == stage) + { + return; + } + else if (m_executionStage > stage && !allowBackward) + { + THROW_HR_MSG(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), "Reporting ExecutionStage to an earlier Stage without allowBackward as true"); + } + + m_executionStage = stage; + } } diff --git a/src/AppInstallerCLICore/ExecutionContext.h b/src/AppInstallerCLICore/ExecutionContext.h index f442a8189f..67777efe9a 100644 --- a/src/AppInstallerCLICore/ExecutionContext.h +++ b/src/AppInstallerCLICore/ExecutionContext.h @@ -101,6 +101,8 @@ namespace AppInstaller::CLI::Execution WI_ClearAllFlags(m_flags, flags); } + virtual void SetExecutionStage(Workflow::ExecutionStage stage, bool); + #ifndef AICLI_DISABLE_TEST_HOOKS // Enable tests to override behavior virtual bool ShouldExecuteWorkflowTask(const Workflow::WorkflowTask&) { return true; } @@ -112,5 +114,6 @@ namespace AppInstaller::CLI::Execution HRESULT m_terminationHR = S_OK; size_t m_CtrlSignalCount = 0; ContextFlag m_flags = ContextFlag::None; + Workflow::ExecutionStage m_executionStage = Workflow::ExecutionStage::Initial; }; } diff --git a/src/AppInstallerCLICore/ExecutionContextData.h b/src/AppInstallerCLICore/ExecutionContextData.h index 7349c6fe16..2bfd806cb6 100644 --- a/src/AppInstallerCLICore/ExecutionContextData.h +++ b/src/AppInstallerCLICore/ExecutionContextData.h @@ -36,7 +36,6 @@ namespace AppInstaller::CLI::Execution InstallerArgs, CompletionData, InstalledPackageVersion, - ExecutionStage, UninstallString, PackageFamilyNames, ProductCodes, @@ -137,12 +136,6 @@ namespace AppInstaller::CLI::Execution using value_t = std::shared_ptr; }; - template <> - struct DataMapping - { - using value_t = Workflow::ExecutionStage; - }; - template <> struct DataMapping { diff --git a/src/AppInstallerCLICore/ExecutionReporter.cpp b/src/AppInstallerCLICore/ExecutionReporter.cpp index 4e4bc298aa..ebffe59efa 100644 --- a/src/AppInstallerCLICore/ExecutionReporter.cpp +++ b/src/AppInstallerCLICore/ExecutionReporter.cpp @@ -20,7 +20,9 @@ namespace AppInstaller::CLI::Execution m_in(inStream), m_progressBar(std::in_place, outStream, IsVTEnabled()), m_spinner(std::in_place, outStream, IsVTEnabled()) - {} + { + SetProgressSink(this); + } Reporter::~Reporter() { @@ -120,6 +122,22 @@ namespace AppInstaller::CLI::Execution m_progressBar->ShowProgress(current, maximum, type); } } + + void Reporter::BeginProgress() + { + GetBasicOutputStream() << VirtualTerminal::Cursor::Visibility::DisableShow; + ShowIndefiniteProgress(true); + }; + + void Reporter::EndProgress(bool hideProgressWhenDone) + { + ShowIndefiniteProgress(false); + if (m_progressBar) + { + m_progressBar->EndProgress(hideProgressWhenDone); + } + GetBasicOutputStream() << VirtualTerminal::Cursor::Visibility::EnableShow; + }; void Reporter::SetProgressCallback(ProgressCallback* callback) { diff --git a/src/AppInstallerCLICore/ExecutionReporter.h b/src/AppInstallerCLICore/ExecutionReporter.h index 16ae7ba327..a6446b31a6 100644 --- a/src/AppInstallerCLICore/ExecutionReporter.h +++ b/src/AppInstallerCLICore/ExecutionReporter.h @@ -88,28 +88,23 @@ namespace AppInstaller::CLI::Execution void ShowIndefiniteProgress(bool running); // IProgressSink + void BeginProgress() override; void OnProgress(uint64_t current, uint64_t maximum, ProgressType type) override; + void EndProgress(bool hideProgressWhenDone) override; // Runs the given callable of type: auto(IProgressCallback&) template auto ExecuteWithProgress(F&& f, bool hideProgressWhenDone = false) { - GetBasicOutputStream() << VirtualTerminal::Cursor::Visibility::DisableShow; - - ProgressCallback callback(this); + IProgressSink* sink = m_progressSink.load(); + ProgressCallback callback(sink); SetProgressCallback(&callback); - ShowIndefiniteProgress(true); + sink->BeginProgress(); auto hideProgress = wil::scope_exit([this, hideProgressWhenDone]() { SetProgressCallback(nullptr); - ShowIndefiniteProgress(false); - if (m_progressBar) - { - m_progressBar->EndProgress(hideProgressWhenDone); - } - - GetBasicOutputStream() << VirtualTerminal::Cursor::Visibility::EnableShow; + m_progressSink.load()->EndProgress(hideProgressWhenDone); }); return f(callback); } @@ -120,6 +115,11 @@ namespace AppInstaller::CLI::Execution // Cancels the in progress task. void CancelInProgressTask(bool force); + void SetProgressSink(IProgressSink* sink) + { + m_progressSink = sink; + } + private: // Gets whether VT is enabled for this reporter. bool IsVTEnabled() const; @@ -136,6 +136,7 @@ namespace AppInstaller::CLI::Execution std::optional m_progressBar; wil::srwlock m_progressCallbackLock; std::atomic m_progressCallback; + std::atomic m_progressSink; }; // Indirection to enable change without tracking down every place diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp index 4cc06f84a2..118afc08d4 100644 --- a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp +++ b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp @@ -692,24 +692,7 @@ namespace AppInstaller::CLI::Workflow void ReportExecutionStage::operator()(Execution::Context& context) const { - if (!context.Contains(Execution::Data::ExecutionStage)) - { - context.Add(m_stage); - } - else if (context.Get() == m_stage) - { - return; - } - else if (context.Get() < m_stage || m_allowBackward) - { - context.Get() = m_stage; - } - else - { - THROW_HR_MSG(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), "Reporting ExecutionStage to an earlier Stage without allowBackward as true"); - } - - Logging::SetExecutionStage(static_cast(context.Get())); + context.SetExecutionStage(m_stage, m_allowBackward); } } diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.h b/src/AppInstallerCLICore/Workflows/WorkflowBase.h index 51fdb10c7b..49ee23efff 100644 --- a/src/AppInstallerCLICore/Workflows/WorkflowBase.h +++ b/src/AppInstallerCLICore/Workflows/WorkflowBase.h @@ -19,6 +19,7 @@ namespace AppInstaller::CLI::Workflow // Values are ordered in a typical workflow stages enum class ExecutionStage : uint32_t { + Initial = 0, ParseArgs = 1000, Discovery = 2000, Download = 3000, diff --git a/src/AppInstallerCLITests/TestCommon.cpp b/src/AppInstallerCLITests/TestCommon.cpp index e6ba3b3b00..f101d196aa 100644 --- a/src/AppInstallerCLITests/TestCommon.cpp +++ b/src/AppInstallerCLITests/TestCommon.cpp @@ -139,6 +139,14 @@ namespace TestCommon } } + void TestProgress::BeginProgress() + { + } + + void TestProgress::EndProgress(bool) + { + } + bool TestProgress::IsCancelled() { return false; diff --git a/src/AppInstallerCLITests/TestCommon.h b/src/AppInstallerCLITests/TestCommon.h index aab515acf3..0ef24f2ffa 100644 --- a/src/AppInstallerCLITests/TestCommon.h +++ b/src/AppInstallerCLITests/TestCommon.h @@ -92,7 +92,12 @@ namespace TestCommon struct TestProgress : public AppInstaller::IProgressCallback { // Inherited via IProgressCallback + void BeginProgress() override; + void OnProgress(uint64_t current, uint64_t maximum, AppInstaller::ProgressType type) override; + + void EndProgress(bool) override; + bool IsCancelled() override; CancelFunctionRemoval SetCancellationFunction(std::function&& f) override; diff --git a/src/AppInstallerCommonCore/Public/AppInstallerProgress.h b/src/AppInstallerCommonCore/Public/AppInstallerProgress.h index 0f346c1f3f..b58b5f4b65 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerProgress.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerProgress.h @@ -18,7 +18,7 @@ namespace AppInstaller } // The semantic meaning of the progress values. - enum class ProgressType + enum class ProgressType: uint32_t { // Progress will not be sent. None, @@ -34,6 +34,12 @@ namespace AppInstaller // Called as progress is made. // If maximum is 0, the maximum is unknown. virtual void OnProgress(uint64_t current, uint64_t maximum, ProgressType type) = 0; + + // Called as progress begins. + virtual void BeginProgress() = 0; + + // Called as progress ends. + virtual void EndProgress(bool hideProgressWhenDone) = 0; }; // Callback interface given to the worker to report to. @@ -55,6 +61,15 @@ namespace AppInstaller ProgressCallback() = default; ProgressCallback(IProgressSink* sink) : m_sink(sink) {} + void BeginProgress() override + { + IProgressSink* sink = GetSink(); + if (sink) + { + sink->BeginProgress(); + } + }; + void OnProgress(uint64_t current, uint64_t maximum, ProgressType type) override { IProgressSink* sink = GetSink(); @@ -64,6 +79,15 @@ namespace AppInstaller } } + void EndProgress(bool hideProgressWhenDone) override + { + IProgressSink* sink = GetSink(); + if (sink) + { + sink->EndProgress(hideProgressWhenDone); + } + }; + bool IsCancelled() override { return m_cancelled.load();