commit aca2fc3d23687ccfebc3e648a3cff1d1a553b9fa
parent 3575fb9368680ef6eecc20ea8e7857bf0c311da8
Author: JohnMcPMS <johnmcp@microsoft.com>
Date: Thu, 27 Mar 2025 13:42:46 -0700
Only download during COM download (#5327)
Fixes #4661
## Issue
When `download` was added, the COM download and install split for
dependencies was accidentally implemented. But it was validated only
with download in mind, so the download part worked for download intent.
With dependencies, the split basically attempted to install the
dependencies twice, causing a failure on the second attempt due to
internal assertions.
## Change
For the COM download command, explicitly force that we are only
downloading.
Diffstat:
3 files changed, 52 insertions(+), 6 deletions(-)
diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp
@@ -609,7 +609,7 @@ namespace AppInstaller::CLI::Workflow
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);
+ Workflow::ProcessMultiplePackages(Resource::String::PackageRequiresDependencies, APPINSTALLER_CLI_ERROR_DOWNLOAD_DEPENDENCIES, {}, true, true, true, false, true);
}
void InstallSinglePackage(Execution::Context& context)
@@ -682,7 +682,7 @@ namespace AppInstaller::CLI::Workflow
return;
}
- bool downloadInstallerOnly = WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerDownloadOnly);
+ bool downloadInstallerOnly = m_downloadOnly ? true : WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerDownloadOnly);
// Show all prompts needed for every package before installing anything
context << Workflow::ShowPromptsForMultiplePackages(m_ensurePackageAgreements, downloadInstallerOnly);
diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.h b/src/AppInstallerCLICore/Workflows/InstallFlow.h
@@ -174,7 +174,8 @@ namespace AppInstaller::CLI::Workflow
bool ensurePackageAgreements = true,
bool ignoreDependencies = false,
bool stopOnFailure = false,
- bool refreshPathVariable = false):
+ bool refreshPathVariable = false,
+ bool downloadOnly = false):
WorkflowTask("ProcessMultiplePackages"),
m_dependenciesReportMessage(dependenciesReportMessage),
m_resultOnFailure(resultOnFailure),
@@ -182,7 +183,8 @@ namespace AppInstaller::CLI::Workflow
m_ignorePackageDependencies(ignoreDependencies),
m_ensurePackageAgreements(ensurePackageAgreements),
m_stopOnFailure(stopOnFailure),
- m_refreshPathVariable(refreshPathVariable){}
+ m_refreshPathVariable(refreshPathVariable),
+ m_downloadOnly(downloadOnly){}
void operator()(Execution::Context& context) const override;
@@ -194,6 +196,7 @@ namespace AppInstaller::CLI::Workflow
bool m_ensurePackageAgreements;
bool m_stopOnFailure;
bool m_refreshPathVariable;
+ bool m_downloadOnly;
};
// Stores the existing set of packages in ARP.
diff --git a/src/AppInstallerCLITests/InstallDependenciesFlow.cpp b/src/AppInstallerCLITests/InstallDependenciesFlow.cpp
@@ -4,8 +4,11 @@
#include "WorkflowCommon.h"
#include "DependenciesTestSource.h"
#include <Commands/InstallCommand.h>
+#include <Commands/COMCommand.h>
#include <Workflows/DependenciesFlow.h>
+#include <Workflows/DownloadFlow.h>
#include <Workflows/InstallFlow.h>
+#include <Workflows/ShellExecuteInstallerHandler.h>
using namespace TestCommon;
using namespace AppInstaller::CLI;
@@ -38,6 +41,15 @@ void OverrideForProcessMultiplePackages(TestContext& context)
} });
}
+void OverrideShellExecute(TestContext& context, std::vector<std::string>& installationOrder)
+{
+ context.Override({ ShellExecuteInstallImpl, [&installationOrder](TestContext& c)
+ {
+ installationOrder.push_back(c.Get<Execution::Data::Manifest>().Id);
+ c.Add<Execution::Data::OperationReturnCode>(0);
+ } });
+}
+
TEST_CASE("DependencyGraph_SkipInstalled", "[InstallFlow][workflow][dependencyGraph][dependencies]")
{
TestCommon::TempFile installResultPath("TestExeInstalled.txt");
@@ -264,7 +276,39 @@ TEST_CASE("InstallFlow_Dependencies", "[InstallFlow][workflow][dependencies]")
REQUIRE(installOutput.str().find("PreviewIIS") != std::string::npos);
}
+TEST_CASE("InstallFlow_Dependencies_COM", "[InstallFlow][workflow][dependencies]")
+{
+ std::vector<std::string> installationOrder;
+
+ std::ostringstream installOutput;
+ TestContext context{ installOutput, std::cin };
+ auto previousThreadGlobals = context.SetForCurrentThread();
+ OverrideForShellExecute(context);
+ OverrideShellExecute(context, installationOrder);
+ OverrideOpenDependencySource(context);
+ OverrideEnableWindowsFeaturesDependencies(context);
+ context.Override({ ReverifyInstallerHash, [](TestContext&) {} });
+
+ context.Add<Execution::Data::Manifest>(YamlParser::CreateFromPath(TestDataFile("InstallFlowTest_MultipleDependencies.yaml")));
+
+ COMDownloadCommand download({});
+ download.Execute(context);
+
+ REQUIRE(installationOrder.size() == 0);
+
+ COMInstallCommand install({});
+ REQUIRE_NOTHROW(install.Execute(context));
+
+ REQUIRE(context.GetTerminationHR() == S_OK);
+
+ // Verify installers are called in order
+ REQUIRE(installationOrder.size() == 3);
+ REQUIRE(installationOrder.at(0) == "Dependency1");
+ REQUIRE(installationOrder.at(1) == "Dependency2");
+ REQUIRE(installationOrder.at(2) == "AppInstallerCliTest.TestExeInstaller.MultipleDependencies");
+}
+
// TODO:
// add dependencies for installer tests to DependenciesTestSource (or a new one)
// add tests for min version dependency solving
-// add tests that check for correct installation of dependencies (not only the order)-
\ No newline at end of file
+// add tests that check for correct installation of dependencies (not only the order)
