From 5c1475d0a7c1c55d2dd7236654727f95a18a64d9 Mon Sep 17 00:00:00 2001 From: John McPherson Date: Mon, 3 Nov 2025 17:36:17 -0800 Subject: [PATCH 1/7] Tree processing; need to generalize tree for testing --- .../Workflows/ConfigurationFlow.cpp | 159 +++++++++++++++--- src/AppInstallerCLICore/pch.h | 1 + 2 files changed, 140 insertions(+), 20 deletions(-) diff --git a/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp b/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp index 055e8fca84..c052a98c48 100644 --- a/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp @@ -1678,6 +1678,125 @@ namespace AppInstaller::CLI::Workflow } } + // Contains a tree of all unit processors by their path. + struct UnitProcessorTree + { + private: + struct SourceAndPackage + { + PackageCollection::Source Source; + PackageCollection::Package Package; + }; + + struct Node + { + // Packages whose installed location is at this node + std::vector Packages; + + // Units whose location is at this node. + std::vector Units; + + // The children of this node. + std::map Children; + }; + + Node m_rootNode; + + Node* FindNode(const std::filesystem::path& path, bool createIfNeeded) + { + const auto& nodePath = std::filesystem::weakly_canonical(path); + Node* currentNode = &m_rootNode; + + for (const auto& pathPart : nodePath) + { + auto& children = currentNode->Children; + + if (createIfNeeded) + { + currentNode = &children[pathPart]; + } + else + { + auto itr = children.find(pathPart); + + if (itr != children.end()) + { + currentNode = &itr->second; + } + else + { + // Not found and should not create + return nullptr; + } + } + } + + return currentNode; + } + + Node& FindNodeForFilePath(const winrt::hstring& filePath) + { + std::filesystem::path path{ std::wstring{ filePath } }; + return *FindNode(path.parent_path(), true); + } + + public: + UnitProcessorTree(std::vector&& unitProcessors) + { + for (auto&& unit : unitProcessors) + { + IConfigurationUnitProcessorDetails3 unitProcessor3; + if (unit.try_as(unitProcessor3)) + { + Node& node = FindNodeForFilePath(unitProcessor3.Path()); + node.Units.emplace_back(std::move(unit)); + } + } + } + + void PlacePackage(const PackageCollection::Source& source, const PackageCollection::Package& package) + { + Node* node = FindNode(package.InstalledLocation, false); + if (node) + { + node->Packages.emplace_back(SourceAndPackage{ source, package }); + } + } + + std::vector GetResourcesForPackage(const PackageCollection::Package& package) + { + std::vector result; + + Node* node = FindNode(package.InstalledLocation, false); + if (node) + { + std::queue nodes; + nodes.push(node); + + while (!nodes.empty()) + { + const Node* currentNode = nodes.front(); + nodes.pop(); + + for (const auto& unit : currentNode->Units) + { + result.emplace_back(unit); + } + + for (const auto& child : currentNode->Children) + { + if (child.second.Packages.empty()) + { + nodes.push(&child.second); + } + } + } + } + + return result; + } + }; + void ProcessPackagesForConfigurationExportAll(Execution::Context& context) { ConfigurationContext& configContext = context.Get(); @@ -1718,6 +1837,17 @@ namespace AppInstaller::CLI::Workflow } } + // Build a tree of the unit processors and place packages onto it to indicate nearest ownership. + UnitProcessorTree unitProcessorTree{ std::move(unitProcessors) }; + + for (const auto& source : context.Get().Sources) + { + for (const auto& package : source.Packages) + { + unitProcessorTree.PlacePackage(source, package); + } + } + for (const auto& source : context.Get().Sources) { // Create WinGetSource unit for non well known source. @@ -1734,30 +1864,19 @@ namespace AppInstaller::CLI::Workflow configContext.Set().Units().Append(packageUnit); // Try package settings export. - for (auto itr = unitProcessors.begin(); itr != unitProcessors.end(); /* itr incremented in the logic */) + auto unitsForPackage = unitProcessorTree.GetResourcesForPackage(package); + for (auto itr = unitsForPackage.begin(); itr != unitsForPackage.end(); ++itr) { - IConfigurationUnitProcessorDetails3 unitProcessor3; - itr->try_as(unitProcessor3); - if (Filesystem::IsParentPath(std::filesystem::path{ std::wstring{ unitProcessor3.Path() } }, package.InstalledLocation)) - { - ConfigurationUnit configUnit = anon::CreateConfigurationUnitFromUnitType( - unitProcessor3.UnitType(), - Utility::ConvertToUTF8(packageUnit.Identifier())); + ConfigurationUnit configUnit = anon::CreateConfigurationUnitFromUnitType( + itr->UnitType(), + Utility::ConvertToUTF8(packageUnit.Identifier())); - auto exportedUnits = anon::ExportUnit(context, configUnit); - anon::AddDependentUnit(exportedUnits, packageUnit); + auto exportedUnits = anon::ExportUnit(context, configUnit); + anon::AddDependentUnit(exportedUnits, packageUnit); - for (auto exportedUnit : exportedUnits) - { - configContext.Set().Units().Append(exportedUnit); - } - - // Remove the unit processor from the list after export. - itr = unitProcessors.erase(itr); - } - else + for (auto exportedUnit : exportedUnits) { - itr++; + configContext.Set().Units().Append(exportedUnit); } } } diff --git a/src/AppInstallerCLICore/pch.h b/src/AppInstallerCLICore/pch.h index c0daddfd06..e13f7ed751 100644 --- a/src/AppInstallerCLICore/pch.h +++ b/src/AppInstallerCLICore/pch.h @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include From 508b6fd012b8cad61e78ed2bb115de09a609bf9f Mon Sep 17 00:00:00 2001 From: John McPherson Date: Tue, 4 Nov 2025 17:03:24 -0800 Subject: [PATCH 2/7] Refactor to testable container; add tests --- .../Workflows/ConfigurationFlow.cpp | 74 ++-------- .../ConfigureExportCommand.cs | 8 +- src/AppInstallerCLITests/Filesystem.cpp | 110 +++++++++++++++ .../AppInstallerSharedLib.vcxproj | 1 + .../AppInstallerSharedLib.vcxproj.filters | 3 + .../Public/winget/PathTree.h | 129 ++++++++++++++++++ src/AppInstallerTestExeInstaller/main.cpp | 23 +++- 7 files changed, 284 insertions(+), 64 deletions(-) create mode 100644 src/AppInstallerSharedLib/Public/winget/PathTree.h diff --git a/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp b/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp index c052a98c48..9170735b5a 100644 --- a/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include using namespace AppInstaller::CLI::Execution; @@ -1695,49 +1696,14 @@ namespace AppInstaller::CLI::Workflow // Units whose location is at this node. std::vector Units; - - // The children of this node. - std::map Children; }; - Node m_rootNode; - - Node* FindNode(const std::filesystem::path& path, bool createIfNeeded) - { - const auto& nodePath = std::filesystem::weakly_canonical(path); - Node* currentNode = &m_rootNode; - - for (const auto& pathPart : nodePath) - { - auto& children = currentNode->Children; - - if (createIfNeeded) - { - currentNode = &children[pathPart]; - } - else - { - auto itr = children.find(pathPart); - - if (itr != children.end()) - { - currentNode = &itr->second; - } - else - { - // Not found and should not create - return nullptr; - } - } - } - - return currentNode; - } + Filesystem::PathTree m_pathTree; Node& FindNodeForFilePath(const winrt::hstring& filePath) { std::filesystem::path path{ std::wstring{ filePath } }; - return *FindNode(path.parent_path(), true); + return m_pathTree.FindOrInsert(path.parent_path()); } public: @@ -1756,42 +1722,30 @@ namespace AppInstaller::CLI::Workflow void PlacePackage(const PackageCollection::Source& source, const PackageCollection::Package& package) { - Node* node = FindNode(package.InstalledLocation, false); + Node* node = m_pathTree.Find(package.InstalledLocation); if (node) { node->Packages.emplace_back(SourceAndPackage{ source, package }); } } - std::vector GetResourcesForPackage(const PackageCollection::Package& package) + std::vector GetResourcesForPackage(const PackageCollection::Package& package) const { std::vector result; - Node* node = FindNode(package.InstalledLocation, false); - if (node) - { - std::queue nodes; - nodes.push(node); - - while (!nodes.empty()) + m_pathTree.VisitIf( + package.InstalledLocation, + [&](const Node& node) { - const Node* currentNode = nodes.front(); - nodes.pop(); - - for (const auto& unit : currentNode->Units) + for (const auto& unit : node.Units) { result.emplace_back(unit); } - - for (const auto& child : currentNode->Children) - { - if (child.second.Packages.empty()) - { - nodes.push(&child.second); - } - } - } - } + }, + [](const Node& node) + { + return node.Packages.empty(); + }); return result; } diff --git a/src/AppInstallerCLIE2ETests/ConfigureExportCommand.cs b/src/AppInstallerCLIE2ETests/ConfigureExportCommand.cs index 58a3085e3f..5bec7f7ac3 100644 --- a/src/AppInstallerCLIE2ETests/ConfigureExportCommand.cs +++ b/src/AppInstallerCLIE2ETests/ConfigureExportCommand.cs @@ -31,7 +31,9 @@ public void BaseSetup() var installDir = TestCommon.GetRandomTestDir(); TestCommon.RunAICLICommand("install", $"AppInstallerTest.TestPackageExport -v 1.0.0.0 --silent -l {installDir}"); this.previousPathValue = System.Environment.GetEnvironmentVariable("PATH"); - System.Environment.SetEnvironmentVariable("PATH", this.previousPathValue + ";" + installDir); + + // The installer puts DSCv3 resources in both locations + System.Environment.SetEnvironmentVariable("PATH", this.previousPathValue + ";" + installDir + ";" + installDir + ".SubDirectory"); DSCv3ResourceTestBase.EnsureTestResourcePresence(); } @@ -175,6 +177,10 @@ public void ExportAll() Assert.True(showResult.StdOut.Contains("AppInstallerTest/TestResource")); Assert.True(showResult.StdOut.Contains($"Dependencies: {Constants.TestSourceName}_AppInstallerTest.TestPackageExport")); Assert.True(showResult.StdOut.Contains("data: TestData")); + + Assert.True(showResult.StdOut.Contains("AppInstallerTest/TestResource.SubDirectory")); + Assert.True(showResult.StdOut.Contains($"Dependencies: {Constants.TestSourceName}_AppInstallerTest.TestPackageExport")); + Assert.True(showResult.StdOut.Contains("data: TestData")); } /// diff --git a/src/AppInstallerCLITests/Filesystem.cpp b/src/AppInstallerCLITests/Filesystem.cpp index f98934de82..38fda9fff6 100644 --- a/src/AppInstallerCLITests/Filesystem.cpp +++ b/src/AppInstallerCLITests/Filesystem.cpp @@ -3,6 +3,7 @@ #include "pch.h" #include "TestCommon.h" #include +#include #include using namespace AppInstaller::Utility; @@ -113,3 +114,112 @@ TEST_CASE("GetExecutablePathForProcess", "[filesystem]") REQUIRE(thisExecutable.has_extension()); REQUIRE(thisExecutable.filename() == L"AppInstallerCLITests.exe"); } + +TEST_CASE("PathTree_InsertAndFind", "[filesystem][pathtree]") +{ + PathTree pathTree; + + std::filesystem::path path1 = L"C:\\test"; + std::filesystem::path path1sub = L"C:\\test\\sub"; + std::filesystem::path path2 = L"C:\\diff"; + std::filesystem::path path3 = L"D:\\test"; + + REQUIRE(nullptr == pathTree.Find(path1)); + pathTree.FindOrInsert(path1) = true; + + REQUIRE(nullptr != pathTree.Find(path1)); + REQUIRE(*pathTree.Find(path1)); + + REQUIRE(nullptr == pathTree.Find(path1sub)); + REQUIRE(nullptr == pathTree.Find(path2)); + REQUIRE(nullptr == pathTree.Find(path3)); +} + +TEST_CASE("PathTree_InsertAndFind_Negative", "[filesystem][pathtree]") +{ + PathTree pathTree; + pathTree.FindOrInsert(L"C:\\a\\aa\\aaa"); + + REQUIRE(nullptr == pathTree.Find({})); + REQUIRE_THROWS_HR(pathTree.FindOrInsert({}), E_INVALIDARG); +} + +size_t CountVisited(const PathTree& pathTree, const std::filesystem::path& path, std::function predicate) +{ + size_t result = 0; + pathTree.VisitIf(path, [&](const bool&) { ++result; }, predicate); + return result; +} + +TEST_CASE("PathTree_VisitIf_Count", "[filesystem][pathtree]") +{ + PathTree pathTree; + + pathTree.FindOrInsert(L"C:\\a\\aa\\aaa") = true; + pathTree.FindOrInsert(L"C:\\a\\aa\\bbb") = true; + pathTree.FindOrInsert(L"C:\\a\\aa\\ccc") = false; + pathTree.FindOrInsert(L"C:\\a\\aa") = true; + + pathTree.FindOrInsert(L"C:\\a\\bb\\aaa") = false; + pathTree.FindOrInsert(L"C:\\a\\bb\\bbb") = true; + pathTree.FindOrInsert(L"C:\\a\\bb\\ccc") = false; + pathTree.FindOrInsert(L"C:\\a\\bb") = true; + + pathTree.FindOrInsert(L"C:\\a\\cc\\aaa") = true; + pathTree.FindOrInsert(L"C:\\a\\cc\\bbb") = false; + pathTree.FindOrInsert(L"C:\\a\\cc\\ccc") = false; + pathTree.FindOrInsert(L"C:\\a\\cc") = false; + + pathTree.FindOrInsert(L"C:\\a") = true; + pathTree.FindOrInsert(L"C:\\b") = false; + pathTree.FindOrInsert(L"D:\\a") = false; + + auto always = [](const bool&) { return true; }; + auto never = [](const bool&) { return false; }; + auto if_input = [](const bool& b) { return b; }; + + REQUIRE(0 == CountVisited(pathTree, {}, always)); + + REQUIRE(15 == CountVisited(pathTree, L"C:\\", always)); + REQUIRE(2 == CountVisited(pathTree, L"D:\\", always)); + REQUIRE(0 == CountVisited(pathTree, L"E:\\", always)); + + REQUIRE(1 == CountVisited(pathTree, L"C:\\", never)); + REQUIRE(1 == CountVisited(pathTree, L"D:\\", never)); + REQUIRE(0 == CountVisited(pathTree, L"E:\\", never)); + + REQUIRE(7 == CountVisited(pathTree, L"C:\\", if_input)); + REQUIRE(6 == CountVisited(pathTree, L"C:\\a", if_input)); + REQUIRE(2 == CountVisited(pathTree, L"C:\\a\\cc", if_input)); + REQUIRE(1 == CountVisited(pathTree, L"D:\\", if_input)); + REQUIRE(0 == CountVisited(pathTree, L"E:\\", if_input)); +} + +TEST_CASE("PathTree_VisitIf_Correct", "[filesystem][pathtree]") +{ + PathTree> pathTree; + + pathTree.FindOrInsert(L"C:\\a\\aa\\aaa") = { true, true }; + pathTree.FindOrInsert(L"C:\\a\\aa\\bbb") = { true, true }; + pathTree.FindOrInsert(L"C:\\a\\aa\\ccc") = { false, false }; + pathTree.FindOrInsert(L"C:\\a\\aa") = { true, true }; + + pathTree.FindOrInsert(L"C:\\a\\bb\\aaa") = { false, false }; + pathTree.FindOrInsert(L"C:\\a\\bb\\bbb") = { true, true }; + pathTree.FindOrInsert(L"C:\\a\\bb\\ccc") = { false, false }; + pathTree.FindOrInsert(L"C:\\a\\bb") = { true, true }; + + pathTree.FindOrInsert(L"C:\\a\\cc\\aaa") = { true, true }; + pathTree.FindOrInsert(L"C:\\a\\cc\\bbb") = { false, false }; + pathTree.FindOrInsert(L"C:\\a\\cc\\ccc") = { false, false }; + pathTree.FindOrInsert(L"C:\\a\\cc") = { false, false }; + + pathTree.FindOrInsert(L"C:\\a") = { true, true }; + pathTree.FindOrInsert(L"C:\\b") = { false, false }; + pathTree.FindOrInsert(L"C:") = { true, false }; + + auto check_input = [](const std::pair& p) { REQUIRE(p.first); }; + auto if_input = [](const std::pair& p) { return p.second; }; + + pathTree.VisitIf(L"C:", check_input, if_input); +} diff --git a/src/AppInstallerSharedLib/AppInstallerSharedLib.vcxproj b/src/AppInstallerSharedLib/AppInstallerSharedLib.vcxproj index 8afee3c685..511de1d87b 100644 --- a/src/AppInstallerSharedLib/AppInstallerSharedLib.vcxproj +++ b/src/AppInstallerSharedLib/AppInstallerSharedLib.vcxproj @@ -346,6 +346,7 @@ + diff --git a/src/AppInstallerSharedLib/AppInstallerSharedLib.vcxproj.filters b/src/AppInstallerSharedLib/AppInstallerSharedLib.vcxproj.filters index 36613a4c92..4c865ad740 100644 --- a/src/AppInstallerSharedLib/AppInstallerSharedLib.vcxproj.filters +++ b/src/AppInstallerSharedLib/AppInstallerSharedLib.vcxproj.filters @@ -146,6 +146,9 @@ Public\winget + + Public\winget + diff --git a/src/AppInstallerSharedLib/Public/winget/PathTree.h b/src/AppInstallerSharedLib/Public/winget/PathTree.h new file mode 100644 index 0000000000..202d9cd5d8 --- /dev/null +++ b/src/AppInstallerSharedLib/Public/winget/PathTree.h @@ -0,0 +1,129 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#pragma once +#include +#include +#include +#include + +namespace AppInstaller::Filesystem +{ + // Container that holds a map of items addressable by path. + template + struct PathTree + { + using value_t = Value; + + PathTree() = default; + + private: + struct Node + { + value_t Value{}; + std::map Children; + }; + + public: + // Returns the value for the given path, inserting it if necessary. + value_t& FindOrInsert(const std::filesystem::path& path) + { + return FindNode(path, true)->Value; + } + + // Finds the value for the given path; returns null if not found. + value_t* Find(const std::filesystem::path& path) + { + Node* node = FindNode(path, false); + return node ? &node->Value : nullptr; + } + + // Finds the value for the given path; returns null if not found. + const value_t* Find(const std::filesystem::path& path) const + { + const Node* node = FindNode(path); + return node ? &node->Value : nullptr; + } + + // Invokes the `visit` function for each value in the tree starting at `initialPath` (unconditionally) + // and recursively continuing on to children for whom the predicate returns true. + void VisitIf(const std::filesystem::path& initialPath, std::function visit, std::function predicate) const + { + const Node* node = FindNode(initialPath); + if (node) + { + std::queue nodes; + nodes.push(node); + + while (!nodes.empty()) + { + const Node* currentNode = nodes.front(); + nodes.pop(); + + visit(currentNode->Value); + + for (const auto& child : currentNode->Children) + { + if (predicate(child.second.Value)) + { + nodes.push(&child.second); + } + } + } + } + } + + private: + // Finds the node for the given path, creating as needed if requested. + Node* FindNode(const std::filesystem::path& path, bool createIfNeeded) + { + if (path.empty()) + { + if (createIfNeeded) + { + THROW_HR(E_INVALIDARG); + } + else + { + return nullptr; + } + } + + const auto& nodePath = std::filesystem::weakly_canonical(path); + Node* currentNode = &m_rootNode; + + for (const auto& pathPart : nodePath) + { + auto& children = currentNode->Children; + + if (createIfNeeded) + { + currentNode = &children[pathPart]; + } + else + { + auto itr = children.find(pathPart); + + if (itr != children.end()) + { + currentNode = &itr->second; + } + else + { + // Not found and should not create + return nullptr; + } + } + } + + return currentNode; + } + + // Finds the node for the given path; returns null if not found. + const Node* FindNode(const std::filesystem::path& path) const + { + return const_cast(this)->FindNode(path, false); + } + + Node m_rootNode; + }; +} diff --git a/src/AppInstallerTestExeInstaller/main.cpp b/src/AppInstallerTestExeInstaller/main.cpp index a67f12eab4..f749cf2db2 100644 --- a/src/AppInstallerTestExeInstaller/main.cpp +++ b/src/AppInstallerTestExeInstaller/main.cpp @@ -128,9 +128,13 @@ path GenerateModifyPath(const path& installDirectory) return modifyScriptPath; } -void GenerateDSCv3ProviderFiles(const path& installDirectory) +void GenerateDSCv3ProviderFiles(const path& installDirectory, const std::wstring_view subDirectory) { path dscResourceExecutablePath = installDirectory; + if (!subDirectory.empty()) + { + dscResourceExecutablePath /= subDirectory; + } dscResourceExecutablePath /= "AppInstallerTestResource.exe"; WCHAR currentExecutable[MAX_PATH]; @@ -139,6 +143,10 @@ void GenerateDSCv3ProviderFiles(const path& installDirectory) copy_file(currentExecutablePath, dscResourceExecutablePath); path dscResourceManifestPath = installDirectory; + if (!subDirectory.empty()) + { + dscResourceManifestPath /= subDirectory; + } dscResourceManifestPath /= "AppInstallerTest.dsc.resource.json"; std::wstring DscResourceJsonContent = @@ -205,7 +213,15 @@ void GenerateDSCv3ProviderFiles(const path& installDirectory) } } }, - "type" : "AppInstallerTest/TestResource", + "type" : "AppInstallerTest/TestResource)"; + + if (!subDirectory.empty()) + { + DscResourceJsonContent += '.'; + DscResourceJsonContent += subDirectory; + } + + DscResourceJsonContent += LR"(, "version" : "1.0.0" } )"; @@ -417,7 +433,8 @@ void HandleInstallationOperation( if (generateDscResourceFiles) { - GenerateDSCv3ProviderFiles(installDirectory); + GenerateDSCv3ProviderFiles(installDirectory, {}); + GenerateDSCv3ProviderFiles(installDirectory, L"SubDirectory"); } path uninstallerPath = GenerateUninstaller(out, installDirectory, productCode, useHKLM); From cf35900ef36acb8a5c052cf7981b57359642673c Mon Sep 17 00:00:00 2001 From: John McPherson Date: Tue, 4 Nov 2025 17:14:32 -0800 Subject: [PATCH 3/7] Spelling --- .github/actions/spelling/expect.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/actions/spelling/expect.txt b/.github/actions/spelling/expect.txt index 9f28a6b5b3..2e41a48dcf 100644 --- a/.github/actions/spelling/expect.txt +++ b/.github/actions/spelling/expect.txt @@ -47,6 +47,7 @@ awgpm awgs azurewebsites Baz +bbb bcp BEBOM BEFACEF @@ -74,6 +75,7 @@ buildtrees cancelledbyuser casemap casemappings +ccc cch centralus certmgr @@ -395,6 +397,7 @@ packageinusebyapplication PACL PARAMETERMAP pathparts +pathtree Patil pbstr pcb From 75ef1f902425814f41b0846198d791ca3648d842 Mon Sep 17 00:00:00 2001 From: John McPherson Date: Wed, 5 Nov 2025 08:15:05 -0800 Subject: [PATCH 4/7] Fix test installer --- src/AppInstallerTestExeInstaller/main.cpp | 41 ++++++++++++----------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/src/AppInstallerTestExeInstaller/main.cpp b/src/AppInstallerTestExeInstaller/main.cpp index f749cf2db2..f0e86644cd 100644 --- a/src/AppInstallerTestExeInstaller/main.cpp +++ b/src/AppInstallerTestExeInstaller/main.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -15,6 +16,7 @@ std::wstring_view RegistrySubkey = L"SOFTWARE\\Microsoft\\Windows\\CurrentVersio std::wstring_view DefaultProductID = L"{A499DD5E-8DC5-4AD2-911A-BCD0263295E9}"; std::wstring_view DefaultDisplayName = L"AppInstallerTestExeInstaller"; std::wstring_view DefaultDisplayVersion = L"1.0.0.0"; +std::wstring_view DscSubDirectoryName = L"SubDirectory"; void WriteModifyRepairScript(std::wofstream& script, const path& repairCompletedTextFilePath, bool isModifyScript) { std::wstring scriptName = isModifyScript ? L"Modify" : L"Uninstaller"; @@ -48,17 +50,16 @@ void WriteUninstallerScript( std::wofstream& uninstallerScript, const path& uninstallerOutputTextFilePath, const std::wstring& registryKey, - const path& modifyScriptPath, - const path& repairCompletedTextFilePath, - const path& dscResourceExecutablePath, - const path& dscResourceManifestPath) { + std::initializer_list paths) { uninstallerScript << "ECHO. >" << uninstallerOutputTextFilePath << "\n"; uninstallerScript << "ECHO AppInstallerTestExeInstaller.exe uninstalled successfully.\n"; uninstallerScript << "REG DELETE " << registryKey << " /f\n"; - uninstallerScript << "if exist \"" << modifyScriptPath.wstring() << "\" del \"" << modifyScriptPath.wstring() << "\"\n"; - uninstallerScript << "if exist \"" << repairCompletedTextFilePath.wstring() << "\" del \"" << repairCompletedTextFilePath.wstring() << "\"\n"; - uninstallerScript << "if exist \"" << dscResourceExecutablePath.wstring() << "\" del \"" << dscResourceExecutablePath.wstring() << "\"\n"; - uninstallerScript << "if exist \"" << dscResourceManifestPath.wstring() << "\" del \"" << dscResourceManifestPath.wstring() << "\"\n"; + + for (const auto& path : paths) + { + std::wstring pathString = path.wstring(); + uninstallerScript << "if exist \"" << pathString << "\" del \"" << pathString << "\"\n"; + } } path GenerateUninstaller(std::wostream& out, const path& installDirectory, const std::wstring& productID, bool useHKLM) @@ -74,15 +75,6 @@ path GenerateUninstaller(std::wostream& out, const path& installDirectory, const path repairCompletedTextFilePath = installDirectory; repairCompletedTextFilePath /= "TestExeRepairCompleted.txt"; - path modifyScriptPath = installDirectory; - modifyScriptPath /= "ModifyTestExe.bat"; - - path dscResourceExecutablePath = installDirectory; - dscResourceExecutablePath /= "AppInstallerTestResource.exe"; - - path dscResourceManifestPath = installDirectory; - dscResourceManifestPath /= "AppInstallerTest.dsc.resource.json"; - std::wstring registryKey{ useHKLM ? L"HKEY_LOCAL_MACHINE\\" : L"HKEY_CURRENT_USER\\" }; registryKey += RegistrySubkey; if (!productID.empty()) @@ -99,7 +91,15 @@ path GenerateUninstaller(std::wostream& out, const path& installDirectory, const uninstallerScript << L"for %%A in (%*) do (\n"; WriteModifyRepairScript(uninstallerScript, repairCompletedTextFilePath, false /*isModifyScript*/); uninstallerScript << ")\n"; - WriteUninstallerScript(uninstallerScript, uninstallerOutputTextFilePath, registryKey, modifyScriptPath, repairCompletedTextFilePath, dscResourceExecutablePath, dscResourceManifestPath); + WriteUninstallerScript(uninstallerScript, uninstallerOutputTextFilePath, registryKey, + { + installDirectory / "ModifyTestExe.bat", + repairCompletedTextFilePath, + installDirectory / "AppInstallerTestResource.exe", + installDirectory / "AppInstallerTest.dsc.resource.json", + installDirectory / DscSubDirectoryName / "AppInstallerTestResource.exe", + installDirectory / DscSubDirectoryName / "AppInstallerTest.dsc.resource.json", + }); uninstallerScript.close(); @@ -134,6 +134,7 @@ void GenerateDSCv3ProviderFiles(const path& installDirectory, const std::wstring if (!subDirectory.empty()) { dscResourceExecutablePath /= subDirectory; + std::filesystem::create_directories(dscResourceExecutablePath); } dscResourceExecutablePath /= "AppInstallerTestResource.exe"; @@ -221,7 +222,7 @@ void GenerateDSCv3ProviderFiles(const path& installDirectory, const std::wstring DscResourceJsonContent += subDirectory; } - DscResourceJsonContent += LR"(, + DscResourceJsonContent += LR"(", "version" : "1.0.0" } )"; @@ -434,7 +435,7 @@ void HandleInstallationOperation( if (generateDscResourceFiles) { GenerateDSCv3ProviderFiles(installDirectory, {}); - GenerateDSCv3ProviderFiles(installDirectory, L"SubDirectory"); + GenerateDSCv3ProviderFiles(installDirectory, DscSubDirectoryName); } path uninstallerPath = GenerateUninstaller(out, installDirectory, productCode, useHKLM); From 2c8896531bce8c5b5c7f16571dbea5ad804ed242 Mon Sep 17 00:00:00 2001 From: John McPherson Date: Thu, 6 Nov 2025 09:55:45 -0800 Subject: [PATCH 5/7] message --- .../Workflows/ConfigurationFlow.cpp | 11 +++++++++-- src/AppInstallerCLIE2ETests/ConfigureExportCommand.cs | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp b/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp index 9170735b5a..bf67681caa 100644 --- a/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp @@ -1714,7 +1714,9 @@ namespace AppInstaller::CLI::Workflow IConfigurationUnitProcessorDetails3 unitProcessor3; if (unit.try_as(unitProcessor3)) { - Node& node = FindNodeForFilePath(unitProcessor3.Path()); + winrt::hstring unitPath = unitProcessor3.Path(); + AICLI_LOG(Config, Verbose, << "Found unit `" << Utility::ConvertToUTF8(unit.UnitType()) << "` at: " << Utility::ConvertToUTF8(unitPath)); + Node& node = FindNodeForFilePath(unitPath); node.Units.emplace_back(std::move(unit)); } } @@ -1814,6 +1816,8 @@ namespace AppInstaller::CLI::Workflow for (const auto& package : source.Packages) { + AICLI_LOG(Config, Verbose, << "Exporting package `" << package.Id << "` at: " << package.InstalledLocation); + auto packageUnit = anon::CreateWinGetPackageUnit(package, source, context.Args.Contains(Args::Type::IncludeVersions), sourceUnit, packageUnitType); configContext.Set().Units().Append(packageUnit); @@ -1821,8 +1825,11 @@ namespace AppInstaller::CLI::Workflow auto unitsForPackage = unitProcessorTree.GetResourcesForPackage(package); for (auto itr = unitsForPackage.begin(); itr != unitsForPackage.end(); ++itr) { + winrt::hstring unitType = itr->UnitType(); + AICLI_LOG(Config, Verbose, << " exporting unit `" << Utility::ConvertToUTF8(unitType)); + ConfigurationUnit configUnit = anon::CreateConfigurationUnitFromUnitType( - itr->UnitType(), + unitType, Utility::ConvertToUTF8(packageUnit.Identifier())); auto exportedUnits = anon::ExportUnit(context, configUnit); diff --git a/src/AppInstallerCLIE2ETests/ConfigureExportCommand.cs b/src/AppInstallerCLIE2ETests/ConfigureExportCommand.cs index 5bec7f7ac3..d7ff31976d 100644 --- a/src/AppInstallerCLIE2ETests/ConfigureExportCommand.cs +++ b/src/AppInstallerCLIE2ETests/ConfigureExportCommand.cs @@ -148,7 +148,7 @@ public void ExportAll() { var exportDir = TestCommon.GetRandomTestDir(); var exportFile = Path.Combine(exportDir, "exported.yml"); - var result = TestCommon.RunAICLICommand(Command, $"--all -o {exportFile}", timeOut: 1200000); + var result = TestCommon.RunAICLICommand(Command, $"--all --verbose -o {exportFile}", timeOut: 1200000); Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode); Assert.True(File.Exists(exportFile)); From 77f3471a124084792bfab851047c9dd00a002e07 Mon Sep 17 00:00:00 2001 From: John McPherson Date: Thu, 6 Nov 2025 13:52:10 -0800 Subject: [PATCH 6/7] PR feedback and fixing test mistake --- src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp | 6 +++--- src/AppInstallerCLIE2ETests/ConfigureExportCommand.cs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp b/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp index bf67681caa..7c23fcab3c 100644 --- a/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp @@ -1823,9 +1823,9 @@ namespace AppInstaller::CLI::Workflow // Try package settings export. auto unitsForPackage = unitProcessorTree.GetResourcesForPackage(package); - for (auto itr = unitsForPackage.begin(); itr != unitsForPackage.end(); ++itr) + for (const auto& unit : unitsForPackage) { - winrt::hstring unitType = itr->UnitType(); + winrt::hstring unitType = unit.UnitType(); AICLI_LOG(Config, Verbose, << " exporting unit `" << Utility::ConvertToUTF8(unitType)); ConfigurationUnit configUnit = anon::CreateConfigurationUnitFromUnitType( @@ -1835,7 +1835,7 @@ namespace AppInstaller::CLI::Workflow auto exportedUnits = anon::ExportUnit(context, configUnit); anon::AddDependentUnit(exportedUnits, packageUnit); - for (auto exportedUnit : exportedUnits) + for (const auto& exportedUnit : exportedUnits) { configContext.Set().Units().Append(exportedUnit); } diff --git a/src/AppInstallerCLIE2ETests/ConfigureExportCommand.cs b/src/AppInstallerCLIE2ETests/ConfigureExportCommand.cs index d7ff31976d..cfc30554b8 100644 --- a/src/AppInstallerCLIE2ETests/ConfigureExportCommand.cs +++ b/src/AppInstallerCLIE2ETests/ConfigureExportCommand.cs @@ -33,7 +33,7 @@ public void BaseSetup() this.previousPathValue = System.Environment.GetEnvironmentVariable("PATH"); // The installer puts DSCv3 resources in both locations - System.Environment.SetEnvironmentVariable("PATH", this.previousPathValue + ";" + installDir + ";" + installDir + ".SubDirectory"); + System.Environment.SetEnvironmentVariable("PATH", this.previousPathValue + ";" + installDir + ";" + installDir + "\\SubDirectory"); DSCv3ResourceTestBase.EnsureTestResourcePresence(); } From 2b59933da64430a623e3992119c2d269b14acd5f Mon Sep 17 00:00:00 2001 From: John McPherson Date: Fri, 7 Nov 2025 10:15:36 -0800 Subject: [PATCH 7/7] Longer timeout, use a more stable URL --- src/AppInstallerCLIE2ETests/ConfigureCommand.cs | 2 +- src/AppInstallerCLITests/Downloader.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/AppInstallerCLIE2ETests/ConfigureCommand.cs b/src/AppInstallerCLIE2ETests/ConfigureCommand.cs index f4048cff45..f90da50627 100644 --- a/src/AppInstallerCLIE2ETests/ConfigureCommand.cs +++ b/src/AppInstallerCLIE2ETests/ConfigureCommand.cs @@ -364,7 +364,7 @@ public void ConfigureFromTestRepo_DSCv3() public void ConfigureFindUnitProcessors() { // Find all unit processors. - var result = TestCommon.RunAICLICommand("test config-find-unit-processors", string.Empty, timeOut: 120000); + var result = TestCommon.RunAICLICommand("test config-find-unit-processors", string.Empty, timeOut: 300000); Assert.AreEqual(0, result.ExitCode); Assert.True(result.StdOut.Contains("Microsoft/OSInfo")); diff --git a/src/AppInstallerCLITests/Downloader.cpp b/src/AppInstallerCLITests/Downloader.cpp index 0965222446..4c65c05bb1 100644 --- a/src/AppInstallerCLITests/Downloader.cpp +++ b/src/AppInstallerCLITests/Downloader.cpp @@ -82,7 +82,7 @@ TEST_CASE("HttpStream_ReadLastFullPage", "[HttpStream]") for (size_t i = 0; i < 10; ++i) { - stream = GetReadOnlyStreamFromURI("https://cdn.winget.microsoft.com/cache/source2.msix"); + stream = GetReadOnlyStreamFromURI("https://aka.ms/win32-x64-user-stable"); stat = { 0 }; REQUIRE(stream->Stat(&stat, STATFLAG_NONAME) == S_OK); @@ -96,7 +96,7 @@ TEST_CASE("HttpStream_ReadLastFullPage", "[HttpStream]") } { - INFO("https://cdn.winget.microsoft.com/cache/source2.msix gave back a 0 byte file"); + INFO("https://aka.ms/win32-x64-user-stable gave back a 0 byte file"); REQUIRE(stream); }