commit 16fd465870a911a5683903abf0af7e277d32a631 parent 02d2f93807c9851d73eaacb4d8811a76b64b7b01 Author: JohnMcPMS <johnmcp@microsoft.com> Date: Mon, 17 Apr 2023 14:38:57 -0700 Improve configure user experience (#3158) This change improves the `configure` user experience by: 1. Ensuring that the configuration file exists before doing any other initialization. This speeds up the time to getting the error dramatically if the file does not exist. 2. Creating a better mechanism to report progress of async activities and using it to give the user information on what is happening during the beginning of the `configure` command. 3. Adding specific error strings for every known configuration error and error source, as well as generic fallbacks. 4. Outputting the result description in the event that it is supplied, and informing the user that there is additional information in the log file in case it is truncated or the `Details` property is not empty. It also logs the entire configuration file input stream to the log in verbose, and improves the progress reporting for the pre-check of the apply flow. The initialization phases are reported as: - "Initializing configuration system" while creating the server process and factory objects. - "Reading configuration file" while parsing the configuration file (this is usually fast enough that you won't see it). - "Retrieving configuration details" while we wait for the detailed information to come in about each configuration unit. This is reported repeatedly any time we are still waiting on more details. Diffstat:
38 files changed, 478 insertions(+), 81 deletions(-)
diff --git a/src/AppInstallerCLICore/ChannelStreams.cpp b/src/AppInstallerCLICore/ChannelStreams.cpp @@ -6,7 +6,6 @@ namespace AppInstaller::CLI::Execution { - using namespace Settings; using namespace VirtualTerminal; size_t GetConsoleWidth() diff --git a/src/AppInstallerCLICore/Command.cpp b/src/AppInstallerCLICore/Command.cpp @@ -4,6 +4,8 @@ #include "Command.h" #include "Resources.h" #include <winget/UserSettings.h> +#include <AppInstallerRuntime.h> +#include <winget/Locale.h> using namespace std::string_view_literals; using namespace AppInstaller::Utility::literals; diff --git a/src/AppInstallerCLICore/Commands/ConfigureCommand.cpp b/src/AppInstallerCLICore/Commands/ConfigureCommand.cpp @@ -54,6 +54,7 @@ namespace AppInstaller::CLI void ConfigureCommand::ExecuteInternal(Execution::Context& context) const { context << + VerifyFile(Execution::Args::Type::ConfigurationFile) << CreateConfigurationProcessor << OpenConfigurationSet << ShowConfigurationSet << diff --git a/src/AppInstallerCLICore/Commands/ConfigureShowCommand.cpp b/src/AppInstallerCLICore/Commands/ConfigureShowCommand.cpp @@ -35,6 +35,7 @@ namespace AppInstaller::CLI void ConfigureShowCommand::ExecuteInternal(Execution::Context& context) const { context << + VerifyFile(Execution::Args::Type::ConfigurationFile) << CreateConfigurationProcessor << OpenConfigurationSet << ShowConfigurationSet; diff --git a/src/AppInstallerCLICore/Commands/RootCommand.cpp b/src/AppInstallerCLICore/Commands/RootCommand.cpp @@ -2,6 +2,7 @@ // Licensed under the MIT License. #include "pch.h" #include "RootCommand.h" +#include <AppInstallerRuntime.h> #include "InstallCommand.h" #include "ShowCommand.h" diff --git a/src/AppInstallerCLICore/Commands/ValidateCommand.cpp b/src/AppInstallerCLICore/Commands/ValidateCommand.cpp @@ -5,6 +5,7 @@ #include "Workflows/WorkflowBase.h" #include "Workflows/DependenciesFlow.h" #include "Resources.h" +#include <winget/ManifestYamlParser.h> namespace AppInstaller::CLI { diff --git a/src/AppInstallerCLICore/CompletionData.cpp b/src/AppInstallerCLICore/CompletionData.cpp @@ -3,12 +3,13 @@ #include "pch.h" #include "CompletionData.h" #include "Resources.h" +#include <AppInstallerLogging.h> +#include <AppInstallerErrors.h> namespace AppInstaller::CLI { using namespace std::string_view_literals; using namespace Utility::literals; - using namespace Settings; // Completion takes in the following values: // Word :: The token from the command line that is being targeted for completion. diff --git a/src/AppInstallerCLICore/ConfigurationSetProcessorFactoryRemoting.cpp b/src/AppInstallerCLICore/ConfigurationSetProcessorFactoryRemoting.cpp @@ -2,6 +2,8 @@ // Licensed under the MIT License. #include "pch.h" #include "ConfigurationSetProcessorFactoryRemoting.h" +#include <AppInstallerLogging.h> +#include <AppInstallerRuntime.h> using namespace winrt::Windows::Foundation; using namespace winrt::Microsoft::Management::Configuration; diff --git a/src/AppInstallerCLICore/ExecutionProgress.cpp b/src/AppInstallerCLICore/ExecutionProgress.cpp @@ -166,7 +166,12 @@ namespace AppInstaller::CLI::Execution void ProgressVisualizerBase::SetMessage(std::string_view message) { - m_message = message; + std::atomic_store(&m_message, std::make_shared<std::string>(message)); + } + + std::shared_ptr<std::string> ProgressVisualizerBase::GetMessage() + { + return std::atomic_load(&m_message); } } @@ -213,7 +218,8 @@ namespace AppInstaller::CLI::Execution ApplyStyle(i % repetitionCount, repetitionCount, true); m_out << '\r' << indent << spinnerChars[i % ARRAYSIZE(spinnerChars)]; m_out.RestoreDefault(); - m_out << ' ' << m_message << std::flush; + std::shared_ptr<std::string> message = this->GetMessage(); + m_out << ' ' << (message ? *message : std::string{}) << std::flush; Sleep(250); } diff --git a/src/AppInstallerCLICore/ExecutionProgress.h b/src/AppInstallerCLICore/ExecutionProgress.h @@ -11,6 +11,7 @@ #include <atomic> #include <future> #include <istream> +#include <memory> #include <ostream> #include <string> #include <vector> @@ -28,11 +29,11 @@ namespace AppInstaller::CLI::Execution void SetStyle(AppInstaller::Settings::VisualStyle style) { m_style = style; } void SetMessage(std::string_view message); + std::shared_ptr<std::string> GetMessage(); protected: BaseStream& m_out; Settings::VisualStyle m_style = AppInstaller::Settings::VisualStyle::Accent; - std::string m_message; bool UseVT() const { return m_enableVT && m_style != AppInstaller::Settings::VisualStyle::NoVT; } @@ -43,6 +44,7 @@ namespace AppInstaller::CLI::Execution private: bool m_enableVT = false; + std::shared_ptr<std::string> m_message; }; } diff --git a/src/AppInstallerCLICore/ExecutionReporter.cpp b/src/AppInstallerCLICore/ExecutionReporter.cpp @@ -2,6 +2,7 @@ // Licensed under the MIT License. #include "pch.h" #include "ExecutionReporter.h" +#include <AppInstallerErrors.h> namespace AppInstaller::CLI::Execution @@ -246,9 +247,50 @@ namespace AppInstaller::CLI::Execution GetBasicOutputStream() << VirtualTerminal::Cursor::Visibility::EnableShow; }; + Reporter::AsyncProgressScope::AsyncProgressScope(Reporter& reporter, IProgressSink* sink, bool hideProgressWhenDone) : + m_reporter(reporter), m_callback(sink) + { + reporter.SetProgressCallback(&m_callback); + sink->BeginProgress(); + m_hideProgressWhenDone = hideProgressWhenDone; + } + + Reporter::AsyncProgressScope::~AsyncProgressScope() + { + m_reporter.get().SetProgressCallback(nullptr); + m_callback.GetSink()->EndProgress(m_hideProgressWhenDone); + } + + ProgressCallback& Reporter::AsyncProgressScope::Callback() + { + return m_callback; + } + + IProgressCallback* Reporter::AsyncProgressScope::operator->() + { + return &m_callback; + } + + bool Reporter::AsyncProgressScope::HideProgressWhenDone() const + { + return m_hideProgressWhenDone; + } + + void Reporter::AsyncProgressScope::HideProgressWhenDone(bool value) + { + m_hideProgressWhenDone.store(value); + } + + std::unique_ptr<Reporter::AsyncProgressScope> Reporter::BeginAsyncProgress(bool hideProgressWhenDone) + { + return std::make_unique<AsyncProgressScope>(*this, m_progressSink.load(), hideProgressWhenDone); + } + void Reporter::SetProgressCallback(ProgressCallback* callback) { auto lock = m_progressCallbackLock.lock_exclusive(); + // Attempting two progress operations at the same time; not supported. + THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), m_progressCallback != nullptr && callback != nullptr); m_progressCallback = callback; } diff --git a/src/AppInstallerCLICore/ExecutionReporter.h b/src/AppInstallerCLICore/ExecutionReporter.h @@ -13,6 +13,7 @@ #include <atomic> #include <iomanip> #include <istream> +#include <memory> #include <optional> #include <ostream> #include <string> @@ -116,23 +117,41 @@ namespace AppInstaller::CLI::Execution void SetProgressMessage(std::string_view message) override; void EndProgress(bool hideProgressWhenDone) override; + // Contains the objects used for async progress and the lifetime of said progress. + struct AsyncProgressScope + { + AsyncProgressScope(Reporter& reporter, IProgressSink* sink, bool hideProgressWhenDone); + ~AsyncProgressScope(); + + AsyncProgressScope(const AsyncProgressScope&) = delete; + AsyncProgressScope& operator=(const AsyncProgressScope&) = delete; + + AsyncProgressScope(AsyncProgressScope&&) = delete; + AsyncProgressScope& operator=(AsyncProgressScope&&) = delete; + + ProgressCallback& Callback(); + IProgressCallback* operator->(); + + bool HideProgressWhenDone() const; + void HideProgressWhenDone(bool value); + + private: + std::reference_wrapper<Reporter> m_reporter; + ProgressCallback m_callback; + std::atomic_bool m_hideProgressWhenDone; + }; + // Runs the given callable of type: auto(IProgressCallback&) template <typename F> auto ExecuteWithProgress(F&& f, bool hideProgressWhenDone = false) { - IProgressSink* sink = m_progressSink.load(); - ProgressCallback callback(sink); - SetProgressCallback(&callback); - sink->BeginProgress(); - - auto hideProgress = wil::scope_exit([this, hideProgressWhenDone]() - { - SetProgressCallback(nullptr); - m_progressSink.load()->EndProgress(hideProgressWhenDone); - }); - return f(callback); + auto progressScope = BeginAsyncProgress(hideProgressWhenDone); + return f(progressScope->Callback()); } + // Begins an asynchronous progress operation. + std::unique_ptr<AsyncProgressScope> BeginAsyncProgress(bool hideProgressWhenDone = false); + // Sets the in progress callback. void SetProgressCallback(ProgressCallback* callback); diff --git a/src/AppInstallerCLICore/PackageCollection.cpp b/src/AppInstallerCLICore/PackageCollection.cpp @@ -1,11 +1,11 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. #include "pch.h" - #include "PackageCollection.h" -#include "AppInstallerRuntime.h" -#include "winget/JsonSchemaValidation.h" +#include <AppInstallerLogging.h> +#include <AppInstallerRuntime.h> +#include <winget/JsonSchemaValidation.h> #include "PackagesSchema.h" diff --git a/src/AppInstallerCLICore/PortableInstaller.cpp b/src/AppInstallerCLICore/PortableInstaller.cpp @@ -10,6 +10,7 @@ #include "Microsoft/PortableIndex.h" #include "Microsoft/Schema/IPortableIndex.h" #include <AppInstallerErrors.h> +#include <AppInstallerRuntime.h> using namespace AppInstaller::Utility; using namespace AppInstaller::Registry; diff --git a/src/AppInstallerCLICore/Resources.h b/src/AppInstallerCLICore/Resources.h @@ -51,6 +51,7 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationApply); WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationAssert); WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationDependencies); + WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationDescriptionWasTruncated); WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationFailedToApply); WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationFailedToGetDetails); WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationFieldInvalid); @@ -58,13 +59,36 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationFileEmpty); WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationFileInvalid); WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationFileVersionUnknown); + WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationGettingDetails); WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationInform); + WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationInitializing); WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationLocal); WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationModuleNameOnly); WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationModuleWithDetails); + WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationReadingConfigFile); WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationSettings); WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationSuccessfullyApplied); + WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitAssertHadNegativeResult); WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitFailed); + WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitFailedConfigSet); + WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitFailedDuringGet); + WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitFailedDuringSet); + WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitFailedDuringTest); + WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitFailedInternal); + WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitFailedPrecondition); + WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitFailedSystemState); + WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitFailedUnitProcessing); + WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitHasDuplicateIdentifier); + WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitHasMissingDependency); + WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitManuallySkipped); + WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitModuleConflict); + WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitModuleImportFailed); + WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitMultipleMatches); + WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitNotFound); + WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitNotFoundInModule); + WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitNotRunDueToDependency); + WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitNotRunDueToFailedAssert); + WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitReturnedInvalidResult); WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitSkipped); WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationWaitingOnAnother); WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationWarning); diff --git a/src/AppInstallerCLICore/VTSupport.cpp b/src/AppInstallerCLICore/VTSupport.cpp @@ -2,6 +2,7 @@ // Licensed under the MIT License. #include "pch.h" #include "VTSupport.h" +#include <AppInstallerLogging.h> namespace AppInstaller::CLI::VirtualTerminal diff --git a/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp b/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp @@ -4,6 +4,7 @@ #include "ConfigurationFlow.h" #include "PromptFlow.h" #include "ConfigurationSetProcessorFactoryRemoting.h" +#include <AppInstallerErrors.h> #include <winrt/Microsoft.Management.Configuration.h> using namespace AppInstaller::CLI::Execution; @@ -361,6 +362,62 @@ namespace AppInstaller::CLI::Workflow } } + struct UnitFailedMessageData + { + Utility::LocIndString Message; + bool ShowDescription = true; + }; + + // TODO: We may need a detailed result code to enable the internal error to be exposed. + // Additionally, some of the processor exceptions that generate these errors should be enlightened to produce better, localized descriptions. + UnitFailedMessageData GetUnitFailedData(const ConfigurationUnit& unit, const ConfigurationUnitResultInformation& resultInformation) + { + int32_t resultCode = resultInformation.ResultCode(); + + switch (resultCode) + { + case WINGET_CONFIG_ERROR_DUPLICATE_IDENTIFIER: return { Resource::String::ConfigurationUnitHasDuplicateIdentifier(Utility::LocIndString{ Utility::ConvertToUTF8(unit.Identifier()) }), false }; + case WINGET_CONFIG_ERROR_MISSING_DEPENDENCY: return { Resource::String::ConfigurationUnitHasMissingDependency(Utility::LocIndString{ Utility::ConvertToUTF8(resultInformation.Details()) }), false }; + case WINGET_CONFIG_ERROR_ASSERTION_FAILED: return { Resource::String::ConfigurationUnitAssertHadNegativeResult(), false }; + case WINGET_CONFIG_ERROR_UNIT_NOT_INSTALLED: return { Resource::String::ConfigurationUnitNotFoundInModule(), false }; + case WINGET_CONFIG_ERROR_UNIT_NOT_FOUND_REPOSITORY: return { Resource::String::ConfigurationUnitNotFound(), false }; + case WINGET_CONFIG_ERROR_UNIT_MULTIPLE_MATCHES: return { Resource::String::ConfigurationUnitMultipleMatches(), false }; + case WINGET_CONFIG_ERROR_UNIT_INVOKE_GET: return { Resource::String::ConfigurationUnitFailedDuringGet(), true }; + case WINGET_CONFIG_ERROR_UNIT_INVOKE_TEST: return { Resource::String::ConfigurationUnitFailedDuringTest(), true }; + case WINGET_CONFIG_ERROR_UNIT_INVOKE_SET: return { Resource::String::ConfigurationUnitFailedDuringSet(), true }; + case WINGET_CONFIG_ERROR_UNIT_MODULE_CONFLICT: return { Resource::String::ConfigurationUnitModuleConflict(), false }; + case WINGET_CONFIG_ERROR_UNIT_IMPORT_MODULE: return { Resource::String::ConfigurationUnitModuleImportFailed(), true }; + case WINGET_CONFIG_ERROR_UNIT_INVOKE_INVALID_RESULT: return { Resource::String::ConfigurationUnitReturnedInvalidResult(), false }; + } + + switch (resultInformation.ResultSource()) + { + case ConfigurationUnitResultSource::ConfigurationSet: return { Resource::String::ConfigurationUnitFailedConfigSet(resultCode), true }; + case ConfigurationUnitResultSource::Internal: return { Resource::String::ConfigurationUnitFailedInternal(resultCode), true }; + case ConfigurationUnitResultSource::Precondition: return { Resource::String::ConfigurationUnitFailedPrecondition(resultCode), true }; + case ConfigurationUnitResultSource::SystemState: return { Resource::String::ConfigurationUnitFailedSystemState(resultCode), true }; + case ConfigurationUnitResultSource::UnitProcessing: return { Resource::String::ConfigurationUnitFailedUnitProcessing(resultCode), true }; + } + + // All other errors use a generic message + return { Resource::String::ConfigurationUnitFailed(resultCode), true }; + } + + Utility::LocIndString GetUnitSkippedMessage(const ConfigurationUnitResultInformation& resultInformation) + { + int32_t resultCode = resultInformation.ResultCode(); + + switch (resultInformation.ResultCode()) + { + case WINGET_CONFIG_ERROR_MANUALLY_SKIPPED: return Resource::String::ConfigurationUnitManuallySkipped(); + case WINGET_CONFIG_ERROR_DEPENDENCY_UNSATISFIED: return Resource::String::ConfigurationUnitNotRunDueToDependency(); + case WINGET_CONFIG_ERROR_ASSERTION_FAILED: return Resource::String::ConfigurationUnitNotRunDueToFailedAssert(); + } + + // If new cases arise and are not handled here, at least have a generic backstop message. + return Resource::String::ConfigurationUnitSkipped(resultCode); + } + // Helper to handle progress callbacks from ApplyConfigurationSetAsync struct ApplyConfigurationSetProgressOutput { @@ -417,6 +474,11 @@ namespace AppInstaller::CLI::Workflow private: void HandleUnitProgress(const ConfigurationUnit& unit, ConfigurationUnitState state, const ConfigurationUnitResultInformation& resultInformation) { + if (UnitHasPreviouslyCompleted(unit)) + { + return; + } + switch (state) { case ConfigurationUnitState::Pending: @@ -435,20 +497,41 @@ namespace AppInstaller::CLI::Workflow } else { - AICLI_LOG(Config, Error, << "Configuration unit " << Utility::ConvertToUTF8(unit.UnitName()) << "[" << Utility::ConvertToUTF8(unit.Identifier()) << "] failed with code 0x" - << Logging::SetHRFormat << resultInformation.ResultCode() << " and error message:\n" << Utility::ConvertToUTF8(resultInformation.Description()) << '\n' - << Utility::ConvertToUTF8(resultInformation.Details())); - // TODO: Improve error reporting for failures: use message, known HRs, getting HR system string, etc. - m_context.Reporter.Error() << " "_liv << Resource::String::ConfigurationUnitFailed << " 0x"_liv << Logging::SetHRFormat << resultInformation.ResultCode() << std::endl; + std::string description = Utility::Trim(Utility::ConvertToUTF8(resultInformation.Description())); + + AICLI_LOG_LARGE_STRING(Config, Error, << "Configuration unit " << Utility::ConvertToUTF8(unit.UnitName()) << "[" << Utility::ConvertToUTF8(unit.Identifier()) << "] failed with code 0x" + << Logging::SetHRFormat << resultInformation.ResultCode() << " and error message:\n" << description, Utility::ConvertToUTF8(resultInformation.Details())); + + UnitFailedMessageData messageData = GetUnitFailedData(unit, resultInformation); + auto error = m_context.Reporter.Error(); + error << " "_liv << messageData.Message << std::endl; + + if (messageData.ShowDescription && !description.empty()) + { + constexpr size_t maximumDescriptionLines = 3; + size_t consoleWidth = GetConsoleWidth(); + std::vector<std::string> lines = Utility::SplitIntoLines(description, maximumDescriptionLines + 1); + bool wasLimited = Utility::LimitOutputLines(lines, consoleWidth, maximumDescriptionLines); + + for (const auto & line : lines) + { + error << line << std::endl; + } + + if (wasLimited || !resultInformation.Details().empty()) + { + error << Resource::String::ConfigurationDescriptionWasTruncated << std::endl; + } + } } OutputUnitCompletionProgress(); + MarkCompleted(unit); break; case ConfigurationUnitState::Skipped: OutputUnitInProgressIfNeeded(unit); - AICLI_LOG(Config, Error, << "Configuration unit " << Utility::ConvertToUTF8(unit.UnitName()) << "[" << Utility::ConvertToUTF8(unit.Identifier()) << "] was skipped with code 0x" + AICLI_LOG(Config, Warning, << "Configuration unit " << Utility::ConvertToUTF8(unit.UnitName()) << "[" << Utility::ConvertToUTF8(unit.Identifier()) << "] was skipped with code 0x" << Logging::SetHRFormat << resultInformation.ResultCode()); - // TODO: Unique message per skip reason? - m_context.Reporter.Warn() << " "_liv << Resource::String::ConfigurationUnitSkipped << " 0x"_liv << Logging::SetHRFormat << resultInformation.ResultCode() << std::endl; + m_context.Reporter.Warn() << " "_liv << GetUnitSkippedMessage(resultInformation) << std::endl; OutputUnitCompletionProgress(); break; } @@ -466,6 +549,18 @@ namespace AppInstaller::CLI::Workflow } } + void MarkCompleted(const ConfigurationUnit& unit) + { + winrt::guid unitInstance = unit.InstanceIdentifier(); + m_unitsCompleted.insert(unitInstance); + } + + bool UnitHasPreviouslyCompleted(const ConfigurationUnit& unit) + { + winrt::guid unitInstance = unit.InstanceIdentifier(); + return m_unitsCompleted.count(unitInstance) != 0; + } + // Sends VT progress to the console void OutputUnitCompletionProgress() { @@ -477,12 +572,22 @@ namespace AppInstaller::CLI::Workflow Context& m_context; std::set<winrt::guid> m_unitsSeen; + std::set<winrt::guid> m_unitsCompleted; bool m_isFirstProgress = true; }; + + std::filesystem::path GetConfigurationFilePath(Execution::Context& context) + { + std::filesystem::path argPath = Utility::ConvertToUTF16(context.Args.GetArg(Args::Type::ConfigurationFile)); + return std::filesystem::weakly_canonical(argPath); + } } void CreateConfigurationProcessor(Context& context) { + auto progressScope = context.Reporter.BeginAsyncProgress(true); + progressScope->Callback().SetProgressMessage(Resource::String::ConfigurationInitializing()); + ConfigurationProcessor processor{ CreateConfigurationSetProcessorFactory() }; // Set the processor to the current level of the logging. @@ -507,13 +612,10 @@ namespace AppInstaller::CLI::Workflow void OpenConfigurationSet(Context& context) { - std::filesystem::path argPath = Utility::ConvertToUTF16(context.Args.GetArg(Args::Type::ConfigurationFile)); - std::filesystem::path absolutePath = std::filesystem::weakly_canonical(argPath); - if (!std::filesystem::exists(absolutePath)) - { - context.Reporter.Error() << Resource::String::FileNotFound << std::endl; - AICLI_TERMINATE_CONTEXT(HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND)); - } + auto progressScope = context.Reporter.BeginAsyncProgress(true); + progressScope->Callback().SetProgressMessage(Resource::String::ConfigurationReadingConfigFile()); + + std::filesystem::path absolutePath = GetConfigurationFilePath(context); Streams::IInputStream inputStream = nullptr; inputStream = Streams::FileRandomAccessStream::OpenAsync(absolutePath.wstring(), FileAccessMode::Read).get(); @@ -561,6 +663,10 @@ namespace AppInstaller::CLI::Workflow AICLI_TERMINATE_CONTEXT(S_FALSE); } + auto gettingDetailString = Resource::String::ConfigurationGettingDetails(); + auto progressScope = context.Reporter.BeginAsyncProgress(true); + progressScope->Callback().SetProgressMessage(gettingDetailString); + auto getDetailsOperation = configContext.Processor().GetSetDetailsAsync(configContext.Set(), ConfigurationUnitDetailLevel::Catalog); OutputStream out = context.Reporter.Info(); @@ -570,6 +676,8 @@ namespace AppInstaller::CLI::Workflow { auto threadContext = context.SetForCurrentThread(); + progressScope.reset(); + auto unitResults = operation.GetResults().UnitResults(); for (unitsShown; unitsShown < unitResults.Size(); ++unitsShown) { @@ -577,12 +685,17 @@ namespace AppInstaller::CLI::Workflow LogFailedGetConfigurationUnitDetails(unitResult.Unit(), unitResult.ResultInformation()); OutputConfigurationUnitInformation(out, unitResult.Unit()); } + + progressScope = context.Reporter.BeginAsyncProgress(true); + progressScope->Callback().SetProgressMessage(gettingDetailString); }); try { GetConfigurationSetDetailsResult result = getDetailsOperation.get(); + progressScope.reset(); + // Handle any missing progress callbacks auto unitResults = result.UnitResults(); for (unitsShown; unitsShown < unitResults.Size(); ++unitsShown) @@ -594,6 +707,8 @@ namespace AppInstaller::CLI::Workflow } CATCH_LOG(); + progressScope.reset(); + // In the event of an exception from GetSetDetailsAsync, show the data we do have if (!unitsShown) { diff --git a/src/AppInstallerCLICore/Workflows/DownloadFlow.cpp b/src/AppInstallerCLICore/Workflows/DownloadFlow.cpp @@ -2,10 +2,11 @@ // Licensed under the MIT License. #include "pch.h" #include "DownloadFlow.h" -#include "winget/Filesystem.h" - +#include <winget/Filesystem.h> +#include <AppInstallerRuntime.h> #include <AppInstallerMsixInfo.h> #include <winget/AdminSettings.h> +#include <AppInstallerDownloader.h> namespace AppInstaller::CLI::Workflow { diff --git a/src/AppInstallerCLICore/Workflows/ImportExportFlow.cpp b/src/AppInstallerCLICore/Workflows/ImportExportFlow.cpp @@ -7,6 +7,7 @@ #include "PackageCollection.h" #include "WorkflowBase.h" #include <winget/RepositorySearch.h> +#include <winget/Runtime.h> namespace AppInstaller::CLI::Workflow { diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp @@ -22,6 +22,7 @@ #include <Argument.h> #include <Command.h> #include <AppInstallerSynchronization.h> +#include <winget/Runtime.h> using namespace winrt::Windows::ApplicationModel::Store::Preview::InstallControl; using namespace winrt::Windows::Foundation; diff --git a/src/AppInstallerCLICore/Workflows/MSStoreInstallerHandler.cpp b/src/AppInstallerCLICore/Workflows/MSStoreInstallerHandler.cpp @@ -1,8 +1,9 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. #include "pch.h" -#include <winget/ManifestCommon.h> #include "MSStoreInstallerHandler.h" +#include <winget/ManifestCommon.h> +#include <winget/Runtime.h> namespace AppInstaller::CLI::Workflow { diff --git a/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp b/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp @@ -1,10 +1,12 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. #include "pch.h" -#include "WorkflowBase.h" -#include "ExecutionContext.h" #include "ManifestComparator.h" +#include "WorkflowBase.h" +#include <AppInstallerLogging.h> #include <winget/UserSettings.h> +#include <winget/Runtime.h> +#include <winget/Locale.h> using namespace AppInstaller::CLI; using namespace AppInstaller::Manifest; diff --git a/src/AppInstallerCLICore/Workflows/ManifestComparator.h b/src/AppInstallerCLICore/Workflows/ManifestComparator.h @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. #pragma once +#include "ExecutionContext.h" #include "ExecutionArgs.h" #include <winget/Manifest.h> #include <winget/RepositorySearch.h> diff --git a/src/AppInstallerCLICore/Workflows/MsiInstallFlow.cpp b/src/AppInstallerCLICore/Workflows/MsiInstallFlow.cpp @@ -2,7 +2,8 @@ // Licensed under the MIT License. #include "pch.h" #include "MsiInstallFlow.h" -#include "winget/MsiExecArguments.h" +#include <winget/MsiExecArguments.h> +#include <winget/Runtime.h> namespace AppInstaller::CLI::Workflow { diff --git a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp @@ -2,7 +2,8 @@ // Licensed under the MIT License. #include "pch.h" #include "ShellExecuteInstallerHandler.h" -#include "AppInstallerFileLogger.h" +#include <AppInstallerFileLogger.h> +#include <AppInstallerRuntime.h> using namespace AppInstaller::CLI; using namespace AppInstaller::Utility; diff --git a/src/AppInstallerCLICore/Workflows/UninstallFlow.cpp b/src/AppInstallerCLICore/Workflows/UninstallFlow.cpp @@ -10,6 +10,7 @@ #include "PortableFlow.h" #include <AppInstallerDeployment.h> #include <AppInstallerSynchronization.h> +#include <winget/Runtime.h> using namespace AppInstaller::CLI::Execution; using namespace AppInstaller::Manifest; diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp @@ -9,6 +9,7 @@ #include <winget/ExperimentalFeature.h> #include <winget/ManifestYamlParser.h> #include <winget/Pin.h> +#include <winget/Runtime.h> using namespace std::string_literals; using namespace AppInstaller::Utility::literals; diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.h b/src/AppInstallerCLICore/Workflows/WorkflowBase.h @@ -5,6 +5,7 @@ #include "ExecutionReporter.h" #include <winget/ExperimentalFeature.h> #include <winget/RepositorySearch.h> +#include <winget/RepositorySource.h> #include <string> #include <string_view> diff --git a/src/AppInstallerCLICore/pch.h b/src/AppInstallerCLICore/pch.h @@ -17,6 +17,7 @@ #pragma warning( pop ) #include <array> +#include <atomic> #include <iostream> #include <fstream> #include <future> @@ -50,18 +51,3 @@ #include <wrl/client.h> #include <AppxPackaging.h> - -#include <AppInstallerArchitecture.h> -#include <AppInstallerDateTime.h> -#include <AppInstallerDownloader.h> -#include <AppInstallerErrors.h> -#include <AppInstallerLogging.h> -#include <AppInstallerRuntime.h> -#include <AppInstallerSHA256.h> -#include <AppInstallerStrings.h> -#include <AppInstallerTelemetry.h> -#include <winget/ExperimentalFeature.h> -#include <winget/Locale.h> -#include <winget/LocIndependent.h> -#include <winget/ManifestYamlParser.h> -#include <winget/RepositorySource.h> diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -1787,12 +1787,6 @@ Please specify one of them using the --source option to proceed.</value> <data name="ConfigurationSuccessfullyApplied" xml:space="preserve"> <value>Configuration successfully applied.</value> </data> - <data name="ConfigurationUnitFailed" xml:space="preserve"> - <value>Failed:</value> - </data> - <data name="ConfigurationUnitSkipped" xml:space="preserve"> - <value>Skipped:</value> - </data> <data name="ConfigurationWaitingOnAnother" xml:space="preserve"> <value>Another configuration is being applied to the system. This configuration will continue as soon as is possible...</value> </data> @@ -1807,9 +1801,6 @@ Please specify one of them using the --source option to proceed.</value> <data name="ConfigurationFileEmpty" xml:space="preserve"> <value>The configuration is empty.</value> </data> - <data name="FileNotFound" xml:space="preserve"> - <value>File not found.</value> - </data> <data name="WindowsFeatureNotFound" xml:space="preserve"> <value>The feature [{0}] was not found.</value> <comment>{Locked="{0}"} Error message displayed when a Windows feature was not found on the system.</comment> @@ -1843,4 +1834,92 @@ Please specify one of them using the --source option to proceed.</value> <data name="InstallWaitingOnAnother" xml:space="preserve"> <value>Waiting for another install/uninstall to complete...</value> </data> + <data name="ConfigurationDescriptionWasTruncated" xml:space="preserve"> + <value><See the log file for additional details></value> + <comment>The brackets are intended to make the value stand out from other text which it will follow. Any locale appropriate mechanism that achieves this is acceptable.</comment> + </data> + <data name="ConfigurationGettingDetails" xml:space="preserve"> + <value>Retrieving configuration details</value> + </data> + <data name="ConfigurationInitializing" xml:space="preserve"> + <value>Initializing configuration system</value> + </data> + <data name="ConfigurationReadingConfigFile" xml:space="preserve"> + <value>Reading configuration file</value> + </data> + <data name="ConfigurationUnitAssertHadNegativeResult" xml:space="preserve"> + <value>The system is not in the desired state asserted by the configuration.</value> + </data> + <data name="ConfigurationUnitFailed" xml:space="preserve"> + <value>This configuration unit failed for an unknown reason: {0}</value> + <comment>{Locked="{0}"} {0} is a placeholder for the unrecognized error code.</comment> + </data> + <data name="ConfigurationUnitFailedConfigSet" xml:space="preserve"> + <value>The configuration unit failed due to the configuration: {0}</value> + <comment>{Locked="{0}"} {0} is a placeholder for the unrecognized error code.</comment> + </data> + <data name="ConfigurationUnitFailedDuringGet" xml:space="preserve"> + <value>The configuration unit failed while attempting to get the current system state.</value> + </data> + <data name="ConfigurationUnitFailedDuringSet" xml:space="preserve"> + <value>The configuration unit failed while attempting to apply the desired state.</value> + </data> + <data name="ConfigurationUnitFailedDuringTest" xml:space="preserve"> + <value>The configuration unit failed while attempting to test the current system state.</value> + </data> + <data name="ConfigurationUnitFailedInternal" xml:space="preserve"> + <value>The configuration unit failed due to an internal error: {0}</value> + <comment>{Locked="{0}"} {0} is a placeholder for the unrecognized error code.</comment> + </data> + <data name="ConfigurationUnitFailedPrecondition" xml:space="preserve"> + <value>The configuration unit failed due to a precondition not being valid: {0}</value> + <comment>{Locked="{0}"} {0} is a placeholder for the unrecognized error code.</comment> + </data> + <data name="ConfigurationUnitFailedSystemState" xml:space="preserve"> + <value>The configuration unit failed due to the system state: {0}</value> + <comment>{Locked="{0}"} {0} is a placeholder for the unrecognized error code.</comment> + </data> + <data name="ConfigurationUnitFailedUnitProcessing" xml:space="preserve"> + <value>The configuration unit failed while attempting to run: {0}</value> + <comment>{Locked="{0}"} {0} is a placeholder for the unrecognized error code.</comment> + </data> + <data name="ConfigurationUnitHasDuplicateIdentifier" xml:space="preserve"> + <value>The configuration contains the identifier `{0}` multiple times.</value> + <comment>{Locked="{0}"} {0} is a placeholder that is replaced by the identifier string from the user input file.</comment> + </data> + <data name="ConfigurationUnitHasMissingDependency" xml:space="preserve"> + <value>The dependency `{0}` was not found within the configuration.</value> + <comment>{Locked="{0}"} {0} is a placeholder that is replaced by the identifier string from the user input file.</comment> + </data> + <data name="ConfigurationUnitManuallySkipped" xml:space="preserve"> + <value>This configuration unit was manually skipped.</value> + </data> + <data name="ConfigurationUnitModuleConflict" xml:space="preserve"> + <value>The module for the configuration unit is available in multiple locations with the same version.</value> + </data> + <data name="ConfigurationUnitModuleImportFailed" xml:space="preserve"> + <value>Loading the module for the configuration unit failed.</value> + </data> + <data name="ConfigurationUnitMultipleMatches" xml:space="preserve"> + <value>Multiple matches were found for the configuration unit; specify the module to select the correct one.</value> + </data> + <data name="ConfigurationUnitNotFound" xml:space="preserve"> + <value>The configuration unit could not be found.</value> + </data> + <data name="ConfigurationUnitNotFoundInModule" xml:space="preserve"> + <value>The configuration unit was not in the module as expected.</value> + </data> + <data name="ConfigurationUnitNotRunDueToDependency" xml:space="preserve"> + <value>This configuration unit was not run because a dependency failed or was not run.</value> + </data> + <data name="ConfigurationUnitNotRunDueToFailedAssert" xml:space="preserve"> + <value>This configuration unit was not run because an assert failed or was false.</value> + </data> + <data name="ConfigurationUnitReturnedInvalidResult" xml:space="preserve"> + <value>The configuration unit returned an unexpected result during execution.</value> + </data> + <data name="ConfigurationUnitSkipped" xml:space="preserve"> + <value>This configuration unit was not run for an unknown reason: {0}</value> + <comment>{Locked="{0}"} {0} is a placeholder for the unrecognized error code.</comment> + </data> </root> \ No newline at end of file diff --git a/src/AppInstallerSharedLib/AppInstallerStrings.cpp b/src/AppInstallerSharedLib/AppInstallerStrings.cpp @@ -703,6 +703,69 @@ namespace AppInstaller::Utility return result; } + std::vector<std::string> SplitIntoLines(std::string_view input, size_t maximum) + { + std::size_t currentOffset = 0; + std::vector<std::string> result; + + while (currentOffset < input.size() && (!maximum || result.size() < maximum)) + { + std::size_t nextOffset = input.find_first_of("\r\n", currentOffset); + if (nextOffset == std::string_view::npos) + { + nextOffset = input.size(); + } + + if (nextOffset - currentOffset > 1) + { + result.emplace_back(input.substr(currentOffset, nextOffset - currentOffset)); + } + + currentOffset = nextOffset; + } + + return result; + } + + bool LimitOutputLines(std::vector<std::string>& lines, size_t lineWidth, size_t maximum) + { + size_t totalLines = 0; + size_t currentLine = 0; + bool result = false; + + for (; currentLine < lines.size() && totalLines < maximum; ++currentLine) + { + size_t currentLineWidth = UTF8ColumnWidth(lines[currentLine]); + // If current line is empty, the cost is 1 line (0 + 1). + // If not, round up to the next line count (by rounding down through integer division after subtracting 1 + 1). + size_t currentLineActualLineCount = (currentLineWidth ? (currentLineWidth - 1) / lineWidth : 0) + 1; + + // The current line may be too big to be the last line. + size_t availableLines = maximum - totalLines; + if (currentLineActualLineCount > availableLines) + { + size_t actualWidth = 0; + std::string trimmedLine = UTF8TrimRightToColumnWidth(lines[currentLine], (availableLines * lineWidth) - 1, actualWidth); + trimmedLine += "\xE2\x80\xA6"; // UTF8 encoding of ellipsis (…) character + lines[currentLine] = trimmedLine; + + currentLineActualLineCount = availableLines; + result = true; + } + + totalLines += currentLineActualLineCount; + } + + // Drop any unprocessed lines + if (currentLine != lines.size()) + { + lines.resize(currentLine); + result = true; + } + + return result; + } + std::string ConvertToHexString(const std::vector<uint8_t>& buffer, size_t byteCount) { if (byteCount && buffer.size() != byteCount) diff --git a/src/AppInstallerSharedLib/Public/AppInstallerErrors.h b/src/AppInstallerSharedLib/Public/AppInstallerErrors.h @@ -164,6 +164,7 @@ #define WINGET_CONFIG_ERROR_ASSERTION_FAILED ((HRESULT)0x8A15C009) #define WINGET_CONFIG_ERROR_MANUALLY_SKIPPED ((HRESULT)0x8A15C00A) #define WINGET_CONFIG_ERROR_WARNING_NOT_ACCEPTED ((HRESULT)0x8A15C00B) +#define WINGET_CONFIG_ERROR_SET_DEPENDENCY_CYCLE ((HRESULT)0x8A15C00C) // Configuration Processor Errors #define WINGET_CONFIG_ERROR_UNIT_NOT_INSTALLED ((HRESULT)0x8A15C101) diff --git a/src/AppInstallerSharedLib/Public/AppInstallerStrings.h b/src/AppInstallerSharedLib/Public/AppInstallerStrings.h @@ -185,6 +185,14 @@ namespace AppInstaller::Utility // Splits the string into words. std::vector<std::string> SplitIntoWords(std::string_view input); + // Splits the string into lines. + // Drops empty lines. + std::vector<std::string> SplitIntoLines(std::string_view input, size_t maximum = 0); + + // Removes lines from the vector (and/or characters from the last line) so that it contains the maximum number of lines. + // Returns true if changes were made, false if not. + bool LimitOutputLines(std::vector<std::string>& lines, size_t lineWidth, size_t maximum); + // Converts a container to a string representation of it. template <typename T, typename U> std::string ConvertContainerToString(const T& container, U toString) diff --git a/src/Microsoft.Management.Configuration.UnitTests/Helpers/Errors.cs b/src/Microsoft.Management.Configuration.UnitTests/Helpers/Errors.cs @@ -15,16 +15,18 @@ namespace Microsoft.Management.Configuration.UnitTests.Helpers #pragma warning disable SA1600 // Elements should be documented #pragma warning disable SA1025 // Code should not contain multiple whitespace in a row - public static readonly int WINGET_CONFIG_ERROR_INVALID_CONFIGURATION_FILE = unchecked((int)0x8A15C001); - public static readonly int WINGET_CONFIG_ERROR_INVALID_YAML = unchecked((int)0x8A15C002); - public static readonly int WINGET_CONFIG_ERROR_INVALID_FIELD = unchecked((int)0x8A15C003); - public static readonly int WINGET_CONFIG_ERROR_UNKNOWN_CONFIGURATION_FILE_VERSION = unchecked((int)0x8A15C004); - public static readonly int WINGET_CONFIG_ERROR_SET_APPLY_FAILED = unchecked((int)0x8A15C005); - public static readonly int WINGET_CONFIG_ERROR_DUPLICATE_IDENTIFIER = unchecked((int)0x8A15C006); - public static readonly int WINGET_CONFIG_ERROR_MISSING_DEPENDENCY = unchecked((int)0x8A15C007); - public static readonly int WINGET_CONFIG_ERROR_DEPENDENCY_UNSATISFIED = unchecked((int)0x8A15C008); - public static readonly int WINGET_CONFIG_ERROR_ASSERTION_FAILED = unchecked((int)0x8A15C009); - public static readonly int WINGET_CONFIG_ERROR_MANUALLY_SKIPPED = unchecked((int)0x8A15C00A); + public static readonly int WINGET_CONFIG_ERROR_INVALID_CONFIGURATION_FILE = unchecked((int)0x8A15C001); + public static readonly int WINGET_CONFIG_ERROR_INVALID_YAML = unchecked((int)0x8A15C002); + public static readonly int WINGET_CONFIG_ERROR_INVALID_FIELD = unchecked((int)0x8A15C003); + public static readonly int WINGET_CONFIG_ERROR_UNKNOWN_CONFIGURATION_FILE_VERSION = unchecked((int)0x8A15C004); + public static readonly int WINGET_CONFIG_ERROR_SET_APPLY_FAILED = unchecked((int)0x8A15C005); + public static readonly int WINGET_CONFIG_ERROR_DUPLICATE_IDENTIFIER = unchecked((int)0x8A15C006); + public static readonly int WINGET_CONFIG_ERROR_MISSING_DEPENDENCY = unchecked((int)0x8A15C007); + public static readonly int WINGET_CONFIG_ERROR_DEPENDENCY_UNSATISFIED = unchecked((int)0x8A15C008); + public static readonly int WINGET_CONFIG_ERROR_ASSERTION_FAILED = unchecked((int)0x8A15C009); + public static readonly int WINGET_CONFIG_ERROR_MANUALLY_SKIPPED = unchecked((int)0x8A15C00A); + public static readonly int WINGET_CONFIG_ERROR_WARNING_NOT_ACCEPTED = unchecked((int)0x8A15C00B); + public static readonly int WINGET_CONFIG_ERROR_SET_DEPENDENCY_CYCLE = unchecked((int)0x8A15C00C); #pragma warning restore SA1025 // Code should not contain multiple whitespace in a row #pragma warning restore SA1600 // Elements should be documented diff --git a/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationProcessorApplyTests.cs b/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationProcessorApplyTests.cs @@ -171,7 +171,7 @@ namespace Microsoft.Management.Configuration.UnitTests.Tests ApplyConfigurationSetResult result = processor.ApplySet(configurationSet, ApplyConfigurationSetFlags.None); Assert.NotNull(result); Assert.NotNull(result.ResultCode); - Assert.Equal(Errors.WINGET_CONFIG_ERROR_DEPENDENCY_UNSATISFIED, result.ResultCode.HResult); + Assert.Equal(Errors.WINGET_CONFIG_ERROR_SET_DEPENDENCY_CYCLE, result.ResultCode.HResult); Assert.Equal(3, result.UnitResults.Count); foreach (var unitResult in result.UnitResults) diff --git a/src/Microsoft.Management.Configuration/ConfigurationSetApplyProcessor.cpp b/src/Microsoft.Management.Configuration/ConfigurationSetApplyProcessor.cpp @@ -45,14 +45,16 @@ namespace winrt::Microsoft::Management::Configuration::implementation { if (PreProcess()) { - // TODO: When cross process is implemented, send Pending until we actually start + // TODO: Send pending when blocked by another configuration run + //SendProgress(ConfigurationSetState::Pending); + SendProgress(ConfigurationSetState::InProgress); ProcessInternal(HasProcessedSuccessfully, &ConfigurationSetApplyProcessor::ProcessUnit, true); - - SendProgress(ConfigurationSetState::Completed); } + SendProgress(ConfigurationSetState::Completed); + m_telemetry.LogConfigProcessingSummaryForApply(*winrt::get_self<implementation::ConfigurationSet>(m_configurationSet), *m_result); } @@ -99,7 +101,11 @@ namespace winrt::Microsoft::Management::Configuration::implementation { AICLI_LOG(Config, Error, << "Found missing dependency: " << dependency); unitInfo.ResultInformation->Initialize(WINGET_CONFIG_ERROR_MISSING_DEPENDENCY, ConfigurationUnitResultSource::ConfigurationSet); + unitInfo.ResultInformation->Details(dependencyHstring); + SendProgress(ConfigurationUnitState::Completed, unitInfo); result = false; + // TODO: Consider collecting all missing dependencies, for now just the first + break; } else { @@ -115,7 +121,16 @@ namespace winrt::Microsoft::Management::Configuration::implementation return false; } - return ProcessInternal(HasPreprocessed, &ConfigurationSetApplyProcessor::MarkPreprocessed); + if (!ProcessInternal(HasPreprocessed, &ConfigurationSetApplyProcessor::MarkPreprocessed)) + { + // The preprocessing simulates processing as if every unit run was successful. + // If it fails, this means that there are unit definitions whose dependencies cannot be satisfied. + // The only reason for that is a cycle in the dependency graph somewhere. + m_result->ResultCode(WINGET_CONFIG_ERROR_SET_DEPENDENCY_CYCLE); + return false; + } + + return true; } bool ConfigurationSetApplyProcessor::AddUnitToMap(UnitInfo& unitInfo, size_t unitInfoIndex) @@ -133,8 +148,10 @@ namespace winrt::Microsoft::Management::Configuration::implementation { AICLI_LOG(Config, Error, << "Found duplicate identifier: " << identifier); // Found a duplicate identifier, mark both as such - unitInfo.ResultInformation->Initialize(WINGET_CONFIG_ERROR_DUPLICATE_IDENTIFIER, ConfigurationUnitResultSource::ConfigurationSet); m_unitInfo[itr->second].ResultInformation->Initialize(WINGET_CONFIG_ERROR_DUPLICATE_IDENTIFIER, ConfigurationUnitResultSource::ConfigurationSet); + SendProgressIfNotComplete(ConfigurationUnitState::Completed, m_unitInfo[itr->second]); + unitInfo.ResultInformation->Initialize(WINGET_CONFIG_ERROR_DUPLICATE_IDENTIFIER, ConfigurationUnitResultSource::ConfigurationSet); + SendProgress(ConfigurationUnitState::Completed, unitInfo); return false; } else @@ -457,4 +474,12 @@ namespace winrt::Microsoft::Management::Configuration::implementation CATCH_LOG(); } } + + void ConfigurationSetApplyProcessor::SendProgressIfNotComplete(ConfigurationUnitState state, const UnitInfo& unitInfo) + { + if (unitInfo.Result->State() != ConfigurationUnitState::Completed) + { + SendProgress(state, unitInfo); + } + } } diff --git a/src/Microsoft.Management.Configuration/ConfigurationSetApplyProcessor.h b/src/Microsoft.Management.Configuration/ConfigurationSetApplyProcessor.h @@ -90,6 +90,7 @@ namespace winrt::Microsoft::Management::Configuration::implementation // TODO: Eventually these functions/call sites will be used for history void SendProgress(ConfigurationSetState state); void SendProgress(ConfigurationUnitState state, const UnitInfo& unitInfo); + void SendProgressIfNotComplete(ConfigurationUnitState state, const UnitInfo& unitInfo); ConfigurationSet m_configurationSet; IConfigurationSetProcessor m_setProcessor; diff --git a/src/Microsoft.Management.Configuration/ConfigurationSetParser.cpp b/src/Microsoft.Management.Configuration/ConfigurationSetParser.cpp @@ -50,6 +50,8 @@ namespace winrt::Microsoft::Management::Configuration::implementation std::unique_ptr<ConfigurationSetParser> ConfigurationSetParser::Create(std::string_view input) { + AICLI_LOG_LARGE_STRING(Config, Verbose, << "Parsing configuration set:", input); + AppInstaller::YAML::Node document; try