Skip to content

Commit 087ea0e

Browse files
authored
Support associating export units with packages in subdirectories of install location (microsoft#5859)
## Change The primary motivation is to support directories below the install location to contain configuration units that we will associate with the package. This is achieved by refactoring the association logic from a Package x Unit loop into a tree structure that is colored by package install locations. This also has the benefit of making a O(N^2) algorithm into an O(N). Units are first inserted into the tree based on their file path. Then the install location of each package is recorded onto that tree as well. Finally, during the export of each package, all resources at the install location and any that are descended from it but not under another package are included.
1 parent ab52c70 commit 087ea0e

File tree

11 files changed

+396
-45
lines changed

11 files changed

+396
-45
lines changed

.github/actions/spelling/expect.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ awgpm
4747
awgs
4848
azurewebsites
4949
Baz
50+
bbb
5051
bcp
5152
BEBOM
5253
BEFACEF
@@ -74,6 +75,7 @@ buildtrees
7475
cancelledbyuser
7576
casemap
7677
casemappings
78+
ccc
7779
cch
7880
centralus
7981
certmgr
@@ -395,6 +397,7 @@ packageinusebyapplication
395397
PACL
396398
PARAMETERMAP
397399
pathparts
400+
pathtree
398401
Patil
399402
pbstr
400403
pcb

src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp

Lines changed: 99 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include <AppInstallerStrings.h>
1818
#include <winget/ExperimentalFeature.h>
1919
#include <winget/SelfManagement.h>
20+
#include <winget/PathTree.h>
2021
#include <winrt/Microsoft.Management.Configuration.h>
2122

2223
using namespace AppInstaller::CLI::Execution;
@@ -1678,6 +1679,80 @@ namespace AppInstaller::CLI::Workflow
16781679
}
16791680
}
16801681

1682+
// Contains a tree of all unit processors by their path.
1683+
struct UnitProcessorTree
1684+
{
1685+
private:
1686+
struct SourceAndPackage
1687+
{
1688+
PackageCollection::Source Source;
1689+
PackageCollection::Package Package;
1690+
};
1691+
1692+
struct Node
1693+
{
1694+
// Packages whose installed location is at this node
1695+
std::vector<SourceAndPackage> Packages;
1696+
1697+
// Units whose location is at this node.
1698+
std::vector<IConfigurationUnitProcessorDetails> Units;
1699+
};
1700+
1701+
Filesystem::PathTree<Node> m_pathTree;
1702+
1703+
Node& FindNodeForFilePath(const winrt::hstring& filePath)
1704+
{
1705+
std::filesystem::path path{ std::wstring{ filePath } };
1706+
return m_pathTree.FindOrInsert(path.parent_path());
1707+
}
1708+
1709+
public:
1710+
UnitProcessorTree(std::vector<IConfigurationUnitProcessorDetails>&& unitProcessors)
1711+
{
1712+
for (auto&& unit : unitProcessors)
1713+
{
1714+
IConfigurationUnitProcessorDetails3 unitProcessor3;
1715+
if (unit.try_as(unitProcessor3))
1716+
{
1717+
winrt::hstring unitPath = unitProcessor3.Path();
1718+
AICLI_LOG(Config, Verbose, << "Found unit `" << Utility::ConvertToUTF8(unit.UnitType()) << "` at: " << Utility::ConvertToUTF8(unitPath));
1719+
Node& node = FindNodeForFilePath(unitPath);
1720+
node.Units.emplace_back(std::move(unit));
1721+
}
1722+
}
1723+
}
1724+
1725+
void PlacePackage(const PackageCollection::Source& source, const PackageCollection::Package& package)
1726+
{
1727+
Node* node = m_pathTree.Find(package.InstalledLocation);
1728+
if (node)
1729+
{
1730+
node->Packages.emplace_back(SourceAndPackage{ source, package });
1731+
}
1732+
}
1733+
1734+
std::vector<IConfigurationUnitProcessorDetails> GetResourcesForPackage(const PackageCollection::Package& package) const
1735+
{
1736+
std::vector<IConfigurationUnitProcessorDetails> result;
1737+
1738+
m_pathTree.VisitIf(
1739+
package.InstalledLocation,
1740+
[&](const Node& node)
1741+
{
1742+
for (const auto& unit : node.Units)
1743+
{
1744+
result.emplace_back(unit);
1745+
}
1746+
},
1747+
[](const Node& node)
1748+
{
1749+
return node.Packages.empty();
1750+
});
1751+
1752+
return result;
1753+
}
1754+
};
1755+
16811756
void ProcessPackagesForConfigurationExportAll(Execution::Context& context)
16821757
{
16831758
ConfigurationContext& configContext = context.Get<Data::ConfigurationContext>();
@@ -1718,6 +1793,17 @@ namespace AppInstaller::CLI::Workflow
17181793
}
17191794
}
17201795

1796+
// Build a tree of the unit processors and place packages onto it to indicate nearest ownership.
1797+
UnitProcessorTree unitProcessorTree{ std::move(unitProcessors) };
1798+
1799+
for (const auto& source : context.Get<Execution::Data::PackageCollection>().Sources)
1800+
{
1801+
for (const auto& package : source.Packages)
1802+
{
1803+
unitProcessorTree.PlacePackage(source, package);
1804+
}
1805+
}
1806+
17211807
for (const auto& source : context.Get<Execution::Data::PackageCollection>().Sources)
17221808
{
17231809
// Create WinGetSource unit for non well known source.
@@ -1730,34 +1816,28 @@ namespace AppInstaller::CLI::Workflow
17301816

17311817
for (const auto& package : source.Packages)
17321818
{
1819+
AICLI_LOG(Config, Verbose, << "Exporting package `" << package.Id << "` at: " << package.InstalledLocation);
1820+
17331821
auto packageUnit = anon::CreateWinGetPackageUnit(package, source, context.Args.Contains(Args::Type::IncludeVersions), sourceUnit, packageUnitType);
17341822
configContext.Set().Units().Append(packageUnit);
17351823

17361824
// Try package settings export.
1737-
for (auto itr = unitProcessors.begin(); itr != unitProcessors.end(); /* itr incremented in the logic */)
1825+
auto unitsForPackage = unitProcessorTree.GetResourcesForPackage(package);
1826+
for (const auto& unit : unitsForPackage)
17381827
{
1739-
IConfigurationUnitProcessorDetails3 unitProcessor3;
1740-
itr->try_as(unitProcessor3);
1741-
if (Filesystem::IsParentPath(std::filesystem::path{ std::wstring{ unitProcessor3.Path() } }, package.InstalledLocation))
1742-
{
1743-
ConfigurationUnit configUnit = anon::CreateConfigurationUnitFromUnitType(
1744-
unitProcessor3.UnitType(),
1745-
Utility::ConvertToUTF8(packageUnit.Identifier()));
1828+
winrt::hstring unitType = unit.UnitType();
1829+
AICLI_LOG(Config, Verbose, << " exporting unit `" << Utility::ConvertToUTF8(unitType));
17461830

1747-
auto exportedUnits = anon::ExportUnit(context, configUnit);
1748-
anon::AddDependentUnit(exportedUnits, packageUnit);
1831+
ConfigurationUnit configUnit = anon::CreateConfigurationUnitFromUnitType(
1832+
unitType,
1833+
Utility::ConvertToUTF8(packageUnit.Identifier()));
17491834

1750-
for (auto exportedUnit : exportedUnits)
1751-
{
1752-
configContext.Set().Units().Append(exportedUnit);
1753-
}
1835+
auto exportedUnits = anon::ExportUnit(context, configUnit);
1836+
anon::AddDependentUnit(exportedUnits, packageUnit);
17541837

1755-
// Remove the unit processor from the list after export.
1756-
itr = unitProcessors.erase(itr);
1757-
}
1758-
else
1838+
for (const auto& exportedUnit : exportedUnits)
17591839
{
1760-
itr++;
1840+
configContext.Set().Units().Append(exportedUnit);
17611841
}
17621842
}
17631843
}

src/AppInstallerCLICore/pch.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include <mutex>
3131
#include <numeric>
3232
#include <optional>
33+
#include <queue>
3334
#include <set>
3435
#include <sstream>
3536
#include <string>

src/AppInstallerCLIE2ETests/ConfigureCommand.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@ public void ConfigureFromTestRepo_DSCv3()
364364
public void ConfigureFindUnitProcessors()
365365
{
366366
// Find all unit processors.
367-
var result = TestCommon.RunAICLICommand("test config-find-unit-processors", string.Empty, timeOut: 120000);
367+
var result = TestCommon.RunAICLICommand("test config-find-unit-processors", string.Empty, timeOut: 300000);
368368
Assert.AreEqual(0, result.ExitCode);
369369
Assert.True(result.StdOut.Contains("Microsoft/OSInfo"));
370370

src/AppInstallerCLIE2ETests/ConfigureExportCommand.cs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,9 @@ public void BaseSetup()
3131
var installDir = TestCommon.GetRandomTestDir();
3232
TestCommon.RunAICLICommand("install", $"AppInstallerTest.TestPackageExport -v 1.0.0.0 --silent -l {installDir}");
3333
this.previousPathValue = System.Environment.GetEnvironmentVariable("PATH");
34-
System.Environment.SetEnvironmentVariable("PATH", this.previousPathValue + ";" + installDir);
34+
35+
// The installer puts DSCv3 resources in both locations
36+
System.Environment.SetEnvironmentVariable("PATH", this.previousPathValue + ";" + installDir + ";" + installDir + "\\SubDirectory");
3537
DSCv3ResourceTestBase.EnsureTestResourcePresence();
3638
}
3739

@@ -146,7 +148,7 @@ public void ExportAll()
146148
{
147149
var exportDir = TestCommon.GetRandomTestDir();
148150
var exportFile = Path.Combine(exportDir, "exported.yml");
149-
var result = TestCommon.RunAICLICommand(Command, $"--all -o {exportFile}", timeOut: 1200000);
151+
var result = TestCommon.RunAICLICommand(Command, $"--all --verbose -o {exportFile}", timeOut: 1200000);
150152
Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode);
151153
Assert.True(File.Exists(exportFile));
152154

@@ -175,6 +177,10 @@ public void ExportAll()
175177
Assert.True(showResult.StdOut.Contains("AppInstallerTest/TestResource"));
176178
Assert.True(showResult.StdOut.Contains($"Dependencies: {Constants.TestSourceName}_AppInstallerTest.TestPackageExport"));
177179
Assert.True(showResult.StdOut.Contains("data: TestData"));
180+
181+
Assert.True(showResult.StdOut.Contains("AppInstallerTest/TestResource.SubDirectory"));
182+
Assert.True(showResult.StdOut.Contains($"Dependencies: {Constants.TestSourceName}_AppInstallerTest.TestPackageExport"));
183+
Assert.True(showResult.StdOut.Contains("data: TestData"));
178184
}
179185

180186
/// <summary>

src/AppInstallerCLITests/Downloader.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ TEST_CASE("HttpStream_ReadLastFullPage", "[HttpStream]")
8282

8383
for (size_t i = 0; i < 10; ++i)
8484
{
85-
stream = GetReadOnlyStreamFromURI("https://cdn.winget.microsoft.com/cache/source2.msix");
85+
stream = GetReadOnlyStreamFromURI("https://aka.ms/win32-x64-user-stable");
8686

8787
stat = { 0 };
8888
REQUIRE(stream->Stat(&stat, STATFLAG_NONAME) == S_OK);
@@ -96,7 +96,7 @@ TEST_CASE("HttpStream_ReadLastFullPage", "[HttpStream]")
9696
}
9797

9898
{
99-
INFO("https://cdn.winget.microsoft.com/cache/source2.msix gave back a 0 byte file");
99+
INFO("https://aka.ms/win32-x64-user-stable gave back a 0 byte file");
100100
REQUIRE(stream);
101101
}
102102

src/AppInstallerCLITests/Filesystem.cpp

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include "pch.h"
44
#include "TestCommon.h"
55
#include <winget/Filesystem.h>
6+
#include <winget/PathTree.h>
67
#include <AppInstallerStrings.h>
78

89
using namespace AppInstaller::Utility;
@@ -113,3 +114,112 @@ TEST_CASE("GetExecutablePathForProcess", "[filesystem]")
113114
REQUIRE(thisExecutable.has_extension());
114115
REQUIRE(thisExecutable.filename() == L"AppInstallerCLITests.exe");
115116
}
117+
118+
TEST_CASE("PathTree_InsertAndFind", "[filesystem][pathtree]")
119+
{
120+
PathTree<bool> pathTree;
121+
122+
std::filesystem::path path1 = L"C:\\test";
123+
std::filesystem::path path1sub = L"C:\\test\\sub";
124+
std::filesystem::path path2 = L"C:\\diff";
125+
std::filesystem::path path3 = L"D:\\test";
126+
127+
REQUIRE(nullptr == pathTree.Find(path1));
128+
pathTree.FindOrInsert(path1) = true;
129+
130+
REQUIRE(nullptr != pathTree.Find(path1));
131+
REQUIRE(*pathTree.Find(path1));
132+
133+
REQUIRE(nullptr == pathTree.Find(path1sub));
134+
REQUIRE(nullptr == pathTree.Find(path2));
135+
REQUIRE(nullptr == pathTree.Find(path3));
136+
}
137+
138+
TEST_CASE("PathTree_InsertAndFind_Negative", "[filesystem][pathtree]")
139+
{
140+
PathTree<bool> pathTree;
141+
pathTree.FindOrInsert(L"C:\\a\\aa\\aaa");
142+
143+
REQUIRE(nullptr == pathTree.Find({}));
144+
REQUIRE_THROWS_HR(pathTree.FindOrInsert({}), E_INVALIDARG);
145+
}
146+
147+
size_t CountVisited(const PathTree<bool>& pathTree, const std::filesystem::path& path, std::function<bool(const bool&)> predicate)
148+
{
149+
size_t result = 0;
150+
pathTree.VisitIf(path, [&](const bool&) { ++result; }, predicate);
151+
return result;
152+
}
153+
154+
TEST_CASE("PathTree_VisitIf_Count", "[filesystem][pathtree]")
155+
{
156+
PathTree<bool> pathTree;
157+
158+
pathTree.FindOrInsert(L"C:\\a\\aa\\aaa") = true;
159+
pathTree.FindOrInsert(L"C:\\a\\aa\\bbb") = true;
160+
pathTree.FindOrInsert(L"C:\\a\\aa\\ccc") = false;
161+
pathTree.FindOrInsert(L"C:\\a\\aa") = true;
162+
163+
pathTree.FindOrInsert(L"C:\\a\\bb\\aaa") = false;
164+
pathTree.FindOrInsert(L"C:\\a\\bb\\bbb") = true;
165+
pathTree.FindOrInsert(L"C:\\a\\bb\\ccc") = false;
166+
pathTree.FindOrInsert(L"C:\\a\\bb") = true;
167+
168+
pathTree.FindOrInsert(L"C:\\a\\cc\\aaa") = true;
169+
pathTree.FindOrInsert(L"C:\\a\\cc\\bbb") = false;
170+
pathTree.FindOrInsert(L"C:\\a\\cc\\ccc") = false;
171+
pathTree.FindOrInsert(L"C:\\a\\cc") = false;
172+
173+
pathTree.FindOrInsert(L"C:\\a") = true;
174+
pathTree.FindOrInsert(L"C:\\b") = false;
175+
pathTree.FindOrInsert(L"D:\\a") = false;
176+
177+
auto always = [](const bool&) { return true; };
178+
auto never = [](const bool&) { return false; };
179+
auto if_input = [](const bool& b) { return b; };
180+
181+
REQUIRE(0 == CountVisited(pathTree, {}, always));
182+
183+
REQUIRE(15 == CountVisited(pathTree, L"C:\\", always));
184+
REQUIRE(2 == CountVisited(pathTree, L"D:\\", always));
185+
REQUIRE(0 == CountVisited(pathTree, L"E:\\", always));
186+
187+
REQUIRE(1 == CountVisited(pathTree, L"C:\\", never));
188+
REQUIRE(1 == CountVisited(pathTree, L"D:\\", never));
189+
REQUIRE(0 == CountVisited(pathTree, L"E:\\", never));
190+
191+
REQUIRE(7 == CountVisited(pathTree, L"C:\\", if_input));
192+
REQUIRE(6 == CountVisited(pathTree, L"C:\\a", if_input));
193+
REQUIRE(2 == CountVisited(pathTree, L"C:\\a\\cc", if_input));
194+
REQUIRE(1 == CountVisited(pathTree, L"D:\\", if_input));
195+
REQUIRE(0 == CountVisited(pathTree, L"E:\\", if_input));
196+
}
197+
198+
TEST_CASE("PathTree_VisitIf_Correct", "[filesystem][pathtree]")
199+
{
200+
PathTree<std::pair<bool, bool>> pathTree;
201+
202+
pathTree.FindOrInsert(L"C:\\a\\aa\\aaa") = { true, true };
203+
pathTree.FindOrInsert(L"C:\\a\\aa\\bbb") = { true, true };
204+
pathTree.FindOrInsert(L"C:\\a\\aa\\ccc") = { false, false };
205+
pathTree.FindOrInsert(L"C:\\a\\aa") = { true, true };
206+
207+
pathTree.FindOrInsert(L"C:\\a\\bb\\aaa") = { false, false };
208+
pathTree.FindOrInsert(L"C:\\a\\bb\\bbb") = { true, true };
209+
pathTree.FindOrInsert(L"C:\\a\\bb\\ccc") = { false, false };
210+
pathTree.FindOrInsert(L"C:\\a\\bb") = { true, true };
211+
212+
pathTree.FindOrInsert(L"C:\\a\\cc\\aaa") = { true, true };
213+
pathTree.FindOrInsert(L"C:\\a\\cc\\bbb") = { false, false };
214+
pathTree.FindOrInsert(L"C:\\a\\cc\\ccc") = { false, false };
215+
pathTree.FindOrInsert(L"C:\\a\\cc") = { false, false };
216+
217+
pathTree.FindOrInsert(L"C:\\a") = { true, true };
218+
pathTree.FindOrInsert(L"C:\\b") = { false, false };
219+
pathTree.FindOrInsert(L"C:") = { true, false };
220+
221+
auto check_input = [](const std::pair<bool, bool>& p) { REQUIRE(p.first); };
222+
auto if_input = [](const std::pair<bool, bool>& p) { return p.second; };
223+
224+
pathTree.VisitIf(L"C:", check_input, if_input);
225+
}

src/AppInstallerSharedLib/AppInstallerSharedLib.vcxproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,7 @@
346346
<ClInclude Include="Public\winget\LocIndependent.h" />
347347
<ClInclude Include="Public\winget\ManagedFile.h" />
348348
<ClInclude Include="Public\winget\ModuleCountBase.h" />
349+
<ClInclude Include="Public\winget\PathTree.h" />
349350
<ClInclude Include="Public\winget\Registry.h" />
350351
<ClInclude Include="Public\winget\Resources.h" />
351352
<ClInclude Include="Public\winget\Runtime.h" />

src/AppInstallerSharedLib/AppInstallerSharedLib.vcxproj.filters

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,9 @@
146146
<ClInclude Include="Public\winget\DetectMismatch.h">
147147
<Filter>Public\winget</Filter>
148148
</ClInclude>
149+
<ClInclude Include="Public\winget\PathTree.h">
150+
<Filter>Public\winget</Filter>
151+
</ClInclude>
149152
</ItemGroup>
150153
<ItemGroup>
151154
<ClCompile Include="pch.cpp">

0 commit comments

Comments
 (0)