Skip to content

Commit a14cbc2

Browse files
authored
Only download during COM download (#5348)
Cherry pick #5327 to 1.10 ## 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.
1 parent 2ef20bf commit a14cbc2

File tree

3 files changed

+52
-5
lines changed

3 files changed

+52
-5
lines changed

src/AppInstallerCLICore/Workflows/InstallFlow.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -609,7 +609,7 @@ namespace AppInstaller::CLI::Workflow
609609
Workflow::GetDependenciesFromInstaller <<
610610
Workflow::ReportDependencies(Resource::String::PackageRequiresDependencies) <<
611611
Workflow::CreateDependencySubContexts(Resource::String::PackageRequiresDependencies) <<
612-
Workflow::ProcessMultiplePackages(Resource::String::PackageRequiresDependencies, APPINSTALLER_CLI_ERROR_DOWNLOAD_DEPENDENCIES, {}, true, true, true, false);
612+
Workflow::ProcessMultiplePackages(Resource::String::PackageRequiresDependencies, APPINSTALLER_CLI_ERROR_DOWNLOAD_DEPENDENCIES, {}, true, true, true, false, true);
613613
}
614614

615615
void InstallSinglePackage(Execution::Context& context)
@@ -682,7 +682,7 @@ namespace AppInstaller::CLI::Workflow
682682
return;
683683
}
684684

685-
bool downloadInstallerOnly = WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerDownloadOnly);
685+
bool downloadInstallerOnly = m_downloadOnly ? true : WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerDownloadOnly);
686686

687687
// Show all prompts needed for every package before installing anything
688688
context << Workflow::ShowPromptsForMultiplePackages(m_ensurePackageAgreements, downloadInstallerOnly);

src/AppInstallerCLICore/Workflows/InstallFlow.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -174,15 +174,17 @@ namespace AppInstaller::CLI::Workflow
174174
bool ensurePackageAgreements = true,
175175
bool ignoreDependencies = false,
176176
bool stopOnFailure = false,
177-
bool refreshPathVariable = false):
177+
bool refreshPathVariable = false,
178+
bool downloadOnly = false):
178179
WorkflowTask("ProcessMultiplePackages"),
179180
m_dependenciesReportMessage(dependenciesReportMessage),
180181
m_resultOnFailure(resultOnFailure),
181182
m_ignorableInstallResults(std::move(ignorableInstallResults)),
182183
m_ignorePackageDependencies(ignoreDependencies),
183184
m_ensurePackageAgreements(ensurePackageAgreements),
184185
m_stopOnFailure(stopOnFailure),
185-
m_refreshPathVariable(refreshPathVariable){}
186+
m_refreshPathVariable(refreshPathVariable),
187+
m_downloadOnly(downloadOnly){}
186188

187189
void operator()(Execution::Context& context) const override;
188190

@@ -194,6 +196,7 @@ namespace AppInstaller::CLI::Workflow
194196
bool m_ensurePackageAgreements;
195197
bool m_stopOnFailure;
196198
bool m_refreshPathVariable;
199+
bool m_downloadOnly;
197200
};
198201

199202
// Stores the existing set of packages in ARP.

src/AppInstallerCLITests/InstallDependenciesFlow.cpp

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,11 @@
44
#include "WorkflowCommon.h"
55
#include "DependenciesTestSource.h"
66
#include <Commands/InstallCommand.h>
7+
#include <Commands/COMCommand.h>
78
#include <Workflows/DependenciesFlow.h>
9+
#include <Workflows/DownloadFlow.h>
810
#include <Workflows/InstallFlow.h>
11+
#include <Workflows/ShellExecuteInstallerHandler.h>
912

1013
using namespace TestCommon;
1114
using namespace AppInstaller::CLI;
@@ -38,6 +41,15 @@ void OverrideForProcessMultiplePackages(TestContext& context)
3841
} });
3942
}
4043

44+
void OverrideShellExecute(TestContext& context, std::vector<std::string>& installationOrder)
45+
{
46+
context.Override({ ShellExecuteInstallImpl, [&installationOrder](TestContext& c)
47+
{
48+
installationOrder.push_back(c.Get<Execution::Data::Manifest>().Id);
49+
c.Add<Execution::Data::OperationReturnCode>(0);
50+
} });
51+
}
52+
4153
TEST_CASE("DependencyGraph_SkipInstalled", "[InstallFlow][workflow][dependencyGraph][dependencies]")
4254
{
4355
TestCommon::TempFile installResultPath("TestExeInstalled.txt");
@@ -264,7 +276,39 @@ TEST_CASE("InstallFlow_Dependencies", "[InstallFlow][workflow][dependencies]")
264276
REQUIRE(installOutput.str().find("PreviewIIS") != std::string::npos);
265277
}
266278

279+
TEST_CASE("InstallFlow_Dependencies_COM", "[InstallFlow][workflow][dependencies]")
280+
{
281+
std::vector<std::string> installationOrder;
282+
283+
std::ostringstream installOutput;
284+
TestContext context{ installOutput, std::cin };
285+
auto previousThreadGlobals = context.SetForCurrentThread();
286+
OverrideForShellExecute(context);
287+
OverrideShellExecute(context, installationOrder);
288+
OverrideOpenDependencySource(context);
289+
OverrideEnableWindowsFeaturesDependencies(context);
290+
context.Override({ ReverifyInstallerHash, [](TestContext&) {} });
291+
292+
context.Add<Execution::Data::Manifest>(YamlParser::CreateFromPath(TestDataFile("InstallFlowTest_MultipleDependencies.yaml")));
293+
294+
COMDownloadCommand download({});
295+
download.Execute(context);
296+
297+
REQUIRE(installationOrder.size() == 0);
298+
299+
COMInstallCommand install({});
300+
REQUIRE_NOTHROW(install.Execute(context));
301+
302+
REQUIRE(context.GetTerminationHR() == S_OK);
303+
304+
// Verify installers are called in order
305+
REQUIRE(installationOrder.size() == 3);
306+
REQUIRE(installationOrder.at(0) == "Dependency1");
307+
REQUIRE(installationOrder.at(1) == "Dependency2");
308+
REQUIRE(installationOrder.at(2) == "AppInstallerCliTest.TestExeInstaller.MultipleDependencies");
309+
}
310+
267311
// TODO:
268312
// add dependencies for installer tests to DependenciesTestSource (or a new one)
269313
// add tests for min version dependency solving
270-
// add tests that check for correct installation of dependencies (not only the order)
314+
// add tests that check for correct installation of dependencies (not only the order)

0 commit comments

Comments
 (0)