From af46517610f54198c42e88193be99ec8b5964e0c Mon Sep 17 00:00:00 2001 From: Yao Sun Date: Thu, 8 Dec 2022 13:24:05 -0800 Subject: [PATCH 1/4] Fix --- .../Workflows/DependenciesFlow.cpp | 12 ++++++------ .../Workflows/DependencyNodeProcessor.cpp | 6 +++--- src/AppInstallerCLICore/Workflows/ShowFlow.cpp | 8 ++++---- src/AppInstallerCLITests/Dependencies.cpp | 16 ++++++++-------- src/AppInstallerCLITests/WorkFlow.cpp | 6 +++--- .../Manifest/ManifestCommon.cpp | 7 ++++--- .../Manifest/ManifestYamlPopulator.cpp | 2 +- .../Public/winget/ManifestCommon.h | 17 +++++++++++++---- .../Microsoft/Schema/1_4/DependenciesTable.cpp | 8 ++++---- .../PackageDependenciesValidation.cpp | 8 ++++---- 10 files changed, 50 insertions(+), 40 deletions(-) diff --git a/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp b/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp index 53bf79f315..3f1f38e587 100644 --- a/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp @@ -48,13 +48,13 @@ 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)) @@ -62,7 +62,7 @@ namespace AppInstaller::CLI::Workflow 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()) diff --git a/src/AppInstallerCLICore/Workflows/DependencyNodeProcessor.cpp b/src/AppInstallerCLICore/Workflows/DependencyNodeProcessor.cpp index 6592c9efc3..9fc28c8f39 100644 --- a/src/AppInstallerCLICore/Workflows/DependencyNodeProcessor.cpp +++ b/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; } diff --git a/src/AppInstallerCLICore/Workflows/ShowFlow.cpp b/src/AppInstallerCLICore/Workflows/ShowFlow.cpp index 1f81ce974a..e3e4820f97 100644 --- a/src/AppInstallerCLICore/Workflows/ShowFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/ShowFlow.cpp @@ -180,13 +180,13 @@ 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)) @@ -194,7 +194,7 @@ namespace AppInstaller::CLI::Workflow 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; }); } } } diff --git a/src/AppInstallerCLITests/Dependencies.cpp b/src/AppInstallerCLITests/Dependencies.cpp index 04f3d6f7bf..02457ae6d0 100644 --- a/src/AppInstallerCLITests/Dependencies.cpp +++ b/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]") diff --git a/src/AppInstallerCLITests/WorkFlow.cpp b/src/AppInstallerCLITests/WorkFlow.cpp index 6c1bf02fa2..1e8935437d 100644 --- a/src/AppInstallerCLITests/WorkFlow.cpp +++ b/src/AppInstallerCLITests/WorkFlow.cpp @@ -3516,9 +3516,9 @@ 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("InstallerWithoutDependencies_RootDependenciesAreUsed", "[dependencies]") diff --git a/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp b/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp index d6c194a5af..c44f020277 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp +++ b/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) diff --git a/src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp b/src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp index aacb3b7f41..d4898e7a55 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp +++ b/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()); return {}; } }, + { "PackageIdentifier", [this](const YAML::Node& value)->ValidationErrors { m_p_packageDependency->SetId(Utility::Trim(value.as())); return {}; } }, { "MinimumVersion", [this](const YAML::Node& value)->ValidationErrors { m_p_packageDependency->MinVersion = Utility::Version(Utility::Trim(value.as())); return {}; } }, }; } diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h index e69bf4e2de..f035bd1013 100644 --- a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h +++ b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h @@ -202,14 +202,15 @@ namespace AppInstaller::Manifest struct Dependency { DependencyType Type; - string_t Id; + string_t Id() const { return m_id; }; std::optional MinVersion; - Dependency(DependencyType type, string_t id, string_t minVersion) : Type(type), Id(std::move(id)), MinVersion(Utility::Version(minVersion)), m_foldedId(FoldCase(Id)) {} - Dependency(DependencyType type, string_t id) : Type(type), Id(std::move(id)), m_foldedId(FoldCase(Id)){} + Dependency(DependencyType type, string_t id, string_t minVersion) : Type(type), m_id(std::move(id)), MinVersion(Utility::Version(minVersion)), m_foldedId(FoldCase(m_id)) {} + Dependency(DependencyType type, string_t id) : Type(type), m_id(std::move(id)), m_foldedId(FoldCase(m_id)){} Dependency(DependencyType type) : Type(type) {} - bool operator==(const Dependency& rhs) const { + bool operator ==(const Dependency& rhs) const + { return Type == rhs.Type && m_foldedId == rhs.m_foldedId && MinVersion == rhs.MinVersion; } @@ -223,8 +224,16 @@ namespace AppInstaller::Manifest return MinVersion <= Utility::Version(version); } + // m_foldedId should be set whenever Id is set + void SetId(string_t id) + { + m_id = std::move(id); + m_foldedId = FoldCase(m_id); + } + private: std::string m_foldedId; + string_t m_id; }; struct DependencyList diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_4/DependenciesTable.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_4/DependenciesTable.cpp index 7aa1bc13b6..51cdc29c50 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_4/DependenciesTable.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_4/DependenciesTable.cpp @@ -48,11 +48,11 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_4 { if (!missingPackageNodes.empty()) { - std::string missingPackages{ missingPackageNodes.begin()->Id }; + std::string missingPackages{ missingPackageNodes.begin()->Id()}; std::for_each( missingPackageNodes.begin() + 1, missingPackageNodes.end(), - [&](auto& dep) { missingPackages.append(", " + dep.Id); }); + [&](auto& dep) { missingPackages.append(", " + dep.Id()); }); THROW_HR_MSG(APPINSTALLER_CLI_ERROR_MISSING_PACKAGE, "Missing packages: %hs", missingPackages.c_str()); } } @@ -70,9 +70,9 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_4 { installer.Dependencies.ApplyToType(dependencyType, [&](Manifest::Dependency dependency) { - auto packageRowId = IdTable::SelectIdByValue(connection, dependency.Id); + auto packageRowId = IdTable::SelectIdByValue(connection, dependency.Id()); std::optional version; - + if (!packageRowId.has_value()) { missingPackageNodes.emplace_back(dependency); diff --git a/src/AppInstallerRepositoryCore/PackageDependenciesValidation.cpp b/src/AppInstallerRepositoryCore/PackageDependenciesValidation.cpp index 7872554951..dca5f8261b 100644 --- a/src/AppInstallerRepositoryCore/PackageDependenciesValidation.cpp +++ b/src/AppInstallerRepositoryCore/PackageDependenciesValidation.cpp @@ -116,23 +116,23 @@ namespace AppInstaller::Repository [&](const Dependency& node) { DependencyList depList; - if (node.Id == rootId.Id) + if (node.Id() == rootId.Id()) { return GetDependencies(manifest, DependencyType::Package); } - auto packageLatest = GetPackageLatestVersion(index, node.Id); + auto packageLatest = GetPackageLatestVersion(index, node.Id()); if (!packageLatest.has_value()) { dependenciesError.emplace_back( - ManifestError::MissingManifestDependenciesNode, "PackageIdentifier", node.Id); + ManifestError::MissingManifestDependenciesNode, "PackageIdentifier", node.Id()); foundErrors = true; return depList; } if (node.MinVersion > packageLatest.value().second) { - dependenciesError.emplace_back(ManifestError::NoSuitableMinVersionDependency, "PackageIdentifier", node.Id); + dependenciesError.emplace_back(ManifestError::NoSuitableMinVersionDependency, "PackageIdentifier", node.Id()); foundErrors = true; return depList; } From 3aa75da4e514687e7da89c2f57ff7cfffc3e6306 Mon Sep 17 00:00:00 2001 From: Yao Sun Date: Thu, 8 Dec 2022 14:25:08 -0800 Subject: [PATCH 2/4] more fix --- src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp | 2 +- src/AppInstallerCLICore/Workflows/InstallFlow.cpp | 6 +++++- src/AppInstallerCLICore/Workflows/InstallFlow.h | 7 +++++-- src/AppInstallerCommonCore/Public/winget/ManifestCommon.h | 2 +- 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp b/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp index 3f1f38e587..a00ce06fe5 100644 --- a/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp @@ -243,6 +243,6 @@ namespace AppInstaller::CLI::Workflow // Install dependencies in the correct order context.Add(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); } } \ No newline at end of file diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp index d94a873392..8b7ded7a6e 100644 --- a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp @@ -510,7 +510,7 @@ namespace AppInstaller::CLI::Workflow bool allSucceeded = true; size_t packagesCount = context.Get().size(); size_t packagesProgress = 0; - + for (auto& packageContext : context.Get()) { 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; + } } } } diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.h b/src/AppInstallerCLICore/Workflows/InstallFlow.h index 7e62becb94..48da383502 100644 --- a/src/AppInstallerCLICore/Workflows/InstallFlow.h +++ b/src/AppInstallerCLICore/Workflows/InstallFlow.h @@ -150,13 +150,15 @@ namespace AppInstaller::CLI::Workflow HRESULT resultOnFailure, std::vector&& 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. diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h index f035bd1013..f21f5bae69 100644 --- a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h +++ b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h @@ -232,8 +232,8 @@ namespace AppInstaller::Manifest } private: - std::string m_foldedId; string_t m_id; + std::string m_foldedId; }; struct DependencyList From d710b79d0d4e03192b8e435f657ca04387800ebd Mon Sep 17 00:00:00 2001 From: Yao Sun Date: Thu, 8 Dec 2022 15:21:00 -0800 Subject: [PATCH 3/4] tests --- .../AppInstallerCLITests.vcxproj | 3 ++ .../AppInstallerCLITests.vcxproj.filters | 27 ++++++++++-------- .../DependenciesTestSource.h | 12 ++++++-- .../InstallFlowTest_MultipleDependencies.yaml | 24 ++++++++++++++++ src/AppInstallerCLITests/WorkFlow.cpp | 28 +++++++++++++++++++ 5 files changed, 80 insertions(+), 14 deletions(-) create mode 100644 src/AppInstallerCLITests/TestData/InstallFlowTest_MultipleDependencies.yaml diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj index 300c14991a..06eb723967 100644 --- a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj +++ b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj @@ -276,6 +276,9 @@ true + + true + true diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters index 2eb85c90d6..2ced205cba 100644 --- a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters +++ b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters @@ -534,6 +534,9 @@ TestData + + TestData + TestData @@ -739,40 +742,40 @@ TestData - true + TestData - true + TestData - true + TestData - true + TestData - true + TestData - true + TestData - true + TestData - true + TestData - true + TestData - true + TestData - true + TestData - true + TestData \ No newline at end of file diff --git a/src/AppInstallerCLITests/DependenciesTestSource.h b/src/AppInstallerCLITests/DependenciesTestSource.h index 7b0a63a473..ae36b03c6e 100644 --- a/src/AppInstallerCLITests/DependenciesTestSource.h +++ b/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 diff --git a/src/AppInstallerCLITests/TestData/InstallFlowTest_MultipleDependencies.yaml b/src/AppInstallerCLITests/TestData/InstallFlowTest_MultipleDependencies.yaml new file mode 100644 index 0000000000..fc3b9e62ce --- /dev/null +++ b/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 \ No newline at end of file diff --git a/src/AppInstallerCLITests/WorkFlow.cpp b/src/AppInstallerCLITests/WorkFlow.cpp index 1e8935437d..f971306326 100644 --- a/src/AppInstallerCLITests/WorkFlow.cpp +++ b/src/AppInstallerCLITests/WorkFlow.cpp @@ -3521,6 +3521,34 @@ TEST_CASE("DependencyGraph_StackOrderIsOk", "[InstallFlow][workflow][dependencyG REQUIRE(installationOrder.at(2).Id() == "StackOrderIsOk"); } +TEST_CASE("DependencyGraph_MultipleDependenciesFromManifest", "[InstallFlow][workflow][dependencyGraph][dependencies]") +{ + std::vector 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({ 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]") { std::ostringstream installOutput; From a57fcf6f7b5aca61f95c33e114a78894cf59c1e9 Mon Sep 17 00:00:00 2001 From: Yao Sun Date: Thu, 8 Dec 2022 20:10:52 -0800 Subject: [PATCH 4/4] pr comments --- src/AppInstallerCommonCore/Public/winget/ManifestCommon.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h index f21f5bae69..478daa9619 100644 --- a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h +++ b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h @@ -202,7 +202,7 @@ namespace AppInstaller::Manifest struct Dependency { DependencyType Type; - string_t Id() const { return m_id; }; + const string_t& Id() const { return m_id; }; std::optional MinVersion; Dependency(DependencyType type, string_t id, string_t minVersion) : Type(type), m_id(std::move(id)), MinVersion(Utility::Version(minVersion)), m_foldedId(FoldCase(m_id)) {}