Fix issue that only 1 dependency package is installed

src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp

@@ -48,21 +48,21 @@ namespace AppInstaller::CLI::Workflow
             if (dependencies.HasAnyOf(DependencyType::WindowsFeature))
             {
                 info << "  - " << Resource::String::WindowsFeaturesDependencies << std::endl;
-                dependencies.ApplyToType(DependencyType::WindowsFeature, [&info](Dependency dependency) {info << "      " << dependency.Id << std::endl; });
+                dependencies.ApplyToType(DependencyType::WindowsFeature, [&info](Dependency dependency) {info << "      " << dependency.Id() << std::endl; });
             }
 
             if (dependencies.HasAnyOf(DependencyType::WindowsLibrary))
             {
                 info << "  - " << Resource::String::WindowsLibrariesDependencies << std::endl;
-                dependencies.ApplyToType(DependencyType::WindowsLibrary, [&info](Dependency dependency) {info << "      " << dependency.Id << std::endl; });
+                dependencies.ApplyToType(DependencyType::WindowsLibrary, [&info](Dependency dependency) {info << "      " << dependency.Id() << std::endl; });
             }
 
             if (dependencies.HasAnyOf(DependencyType::Package))
             {
                 info << "  - " << Resource::String::PackageDependencies << std::endl;
                 dependencies.ApplyToType(DependencyType::Package, [&info](Dependency dependency)
                     {
-                        info << "      " << dependency.Id;
+                        info << "      " << dependency.Id();
                         if (dependency.MinVersion)
                         {
                             info << " [>= " << dependency.MinVersion.value().ToString() << "]";
@@ -74,7 +74,7 @@ namespace AppInstaller::CLI::Workflow
             if (dependencies.HasAnyOf(DependencyType::External))
             {
                 context.Reporter.Warn() << "  - " << Resource::String::ExternalDependencies << std::endl;
-                dependencies.ApplyToType(DependencyType::External, [&info](Dependency dependency) {info << "      " << dependency.Id << std::endl; });
+                dependencies.ApplyToType(DependencyType::External, [&info](Dependency dependency) {info << "      " << dependency.Id() << std::endl; });
             }
         }
     }
@@ -180,7 +180,7 @@ namespace AppInstaller::CLI::Workflow
                         std::move(nodeProcessor.GetPackageInstalledVersion()),
                         std::move(nodeProcessor.GetManifest()),
                         std::move(nodeProcessor.GetPreferredInstaller()) };
-                    idToPackageMap.emplace(node.Id, std::move(dependencyPackageCandidate));
+                    idToPackageMap.emplace(node.Id(), std::move(dependencyPackageCandidate));
                 };
 
                 return list;
@@ -205,7 +205,7 @@ namespace AppInstaller::CLI::Workflow
 
         for (auto const& node : installationOrder)
         {
-            auto itr = idToPackageMap.find(node.Id);
+            auto itr = idToPackageMap.find(node.Id());
             // if the package was already installed (with a useful version) or is the root
             // then there will be no installer for it on the map.
             if (itr != idToPackageMap.end())
@@ -243,6 +243,6 @@ namespace AppInstaller::CLI::Workflow
 
         // Install dependencies in the correct order
         context.Add<Execution::Data::PackagesToInstall>(std::move(dependencyPackageContexts));
-        context << Workflow::InstallMultiplePackages(m_dependencyReportMessage, APPINSTALLER_CLI_ERROR_INSTALL_DEPENDENCIES, {}, false, true);
+        context << Workflow::InstallMultiplePackages(m_dependencyReportMessage, APPINSTALLER_CLI_ERROR_INSTALL_DEPENDENCIES, {}, false, true, true);
     }
 }

src/AppInstallerCLICore/Workflows/DependencyNodeProcessor.cpp

@@ -19,7 +19,7 @@ namespace AppInstaller::CLI::Workflow
         auto error = m_context.Reporter.Error();
         auto info = m_context.Reporter.Info();
 
-        searchRequest.Filters.emplace_back(PackageMatchFilter(PackageMatchField::Id, MatchType::CaseInsensitive, dependencyNode.Id));
+        searchRequest.Filters.emplace_back(PackageMatchFilter(PackageMatchField::Id, MatchType::CaseInsensitive, dependencyNode.Id()));
 
         const auto& matches = source.Search(searchRequest).Matches;
 
@@ -31,8 +31,8 @@ namespace AppInstaller::CLI::Workflow
 
         if (matches.size() > 1)
         {
-            error << Resource::String::DependenciesFlowSourceTooManyMatches << " " << Utility::Normalize(dependencyNode.Id);
-            AICLI_LOG(CLI, Error, << "Too many matches for package " << dependencyNode.Id);
+            error << Resource::String::DependenciesFlowSourceTooManyMatches << " " << Utility::Normalize(dependencyNode.Id());
+            AICLI_LOG(CLI, Error, << "Too many matches for package " << dependencyNode.Id());
             return DependencyNodeProcessorResult::Error;
         }
 

src/AppInstallerCLICore/Workflows/InstallFlow.cpp

@@ -510,7 +510,7 @@ namespace AppInstaller::CLI::Workflow
         bool allSucceeded = true;
         size_t packagesCount = context.Get<Execution::Data::PackagesToInstall>().size();
         size_t packagesProgress = 0;
-        
+
         for (auto& packageContext : context.Get<Execution::Data::PackagesToInstall>())
         {
             packagesProgress++;
@@ -552,6 +552,10 @@ namespace AppInstaller::CLI::Workflow
                 if (m_ignorableInstallResults.end() == std::find(m_ignorableInstallResults.begin(), m_ignorableInstallResults.end(), installContext.GetTerminationHR()))
                 {
                     allSucceeded = false;
+                    if (m_stopOnFailure)
+                    {
+                        break;
+                    }
                 }
             }
         }

src/AppInstallerCLICore/Workflows/InstallFlow.h

@@ -150,13 +150,15 @@ namespace AppInstaller::CLI::Workflow
             HRESULT resultOnFailure,
             std::vector<HRESULT>&& ignorableInstallResults = {},
             bool ensurePackageAgreements = true,
-            bool ignoreDependencies = false) :
+            bool ignoreDependencies = false,
+            bool stopOnFailure = false) :
             WorkflowTask("InstallMultiplePackages"),
             m_dependenciesReportMessage(dependenciesReportMessage),
             m_resultOnFailure(resultOnFailure),
             m_ignorableInstallResults(std::move(ignorableInstallResults)),
             m_ignorePackageDependencies(ignoreDependencies),
-            m_ensurePackageAgreements(ensurePackageAgreements) {}
+            m_ensurePackageAgreements(ensurePackageAgreements),
+            m_stopOnFailure(stopOnFailure) {}
 
         void operator()(Execution::Context& context) const override;
 
@@ -166,6 +168,7 @@ namespace AppInstaller::CLI::Workflow
         StringResource::StringId m_dependenciesReportMessage;
         bool m_ignorePackageDependencies;
         bool m_ensurePackageAgreements;
+        bool m_stopOnFailure;
     };
 
     // Stores the existing set of packages in ARP.

src/AppInstallerCLICore/Workflows/ShowFlow.cpp

@@ -180,21 +180,21 @@ namespace AppInstaller::CLI::Workflow
                     if (dependencies.HasAnyOf(Manifest::DependencyType::WindowsFeature))
                     {
                         info << "    - "_liv << Resource::String::ShowLabelWindowsFeaturesDependencies << ' ' << std::endl;
-                        dependencies.ApplyToType(Manifest::DependencyType::WindowsFeature, [&info](Manifest::Dependency dependency) {info << "        "_liv << dependency.Id << std::endl; });
+                        dependencies.ApplyToType(Manifest::DependencyType::WindowsFeature, [&info](Manifest::Dependency dependency) {info << "        "_liv << dependency.Id() << std::endl; });
                     }
 
                     if (dependencies.HasAnyOf(Manifest::DependencyType::WindowsLibrary))
                     {
                         info << "    - "_liv << Resource::String::ShowLabelWindowsLibrariesDependencies << ' ' << std::endl;
-                        dependencies.ApplyToType(Manifest::DependencyType::WindowsLibrary, [&info](Manifest::Dependency dependency) {info << "        "_liv << dependency.Id << std::endl; });
+                        dependencies.ApplyToType(Manifest::DependencyType::WindowsLibrary, [&info](Manifest::Dependency dependency) {info << "        "_liv << dependency.Id() << std::endl; });
                     }
 
                     if (dependencies.HasAnyOf(Manifest::DependencyType::Package))
                     {
                         info << "    - "_liv << Resource::String::ShowLabelPackageDependencies << ' ' << std::endl;
                         dependencies.ApplyToType(Manifest::DependencyType::Package, [&info](Manifest::Dependency dependency)
                             {
-                                info << "        "_liv << dependency.Id;
+                                info << "        "_liv << dependency.Id();
                                 if (dependency.MinVersion)
                                 {
                                     info << " [>= " << dependency.MinVersion.value().ToString() << "]";
@@ -206,7 +206,7 @@ namespace AppInstaller::CLI::Workflow
                     if (dependencies.HasAnyOf(Manifest::DependencyType::External))
                     {
                         info << "    - "_liv << Resource::String::ShowLabelExternalDependencies << ' ' << std::endl;
-                        dependencies.ApplyToType(Manifest::DependencyType::External, [&info](Manifest::Dependency dependency) {info << "        "_liv << dependency.Id << std::endl; });
+                        dependencies.ApplyToType(Manifest::DependencyType::External, [&info](Manifest::Dependency dependency) {info << "        "_liv << dependency.Id() << std::endl; });
                     }
                 }
             }

src/AppInstallerCLITests/AppInstallerCLITests.vcxproj

@@ -276,6 +276,9 @@
     <CopyFileToFolders Include="TestData\InstallFlowTest_MSStore.yaml">
       <DeploymentContent>true</DeploymentContent>
     </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\InstallFlowTest_MultipleDependencies.yaml">
+      <DeploymentContent>true</DeploymentContent>
+    </CopyFileToFolders>
     <CopyFileToFolders Include="TestData\InstallFlowTest_AbortsTerminal.yaml">
       <DeploymentContent>true</DeploymentContent>
     </CopyFileToFolders>

src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters

@@ -534,6 +534,9 @@
     <CopyFileToFolders Include="TestData\InstallFlowTest_MSStore.yaml">
       <Filter>TestData</Filter>
     </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\InstallFlowTest_MultipleDependencies.yaml">
+      <Filter>TestData</Filter>
+    </CopyFileToFolders>
     <CopyFileToFolders Include="TestData\InstallFlowTest_AbortsTerminal.yaml">
       <Filter>TestData</Filter>
     </CopyFileToFolders>
@@ -739,40 +742,40 @@
       <Filter>TestData</Filter>
     </CopyFileToFolders>
     <CopyFileToFolders Include="TestData\Manifest-Bad-NoSupportedPlatforms.yaml">
-      <DeploymentContent>true</DeploymentContent>
+      <Filter>TestData</Filter>
     </CopyFileToFolders>
     <CopyFileToFolders Include="TestData\Installer-Bad-NoSupportedPlatforms.msix">
-      <DeploymentContent>true</DeploymentContent>
+      <Filter>TestData</Filter>
     </CopyFileToFolders>
     <CopyFileToFolders Include="TestData\Installer-Good.msix">
-      <DeploymentContent>true</DeploymentContent>
+      <Filter>TestData</Filter>
     </CopyFileToFolders>
     <CopyFileToFolders Include="TestData\Installer-Good.msixbundle">
-      <DeploymentContent>true</DeploymentContent>
+      <Filter>TestData</Filter>
     </CopyFileToFolders>
     <CopyFileToFolders Include="TestData\Installer-Signed-Good.msix">
-      <DeploymentContent>true</DeploymentContent>
+      <Filter>TestData</Filter>
     </CopyFileToFolders>
     <CopyFileToFolders Include="TestData\Installer-Signed-Good.msixbundle">
-      <DeploymentContent>true</DeploymentContent>
+      <Filter>TestData</Filter>
     </CopyFileToFolders>
     <CopyFileToFolders Include="TestData\Manifest-Bad-InconsistentSignedMsixInstallerFields.yaml">
-      <DeploymentContent>true</DeploymentContent>
+      <Filter>TestData</Filter>
     </CopyFileToFolders>
     <CopyFileToFolders Include="TestData\Manifest-Good-SignedMsixInstaller.yaml">
-      <DeploymentContent>true</DeploymentContent>
+      <Filter>TestData</Filter>
     </CopyFileToFolders>
     <CopyFileToFolders Include="TestData\Manifest-Good-MsixBundleInstaller.yaml">
-      <DeploymentContent>true</DeploymentContent>
+      <Filter>TestData</Filter>
     </CopyFileToFolders>
     <CopyFileToFolders Include="TestData\Manifest-Bad-InconsistentMsixBundleInstallerFields.yaml">
-      <DeploymentContent>true</DeploymentContent>
+      <Filter>TestData</Filter>
     </CopyFileToFolders>
     <CopyFileToFolders Include="TestData\Manifest-Good-SignedMsixBundleInstaller.yaml">
-      <DeploymentContent>true</DeploymentContent>
+      <Filter>TestData</Filter>
     </CopyFileToFolders>
     <CopyFileToFolders Include="TestData\Manifest-Bad-InconsistentSignedMsixBundleInstallerFields.yaml">
-      <DeploymentContent>true</DeploymentContent>
+      <Filter>TestData</Filter>
     </CopyFileToFolders>
   </ItemGroup>
 </Project>

src/AppInstallerCLITests/Dependencies.cpp

@@ -53,9 +53,9 @@ TEST_CASE("DependencyGraph_BFirst", "[dependencyGraph][dependencies]")
     installationOrder = graph.GetInstallationOrder();
 
     REQUIRE(installationOrder.size() == 3);
-    REQUIRE(installationOrder.at(0).Id == "C");
-    REQUIRE(installationOrder.at(1).Id == "B");
-    REQUIRE(installationOrder.at(2).Id == "NeedsToInstallBFirst");
+    REQUIRE(installationOrder.at(0).Id() == "C");
+    REQUIRE(installationOrder.at(1).Id() == "B");
+    REQUIRE(installationOrder.at(2).Id() == "NeedsToInstallBFirst");
 }
 
 TEST_CASE("DependencyGraph_InStackNoLoop", "[dependencyGraph][dependencies]")
@@ -87,9 +87,9 @@ TEST_CASE("DependencyGraph_InStackNoLoop", "[dependencyGraph][dependencies]")
     installationOrder = graph.GetInstallationOrder();
 
     REQUIRE(installationOrder.size() == 3);
-    REQUIRE(installationOrder.at(0).Id == "F");
-    REQUIRE(installationOrder.at(1).Id == "C");
-    REQUIRE(installationOrder.at(2).Id == "DependencyAlreadyInStackButNoLoop");
+    REQUIRE(installationOrder.at(0).Id() == "F");
+    REQUIRE(installationOrder.at(1).Id() == "C");
+    REQUIRE(installationOrder.at(2).Id() == "DependencyAlreadyInStackButNoLoop");
 }
 
 TEST_CASE("DependencyGraph_EasyToSeeLoop", "[dependencyGraph][dependencies]")
@@ -124,8 +124,8 @@ TEST_CASE("DependencyGraph_EasyToSeeLoop", "[dependencyGraph][dependencies]")
     REQUIRE(hasLoop);
 
     REQUIRE(installationOrder.size() == 2);
-    REQUIRE(installationOrder.at(0).Id == "D");
-    REQUIRE(installationOrder.at(1).Id == "EasyToSeeLoop");
+    REQUIRE(installationOrder.at(0).Id() == "D");
+    REQUIRE(installationOrder.at(1).Id() == "EasyToSeeLoop");
 }
 
 TEST_CASE("DependencyNodeProcessor_SkipInstalled", "[dependencies]")

src/AppInstallerCLITests/DependenciesTestSource.h

@@ -157,7 +157,7 @@ namespace TestCommon
             else if (!request.Filters.empty())
             {
                 input = request.Filters[0].Value;
-            }// else: default?
+            }
 
             bool installed = false;
             if (input == "installed1")
@@ -170,7 +170,15 @@ namespace TestCommon
                 return result;
             }
 
-            Manifest manifest = CreateFakeManifestWithDependencies(input);
+            Manifest manifest;
+            if (input == "MultipleDependenciesFromManifest")
+            {
+                manifest = YamlParser::CreateFromPath(TestDataFile("InstallFlowTest_MultipleDependencies.yaml"));
+            }
+            else
+            {
+                manifest = CreateFakeManifestWithDependencies(input);
+            }
 
             //TODO:
             // test for installed packages and packages that need upgrades

src/AppInstallerCLITests/TestData/InstallFlowTest_MultipleDependencies.yaml

@@ -0,0 +1,24 @@
+PackageIdentifier: AppInstallerCliTest.TestExeInstaller.MultipleDependencies
+PackageVersion: 1.0.0.0
+PackageName: AppInstaller Test Installer
+PackageLocale: en-US
+Publisher: Microsoft Corporation
+ShortDescription: Installs exe installer with multiple dependencies
+Moniker: AICLITestExe
+License: Test
+InstallerSwitches:
+  Custom: /custom
+  SilentWithProgress: /silentwithprogress
+  Silent: /silence
+  Upgrade: /upgrade
+Installers: 
+    - Architecture: x86
+      InstallerUrl: https://ThisIsNotUsed
+      InstallerType: exe
+      InstallerSha256: 65DB2F2AC2686C7F2FD69D4A4C6683B888DC55BFA20A0E32CA9F838B51689A3B
+      Dependencies:
+        PackageDependencies:
+        - PackageIdentifier: Dependency1
+        - PackageIdentifier: Dependency2
+ManifestType: singleton
+ManifestVersion: 1.3.0

src/AppInstallerCLITests/WorkFlow.cpp

@@ -3516,9 +3516,37 @@ TEST_CASE("DependencyGraph_StackOrderIsOk", "[InstallFlow][workflow][dependencyG
 
     // Verify installers are called in order
     REQUIRE(installationOrder.size() == 3);
-    REQUIRE(installationOrder.at(0).Id == "B");
-    REQUIRE(installationOrder.at(1).Id == "C");
-    REQUIRE(installationOrder.at(2).Id == "StackOrderIsOk");
+    REQUIRE(installationOrder.at(0).Id() == "B");
+    REQUIRE(installationOrder.at(1).Id() == "C");
+    REQUIRE(installationOrder.at(2).Id() == "StackOrderIsOk");
+}
+
+TEST_CASE("DependencyGraph_MultipleDependenciesFromManifest", "[InstallFlow][workflow][dependencyGraph][dependencies]")
+{
+    std::vector<Dependency> installationOrder;
+
+    std::ostringstream installOutput;
+    TestContext context{ installOutput, std::cin };
+    auto previousThreadGlobals = context.SetForCurrentThread();
+    OverrideOpenSourceForDependencies(context);
+    OverrideForShellExecute(context, installationOrder);
+
+    context.Args.AddArg(Execution::Args::Type::Query, "MultipleDependenciesFromManifest"sv);
+
+    TestUserSettings settings;
+    settings.Set<AppInstaller::Settings::Setting::EFDependencies>({ true });
+
+    InstallCommand install({});
+    install.Execute(context);
+    INFO(installOutput.str());
+
+    REQUIRE(installOutput.str().find(Resource::LocString(Resource::String::DependenciesFlowContainsLoop)) == std::string::npos);
+
+    // Verify installers are called in order
+    REQUIRE(installationOrder.size() == 3);
+    REQUIRE(installationOrder.at(0).Id() == "Dependency1");
+    REQUIRE(installationOrder.at(1).Id() == "Dependency2");
+    REQUIRE(installationOrder.at(2).Id() == "AppInstallerCliTest.TestExeInstaller.MultipleDependencies");
 }
 
 TEST_CASE("InstallerWithoutDependencies_RootDependenciesAreUsed", "[dependencies]")

src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp

@@ -694,8 +694,9 @@ namespace AppInstaller::Manifest
 
     Dependency* DependencyList::HasDependency(const Dependency& dependencyToSearch)
     {
-        for (auto& dependency : m_dependencies) {
-            if (dependency.Type == dependencyToSearch.Type && ICUCaseInsensitiveEquals(dependency.Id, dependencyToSearch.Id))
+        for (auto& dependency : m_dependencies)
+        {
+            if (dependency.Type == dependencyToSearch.Type && ICUCaseInsensitiveEquals(dependency.Id(), dependencyToSearch.Id()))
             {
                 return &dependency;
             }
@@ -708,7 +709,7 @@ namespace AppInstaller::Manifest
     {
         for (const auto& dependency : m_dependencies)
         {
-            if (dependency.Type == type && Utility::ICUCaseInsensitiveEquals(dependency.Id, id))
+            if (dependency.Type == type && Utility::ICUCaseInsensitiveEquals(dependency.Id(), id))
             {
                 if (dependency.MinVersion) {
                     if (dependency.MinVersion.value() == minVersion)

src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp

@@ -494,7 +494,7 @@ namespace AppInstaller::Manifest
         {
             result =
             {
-                { "PackageIdentifier", [this](const YAML::Node& value)->ValidationErrors { m_p_packageDependency->Id = Utility::Trim(value.as<std::string>()); return {}; } },
+                { "PackageIdentifier", [this](const YAML::Node& value)->ValidationErrors { m_p_packageDependency->SetId(Utility::Trim(value.as<std::string>())); return {}; } },
                 { "MinimumVersion", [this](const YAML::Node& value)->ValidationErrors { m_p_packageDependency->MinVersion = Utility::Version(Utility::Trim(value.as<std::string>())); return {}; } },
             };
         }