commit 6096b5e9f3ed13726e5ec36947b0ce88a040cff4
parent 511d4f9815af8070b4d0d80fe9fbd2024f5f96e3
Author: JohnMcPMS <johnmcp@microsoft.com>
Date: Mon, 31 Mar 2025 17:04:38 -0700
Refactor ProcessMultiplePackages for clarity (#5340)
## Change
A refactor of `ProcessMultiplePackages` to increase the clarity of what
is being requested as options. This is done by passing a flags enum
rather than 5 booleans.
Also removes some extraneous `Workflow::` namespaces and improves the
logging when going backward in execution phase.
Diffstat:
8 files changed, 103 insertions(+), 72 deletions(-)
diff --git a/src/AppInstallerCLICore/Commands/InstallCommand.cpp b/src/AppInstallerCLICore/Commands/InstallCommand.cpp
@@ -119,47 +119,49 @@ namespace AppInstaller::CLI
{
context.SetFlags(ContextFlag::ShowSearchResultsOnPartialFailure);
- context << Workflow::InitializeInstallerDownloadAuthenticatorsMap;
+ context << InitializeInstallerDownloadAuthenticatorsMap;
if (context.Args.Contains(Execution::Args::Type::Manifest))
{
context <<
- Workflow::ReportExecutionStage(ExecutionStage::Discovery) <<
- Workflow::GetManifestFromArg <<
- Workflow::SelectInstaller <<
- Workflow::EnsureApplicableInstaller <<
- Workflow::Checkpoint("PreInstallCheckpoint", {}) << // TODO: Capture context data
- Workflow::InstallSinglePackage;
+ ReportExecutionStage(ExecutionStage::Discovery) <<
+ GetManifestFromArg <<
+ SelectInstaller <<
+ EnsureApplicableInstaller <<
+ Checkpoint("PreInstallCheckpoint", {}) << // TODO: Capture context data
+ InstallSinglePackage;
}
else
{
context <<
- Workflow::ReportExecutionStage(ExecutionStage::Discovery) <<
- Workflow::OpenSource();
+ ReportExecutionStage(ExecutionStage::Discovery) <<
+ OpenSource();
if (!context.Args.Contains(Execution::Args::Type::Force))
{
context <<
- Workflow::OpenCompositeSource(Workflow::DetermineInstalledSource(context), false, Repository::CompositeSearchBehavior::AvailablePackages);
+ OpenCompositeSource(DetermineInstalledSource(context), false, Repository::CompositeSearchBehavior::AvailablePackages);
}
if (context.Args.Contains(Execution::Args::Type::MultiQuery))
{
- bool skipDependencies = Settings::User().Get<Settings::Setting::InstallSkipDependencies>() || context.Args.Contains(Execution::Args::Type::SkipDependencies);
+ ProcessMultiplePackages::Flags flags = ProcessMultiplePackages::Flags::None;
+ if (Settings::User().Get<Settings::Setting::InstallSkipDependencies>() || context.Args.Contains(Execution::Args::Type::SkipDependencies))
+ {
+ flags = ProcessMultiplePackages::Flags::IgnoreDependencies;
+ }
+
context <<
- Workflow::GetMultiSearchRequests <<
- Workflow::SearchSubContextsForSingle() <<
- Workflow::ReportExecutionStage(Workflow::ExecutionStage::Execution) <<
- Workflow::ProcessMultiplePackages(
- Resource::String::PackageRequiresDependencies,
- APPINSTALLER_CLI_ERROR_MULTIPLE_INSTALL_FAILED,
- {}, true, skipDependencies);
+ GetMultiSearchRequests <<
+ SearchSubContextsForSingle() <<
+ ReportExecutionStage(ExecutionStage::Execution) <<
+ ProcessMultiplePackages(Resource::String::PackageRequiresDependencies, APPINSTALLER_CLI_ERROR_MULTIPLE_INSTALL_FAILED, flags);
}
else
{
context <<
- Workflow::Checkpoint("PreInstallCheckpoint", {}) << // TODO: Capture context data
- Workflow::InstallOrUpgradeSinglePackage(OperationType::Install);
+ Checkpoint("PreInstallCheckpoint", {}) << // TODO: Capture context data
+ InstallOrUpgradeSinglePackage(OperationType::Install);
}
}
}
diff --git a/src/AppInstallerCLICore/Commands/UpgradeCommand.cpp b/src/AppInstallerCLICore/Commands/UpgradeCommand.cpp
@@ -158,10 +158,10 @@ namespace AppInstaller::CLI
}
context <<
- Workflow::InitializeInstallerDownloadAuthenticatorsMap <<
- Workflow::ReportExecutionStage(ExecutionStage::Discovery) <<
- Workflow::OpenSource() <<
- Workflow::OpenCompositeSource(Workflow::DetermineInstalledSource(context));
+ InitializeInstallerDownloadAuthenticatorsMap <<
+ ReportExecutionStage(ExecutionStage::Discovery) <<
+ OpenSource() <<
+ OpenCompositeSource(DetermineInstalledSource(context));
if (ShouldListUpgrade(context.Args))
{
@@ -200,19 +200,21 @@ namespace AppInstaller::CLI
// The remaining case: search for specific packages to update
if (!context.Args.Contains(Execution::Args::Type::MultiQuery))
{
- context << Workflow::InstallOrUpgradeSinglePackage(OperationType::Upgrade);
+ context << InstallOrUpgradeSinglePackage(OperationType::Upgrade);
}
else
{
- bool skipDependencies = Settings::User().Get<Settings::Setting::InstallSkipDependencies>() || context.Args.Contains(Execution::Args::Type::SkipDependencies);
+ ProcessMultiplePackages::Flags flags = ProcessMultiplePackages::Flags::None;
+ if (Settings::User().Get<Settings::Setting::InstallSkipDependencies>() || context.Args.Contains(Execution::Args::Type::SkipDependencies))
+ {
+ flags = ProcessMultiplePackages::Flags::IgnoreDependencies;
+ }
+
context <<
- Workflow::GetMultiSearchRequests <<
- Workflow::SearchSubContextsForSingle(OperationType::Upgrade) <<
- Workflow::ReportExecutionStage(Workflow::ExecutionStage::Execution) <<
- Workflow::ProcessMultiplePackages(
- Resource::String::PackageRequiresDependencies,
- APPINSTALLER_CLI_ERROR_MULTIPLE_INSTALL_FAILED,
- {}, true, skipDependencies);
+ GetMultiSearchRequests <<
+ SearchSubContextsForSingle(OperationType::Upgrade) <<
+ ReportExecutionStage(ExecutionStage::Execution) <<
+ ProcessMultiplePackages(Resource::String::PackageRequiresDependencies, APPINSTALLER_CLI_ERROR_MULTIPLE_INSTALL_FAILED, flags);
}
}
}
diff --git a/src/AppInstallerCLICore/ExecutionContext.cpp b/src/AppInstallerCLICore/ExecutionContext.cpp
@@ -465,7 +465,7 @@ namespace AppInstaller::CLI::Execution
}
else if (m_executionStage > stage)
{
- THROW_HR_MSG(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), "Reporting ExecutionStage to an earlier Stage without allowBackward as true");
+ THROW_HR_MSG(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), "Reporting ExecutionStage to an earlier Stage: current[%d], new[%d]", ToIntegral(m_executionStage), ToIntegral(stage));
}
m_executionStage = stage;
diff --git a/src/AppInstallerCLICore/Workflows/ImportExportFlow.cpp b/src/AppInstallerCLICore/Workflows/ImportExportFlow.cpp
@@ -319,9 +319,9 @@ namespace AppInstaller::CLI::Workflow
context.Add<Execution::Data::Dependencies>(allDependencies);
context <<
- Workflow::ReportDependencies(Resource::String::ImportCommandReportDependencies) <<
- Workflow::ProcessMultiplePackages(
- Resource::String::ImportCommandReportDependencies, APPINSTALLER_CLI_ERROR_IMPORT_INSTALL_FAILED, {}, true, true);
+ ReportDependencies(Resource::String::ImportCommandReportDependencies) <<
+ ProcessMultiplePackages(
+ Resource::String::ImportCommandReportDependencies, APPINSTALLER_CLI_ERROR_IMPORT_INSTALL_FAILED, ProcessMultiplePackages::Flags::IgnoreDependencies);
if (context.GetTerminationHR() == APPINSTALLER_CLI_ERROR_IMPORT_INSTALL_FAILED)
{
diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp
@@ -584,6 +584,8 @@ namespace AppInstaller::CLI::Workflow
void InstallDependencies(Execution::Context& context)
{
+ using Flags = ProcessMultiplePackages::Flags;
+
if (Settings::User().Get<Settings::Setting::InstallSkipDependencies>() || context.Args.Contains(Execution::Args::Type::SkipDependencies))
{
context.Reporter.Warn() << Resource::String::DependenciesSkippedMessage << std::endl;
@@ -591,14 +593,16 @@ namespace AppInstaller::CLI::Workflow
}
context <<
- Workflow::GetDependenciesFromInstaller <<
- Workflow::ReportDependencies(Resource::String::PackageRequiresDependencies) <<
- Workflow::EnableWindowsFeaturesDependencies <<
- Workflow::ProcessMultiplePackages(Resource::String::PackageRequiresDependencies, APPINSTALLER_CLI_ERROR_INSTALL_DEPENDENCIES, {}, true, true, true, true);
+ GetDependenciesFromInstaller <<
+ ReportDependencies(Resource::String::PackageRequiresDependencies) <<
+ EnableWindowsFeaturesDependencies <<
+ ProcessMultiplePackages(Resource::String::PackageRequiresDependencies, APPINSTALLER_CLI_ERROR_INSTALL_DEPENDENCIES, Flags::IgnoreDependencies | Flags::StopOnFailure | Flags::RefreshPathVariable);
}
void DownloadPackageDependencies(Execution::Context& context)
{
+ using Flags = ProcessMultiplePackages::Flags;
+
if (Settings::User().Get<Settings::Setting::InstallSkipDependencies>() || context.Args.Contains(Execution::Args::Type::SkipDependencies))
{
context.Reporter.Warn() << Resource::String::DependenciesSkippedMessage << std::endl;
@@ -606,10 +610,10 @@ namespace AppInstaller::CLI::Workflow
}
context <<
- Workflow::GetDependenciesFromInstaller <<
- Workflow::ReportDependencies(Resource::String::PackageRequiresDependencies) <<
- Workflow::CreateDependencySubContexts(Resource::String::PackageRequiresDependencies) <<
- Workflow::ProcessMultiplePackages(Resource::String::PackageRequiresDependencies, APPINSTALLER_CLI_ERROR_DOWNLOAD_DEPENDENCIES, {}, true, true, true, false, true);
+ GetDependenciesFromInstaller <<
+ ReportDependencies(Resource::String::PackageRequiresDependencies) <<
+ CreateDependencySubContexts(Resource::String::PackageRequiresDependencies) <<
+ ProcessMultiplePackages(Resource::String::PackageRequiresDependencies, APPINSTALLER_CLI_ERROR_DOWNLOAD_DEPENDENCIES, Flags::IgnoreDependencies | Flags::StopOnFailure | Flags::DownloadOnly);
}
void InstallSinglePackage(Execution::Context& context)
@@ -675,6 +679,25 @@ namespace AppInstaller::CLI::Workflow
Workflow::EnsureValidNestedInstallerMetadataForArchiveInstall;
}
+ ProcessMultiplePackages::ProcessMultiplePackages(
+ StringResource::StringId dependenciesReportMessage,
+ HRESULT resultOnFailure,
+ Flags flags,
+ std::vector<HRESULT>&& ignorableInstallResults) :
+ WorkflowTask("ProcessMultiplePackages"),
+ m_dependenciesReportMessage(dependenciesReportMessage),
+ m_resultOnFailure(resultOnFailure),
+ m_ignorableInstallResults(std::move(ignorableInstallResults))
+ {
+ // Inverted
+ m_ensurePackageAgreements = !WI_IsFlagSet(flags, Flags::SkipPackageAgreements);
+
+ m_ignorePackageDependencies = WI_IsFlagSet(flags, Flags::IgnoreDependencies);
+ m_stopOnFailure = WI_IsFlagSet(flags, Flags::StopOnFailure);
+ m_refreshPathVariable = WI_IsFlagSet(flags, Flags::RefreshPathVariable);
+ m_downloadOnly = WI_IsFlagSet(flags, Flags::DownloadOnly);
+ }
+
void ProcessMultiplePackages::operator()(Execution::Context& context) const
{
if (!context.Contains(Execution::Data::PackageSubContexts))
@@ -692,11 +715,11 @@ namespace AppInstaller::CLI::Workflow
return;
}
+ auto& packageSubContexts = context.Get<Execution::Data::PackageSubContexts>();
+
// Report dependencies
if (!m_ignorePackageDependencies)
{
- auto& packageSubContexts = context.Get<Execution::Data::PackageSubContexts>();
-
DependencyList allDependencies;
for (auto& packageContext : packageSubContexts)
@@ -721,10 +744,10 @@ namespace AppInstaller::CLI::Workflow
}
bool allSucceeded = true;
- size_t packagesCount = context.Get<Execution::Data::PackageSubContexts>().size();
+ size_t packagesCount = packageSubContexts.size();
size_t packagesProgress = 0;
- for (auto& packageContext : context.Get<Execution::Data::PackageSubContexts>())
+ for (auto& packageContext : packageSubContexts)
{
packagesProgress++;
context.Reporter.Info() << '(' << packagesProgress << '/' << packagesCount << ") "_liv;
@@ -744,7 +767,7 @@ namespace AppInstaller::CLI::Workflow
currentContext <<
Workflow::EnableWindowsFeaturesDependencies <<
Workflow::CreateDependencySubContexts(m_dependenciesReportMessage) <<
- Workflow::ProcessMultiplePackages(m_dependenciesReportMessage, APPINSTALLER_CLI_ERROR_INSTALL_DEPENDENCIES, {}, true, true, true, true);
+ Workflow::ProcessMultiplePackages(m_dependenciesReportMessage, APPINSTALLER_CLI_ERROR_INSTALL_DEPENDENCIES, Flags::IgnoreDependencies | Flags::StopOnFailure | Flags::RefreshPathVariable);
}
currentContext << Workflow::DownloadInstaller;
diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.h b/src/AppInstallerCLICore/Workflows/InstallFlow.h
@@ -167,24 +167,22 @@ namespace AppInstaller::CLI::Workflow
// Outputs: None
struct ProcessMultiplePackages : public WorkflowTask
{
+ // Flags to signal change from default behavior of the task.
+ enum class Flags : uint32_t
+ {
+ None = 0x00,
+ SkipPackageAgreements = 0x01,
+ IgnoreDependencies = 0x02,
+ StopOnFailure = 0x04,
+ RefreshPathVariable = 0x08,
+ DownloadOnly = 0x10,
+ };
+
ProcessMultiplePackages(
StringResource::StringId dependenciesReportMessage,
HRESULT resultOnFailure,
- std::vector<HRESULT>&& ignorableInstallResults = {},
- bool ensurePackageAgreements = true,
- bool ignoreDependencies = false,
- bool stopOnFailure = false,
- bool refreshPathVariable = false,
- bool downloadOnly = false):
- WorkflowTask("ProcessMultiplePackages"),
- m_dependenciesReportMessage(dependenciesReportMessage),
- m_resultOnFailure(resultOnFailure),
- m_ignorableInstallResults(std::move(ignorableInstallResults)),
- m_ignorePackageDependencies(ignoreDependencies),
- m_ensurePackageAgreements(ensurePackageAgreements),
- m_stopOnFailure(stopOnFailure),
- m_refreshPathVariable(refreshPathVariable),
- m_downloadOnly(downloadOnly){}
+ Flags flags = Flags::None,
+ std::vector<HRESULT>&& ignorableInstallResults = {});
void operator()(Execution::Context& context) const override;
@@ -199,6 +197,8 @@ namespace AppInstaller::CLI::Workflow
bool m_downloadOnly;
};
+ DEFINE_ENUM_FLAG_OPERATORS(ProcessMultiplePackages::Flags);
+
// Stores the existing set of packages in ARP.
// Required Args: None
// Inputs: Installer
diff --git a/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp b/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp
@@ -269,13 +269,19 @@ namespace AppInstaller::CLI::Workflow
{
context.Add<Execution::Data::PackageSubContexts>(std::move(packageSubContexts));
context.Reporter.Info() << std::endl;
- bool skipDependencies = Settings::User().Get<Settings::Setting::InstallSkipDependencies>() || context.Args.Contains(Execution::Args::Type::SkipDependencies);
+
+ ProcessMultiplePackages::Flags flags = ProcessMultiplePackages::Flags::None;
+ if (Settings::User().Get<Settings::Setting::InstallSkipDependencies>() || context.Args.Contains(Execution::Args::Type::SkipDependencies))
+ {
+ flags = ProcessMultiplePackages::Flags::IgnoreDependencies;
+ }
+
context <<
ProcessMultiplePackages(
Resource::String::PackageRequiresDependencies,
APPINSTALLER_CLI_ERROR_UPDATE_ALL_HAS_FAILURE,
- { APPINSTALLER_CLI_ERROR_UPDATE_NOT_APPLICABLE },
- true, skipDependencies);
+ flags,
+ { APPINSTALLER_CLI_ERROR_UPDATE_NOT_APPLICABLE });
}
if (packagesWithUnknownVersionSkipped > 0)
diff --git a/src/AppInstallerCLITests/InstallDependenciesFlow.cpp b/src/AppInstallerCLITests/InstallDependenciesFlow.cpp
@@ -30,12 +30,10 @@ void OverrideOpenSourceForDependencies(TestContext& context)
void OverrideForProcessMultiplePackages(TestContext& context)
{
- context.Override({ Workflow::ProcessMultiplePackages(
+ context.Override({ ProcessMultiplePackages(
Resource::String::PackageRequiresDependencies,
APPINSTALLER_CLI_ERROR_INSTALL_DEPENDENCIES,
- {},
- false,
- true), [](TestContext&)
+ ProcessMultiplePackages::Flags::SkipPackageAgreements | ProcessMultiplePackages::Flags::IgnoreDependencies), [](TestContext&)
{
} });
